[Android] Auto-generate API 14 resources from the existing API 17 resources.

build/android/gyp/mirror_resources.py

+#!/usr/bin/env python
+#
+# Copyright 2013 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.
+
+import optparse
+import os
+import re
+import sys
+import xml.etree.cElementTree as ET

[newt (away), 2013/04/26 18:46:26]

remove this

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+import xml.dom.minidom as minidom
+
+from util import build_utils
+
+
+ATTRIBUTE_NAMESPACE = u'http://schemas.android.com/apk/res/android'
+
+# The list of the attributes were taken from go/android-rtl
+ATTRIBUTES_TO_MAP = {'paddingStart' : 'paddingLeft',
+                     'drawableStart' : 'drawableLeft',
+                     'layout_alignStart' : 'layout_alignLeft',
+                     'layout_marginStart' : 'layout_marginLeft',
+                     'layout_alignParentStart' : 'layout_alignParentLeft',
+                     'layout_toStartOf' : 'layout_toLeftOf',
+                     'paddingEnd' : 'paddingRight',
+                     'drawableEnd' : 'drawableRight',
+                     'layout_alignEnd' : 'layout_alignRight',
+                     'layout_marginEnd' : 'layout_marginRight',
+                     'layout_alignParentEnd' : 'layout_alignParentRight',
+                     'layout_toEndOf' : 'layout_toRightOf'}
+
+ATTRIBUTES_TO_MAP = {(ATTRIBUTE_NAMESPACE,  k) : (ATTRIBUTE_NAMESPACE , v)

[newt (away), 2013/04/26 18:46:26]

dict comprehensions are python 2.7+.  not sure if that's allowed in Chromium

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+                    for k, v in ATTRIBUTES_TO_MAP.items()}
+
+GRAVITY_ATTRIBUTES = ('gravity', 'layout_gravity')
+
+GRAVITY_ATTRIBUTES = [(ATTRIBUTE_NAMESPACE, i) for i in GRAVITY_ATTRIBUTES]
+
+
+def iterate_xml_elements(node):
+  """minidom helper function that iterates all the element nodes. Depth-first.
+  """
+  if node.nodeType == node.ELEMENT_NODE:
+    yield node
+  for child_node in node.childNodes:
+    for child_node_element in iterate_xml_elements(child_node):
+      yield child_node_element
+
+
+def generate_pre_v17_resource(input_filename, output_filename):
+  """Convert resource to API 14 compatible resource.
+
+  It's mostly a simple replacement, s/Start/Left s/End/Right,
+  on the attribute names.
+  """
+  dom = minidom.parse(input_filename)
+
+  for element in iterate_xml_elements(dom):
+    all_names = element.attributes.keysNS()
+
+    # Iterate all the attributes to find attributes to convert.
+    # Note that name variable is actually a tuple that has namespace and name.
+    # For example,
+    # name == ('http://schemas.android.com/apk/res/android', 'paddingStart')
+    for name, value in list(element.attributes.itemsNS()):
+      # For gravity attributes, replace start/end to left/right.
+      # For example, gravity="start" to gravity="end"
+      # This is actually not necessary because start/end is backward-compatible.

[newt (away), 2013/04/26 18:46:26]

then why do it? :)

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

yeah.. actually I was on the edge, because it didn't matter; but I guess less
code is better.

+      if name in GRAVITY_ATTRIBUTES:
+        replaced_value = re.sub('start|end',
+            lambda x: 'left' if x.group(0) == 'start' else 'right', value)
+        element.setAttributeNS(name[0], name[1], replaced_value)
+      # Convert any other API 17 Start/End attributes to Left/Right attributes.
+      # For example, from paddingStart="10dp" to paddingLeft="10dp"
+      elif name in ATTRIBUTES_TO_MAP.keys():

[newt (away), 2013/04/26 18:46:26]

don't need ".keys()"

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+        mapped_name = ATTRIBUTES_TO_MAP[name]
+
+        # If, for example, paddingLeft and paddingStart are used
+        # but have different values, then it is definitely not right.
+        if mapped_name in all_names:
+          if value != element.getAttributeNS(*mapped_name):

[newt (away), 2013/04/26 18:46:26]

I'd skip this check.  We can just make it a rule that paddingLeft and
paddingStart should never both be defined.

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+            error_message = '[ERROR]' + input_filename + '\n'

[newt (away), 2013/04/26 18:46:26]

I'd write the error like this:

Error: some_layout.xml: These attributes are...

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done. (Removed)

+            error_message += ('These Attributes are defined'
+                             'but the values are different.\n')
+            error_message += name[1] + ' = '
+            error_message += element.getAttributeNS(*name) + '\n'
+            error_message += mapped_name[1] + ' = '
+            error_message += element.getAttributeNS(*mapped_name) + '\n'
+            print >> sys.stderr, error_message
+            sys.exit(1)
+
+        # Add the new mapped attribute and remove the original attribute.
+        # For example, add paddingLeft and remove paddingStart.
+        element.setAttributeNS(mapped_name[0], mapped_name[1], value)
+        del element.attributes[name]
+

[newt (away), 2013/04/26 18:46:26]

I think it makes sense to check for non-RTL-friendly attributes and emit a
warning or error if they're found.

    elif name in ATTRIBUTES_TO_MAP.values():
        # TODO(kkimlabs): enable this warning once layouts have been converted
        #print >> sys.stderror, 'Warning: layout should use xxx instead of yyy'
        pass

It's possible (though I think unlikely) that we'll have some super fancy layout
in the future where we need to specify paddingLeft for some reason. If that
happens, we can change the warning system then. The extremely likely case,
however, is that someone in unaware of the RTL issues, and uses paddingLeft
because that's what we've always done.

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+  build_utils.MakeDirectory(os.path.dirname(output_filename))
+  dom.writexml(open(output_filename,"w"), '  ', '\n')
+
+
+def mirror_resources_in_dir(input_dir, output_dir):
+  """Convert resources to API 14 compatible resources in the directory
+  recursively.
+  """
+  for root, dirs, files in os.walk(input_dir, followlinks=True):

[newt (away), 2013/04/26 18:46:26]

I'd remove followlinks=True. actually, this doesn't need to be recursive at all,
so you could just use

    for name in os.listdir(input_dir):
        if os.isfile(name):
            ...

and get rid of relative_dir

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+    relative_dir = os.path.relpath(root, input_dir)
+    for file_name in files:
+      if file_name.lower().endswith('.xml'):
+        input_filename = os.path.join(input_dir, relative_dir, file_name)
+        output_filename = os.path.join(output_dir, relative_dir, file_name)
+        generate_pre_v17_resource(input_filename, output_filename)
+
+
+def ParseArgs():
+  """Parses command line options.
+
+  Returns:
+    An options object as from optparse.OptionsParser.parse_args()
+  """
+  parser = optparse.OptionParser()
+  parser.add_option('--res-dir',
+                    help='directory containing resources to be mirrored to RTL')

[newt (away), 2013/04/26 18:46:26]

these help messages need updating

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+  parser.add_option('--res-mirrored-dir',
+                    help='directory containing RTL resources to be generated')
+
+  (options, args) = parser.parse_args()
+
+  if args:
+    parser.error('No positional arguments should be given.')
+
+  # Check that required options have been provided.
+  required_options = ('res_dir', 'res_mirrored_dir')
+  build_utils.CheckOptions(options, parser, required=required_options)
+  return options
+
+
+def main(argv):
+  """Convert Android xml resources to API 14 compatible.

[newt (away), 2013/04/26 18:46:26]

I'd mention the issue with Galaxy Note 2 (or whatever it is), and explain when
this script can be removed (once we Galaxy Note 2 market share is 0? once we
drop support for JB MR0?)

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+
+  """
+  options = ParseArgs()
+
+  res_sub_dir_names = os.walk(options.res_dir).next()[1]

[newt (away), 2013/04/26 18:46:26]

add a comment explaining what this non-obvious call does

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done. replaced to listdir(...)

+
+  for res_sub_dir_name in res_sub_dir_names:
+    dir_pieces = res_sub_dir_name.split('-')
+    resource_type = dir_pieces[0]
+    qualifiers = dir_pieces[1:]
+

[newt (away), 2013/04/26 18:46:26]

only one blank line here

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+
+    # We only convert resources under layout*/ and xml*/.
+    if resource_type in ('layout', 'xml'):
+      res_sub_dir = os.path.join(options.res_dir, res_sub_dir_name)
+      to_path = os.path.join(options.res_mirrored_dir,
+                             res_sub_dir_name + '-v17')
+
+      # Android pre-v17 API doesn't support RTL. Skip.
+      if 'ldrtl' in qualifiers:

[newt (away), 2013/04/26 18:46:26]

move this to the top of the if block

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done. (I just made the above if resource_type... to call continue instead)

+        continue
+
+      # Convert all the resource files.
+      mirror_resources_in_dir(res_sub_dir,
+                              os.path.join(options.res_mirrored_dir,

[newt (away), 2013/04/26 18:46:26]

maybe assign this to a variable and pass in the variable name, to make it
clearer what this argument represents

[Kibeom Kim (inactive), 2013/04/26 19:37:44]

Done.

+                                           res_sub_dir_name))
+
+
+if __name__ == '__main__':
+  sys.exit(main(sys.argv))
+