Skip to content

Commit

Permalink
feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny committed Jul 30, 2018
1 parent 32e1857 commit a4475e1
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 8 deletions.
7 changes: 5 additions & 2 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -407,15 +407,18 @@ class Config {
* @return {LH.Config.Settings}
*/
static initSettings(settingsJson = {}, flags) {
// Use locale specified in flags or settings, allowing i18n system to select fallback if needed.

This comment has been minimized.

Copy link
@paulirish

paulirish Jul 30, 2018

Member

If a locale is requested in flags or settings, use it. A typical CLI run will not have one, however lookupLocale will always determine which of our supported locales to use (falling back if necessary).

const locale = i18n.lookupLocale((flags && flags.locale) || settingsJson.locale);

// Fill in missing settings with defaults
const {defaultSettings} = constants;
const settingWithDefaults = merge(deepClone(defaultSettings), settingsJson, true);

// Override any applicable settings with CLI flags
const settingsWithFlags = merge(settingWithDefaults || {}, cleanFlagsForSettings(flags), true);

// Use locale specified in flags or settings, allowing i18n system to select fallback if needed.
settingsWithFlags.locale = i18n.lookupLocale((flags && flags.locale) || settingsJson.locale);
// Locale is special and comes only from flags/settings
settingsWithFlags.locale = locale;

return settingsWithFlags;
}
Expand Down
13 changes: 8 additions & 5 deletions lighthouse-core/lib/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,17 @@ const formats = {
};

/**
* Look up the best available locale for the requested language, falling back by
* looking at progressively shorter prefixes (`de-CH-1996` -> `de-CH` -> `de`),
* and finally returning the default locale ('en-US') if no match is found.
* Look up the best available locale for the requested language through these fall backs:
* - exact match
* - progressively shorter prefixes (`de-CH-1996` -> `de-CH` -> `de`)
* - the default locale ('en-US') if no match is found
*
* If `locale` isn't provided, the default is used.
* @param {string=} locale
* @return {LH.Locale}
*/
function lookupLocale(locale = MessageFormat.defaultLocale) {
// TODO: could do more work to sniff out defaultLocale
function lookupLocale(locale) {
// TODO: could do more work to sniff out default locale
const canonicalLocale = Intl.getCanonicalLocales(locale)[0];

const closestLocale = lookupClosestLocale(canonicalLocale, LOCALES);
Expand Down
2 changes: 1 addition & 1 deletion typings/externs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ declare global {
// Augment Intl to include
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl/getCanonicalLocales
namespace Intl {
var getCanonicalLocales: (locales: string | Array<string>) => Array<string>;
var getCanonicalLocales: (locales?: string | Array<string>) => Array<string>;
}

/** Make properties K in T optional. */
Expand Down

0 comments on commit a4475e1

Please sign in to comment.