zaach / jsxgettext

Extract gettext calls from JavaScript, EJS, and other template formats.
Mozilla Public License 2.0
105 stars 56 forks source link

Added extracting plural forms using standard "n"-prefixed gettext functions #92

Closed vrouet closed 9 years ago

vrouet commented 9 years ago

This adds basic plural forms extraction, it suits my need well, it'd be great to have merged. I'm sure there are stuff I didn't think about and that would need fixing before, so please comment on this.

vrouet commented 9 years ago

I've noticed that one test if failing, but I checked on the master version and it does fail as well, so it's not something I broke ;)

BYK commented 9 years ago

First of all this is great! Thank you since we have been wanting this for quite some time now. I have a few requests though:

  1. Please add some tests
  2. Squash your commits
  3. May be put an example usage/expected format somewhere since even I am not very familiar with ngettext usage. Adding tests may cover this.
BYK commented 9 years ago

Fixes #51

vrouet commented 9 years ago

Thanks for your reviews, I will definitely add some tests and doc for that, I made the pull request first basically to see if my approach was fitting your standards, and did rthings quickly because I needed to tel my dev team what to use for i18n on our project. I will update soon

vrouet commented 9 years ago

Ok, I have taken your comments into account, added some notes, integrated the plural functions to all of the relevant test cases, and squashed my commits

BYK commented 9 years ago

And rebase to get tests fixed :)

vrouet commented 9 years ago

Alright, rebase is done. Nice to see the whole test suite passing ;)

BYK commented 9 years ago

Alright, rebase is done. Nice to see the whole test suite passing ;)

Definitely. You triggered me to do it :)

Final thing before merge: fix those %s lines in the README please?

vrouet commented 9 years ago

OK now

BYK commented 9 years ago

aw yiss