[Translation] Fix InvalidArgumentException when using untranslated plural forms from .po files #24594
Conversation
This file contains hidden or 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
stof
requested changes
Oct 17, 2017
| preg_match_all('/(?:\|\||[^\|])++/', $message, $parts); | ||
| $parts = array(); | ||
| if (preg_match('/^\|+$/', $message)) { | ||
| $parts = explode('|', $message); |
There was a problem hiding this comment.
this is broken if previous parts of the message have escaped pipes in them
Tasiobg
approved these changes
Oct 20, 2017
stof
requested changes
Oct 20, 2017
| { | ||
| preg_match_all('/(?:\|\||[^\|])++/', $message, $parts); | ||
| $parts = array(); | ||
| if (preg_match('/^\|+$/', $message)) { |
There was a problem hiding this comment.
please change the regex to ^\|++$ to avoid useless backtracking (which could cause issues with long strings). Backtracking will never allow to find a match in a different way for this regex.
|
@stof , changes made as requested :) |
|
But tests are failing. |
…translated plural forms from .po files
|
@nicolas-grekas I've rebased the branch on 3.3 and all tests pass now. I think there were some failing tests in the 3.2 branch |
|
@nicolas-grekas @stof tests are passing, is this okay to merge now? |
|
This has been rebased on 3.3 and tests are passing. Can this be merged please? |
|
Are 2.7 and 2.8 not affected? |
fabpot
approved these changes
Dec 11, 2017
|
Thank you @BjornTwachtmann. |
fabpot
added a commit
that referenced
this pull request
Dec 11, 2017
…anslated plural forms from .po files (BjornTwachtmann) This PR was squashed before being merged into the 3.3 branch (closes #24594). Discussion ---------- [Translation] Fix InvalidArgumentException when using untranslated plural forms from .po files | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #24593 | License | MIT This PR fixes the bug in #24593. It's not the absolutely ideal way to address the issue, but I don't see how else to handle it without either dropping support for pips in translation strings, or rearchitecting the code that feeds translations to the MessageSelector as pipe separated in the first place. Commits ------- fea815b [Translation] Fix InvalidArgumentException when using untranslated plural forms from .po files
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
This PR fixes the bug in #24593. It's not the absolutely ideal way to address the issue, but I don't see how else to handle it without either dropping support for pips in translation strings, or rearchitecting the code that feeds translations to the MessageSelector as pipe separated in the first place.