[gen_l10n] Handle single, double quotes, and dollar signs in strings #54185
Conversation
fluttergithubbot
added
c: contributor-productivity
perclasson
changed the title
dev/tools/localization/gen_l10n.dart
Outdated
| } | ||
| } | ||
| pluralLogicArgs.add(" ${pluralIds[pluralKey]}: '$argValue'"); | ||
| pluralLogicArgs.add(' ${pluralIds[pluralKey]}: ${generateString(argValue, escapeDollar: false)}'); |
There was a problem hiding this comment.
I realize that this is intended to behave mostly the way the existing code behaved. Are we missing another error here? What about a plural that contains dollar signs?
"singleQuotePlural": "{count,plural, $ =1{Flutter's amazing $, times 1!} other{Flutter's amazing $, times {count}!}}",
There was a problem hiding this comment.
@HansMuller Did you mean something like:
"dollarSignPlural": "{count,plural, =1{Flutter's amazing $, times {count}!} other{Flutter's amazing $, times {count}!}}",
I think the current way generateString works, it doesn't properly account for $ correctly because we use $ in the generated strings to interpolate variables into the messages. I'll file a separate issue for this, but we should unblock having this work for the other special JSON characters like quotes, return carriage, etc for now.
There was a problem hiding this comment.
I think by moving up the generateString function to before we do the interpolation of the variables, we can fix this. But it's probably best to handle it in a separate issue and add the extra test coverage that is needed.
There was a problem hiding this comment.
Created the issue here: #54203, had a discussion with @perclasson via DM -- We can solve the issue and add test coverage for it in the same PR. We would probably want to change the PR title and description to reflect the fixes this would address
perclasson
changed the title
shihaohong
left a comment
shihaohong
left a comment
There was a problem hiding this comment.
LGTM. Thanks for figuring this one out!
6 participants
Description
Handle single quotes, double quotes and dollar signs in strings for gen_l10n script.
Related Issues
Closes #54184
Closes #54203
Tests
I updated the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.