Make sure rosetta can read both flat and structured string keys.

[samreid]

From #193, @zepumph and I have been testing flat "a.b" keys and structured "a:{b}" keys. Things have been working in build and requirejs, but we haven't yet tested in rosetta.

This should work because, if we understand correctly, rosetta is not using the english string file at all, which is the only place that now supports structured string keys. A build flattens everything, so perhaps rosetta will just work.

[zepumph]

Tagging @jbphet for this issue. It may be something that he is able to test much easier than us. I think we just want to test publishing a test sim from master with nested strings (perhaps chains?), and then translating it to make sure that things are business as usual.

[zepumph]

Assigning to @jbphet after discussing with him. When he is in rosetta and testing #214, this will likely be done organically. @jbphet please look specifically for places in rosetta that may be reading directly from the en.json files checked in to the sim repos. We didn't think this would be a problem, but it would be good to make sure because right now rosetta doesn't include the logic to get a string from the nested object structure like is in ChipperStringUtils.js

[zepumph]

This logic is now on master, so the burden is on @jbphet to make sure that rosetta can support reading this nesting. Just because the logic has been merged doesn't mean we support it. I will wait for rosetta's side to be finished before committing any nesting to an en json file.

[zepumph]

When I get back to testing this, note that this patch was helpful in preliminary testing of nested objects:

Index: js/chains/view/ChainsView.js
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- js/chains/view/ChainsView.js	(revision 3bbea9fb29df1bbbd73020e6da6b90f92c63322c)
+++ js/chains/view/ChainsView.js	(date 1576001790078)
@@ -11,6 +11,7 @@
   // modules
   const Bounds2 = require( 'DOT/Bounds2' );
   const chains = require( 'CHAINS/chains' );
+  const HBox = require( 'SCENERY/nodes/HBox' );
   const inherit = require( 'PHET_CORE/inherit' );
   const LayoutBox = require( 'SCENERY/nodes/LayoutBox' );
   const MultiLineText = require( 'SCENERY_PHET/MultiLineText' );
@@ -22,6 +23,8 @@
   const Text = require( 'SCENERY/nodes/Text' );
 
   // strings
+  const heightShortString = require( 'string!CHAINS/height.short' );
+  const heightTallString = require( 'string!CHAINS/height.tall' );
   const htmlStringString = require( 'string!CHAINS/htmlString' );
   const multilineStringString = require( 'string!CHAINS/multilineString' );
   const namedPlaceholdersStringString = require( 'string!CHAINS/namedPlaceholdersString' );
@@ -68,6 +71,11 @@
           font: FONT,
           fill: '#990000',
           tandem: tandem.createTandem( 'namedPlaceholdersString' )
+        } ),
+        new HBox( {
+          children: [
+            new Text( heightTallString, { font: FONT } ),
+            new Text( heightShortString, { font: FONT } ) ]
         } )
       ],
       center: this.layoutBounds.center
Index: chains-strings_en.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- chains-strings_en.json	(revision 3bbea9fb29df1bbbd73020e6da6b90f92c63322c)
+++ chains-strings_en.json	(date 1576001718916)
@@ -19,5 +19,13 @@
   },
   "size": {
     "value": "Size"
+  },
+  "height": {
+    "short": {
+      "value": "short"
+    },
+    "tall": {
+      "value": "tall"
+    }
   }
 }
\ No newline at end of file

[jbphet]

@samreid said:

This should work because, if we understand correctly, rosetta is not using the english string file at all

This is incorrect. Rosetta extracts the string keys used by the sim from the live sim itself, but it obtains those values from the GitHub file containing the English strings. That means it will need to dealing with the nested string structure.

I haven't thought about this a ton, but my initial preference would be to keep the key structures the same between the English and translated string files files, rather than "flattening" them in the translations. I can't think of a situation off hand where having the key structure differ would cause immediate problems, but it just feels to me like one of those things that we will regret at some point if we allow them to diverge. For instance, if we someday need to write some tools to navigate through string files, we'd have to handle things differently in the translated files, and may increase the amount of work needed and tools that we'd have to maintain.

[jbphet]

This was discussed in the 12/12/2019 dev meeting, here is a summary:

• There is no objection to storing the translated strings using the hierarchical string keys that are used in the English file. @zepumph had suggested using flattened keys because he thought that Rosetta was pulling the string values from the live sim (instead of just the keys), so the information about what was hierarchical and what wasn't wouldn't be available.
• There is code in chipper that is doing some of the sorting out of flattened versus non-flattened string keys, see ChipperStringUtils.js and search for _.at.
• @zepumph will add an assertion to verify that we don't accidentally create nested and non-nested keys that would overlap.
• This feature - i.e. support for nested string keys - was proposed because it would be a nice organizational improvement to the string files and we were thinking that it would essentially "come along for free" with the work being done to ignore a11y strings in Rosetta should ignore the "a11y" key in strings files #214. This isn't quite true, however, since initially the only thing that needs to be implemented in Rosetta is the ability to ignore such strings, and not interpret them. Given this situation, @zepumph and I decided that we'd look at adding the support after the first of the year and see if it can be done fairly easily (@zepumph is pretty familiar with the problem since he's been adding support in chipper) and, if so, we'll do it. If not, this will wait until we need to add more comprehensive support for the translation of a11y strings to Rosetta.

[jbphet]

In #214 (comment) I describe some testing that was done with nested strings in place. What I found during that testing applies to this issue too, which is that, in its current state, nested strings will be ignored by Rosetta because it won't be able to identify them in the English strings file.

[jbphet]

Deferring until after the first of the year. I have a reminder on my calendar to pick it back up.

[zepumph]

This is tracked in #221, unassigning until later.

[jbphet]

We have had the a11y key in the strings file for a while now and rosetta has been ignoring them, see #214. This seems to work fine, so I believe it is safe to close this issue.

[zepumph]

This isn't wasn't for the a11y substructure, but was for outfitting the non-a11y strings with this same support. For example if we could convert "screen.intro" into:

{ 
  screen: {
    intro: {
      value: "Intro"
    }
}

I would not recommend closing this issue.

[marlitas]

Meeting 12/4
We are deferring until we know more about what we are doing with Fluent.js which is being investigated here: phetsims/joist#992