quipper / i18n-dts

A d.ts file generator for i18n-js.
Apache License 2.0
48 stars 6 forks source link

Pluralization support #15

Closed JoelBesada closed 4 years ago

JoelBesada commented 4 years ago

Add support for strings that use the pluralization feature of i18n-js. From this input:

{
  "common": {
    "items": {
      "one": "One item.",
      "other": "{{count}} items."
  }
}

We don't want to interpret this as two strings "common.items.one" and "common.items.other" as it does currently, but rather collapse them into one string "common.items" with a count parameter, returning either "One item." or "{{count}} items.".

To do this, we assume that a key that contains an object with only keys "zero", "one" or "other" to be in pluralized form. This might however end up being a breaking change if someone was previously using this library and not expecting these keys to be collapsed into the parent as they will with this PR.

We needed to add this feature to make this library fit our needs, and it's been working out great. Happy to hear if you'll consider merging this so we can drop our fork. :)

hotchemi commented 4 years ago

let me take a look, thank you!

hotchemi commented 4 years ago

@yuya-takeyama @christineoo sorry for mentioning, could you take a look when you have time(and merge)? 🙇 I don't have ownership anymore.

JoelBesada commented 4 years ago

Btw just found i18n-js supports some exceptional lang rule(like Russian). presumably it'd be great if we can support it as well(haven't thought so much) 😉

I'll see if I can add a CLI flag to override the PLURALIZATION_KEYS value, that way you should be able to customize according to any extra rules you've added yourself.

hotchemi commented 4 years ago

@JoelBesada thank you so much! Btw, the current situation is the library has not been used so much internally(and I've already left Quipper and it nearly means there's no active maintainer in the company). Considering that situation if you're using the library actively you can just fork and continue to maintain/develop😉 Then we'll just archive the repo and update README to deprecate it.

JoelBesada commented 4 years ago

@JoelBesada thank you so much! Btw, the current situation is the library has not been used so much internally(and I've already left Quipper and it nearly means there's no active maintainer in the company). Considering that situation if you're using the library actively you can just fork and continue to maintain/develop😉 Then we'll just archive the repo and update README to deprecate it.

I've updated the PR adding in a CLI option to override the pluralization keys.

Regarding taking over the active development of this library through our fork, I'm not sure we're going to continue adding much more after these changes, which would mean our fork wouldn't be that actively maintained either. I appreciate the offer, and we'll reconsider it again if we find ourselves making more additions to the library, but for now I think it would be best to just have this update merged and pushed as a new version to NPM within this repo if possible.

hotchemi commented 4 years ago

Thank you, let us take a look.

yuya-takeyama commented 4 years ago

@JoelBesada Your change has been released as 0.3.0. https://www.npmjs.com/package/i18n-dts/v/0.3.0

Thanks for your contribution!