Skip to content

Remove one and only use of ngettext for now#12448

Merged
michaelDCurran merged 1 commit into
betafrom
noPlurals
May 21, 2021
Merged

Remove one and only use of ngettext for now#12448
michaelDCurran merged 1 commit into
betafrom
noPlurals

Conversation

@michaelDCurran

@michaelDCurran michaelDCurran commented May 21, 2021

Copy link
Copy Markdown
Member

Link to issue number:

None

Summary of the issue:

Recently NVDA began using ngettext to allow translators to provide both singular and plural translations for a particular message.
However, it seems that our PO validation code in the translation system is not handling plural forms well, and is incorrectly classing translations as missing brace format variables, as it is incorrectly comparing the translation with the message before that one.
It then outputs the following error:

Message starting on line 7169
Original: "{startTime} to {endTime}"
Translated: "category {categories}"
Error: missing brace format interpolation, extra brace format interpolation
Expected: these brace format interpolations: {endTime}, {startTime}
Got: these brace format interpolations: {categories} 

ngettext was so far only used for one string in NVDA, in source/appModules/outlook.py. Seen by doing git grep ngettext.

We have many many strings in NVDA which would benefit from ngettext, but we need to know it works correctly. So until we are sure we can validate this correctly, we should stop using it in the one and only place it exists today.

Description of how this pull request fixes the issue:

Stop using ngettext for the one string in the Outlook appModule, instead replacing it with a normal use of _() providing just the plural form.

Testing strategy:

  • Manually tested by Opening Outlook. Going to a calendar. Creating an appointment and adding 0, 1 and 2 categories, verifying each time that focusing the appointment NVDA reports the categories.

Known issues with pull request:

None.

Change log entries:

None needed.

New features
Changes
Bug fixes
For Developers

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

…etting confused by it, and this was not the first and only issue.
@michaelDCurran michaelDCurran requested a review from a team as a code owner May 21, 2021 06:15
@michaelDCurran michaelDCurran requested review from seanbudd and removed request for a team May 21, 2021 06:15
@michaelDCurran michaelDCurran merged commit 76fc1f7 into beta May 21, 2021
@michaelDCurran michaelDCurran deleted the noPlurals branch May 21, 2021 06:32
@nvaccessAuto nvaccessAuto added this to the 2021.2 milestone May 21, 2021
@michaelDCurran michaelDCurran modified the milestones: 2021.2, 2021.1 May 21, 2021
@feerrenrut

Copy link
Copy Markdown
Contributor

seanbudd pushed a commit to nvaccess/mrconfig that referenced this pull request Jun 13, 2023
Preliminary to nvaccess/nvda#12445

Context
A first use of ngettext had been introduced in NVDA in nvaccess/nvda#11598 and nvaccess/nvda#12432.
However these changes were removed in nvaccess/nvda#12448.

Issue
As written by @michaelDCurran in nvaccess/nvda#12448:

However, it seems that our PO validation code in the translation system is not handling plural forms well, and is incorrectly classing translations as missing brace format variables, as it is incorrectly comparing the translation with the message before that one.
It then outputs the following error:
Message starting on line 7169
Original: "{startTime} to {endTime}"
Translated: "category {categories}"
Error: missing brace format interpolation, extra brace format interpolation
Expected: these brace format interpolations: {endTime}, {startTime}
Got: these brace format interpolations: {categories}

Changes in this PR
This PR adds the support of ngettext messages in the PO validation script.
CyrilleB79 added a commit to CyrilleB79/nvda that referenced this pull request Jun 13, 2023
… to be getting confused by it, and this was not the first and only issue. (nvaccess#12448)"

This reverts commit 76fc1f7.
CyrilleB79 added a commit to CyrilleB79/nvda that referenced this pull request Jun 15, 2023
… to be getting confused by it, and this was not the first and only issue. (nvaccess#12448)"

This reverts commit 76fc1f7.
seanbudd pushed a commit that referenced this pull request Jun 19, 2023
This PR is the first step to reintroduce the distinction of singulare/plural forms when formatting UI strings. The project is as follow:
1. This PR introduce three strings to be able to test the feature.
2. 2023.2 beta phase should be used to validate more widely the feature with translators
3. A second PR during 2023.3 dev cycle will implement the plural forms for all UI strings containing only one value to format them.
4. A third PR should implement more complex cases of string formatted by two or more values, e.g. such as "table with x rows and y columns".

This step-by-step approach should allow to lose less work in case an issue occurs and requires to revert changes.

### Link to issue number:
First step to implement #12445.
Restoring #11598, #12432 and #12435 that were reverted in #12448.
Unblocked by nvaccess/mrconfig#97.
### Summary of the issue:
Some UI messages or strings are reported with plural form, no matter the number passed as parameter to format them, e.g. NVDA reports "list with 1 items" with "s" even if there is only one item.

### Description of user facing changes
The following strings will be reported using singular or plural form depending on the number used to format them:
* "with %s items" (used to describe the number of items in a list on the web)
* ".1f lines" (used when reporting multiple line spacing formatting in MS Word)
* ""categories {categories}" used to report categories of appointments in MS Outlook

Translators will be able to translate singular or plural forms.

### Description of development approach
* First revert #12448 to restore the first attempt that was made to introduce `ngettext`.
* Develop a custom `npgettext` function the same way as `pgettext` was introduced in NVDA since `npgettext` is not available in Python 3.7; `pgettext` and `npgettext` are available natively in Python 3.8.
* `ngettext` and `npgettext` are made accessible without importing them; `pgettext` was already added previously in builtins.
* Implement the plural form for two strings that are more commonly used than Outlook appointments category reporting:
  * "with %s items", using `ngettext`
  * "%.1f lines", using `npgettext`
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