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

i18n: add es-419 #9171

Merged
merged 5 commits into from
Jun 12, 2019
Merged

i18n: add es-419 #9171

merged 5 commits into from
Jun 12, 2019

Conversation

exterkamp
Copy link
Member

Summary
import es-419 and a pt correction.

patrickhulce
patrickhulce previously approved these changes Jun 10, 2019
Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

for everyone else not sure what es-419 is, it's "Latin American Spanish" :)

http://www.localeplanet.com/icu/es-419/index.html

@exterkamp
Copy link
Member Author

exterkamp commented Jun 10, 2019

I've never added a new locale! Didn't know to add it to the locale.js list & externs! Should be g2g now, --locale=es-419 is different than --locale=es.

@patrickhulce
Copy link
Collaborator

Didn't know to add it to the locale.js list & externs

Is there a way to edit my LGTM to make it look like I didn't forget that too? 😆

@@ -40,6 +40,7 @@ const locales = {
'el': require('./locales/el.json'),
'en-XA': require('./locales/en-XA.json'), // psuedolocalization
'es': require('./locales/es.json'),
'es-419': require('./locales/es-419.json'),
Copy link
Member

@paulirish paulirish Jun 11, 2019

Choose a reason for hiding this comment

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

much like en-gb, it appears we have to account for the parent/child locales ourselves. Why?? I have no idea.

But here's the list of es-419 child locales: es-AR, es-BO, es-BR, es-BZ, es-CL, es-CO, es-CR, es-CU, es-DO, es-EC, es-GT, es-HN, es-MX, es-NI, es-PA, es-PE, es-PR, es-PY, es-SV, es-US, es-UY, es-VE

Source in the CLDR: https://raw.githubusercontent.com/unicode-cldr/cldr-core/master/supplemental/parentLocales.json

( And tc39/ecma402#87 isn't a thing yet.)

Copy link
Member

Choose a reason for hiding this comment

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

I know this because I started writing this patch yesterday, before I saw your PR.

But where I went is a dark dark place.... (syntax-wise, at least)

@@ -14,6 +14,9 @@
 
 /** @typedef {Record<string, {message: string}>} LocaleMessages */
 
+const es419 = require('./locales/es-419.json');
+const es419locales = 'es-AR, es-BO, es-BR, es-BZ, es-CL, es-CO, es-CR, es-CU, es-DO, es-EC, es-GT, es-HN, es-MX, es-NI, es-PA, es-PE, es-PR, es-PY, es-SV, es-US, es-UY, es-VE';
+
 // The keys within this const must exactly match the LH.Locale type in externs.d.ts
 /** @type {Record<LH.Locale, LocaleMessages>} */
 const locales = {
@@ -28,6 +31,9 @@ const locales = {
   'en-ZA': require('./locales/en-GB.json'), // Alias of 'en-GB'
   'en-IN': require('./locales/en-GB.json'), // Alias of 'en-GB'
 
+  // https://raw.githubusercontent.com/unicode-cldr/cldr-core/master/supplemental/parentLocales.json
+  ...Object.fromEntries(es419locales.split(', ').map(locale => [locale, es419])),
+
   // All locales from here have a messages file, though we allow fallback to the base locale when the files are identical
   'ar-XB': require('./locales/ar-XB.json'), // psuedolocalization
   'ar': require('./locales/ar.json'),

🤢

Copy link
Collaborator

Choose a reason for hiding this comment

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

i like it

Copy link
Member

Choose a reason for hiding this comment

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

no fromEntries yet (Node 10)

Copy link
Member

Choose a reason for hiding this comment

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

yah fromEntries ended up being a pain in the ass. Node 10 and TSC both being real cranky about it. Not worth it.

Copy link
Member Author

Choose a reason for hiding this comment

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

TL;DR fancy js is fancy, but fancy regex replacements is also fancy, and much easier.

@patrickhulce patrickhulce dismissed their stale review June 11, 2019 17:03

I don't know what we need for i18n these days :)

@paulirish paulirish changed the title i18n: es-419 i18n: add es-419 Jun 12, 2019
@paulirish paulirish merged commit 6c90731 into master Jun 12, 2019
@paulirish paulirish deleted the import-i18n-now branch June 12, 2019 15:40
@exterkamp
Copy link
Member Author

Tagging as part of: #7238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants