Skip to content

Add-on store: pluralize one string#19059

Merged
seanbudd merged 5 commits into
nvaccess:masterfrom
CyrilleB79:plural
Nov 3, 2025
Merged

Add-on store: pluralize one string#19059
seanbudd merged 5 commits into
nvaccess:masterfrom
CyrilleB79:plural

Conversation

@CyrilleB79

@CyrilleB79 CyrilleB79 commented Oct 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Link to issue number:

Fix-up of #18974

Summary of the issue:

One newly translatable string needs to support plural forms:
"{malicious} out of {total} malware scanners detected this add-on as potentially malicious."

In English, there is no difference, since "detected" uses the same form at singular or plural; but in other languages, not. There are also languages where scanners pluralization needs to change.

Description of user facing changes:

The information will have correct pluralization form.

After discussion, I have tried to find a wording where only one of the two numbers needs an agreement to avoid splitting and re-building the sentence.

Description of developer facing changes:

N/A

Description of development approach:

Use npgettext instead of pgettext.

Testing strategy:

Checked the add-on store's details panel. Found add-ons with 0, 1, 4 and 10 scanners that detected add-on as malicious; so I have been able to check pluralization working in English.

Known issues with pull request:

None

Code Review Checklist:

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

@CyrilleB79 CyrilleB79 requested a review from a team as a code owner October 7, 2025 10:54
@CyrilleB79

CyrilleB79 commented Oct 7, 2025

Copy link
Copy Markdown
Contributor Author

@SaschaCowley:
As written in test section, I have not checked with 1 malicious scanner result.
Is there a way to spoof this information?
If we If we could have a test plan or a script — similar to the one we already use to spoof the add-on version — that would be great.
I can add one here if you let me know which information needs to be modified and in which file. Thanks.

@zstanecic, are there languages where both variables can affect singular/plural/other forms?
I’m afraid yes. For example, if malicious = 1 and total = 72, I guess that in some languages you would need:
•
“{total} malware scanners” to agree in number differently depending on whether total is 69 (as today), 71, or 72…
•
“detected” to agree in number with {malicious}, so differently depending on whether it’s 0, 1, 2, 3, 6, etc.
Or do you plan to use alternative wording for those languages?
Maybe a strict translation of this sentence would lead to undesirable declension or agreement issues, so translators might need to rephrase it.

@zstanecic

zstanecic commented Oct 7, 2025

Copy link
Copy Markdown
Contributor

@CyrilleB79 are you sure that you know what are you doing? The number should agree with the noun. I would rather rephrase this message, so that it sounds: x of y malware scanners flagged the addon [addonname] as malicious.

@seanbudd

seanbudd commented Oct 7, 2025

Copy link
Copy Markdown
Member

@CyrilleB79 - there is something incredibly wrong with the diff here

@seanbudd seanbudd marked this pull request as draft October 7, 2025 23:12
@CyrilleB79

CyrilleB79 commented Oct 8, 2025

Copy link
Copy Markdown
Contributor Author

@seanbudd:
Sorry, probably a very bad error with so many changes! I have force pushed a new version of the intended change.
Though, I'll keep this PR as draft until I have finished discussing with @zstanecic about correct wording.

@zstanecic wrote:

@CyrilleB79 are you sure that you know what are you doing? The number should agree with the noun.

The sentence was first written by @seanbudd. I have checked with ChatGPT; according to it, in English, the number "{total}" should agree with the noun "scanner" and the number "{malwares}" should agree with the verb "detected". Though "detected" past form does not change in English, no matter the number "{malwares}"; but if the sentence would have been at present tense, it would change ("detects" vs "detect").

If you disagree with this, in English or in another language, please let me know, explaining clearly.

I would rather rephrase this message, so that it sounds: x of y malware scanners flagged the addon [addonname] as malicious.

I am more than open to a better wording. But why did you add "[addonname]" here? It is useless.
If the sentence should agree with both numbers, maybe we can split the sentence in two translatable ones:

  • "{malicious} malware scanners detected this add-on as potentially malicious."
  • "({total} scanners used)"

We can then concatenate the two sentences.

What do you think?

@zstanecic

zstanecic commented Oct 9, 2025

Copy link
Copy Markdown
Contributor

Yes, we need to split this. In some languages, nouns not only agree with gender and case, but with the numbers.

@seanbudd

seanbudd commented Oct 14, 2025

Copy link
Copy Markdown
Member

@CyrilleB79 - is this ready for re-review?

@CyrilleB79

