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

Write proper plural msgstr[0] #24

Merged
merged 3 commits into from
Feb 10, 2017
Merged

Write proper plural msgstr[0] #24

merged 3 commits into from
Feb 10, 2017

Conversation

rosston
Copy link
Contributor

@rosston rosston commented Feb 10, 2017

Fixes the same problem as #21 (with tests!).

Before this PR, untranslated plurals were written as follows:

msgid "1 destination"
msgid_plural "{{$count}} destinations"
msgstr ""

POEdit is unable to open a file containing such plural entries. And gettext utility msgcat throws the following error when trying to read the file:

zh-CN.po:44:10: syntax error
zh-CN.po:49:10: syntax error
zh-CN.po:54:10: syntax error
zh-CN.po:6337:10: syntax error
msgcat: found 4 fatal errors

My change will generate this result:

msgid "1 destination"
msgid_plural "{{$count}} destinations"
msgstr[0] ""

The gettext docs aren't totally clear on this point, but that does appear to be the correct syntax. And neither msgcat nor POEdit throw any errors when reading that kind of plural entry.


This PR has a couple other mostly-unrelated changes. (Sorry! I normally hate that.)

  • I upgraded all devDependencies because I couldn't even npm install with the previous ones
    • grunt-jscs-checker was renamed to grunt-jscs (jscs-dev/grunt-jscs@033b836)
    • alias option in grunt-browserify doesn't seem necessary anymore? Browserify v3 was 3+ years ago now!
  • I removed lodash.isarray in favor of the native Array.isArray. There was already code depending on Object.keys and Array.prototype.filter (https://github.com/rubenv/pofile/blob/v1.0.2/lib/po.js#L309), which are also ES5-only, so I didn't see any reason to not use Array.isArray.

I would be happy to remove any of those changes if you want a cleaner PR. Just let me know!

Other code was already depending on Object.keys() and
Array.prototype.filter(), which are also ES5 along with
Array.isArray().
@rubenv
Copy link
Owner

rubenv commented Feb 10, 2017

Love it!

I normally hate it as well when people tack on extra changes, but let's admit it: some maintenance was probably overdue (sucks that we have to do it though!).

See no reason not to merge it.

@rubenv rubenv merged commit 52c11f7 into rubenv:master Feb 10, 2017
@rosston
Copy link
Contributor Author

rosston commented Feb 10, 2017

Thanks for the quick merge!

@rubenv
Copy link
Owner

rubenv commented Feb 10, 2017

Published to NPM as v1.0.3. Thanks a lot!

@vslavik
Copy link

vslavik commented Feb 11, 2017

My change will generate this result:

msgid "1 destination"
msgid_plural "{{$count}} destinations"
msgstr[0] ""

Poedit dev here. My understanding of the file format is that this is still incorrect, the number of msgstr[] entries must match the nplurals value from the Plural-Forms header. See POT files generated by GNU gettext's xgettext, they output English Plural-Forms with nplurals=2 and plural entries are always output as

msgid "%d foo"
msgid_plural "%d foos"
msgstr[0] ""
msgstr[1] ""

by it. Poedit tries not to choke on bad inputs, so that alone is not proof of correctness ;)

The gettext docs aren't totally clear on this point

GNU gettext manual is indeed not very good as a PO format reference. It's best to consult "official" GNU gettext tools' output as examples or consult Pology's unofficial, but great PO format reference.

@rubenv
Copy link
Owner

rubenv commented Feb 11, 2017

@vslavik Thanks for weighing in!

@rosston
Copy link
Contributor Author

rosston commented Feb 11, 2017

Thanks @vslavik! After testing, I think you are correct about the number of msgstrs matching the nplurals. Here's what I've come up with:

messages.po file (i.e., POT), generated by xgettext as nplurals=INTEGER:

msgid "1 thing"
msgid_plural "%i things"
msgstr[0] ""
msgstr[1] ""

zh-CN.po file (nplurals=1), after using msgmerge to pull the POT changes in:

msgid "1 thing"
msgid_plural "%i things"
msgstr[0] ""

pl-PL.po file (nplurals=3), after using msgmerge to pull the POT changes in:

msgid "1 thing"
msgid_plural "%i things"
msgstr[0] ""
msgstr[1] ""
msgstr[2] ""

So the reason my PR worked for me was that zh-CN was the only problem language in my project. Whoops. 😊

I'll work on getting another PR soon to output the proper number based on nplurals.

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.

3 participants