Skip to content

Fixed a string that was not translatable.#14691

Merged
seanbudd merged 1 commit into
nvaccess:masterfrom
CyrilleB79:makeTranslatable
Mar 7, 2023
Merged

Fixed a string that was not translatable.#14691
seanbudd merged 1 commit into
nvaccess:masterfrom
CyrilleB79:makeTranslatable

Conversation

@CyrilleB79

@CyrilleB79 CyrilleB79 commented Mar 2, 2023

Copy link
Copy Markdown
Contributor

Link to issue number:

This thread on the translators' mailing list.

Summary of the issue:

The string "has %s" is not translatable whereas it should.

This is due to the fact that the formatting has been included in the gettext (underscore function)'s argument.

Description of user facing changes

For translators: the translation of the string "has %s" should now be working.

Description of development approach

First call gettext _ (underscore) function, and then format the result.

Testing strategy:

Manual test:
These strings have already been translated. Thus, test with the appveyor build or from source that translation is working:

Known issues with pull request:

None

Change log entries:

Not needed for this small fix.

Code Review Checklist:

  • Pull Request description:
    • description is up to date
    • change log entries
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • API is compatible with existing add-ons.
  • Documentation:
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • Security precautions taken.

@XLTechie

XLTechie commented Mar 2, 2023 via email

Copy link
Copy Markdown
Collaborator

@CyrilleB79

Copy link
Copy Markdown
Contributor Author

Why not change that to a .format call while you're at it? My understanding was that we're trying to remove % substitutions wherever we find them (all over the place).

It seems indeed that the trend is more to .format(...) for translatable strings and f-string for other string formatting, especially for new code. But this does not seem a strong requirement from NVAccess and I have seen situations where % was still used in recent PRs.

Regarding the current PR, keeping the string as is allows translators not to have to re-translate it.

Let's wait for the feedback from NVAccess' review.

@CyrilleB79 CyrilleB79 marked this pull request as ready for review March 3, 2023 10:26
@CyrilleB79 CyrilleB79 requested a review from a team as a code owner March 3, 2023 10:26
@seanbudd seanbudd merged commit 50cb4de into nvaccess:master Mar 7, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Mar 7, 2023
@CyrilleB79 CyrilleB79 deleted the makeTranslatable branch March 27, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants