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

feat: Allow alternative country name spellings via array #175

Merged

Conversation

danilofuchs
Copy link
Contributor

Closes #174

I added test cases and updated the translation maps for the example in the issue.

countries.getName("US", "pt") // Estados Unidos
countries.getAlpha2Code("Estados Unidos da América", "pt")) // US
countries.getAlpha2Code("Estados Unidos", "pt")) // US
countries.getSimpleAlpha2Code("Estados Unidos da America", "pt")) // US

While building this, I found the code a little hard to follow, using var everywhere. I changed this bits to use const instead (supported since Node 6)

@michaelwittig
Copy link
Owner

LGTM

@michaelwittig michaelwittig merged commit 9fec0a9 into michaelwittig:master May 28, 2020
@michaelwittig
Copy link
Owner

released as [email protected]

@danilofuchs danilofuchs deleted the feat/alternative-names branch May 28, 2020 22:01
@sebastien-prudhomme
Copy link

Hi,

Any reason to break compatibility for getNames function and not getName function?

I had to fix my code with this to have the same result than before:

Object.fromEntries(Object.entries(countries.getNames('en')).map(([key, value]) => [key, Array.isArray(value) ? value[0] : value]))

The new feature is good, but I would prefer to have a new function that get all known translations instead of changing the current one.

@danilofuchs
Copy link
Contributor Author

Hi,

Any reason to break compatibility for getNames function and not getName function?

I had to fix my code with this to have the same result than before:

Object.fromEntries(Object.entries(countries.getNames('en')).map(([key, value]) => [key, Array.isArray(value) ? value[0] : value]))

The new feature is good, but I would prefer to have a new function that get all known translations instead of changing the current one.

I did not realise getNames interface would be changed, sorry about that. I think the user has more possibilities than before by having the names array.

Maybe we could add a options.alternatives optional argument in getName and getNames which either returns an array or the first match.

Either way, create a new issue so we can discuss it apart from this PR.

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.

Allow for multiple possible spellings of names of a country
3 participants