Skip to content

Conversation

sgiehl
Copy link
Member

@sgiehl sgiehl commented Nov 27, 2016

Using unescaped % symbols in translations can cause problems on transifex: see #10877

Currently escaped % weren't always unescaped.

fixes #10877

@sgiehl sgiehl added the Needs Review PRs that need a code review label Nov 27, 2016
@sgiehl sgiehl added this to the 3.0.0-b4 milestone Nov 27, 2016
@mattab
Copy link
Member

mattab commented Dec 1, 2016

Looks good, but when do we need to use %% VS simply %? noticed there are more % used in translations (in Insights plugin, CustomAlerts, Intl, AbTesting).

Should we change them all maybe? to find them all I did a regex search in phpstorm %[^sd1-9] restricted to file masks en.json

@mattab mattab modified the milestones: 3.0.0-b5, 3.0.0-b4 Dec 1, 2016
@mattab
Copy link
Member

mattab commented Dec 1, 2016

Also maybe we could add an integration test that fails when a % sign isn't escaped in english translation?

@sgiehl
Copy link
Member Author

sgiehl commented Dec 1, 2016

There are already quite a few % escaped in some translations. That already works fine when there are other placeholders being used.

The only reason for escaping all % symbols in translations is to get them translatable in transifex. Transifex won't accept translations if the number of placeholder is changed. As it sometime recognizes % d as placeholder, even if there is a space between, it's impossible to add a translations. %% should work without problems on Transifex.

I'll check and add a little test for that.

@sgiehl
Copy link
Member Author

sgiehl commented Dec 1, 2016

Added a little test. Might maybe fail because of some translations in CustomAlerts plugin, but I would fix those, after this PR was merged

@mattab
Copy link
Member

mattab commented Dec 5, 2016

LGTM, +1 to merge & fix tests

@mattab mattab modified the milestones: 3.0.0-b5, 3.0.0-rc Dec 5, 2016
@sgiehl sgiehl force-pushed the translationescape branch from 4c86ed0 to 407ac6a Compare December 5, 2016 17:14
@sgiehl sgiehl merged commit cbec998 into 3.x-dev Dec 5, 2016
@sgiehl sgiehl deleted the translationescape branch December 5, 2016 17:37
@mattab mattab added the Bug For errors / faults / flaws / inconsistencies etc. label Dec 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For errors / faults / flaws / inconsistencies etc. Needs Review PRs that need a code review
Development

Successfully merging this pull request may close these issues.

I can't translate some fields
2 participants