Run string formatting after gettext() call #6368
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't interpolate parameters before looking up the string in gettext, otherwise it won't find anything and fallback to English. Fixes #6367.
Codecov Report
@@ Coverage Diff @@
## develop #6368 +/- ##
========================================
Coverage 83.98% 83.98%
========================================
Files 61 61
Lines 4301 4301
Branches 522 522
========================================
Hits 3612 3612
Misses 565 565
Partials 124 124
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
zenmonkeykstop
approved these changes
Mar 29, 2022
| @@ -46,11 +46,11 @@ <h2 id="submit-heading" class="headline">{{ gettext('Submit Messages') }}</h2> | |||
| {{ form.msg(class="fill-parent") }} | |||
| {% if allow_document_uploads %} | |||
| {% if min_len > 0 %} | |||
| <p class="center" id="min-msg-length">{{ gettext('If you are only sending a message, it must be at least {} characters long.'.format(min_len)) }}</p> | |||
| <p class="center" id="min-msg-length">{{ gettext('If you are only sending a message, it must be at least {} characters long.').format(min_len) }}</p> | |||
There was a problem hiding this comment.
Testing: this case did not work correctly, but that's due to this string not being collected for translation, the code change is valid.
| @@ -204,7 +204,7 @@ def submit(logged_in_source: SourceUser) -> werkzeug.Response: | |||
| min_len = InstanceConfig.get_default().initial_message_min_len | |||
| if (min_len > 0) and (msg and not fh) and (len(msg) < min_len): | |||
| flash(gettext( | |||
| "Your first message must be at least {} characters long.".format(min_len)), | |||
| "Your first message must be at least {} characters long.").format(min_len), | |||
There was a problem hiding this comment.
testing - confirmed that this case works correctly when rebased onto the i18n-merge branch.
| {% endif %} | ||
| {% else %} | ||
| {% if min_len > 0 %} | ||
| <p class="center" id="min-msg-length">{{ gettext('Your first message must be at least {} characters long.'.format(min_len)) }}</p> | ||
| <p class="center" id="min-msg-length">{{ gettext('Your first message must be at least {} characters long.').format(min_len) }}</p> |
There was a problem hiding this comment.
Testing: this case works correctly (with min-length set and document uploads disabled).
3 tasks
35 tasks
This was referenced May 23, 2022
lsd-cat
pushed a commit
to lsd-cat/securedrop-public
that referenced
this pull request
Sep 14, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Status
Ready for review
Description of Changes
Don't interpolate parameters before looking up the string in gettext,
otherwise it won't find anything and fallback to English.
Fixes #6367.
Testing
{}appear in the error message when inputting too short of a message, and the correct character count is shown.Deployment
Any special considerations for deployment? No.
Checklist
make lint) and tests (make test) pass in the development container