CyrilleB79 commented Oct 17, 2025

Copy link
Copy Markdown
Contributor Author

@zstanecic what about the following string:
"{malicious} malware scanner detected this add-on as potentially malicious (out of {total})."

This wording would allow to pluralize this string according to malicious, but the fact that total is in brackets prevents us from needing to have a double agreement in the sentence.
In French I would translate the brackets content by "{total} au total", with no plural agreement needed.

If it is not suitable for other languages, would you think to a wording that would allow this double pluralization.
If not possible, I'll split the string, but in any case, the wording should be reorganized to ease translation.

Let me know. Thanks!

@zstanecic

zstanecic commented Oct 19, 2025

Copy link
Copy Markdown
Contributor

Okay. when thinking more about⠀ this, only second number, if mo⠀re tha⠀n 10⠀, will need declension, so it is fairly safe to proceed.

@CyrilleB79

CyrilleB79 commented Oct 19, 2025

Copy link
Copy Markdown
Contributor Author

@zstanecic, it's not very clear to me: to which proposal are you replying to? Is the following OK for you?
"{malicious} malware scanner detected this add-on as potentially malicious (out of {total})."

@zstanecic

zstanecic commented Oct 29, 2025

Copy link
Copy Markdown
Contributor

Hi @CyrilleB79
Yes, the following is okay for me:
"{malicious} malware scanner detected this add-on as potentially malicious (out of {total})."
Sorry for my late reply.

@CyrilleB79 CyrilleB79 marked this pull request as ready for review November 1, 2025 22:15
@CyrilleB79 CyrilleB79 mentioned this pull request Nov 2, 2025
Merged
5 tasks
@CyrilleB79
Revert "Fix checkPot"
200f8c4 
This reverts commit 357a761.
@CyrilleB79

CyrilleB79 commented Nov 2, 2025

Copy link
Copy Markdown
Contributor Author

There was an issue with the checkPot.

Finally, I have fixed it in #19158 rather than here to separate both works. So please merge #19158 before this PR. Thanks.

seanbudd pushed a commit that referenced this pull request Nov 3, 2025
@CyrilleB79
Fix checkPot for plural (#19158)
97f5033 
Fixes issue found while working on #19059
Summary of the issue:

In #19059, commit 9595711, an error is raised during check Pot:

scons: *** [tests\checkPot] IndexError : list index out of range
Traceback (most recent call last):
  File "D:\a\nvda\nvda\.venv\Lib\site-packages\SCons\Action.py", line 1434, in execute
    result = self.execfunction(target=target, source=rsources, env=env)
  File "D:\a\nvda\nvda\tests\sconscript", line 21, in checkPotAction
    return checkPot.checkPot(source[0].abspath)
           ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^
  File "D:\a\nvda\nvda\tests\checkPot.py", line 142, in checkPot
    msgid += getStringFromLine(line)
             ~~~~~~~~~~~~~~~~~^^^^^^
  File "D:\a\nvda\nvda\tests\checkPot.py", line 201, in getStringFromLine
    quoted = line.split(" ", 1)[1]
             ~~~~~~~~~~~~~~~~~~^^^
IndexError: list index out of range
Error: Process completed with exit code 1.

The test should pass or fail but no error should be raised.

The error occurs if a msgid command containing a multi-line string is followed by a msgid_plural command. This had probably never happened before the string introduced in #19059.
Description of user facing changes:

The Python script to check Pot does not raise an error before completing.
Description of developer facing changes:

N/A
Description of development approach:

Take into account the presence of msgid_plural token. Taken into account that a multi-line string in the msgid command can end:

    either with a msgstr command
    or a msgid_plural command
@seanbudd seanbudd closed this Nov 3, 2025
@seanbudd seanbudd reopened this Nov 3, 2025
@seanbudd seanbudd enabled auto-merge (squash) November 3, 2025 00:59
@seanbudd seanbudd merged commit 70234cb into nvaccess:master Nov 3, 2025
54 of 57 checks passed
@github-actions github-actions Bot added this to the 2026.1 milestone Nov 3, 2025
@CyrilleB79 CyrilleB79 deleted the plural branch November 3, 2025 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@seanbudd seanbudd seanbudd approved these changes

@SaschaCowley SaschaCowley Awaiting requested review from SaschaCowley SaschaCowley is a code owner automatically assigned from nvaccess/developers

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2026.1

Development

Successfully merging this pull request may close these issues.

3 participants

@CyrilleB79 @zstanecic @seanbudd