1368449 - [compare-locales] change data storage / json output to to show entity data in the order received

Status: RESOLVED FIXED

(Localization Infrastructure and Tools :: compare-locales, enhancement)

Description

[Axel Hecht [:Pike]]

Right now, we store each facet of compare-locales per file in a separate list.

That makes it hard to change things, and also I think it makes our data structure way more complicated than it should be.

I'll need this before tackling bug 1368444 (order entries by file order).

Comment 1

[Axel Hecht [:Pike]]

Attachment: bug 1368449, data storage and iteration,

This changes the data storage from a dict like
{
 'missingEntity': [
  'onekey',
  'zoom'
 ],
 'obsoleteEntity': [
  'more'
 ]
}
to a list like
[
 {'missingEntity': 'onekey'},
 {'obsoleteEntity': 'more'},
 {'missingEntity': 'zoom'}
]

With this patch there is only a single location where we
sort, which is going to be good for bug 1368444.
I had to rip out the use of SequenceMatcher, because that doesn't
persist order of the keys, it always issues "delete" first, and
then "add", regardless whether the added key would come first
or second. And while I was at that, I dropped the whole
construct of yielding the equal key twice. I did that to mimic
SequenceMatcher, and I don't have to. Makes the code easier
to read, too.

Review commit: https://reviewboard.mozilla.org/r/143824/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/143824/

Comment 2

[Axel Hecht [:Pike]]

Matjaz, tackling you with review on this one, as pontoon might become a consumer of the json data at a point.

Files would look like:

{
 "details": {
  "de": {
   "toolkit/chrome": {
    "passwordmgr/passwordmgr.properties": [
     {
      "missingEntity": "userSelectText2"
     }, 
     {
      "obsoleteEntity": "userSelectText"
     }
    ], 
    "mozapps": {
     "downloads/downloads.properties": [
      {
       "obsoleteEntity": "canceled"
      }, 
      {
       "obsoleteEntity": "downloading"
      }, 
      {
       "obsoleteEntity": "downloadsCompleteMsg"
      }, 
      {
       "obsoleteEntity": "downloadsCompleteTitle"
      }, 
      {
       "obsoleteEntity": "failed"
      }, 
      {
       "obsoleteEntity": "finished"
      }, 
      {
       "obsoleteEntity": "notStarted"
      }, 
      {
       "obsoleteEntity": "paused"
      }
     ], 
     "extensions": {
      "extensions.dtd": [
       {
        "missingEntity": "addon.legacy.label"
       }
      ], 
      "extensions.properties": [
       {
        "missingEntity": "details.notification.nonMpcDisabled"
       }, 
       {
        "missingEntity": "details.notification.nonMpcDisabled.link"
       }, 
       {
        "missingEntity": "notification.nonMpcDisabled"
       }, 
       {
        "missingEntity": "notification.nonMpcDisabled.link"
       }
      ]
     }
    }, 
    "global": {
     "config.properties": [
      {
       "missingEntity": "modified"
      }, 
      {
       "obsoleteEntity": "user"
      }
     ], 
     "datetimebox.dtd": [
      {
       "missingFile": "error"
      }
     ], 
     "aboutNetworking.dtd": [
      {
       "missingEntity": "aboutNetworking.http2"
      }, 
      {
       "missingEntity": "aboutNetworking.rcwn"
      }, 
      {
       "missingEntity": "aboutNetworking.rcwnCacheWonCount"
      }, 
      {
       "missingEntity": "aboutNetworking.rcwnNetWonCount"
      }, 
      {
       "missingEntity": "aboutNetworking.rcwnStatus"
      }, 
      {
       "obsoleteEntity": "aboutNetworking.spdy"
      }, 
      {
       "missingEntity": "aboutNetworking.totalNetworkRequests"
      }
     ], 
     "aboutUrlClassifier.properties": [
      {
       "missingFile": "error"
      }
     ], 
     "resetProfile.dtd": [
      {
       "warning": "Referencing unknown entity `brandShorterName` (brandShortName used in context) at line 12, column 0 for refreshProfile.button.label"
      }, 
      {
       "warning": "Referencing unknown entity `brandShorterName` (brandShortName used in context) at line 9, column 0 for refreshProfile.dialog.button.label"
      }, 
      {
       "warning": "Referencing unknown entity `brandShorterName` (brandShortName known) at line 5, column 0 for refreshProfile.dialog.description1"
      }, 
      {
       "warning": "Referencing unknown entity `brandShorterName` (brandShortName used in context) at line 4, column 0 for refreshProfile.dialog.title"
      }, 
      {
       "warning": "Referencing unknown entity `brandShorterName` (brandShortName used in context) at line 11, column 0 for refreshProfile.title"
      }
     ], 
...

Comment 3

[Matjaz Horvat [:mathjazz]]

Comment on attachment 8872337 [details]
bug 1368449, data storage and iteration,

https://reviewboard.mozilla.org/r/143824/#review147656

The current JSON structure actually looks simpler to me. :)

I understand the motivation behind the change is bug 1368444, but I wonder what's the motivation behind that bug.

On the surface, the code makes sense, but I didn't run it. If you want me to dive deeper, I can do that, but it will take some time unless it's urgent.

Comment 4

[Axel Hecht [:Pike]]

Comment on attachment 8872337 [details]
bug 1368449, data storage and iteration,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/143824/diff/1-2/

Comment 5

[Axel Hecht [:Pike]]

https://hg.mozilla.org/l10n/compare-locales/rev/9a8edbd6e4577263427086388502d4a413b21fae, FIXED.

The intent of ordering by file order is to make compare-locales for friendly for tooling that's targeting humans. I'll probably re-use the algorithm for the diff view in elmo, too.