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

[Translation] Added unescaping of ids in PoFileLoader #11238

Closed
wants to merge 2 commits into from
Closed

[Translation] Added unescaping of ids in PoFileLoader #11238

wants to merge 2 commits into from

Conversation

JustBlackBird
Copy link
Contributor

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Although it is not directly described in gettext docs, msgid should be unescaped too. The other reason to unescape msgid is symmetry between PoFileLoader and PoFileDumper. The dumper escapes both msgid and msgstr values, but the loader unescapes only msgstr.

@jakzal
Copy link
Contributor

jakzal commented Jul 1, 2014

👍

@JustBlackBird
Copy link
Contributor Author

May be i should add tests for \n characters too?

@jakzal
Copy link
Contributor

jakzal commented Jul 1, 2014

@JustBlackBird quotes are fine. I'd rather add tests for plurals (see my PR, but use quotes like in your case).

@jakzal
Copy link
Contributor

jakzal commented Jul 1, 2014

Since this is a bug fix, it should be merged into 2.3.

@JustBlackBird
Copy link
Contributor Author

@jakzal I'll add tests for plurals. Should I close this PR and create a new one to 2.3 branch?

@jakzal
Copy link
Contributor

jakzal commented Jul 1, 2014

@JustBlackBird no need for closing. I only left the message as a reminder. We can still merge it to 2.3.

@stof
Copy link
Member

stof commented Jul 3, 2014

👍

1 similar comment
@fabpot
Copy link
Member

fabpot commented Jul 4, 2014

👍

@fabpot
Copy link
Member

fabpot commented Jul 4, 2014

Thank you @JustBlackBird.

fabpot added a commit that referenced this pull request Jul 4, 2014
…tBlackBird)

This PR was submitted for the 2.5 branch but it was merged into the 2.3 branch instead (closes #11238).

Discussion
----------

[Translation] Added unescaping of ids in PoFileLoader

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | n/a
| License       | MIT
| Doc PR        | n/a

Although it is not directly described in gettext docs, _msgid_ should be unescaped too. The other reason to unescape _msgid_ is symmetry between ```PoFileLoader``` and ```PoFileDumper```. The dumper escapes both _msgid_ and _msgstr_ values, but the loader unescapes only _msgstr_.

Commits
-------

816a4a9 [Translation] Added unescaping of ids in PoFileLoader
@fabpot fabpot closed this Jul 4, 2014
@JustBlackBird
Copy link
Contributor Author

You are welcome, @fabpot

@JustBlackBird JustBlackBird deleted the translation-po-loader-unescape-ids branch July 4, 2014 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants