GritResourceMap: output sorted by keys

grit/format/resource_map.py

 #!/usr/bin/env python
 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 '''This file contains item formatters for resource_map_header and
 resource_map_source files.  A resource map is a mapping between resource names
-(string) and the internal resource ID.'''
+(string) and the internal resource ID. It is output with an alphabetical order
+of resource names.'''
 
 import os
 from functools import partial
 
 from grit import util
 
 
 def GetFormatter(type):
   if type == 'resource_map_header':
     return _FormatHeader
@@ skipping 82 matching lines @@
 const size_t %(map_name)sSize = arraysize(%(map_name)s);
 ''' % { 'map_name': GetMapName(root) }
 
 
 def _FormatSource(get_key, root, lang, output_dir):
   from grit.format import rc_header
   from grit.node import include, structure, message
   yield _FormatSourceHeader(root)
   tids = rc_header.GetIds(root)
   seen = set()
+  items = []
   active_descendants = [item for item in root.ActiveDescendants()]
   output_all_resource_defines = root.ShouldOutputAllResourceDefines()
   for item in root:
     if not item.IsResourceMapSource():
       continue
+    items.append(item)

[flackr, 2015/08/27 22:03:15]

nit: Prefer list comprehension over looping to build lists:
e.g.
items = sorted([item for item in root if not item.IsResourceMapSource()],
key=get_key)

+  items = sorted(items, key=get_key)
+  for item in items:
     key = get_key(item)
     tid = item.attrs['name']
     if tid not in tids or key in seen:

[flackr, 2015/08/27 22:03:15]

now that we're sorted by key we should just compare against the last key rather
than build a set of seen keys.

As an aside, skipping duplicate keys where key is defined by the get_key
function (in our cases to be either item name or path) seems dangerous. I'd
prefer that we either output duplicate entries (Which seems to be the case for
some resourecs: https://code.google.com/p/grit-i18n/issues/detail?id=27 ) or
raise an exception to enforce that you can't generate a lossy map file. I filed
https://code.google.com/p/grit-i18n/issues/detail?id=33 for this and it can be
done later.

       continue
     seen.add(key)
     if item.GeneratesResourceMapEntry(output_all_resource_defines,
                                       item in active_descendants):

[flackr, 2015/08/27 22:03:15]

nit: looks like active_descendants should be a set as I'm pretty sure we can't
answer in for a python list efficiently.

       yield '  {"%s", %s},\n' % (key, tid)
   yield _FormatSourceFooter(root)
 
 
 def _GetItemName(item):
   return item.attrs['name']
 
 
 def _GetItemPath(item):
   return item.GetInputPath().replace("\\", "/")