Skip to content

Commit

Permalink
Simplify procedure for getting strings from loaded files, see #786
Browse files Browse the repository at this point in the history
  • Loading branch information
@samreid
samreid committed Sep 23, 2019
1 parent 8ddfb08 commit 051c1d1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 63 deletions.
51 changes: 17 additions & 34 deletions js/common/ChipperStringUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,50 +146,33 @@
},

/**
* Given a key, get the appropriate string from the "map" object. This method is called during requirejs mode from
* the string plugin and during the build via CHIPPER/getStringMap, which adds the REPO prefix to all string keys
* in the map. This method supports recursing through keys that support string nesting. This method was created to
* support nested string keys in https://github.com/phetsims/rosetta/issues/193
* Given a key, get the appropriate string from the "map" object, or null if the key does not appear in the map.
* This method is called during requirejs mode from the string plugin and during the build via CHIPPER/getStringMap.
* This method supports recursing through keys that support string nesting. This method was created to support
* nested string keys in https://github.com/phetsims/rosetta/issues/193
* @param {Object.<string, Object|{value: string}>} map - where an "intermediate" Object should hold nested strings
* @param {string} key - like `friction.title` or `FRICTION/friction.title` or using nesting like `a11y.nested.string.here`
* @returns {string} - the string value of the key
* @param {string} key - like `FRICTION/friction.title` or using nesting like `a11y.nested.string.here`
* @returns {string|null} - the string value of the key, or null if the key does not appear in the map
* @throws {Error} - if the key doesn't hold a string value in the map
* @public
*/
getStringFromMap( map, key ) {

// Case where the map is "flat," and the key holds an object with the string in the "value" key, i.e. "friction.title": { "value": "Friction" },
if ( map[ key ] !== undefined ) {
if ( map[ key ].value === undefined ) {
throw new Error( `no value entry for string key: ${key}` );
}
return map[ key ].value;
}

// If not directly accessing the top level of the map, then treat the map like a nested object
// Test for the presence of the namespace prefix, as that will dictate how the string is accessed
const dividerMatches = key.match( new RegExp( NAMESPACE_PREFIX_DIVIDER, 'g' ) );
if ( dividerMatches && dividerMatches.length > 1 ) {
throw new Error( `more forward slashes than expected in key: ${key}` );
}

// In this case the map and key have a namespace prefix, but the map is using nested objects, so the following are
// needed:
// Take the string key `FRICTION/a11y.temperature.hotter
// The map looks like `FRICTION/a11y: { temperature: { hotter: { value: '. . .' } } }`
// So we need to peel off the first key
if ( key.indexOf( NAMESPACE_PREFIX_DIVIDER ) >= 0 ) {
const keyParts = key.split( '.' );
const firstKeyPart = keyParts.splice( 0, 1 );
map = map[ firstKeyPart ];
key = keyParts.join( '.' ); // this no longer has the first key in it
throw new Error( 'getStringFromMap key should not have REPO/' );
}

const string = _.at( map, key )[ 0 ];
if ( string === undefined || string.value === undefined ) {
throw new Error( `no entry for nested string key: ${key}` );
// Lodash gives precedence to "key1.key2" over "key1:{key2}", so we do too.
const result = _.at( map, key )[ 0 ];
if ( result ) {
if ( result.value === undefined ) {
throw new Error( `no value for string: ${key}` );
}
return result.value;
}
return string.value;

// They key does not appear in the map
return null;
},

/**
Expand Down
33 changes: 10 additions & 23 deletions js/grunt/getStringMap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ module.exports = function( locales, phetLibs ) {

// Get metadata of repositories that we want to load strings from (that were referenced in the sim)
const stringRepositories = []; // { name: {string}, path: {string}, requirejsNamespace: {string} }

// TODO: Use map/unique to get repository names
for ( const stringKey in global.phet.chipper.strings ) {
const repositoryName = global.phet.chipper.strings[ stringKey ].repositoryName;

Expand Down Expand Up @@ -78,17 +80,11 @@ module.exports = function( locales, phetLibs ) {
grunt.log.debug( `missing string file: ${stringsFilename}` );
fileContents = {};
}
const fileMap = stringFilesContents[ repository.name ][ locale ] = {};

// Format the string values
ChipperStringUtils.formatStringValues( fileContents, isRTL );

// Loop through all top level keys and add the repo prefix, since fileMap is cross repo for all the sim's strings.
for ( const stringKeyMissingPrefix in fileContents ) {

// Add the requirejs namespaces (eg, JOIST) to the key
fileMap[ repository.requirejsNamespace + '/' + stringKeyMissingPrefix ] = fileContents[ stringKeyMissingPrefix ];
}
stringFilesContents[ repository.name ][ locale ] = fileContents;
} );
} );

Expand All @@ -100,24 +96,15 @@ module.exports = function( locales, phetLibs ) {
stringMap[ locale ] = {};

for ( const stringKey in global.phet.chipper.strings ) {
const repositoryName = global.phet.chipper.strings[ stringKey ].repositoryName;

// English fallback
const fallbackStringFileContents = stringFilesContents[ repositoryName ][ fallbackLocale ];

// This method will recurse if needed to find nested string values as well.
stringMap[ locale ][ stringKey ] = ChipperStringUtils.getStringFromMap( fallbackStringFileContents, stringKey );
const stringMetadata = global.phet.chipper.strings[ stringKey ];
const repositoryName = stringMetadata.repositoryName;
const key = stringMetadata.key;

// Extract 'value' field from non-fallback (babel) strings file, and overwrites the default if available.
if ( locale !== fallbackLocale &&
stringFilesContents[ repositoryName ] &&
stringFilesContents[ repositoryName ][ locale ] &&
stringFilesContents[ repositoryName ][ locale ][ stringKey ] &&

// if the string in rosetta is empty we want to use the fallback english string
stringFilesContents[ repositoryName ][ locale ][ stringKey ].value.length > 0 ) {
stringMap[ locale ][ stringKey ] = ChipperStringUtils.getStringFromMap( stringFilesContents[ repositoryName ][ locale ], stringKey );
}
const value = ChipperStringUtils.getStringFromMap( stringFilesContents[ repositoryName ][ locale ], key ) ||
ChipperStringUtils.getStringFromMap( stringFilesContents[ repositoryName ][ fallbackLocale ], key );

stringMap[ locale ][ stringKey ] = value;
}
} );

Expand Down
15 changes: 9 additions & 6 deletions js/requirejs-plugins/string.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,16 @@ define( require => {
};

/**
*
* @param {function} onload see doc for load()
* Only called in requirejs mode
* @param {Object} fileContents - loaded from a string file
* @param {string} key - the key of the string to be loaded
* @param {string} fileURL - for better error messaging
*/
const getStringFromFileContents = ( fileContents, key ) => {
return window.phet.chipper.mapString( ChipperStringUtils.getStringFromMap( fileContents, key ), stringTest );
const stringFromMap = ChipperStringUtils.getStringFromMap( fileContents, key );
if ( stringFromMap === null ) {
throw new Error( `String not found: ${key}` );
}
return window.phet.chipper.mapString( stringFromMap, stringTest );
};

return {
Expand Down Expand Up @@ -139,7 +141,8 @@ define( require => {
requirejsNamespace: requirejsNamespace, // 'SOME_SIM'
requirePath: requirePath, // '/Users/something/phet/git/some-sim/js'
repositoryPath: repositoryPath, // '/Users/something/phet/git/some-sim'
repositoryName: repositoryName // 'some-sim'
repositoryName: repositoryName, // 'some-sim'
key: key // 'string.title
};

// tell require.js we're done processing
Expand Down Expand Up @@ -184,7 +187,7 @@ define( require => {
getWithCache( localeSpecificPath, parsed => {

// Combine the primary and fallback strings into one object hash.
const parsedStrings = _.extend( parsedFallbackStrings, parsed );
const parsedStrings = _.extend( {}, parsedFallbackStrings, parsed );
onload( getStringFromFileContents( parsedStrings, key ) );
},

Expand Down

0 comments on commit 051c1d1

Please sign in to comment.