Skip to content

Commit

Permalink
add a11y nested strings support to reportUnusedStrings, #780
Browse files Browse the repository at this point in the history
  • Loading branch information
zepumph committed Jul 23, 2019
1 parent e504e09 commit 2083eb8
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 7 deletions.
46 changes: 43 additions & 3 deletions js/common/ChipperStringUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
/**
* String utilities used throughout chipper.
*
* @Chris Malley (PixelZoom, Inc.)
* @author Chris Malley (PixelZoom, Inc.)
* @author Michael Kauzmann (PhET Interactive Simulations)
*/

/* eslint-env browser, node */
Expand Down Expand Up @@ -64,7 +65,7 @@
return str.replace( new RegExp( find.replace( /[-\/\\^$*+?.()|[\]{}]/g, '\\$&' ), 'g' ), replaceWith );
},

//TODO chipper#316 determine why this behaves differently than str.replace for some cases (eg, 'MAIN_INLINE_JAVASCRIPT')
// TODO chipper#316 determine why this behaves differently than str.replace for some cases (eg, 'MAIN_INLINE_JAVASCRIPT')
/**
* Replaces the first occurrence of {string} find with {string} replaceWith in {string} str
*
Expand Down Expand Up @@ -122,7 +123,7 @@

/**
* Recurse through a string file and format each string value appropriately
* @param {Object.<string, intermediary:Object|{value:string}>} stringObject - if "intermediary", then recurse to
* @param {Object.<string, intermediary:Object|{value:string}>} stringsMap - if "intermediary", then recurse to
* find more nested keys
* @param {boolean} isRTL - is right to left language
*/
Expand Down Expand Up @@ -216,6 +217,45 @@
// It would be really strange for there to be no fallback for a certain string, that means it exists in the translation but not the original English
throw new Error( `no entry for string key: ${key}` );
}
},

/**
* Call a function on each string object in a string map. Recursively dive into each object that doesn't have a
* `value` to find nested string objects too.
* @param {Object.<string, Object|{value:string}>} map - string map, like a loaded JSON strings file
* @param {function(key:string, {value:string})} func
* @public
*/
forEachString( map, func ) {
forEachStringImplementation( '', map, func );
},

/**
* @type {string}
* @pubic
*/
A11Y_STRING_KEY_NAME: A11Y_STRING_KEY_NAME
};

/**
* This implementation function helps keep a better api for `forEachString`.
* @param {string} keySoFar - as we recurse down, build up a string of the key separated with dots.
* @param {Object} map - string key map
* @param {function(key:string, {value:string})} func
*/
const forEachStringImplementation = ( keySoFar, map, func ) => {
for ( const key in map ) {
if ( map.hasOwnProperty( key ) ) {
const nextKey = keySoFar ? `${keySoFar}.${key}` : key; // don't start with period
if ( map[ key ].value ) {
func( nextKey, map[ key ] );
}
else {

// recurse to the next level since this wasn't the `value` key
forEachStringImplementation( nextKey, map[ key ], func );
}
}
}
};

Expand Down
33 changes: 29 additions & 4 deletions js/grunt/reportUnusedStrings.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
'use strict';

const grunt = require( 'grunt' );
const ChipperStringUtils = require( '../common/ChipperStringUtils' );

/**
* @param {string} repo
Expand All @@ -28,17 +29,41 @@ module.exports = function( repo, requirejsNamespace ) {
// iterate over the strings
for ( const key in jsStrings ) {

if ( jsStrings.hasOwnProperty( key ) && key !== 'a11y' ) {
if ( jsStrings.hasOwnProperty( key ) ) {

const string = jsStrings[ key ].value;
const requireStringKey = requirejsNamespace + '/' + key;

// global.phet.chipper.strings is initialized by the string plugin
const chipperStrings = global.phet.chipper.strings || {};

// If this string was not added to the global chipperStrings, it was not required in the sim
if ( !chipperStrings.hasOwnProperty( requireStringKey ) ) {
grunt.log.warn( `Unused string: key=${requireStringKey}, value=${string}` );
/**
* Warn if the string is not used.
* @param {string} fullKey - with the `REPO/` included
* @param {string} key - just the key, no `REPO/`
* @param {string} value - the value of the string
*/
const warnIfStringUnused = ( fullKey, key, value ) => {

// If this string was not added to the global chipperStrings, it was not required in the sim
if ( !chipperStrings.hasOwnProperty( fullKey ) ) {
grunt.log.warn( `Unused string: key=${key}, value=${value}` );
}
};

// for top level strings
warnIfStringUnused( requireStringKey, key, string );

// support nesting into a11y strings
if ( key === ChipperStringUtils.A11Y_STRING_KEY_NAME ) {

const a11yStrings = jsStrings[ key ];

ChipperStringUtils.forEachString( a11yStrings, ( a11ySubKey, stringObject ) => {
const keyWithRepo = `${requireStringKey}.${a11ySubKey}`;
const fullKeyNoRepo = `${ChipperStringUtils.A11Y_STRING_KEY_NAME}.${a11ySubKey}`;
warnIfStringUnused( keyWithRepo, fullKeyNoRepo, stringObject.value );
} );
}
}
}
Expand Down

0 comments on commit 2083eb8

Please sign in to comment.