martinandert / counterpart

A translation and localization library for Node.js and the browser.
MIT License
242 stars 25 forks source link

Allow trailing dot in translation keys #28

Closed yanndinendal closed 8 years ago

yanndinendal commented 8 years ago

We use gettext-style translation keys (the translation key is the full text in the source lang), but prefixed with the default Counterpart's separator (.), so we encountered a bug when a translation key is a sentence that ends with a dot. The translation is never found.

As I saw in the code that it might have been intentional to treat the last separator as a real superfluous separator, this PR implements this with an option. But if you are ok to always keep the last dot (or other separator) as being part of the key, I can remove that setting (_registry.keepTrailingDot).

If you want to keep this as a setting and are ok with this PR, I can add a getter and a setter like you do for other _registry params. Also, I'm open to other suggestions for that param's name.

I added a test, but I was not sure if it was the right place, tell me if it's ok like this.

yanndinendal commented 8 years ago

I added support for $ npm test, because it took me a while to find how to launch the tests as I didn't see the Makefile. ^^

martinandert commented 8 years ago

@yanndinendal I'm happy to accept this but could you add another assertion to your test for when the key looks like foo..bar?

yanndinendal commented 8 years ago

Thanks @martinandert. :)

I wrote the tests, but I'm not sure that's the expected result: a70bba1.

I will try to only keep the last trailing dot in a new patch.

yanndinendal commented 8 years ago

@martinandert : ok, now that looks better: 2a9fb3f. :)

What do you think?

yanndinendal commented 8 years ago

And the final squashed commit: 891cc57.

martinandert commented 8 years ago

Thanks! I've just released a new patch version. Enjoy!

yanndinendal commented 8 years ago

@martinandert : Great! Thanks. :)