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

Make pluralization rules configurable #138

Merged
merged 1 commit into from Sep 10, 2019

Conversation

@schleyfox
Copy link
Member

commented Sep 9, 2019

Users may wish to support different locales without requiring a change
or fork to the library. Additionally, some users may prefer slightly
different pluralization rules for some locales.

Follow up to #137

/cc @ljharb @mereskin-zz

Make pluralization rules configurable.
Users may wish to support different locales without requiring a change
or fork to the library. Additionally, some users may prefer slightly
different pluralization rules for some locales.
index.js Show resolved Hide resolved
@schleyfox

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2019

@spikebrehm

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

Don't forget to also update the transformPhrase() usage on line 387

@spikebrehm
Copy link
Member

left a comment

LGTM minus updating the Polyglot.transformPhrase() method at the end of the file.

@schleyfox

This comment has been minimized.

Copy link
Member Author

commented Sep 10, 2019

@spikebrehm

Polyglot.transformPhrase = function transform(phrase, substitutions, locale) {
doesn't support the other polyglot option of tokenRegex, so I don't think making it configurable is within scope.

@spikebrehm

This comment has been minimized.

Copy link
Member

commented Sep 10, 2019

The omission of the tokenRegex argument in that method was probably an oversight that we could fix here, but if you think it's out of scope, then 👍.

@ljharb

This comment has been minimized.

Copy link
Collaborator

commented Sep 10, 2019

It wasn’t an oversight :-) it was intentional to not expose that internal implementation detail, and i think it’s fine to add the overrides later if it’s desired.

@ljharb
ljharb approved these changes Sep 10, 2019

@schleyfox schleyfox merged commit f7c7a96 into master Sep 10, 2019

3 checks passed

Tidelift Dependencies checked
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ljharb ljharb deleted the make-pluralization-rules-configurable branch Sep 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.