Skip to content

Fix missing translator comment warning for split translatable strings#11093

Merged
feerrenrut merged 2 commits into
nvaccess:masterfrom
accessolutions:i11071-checkPot
May 4, 2020
Merged

Fix missing translator comment warning for split translatable strings#11093
feerrenrut merged 2 commits into
nvaccess:masterfrom
accessolutions:i11071-checkPot

Conversation

@JulienCochuyt

Copy link
Copy Markdown
Contributor

Link to issue number:

Fix-up of PR #7492
Found thanks to PR #10941
Fixes #11071
Follow-up of PR #11092

Summary of the issue:

When xgettext extracts lengthy translatable strings, it starts its entry in the POT file with

msgid ""

checkPot misinterpret this as the header line and thus triggers no error for missing translators comment for this message.

Description of how this pull request fixes the issue:

  • Ensured checkPot does not mistake a lengthy msgid with the POT file header
  • Added 5 additional known messages without translators comment with reference to their location in the source
  • Added 1 missing translators comment
  • Linted 25 existing translators comment so that they now properly land in the POT file

Testing performed:

Ran scons checkPot with its sole fix.
Result: 31 errors
Addressed the 31 errors as detailed above.
Ran scons checkPot again.
Result: 0 errors
Manually checked in the POT file some of the previously reported errors and ensured their translators comment has indeed been copied.
Finally, ran scons tests, built a local launcher and checked in the GUI that some of the 31 incriminated messages were still translated as expected.

Known issues with pull request:

Change log entry:

I don't think this deserves a change log entry.

…slatable strings

 - Ensured `checkPot` does not mistake a lengthy `msgid` with the POT file header
 - Added 5 additional known messages without translators comment with reference to their location in the source
 - Added 1 missing translators comment
 - Linted 25 existing translators comment so that they now properly land in the POT file
@JulienCochuyt JulienCochuyt changed the title Fix-up of #7492 / Fix #11071: Allow splitting of translatable strings Fix-up of PR #7492 / Fix #11071: Allow splitting of translatable strings May 3, 2020
@feerrenrut feerrenrut changed the title Fix-up of PR #7492 / Fix #11071: Allow splitting of translatable strings Fix missing translator comment warning for split translatable strings May 4, 2020
Comment thread tests/checkPot.py
Comment thread tests/checkPot.py
'Primary header',
'Text frame',
# core.py:87
r'Your gesture map file contains errors.\n"More details about the errors can be found in the log file."',

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you create an issue for these missing translator comment strings? This could be fairly straightforward work for a new contributor.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: #11104
Could you please add the label "goodForNewDev"?
(Or allow me to label issues...)

Comment thread source/gui/settingsDialogs.py
Comment thread tests/checkPot.py Outdated

@feerrenrut feerrenrut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@feerrenrut feerrenrut merged commit aa54784 into nvaccess:master May 4, 2020
@nvaccessAuto nvaccessAuto added this to the 2020.1 milestone May 4, 2020
@JulienCochuyt JulienCochuyt deleted the i11071-checkPot branch May 4, 2020 16:28
JulienCochuyt added a commit to accessolutions/nvda that referenced this pull request May 19, 2020
feerrenrut pushed a commit that referenced this pull request Jun 3, 2020
PR #10941 brought to light flaws in checkPot regarding handling of long messages - fixed by PR #11093
In #11093 (comment) the creation of unit tests was discussed to help limit occurrence of future regressions.
Discussion: #11093 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/i18n existing localisations or internationalisation maintenance

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow splitting of translatable strings

3 participants