@sgiehl opened this Pull Request on November 27th 2016 Member

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

Currently escaped % weren't always unescaped.

fixes #10877

@mattab commented on December 1st 2016 Owner

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 commented on December 1st 2016 Owner

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

@sgiehl commented on December 1st 2016 Member

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 commented on December 1st 2016 Member

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 commented on December 5th 2016 Owner

LGTM, +1 to merge & fix tests

This Pull Request was closed on December 5th 2016
Powered by GitHub Issue Mirror