Skip to content
This repository has been archived by the owner on Nov 21, 2024. It is now read-only.

Handle deprecated locales #5

Merged
merged 4 commits into from
May 1, 2018
Merged

Handle deprecated locales #5

merged 4 commits into from
May 1, 2018

Conversation

bsudekum
Copy link

@bsudekum bsudekum commented May 1, 2018

Closes: #4

This handles older, deprecated locales by inspecting the language component of a locale and seeing if it is one of the deprecated locales. If it is, the current locale is returned instead.

/cc @danpaz @1ec5

index.js Outdated
@@ -88,6 +90,14 @@ function parseLocaleIntoCodes (locale) {
};
}

function handleDeprecatedLocales(language) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“Handle” isn’t a useful word to put in a function name. How about getStandardLanguage()?

@@ -0,0 +1,10 @@
module.exports = [{
previously: 'iw',
current: 'he'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn’t need to be an array of objects. A simple object mapping deprecated codes to standard codes would do. If we need to attach more information to each entry in the future, we can use nested objects again. But for now it just makes getStandardLanguage() more complicated and less performant.

@@ -22,6 +25,11 @@ tape('test bestMatchingLocale', function(t) {
t.equal(locale.bestMatchingLocale('es-mx', availableLocales), 'es');
t.equal(locale.bestMatchingLocale('zh-Hans-region', availableLocales), 'zh-Hans');
t.equal(locale.bestMatchingLocale('pt-BR', availableLocales), 'pt-PT');

t.equal(locale.bestMatchingLocale('iw', availableLocales), 'he', 'Test deprecated locale');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test case for a deprecated language plus a country code, such as iw-IL.

@bsudekum
Copy link
Author

bsudekum commented May 1, 2018

@1ec5 @danpaz updated tests.

Copy link

@danpaz danpaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@bsudekum bsudekum merged commit 08d7017 into master May 1, 2018
@bsudekum bsudekum deleted the older-locales branch May 1, 2018 20:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants