Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue #427: fix edge case bug in plural pseudolocalization #428

Merged

Conversation

professorplumb
Copy link
Contributor

@professorplumb professorplumb commented Jan 9, 2019

Fixes issue #427.
Depends on PR #419 - the test added here will fail on this branch until that PR is incorporated.

This fix will still fail in the case that a variable placeholder in a template string inside a plural macro is followed by a single word which matches one of the plural rules, e.g. {count, plural, one {{countString} other} other {{countString} others}}.

This is a much more extreme edge case, though - it must be

  1. A template string
  2. Inside a plural expression
  3. With a variable placeholder
  4. Followed by zero|one|two|few|many|other
  5. With no other words or punctuation after that word
  6. With pseudolocalization enabled

IMO this is an acceptable tradeoff for a more readable regular expression.

@tricoder42
Copy link
Contributor

Thank you, @professorplumb. I think this is the best what we can get with regular expressions. 👍

Other method is use message-parser tokenizer, traverse through text tokens, pseudolocalize them and then join them back to message format, but that seems to be overengineered at the moment.

This patch looks good, thanks!

@tricoder42
Copy link
Contributor

Released in 2.7.3

Photonios pushed a commit to SectorLabs/js-lingui that referenced this pull request Dec 9, 2019
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.

2 participants