Refactor make-file-arrays.py to ease reuse

Source/web/scripts/make-file-arrays.py

 #!/usr/bin/env python
 # Copyright (C) 2012 Google Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions are
 # met:
 #
 #     * Redistributions of source code must retain the above copyright
 # notice, this list of conditions and the following disclaimer.
 #     * Redistributions in binary form must reproduce the above
@@ skipping 18 matching lines @@
 
 # Usage: make-file-arrays.py [--condition=condition-string] --out-h=<header-file-name> --out-cpp=<cpp-file-name> <input-file>...
 
 import os.path
 import re
 import sys
 from optparse import OptionParser
 
 
 def make_variable_name_and_read(file_name):
-    result = re.match(r"([\w\d_]+)\.([\w\d_]+)", os.path.basename(file_name))
+    result = re.match(r'([\w\d_]+)\.([\w\d_]+)', os.path.basename(file_name))
     if not result:
-        print "Invalid input file name:", os.path.basename(file_name)
+        print 'Invalid input file name:', os.path.basename(file_name)
         sys.exit(1)
     variable_name = result.group(1)[0].lower() + result.group(1)[1:] + result.group(2).capitalize()
-    file = open(file_name, "rb")
-    content = file.read()
-    file.close()
-    return (variable_name, content)
+    with open(file_name, 'rb') as f:
+        content = f.read()
+    return variable_name, content
 
 
 def strip_whitespace_and_comments(file_name, content):
-    result = re.match(r".*\.([^.]+)", file_name)
+    result = re.match(r'.*\.([^.]+)', file_name)
     if not result:
-        print "The file name has no extension:", file_name
+        print 'The file name has no extension:', file_name
         sys.exit(1)
     extension = result.group(1).lower()
-    multi_line_comment = re.compile(r"/\*.*?\*/", re.MULTILINE | re.DOTALL)
-    single_line_comment = re.compile(r"//.*$", re.MULTILINE)
-    repeating_space = re.compile(r"[ \t]+", re.MULTILINE)
-    leading_space = re.compile(r"^[ \t]+", re.MULTILINE)
-    trailing_space = re.compile(r"[ \t]+$", re.MULTILINE)
-    empty_line = re.compile(r"\n+")
-    if extension == "js":
-        content = multi_line_comment.sub("", content)
-        content = single_line_comment.sub("", content)
-        content = repeating_space.sub(" ", content)
-        content = leading_space.sub("", content)
-        content = trailing_space.sub("", content)
-        content = empty_line.sub("\n", content)
-    elif extension == "css":
-        content = multi_line_comment.sub("", content)
-        content = repeating_space.sub(" ", content)
-        content = leading_space.sub("", content)
-        content = trailing_space.sub("", content)
-        content = empty_line.sub("\n", content)
+    multi_line_comment = re.compile(r'/\*.*?\*/', re.MULTILINE | re.DOTALL)
+    single_line_comment = re.compile(r'//.*$', re.MULTILINE)

[tkent, 2013/10/25 04:06:07]

This pattern will break XMLView.js.

http://src.chromium.org/viewvc/blink/trunk/Source/core/xml/XMLViewer.js?revis...

[abarth-chromium, 2013/10/25 04:09:35]

That's a good point.  Currently XMLViewer.js goes through xxd rather than this
file.  Maybe we should use an off-the-shelf JS minifier instead of trying to
write our own?

[Nils Barth (inactive), 2013/10/25 05:03:57]

Rather, XMLViewer.js.
Good point, thanks!
I've added another pattern to avoid matching '://'.

[Nils Barth (inactive), 2013/10/25 05:03:57]

That would be much better; using regexes for parsing is really
error-prone, with comments being a serious problem.

+    repeating_space = re.compile(r'[ \t]+', re.MULTILINE)
+    leading_space = re.compile(r'^[ \t]+', re.MULTILINE)
+    trailing_space = re.compile(r'[ \t]+$', re.MULTILINE)
+    empty_line = re.compile(r'\n+')
+    if extension == 'js':
+        content = multi_line_comment.sub('', content)
+        content = single_line_comment.sub('', content)
+        content = repeating_space.sub(' ', content)
+        content = leading_space.sub('', content)
+        content = trailing_space.sub('', content)
+        content = empty_line.sub('\n', content)
+    elif extension == 'css':
+        content = multi_line_comment.sub('', content)
+        content = repeating_space.sub(' ', content)
+        content = leading_space.sub('', content)
+        content = trailing_space.sub('', content)
+        content = empty_line.sub('\n', content)
     return content
 
 
+def process_file(file_name):
+    variable_name, content = make_variable_name_and_read(file_name)
+    content = strip_whitespace_and_comments(file_name, content)
+    size = len(content)
+    return variable_name, content
+
+
+def write_header_file(header_file_name, flag, names_and_contents):
+    with open(header_file_name, 'w') as header_file:
+        if flag:
+            header_file.write('#if ' + flag + '\n')
+        header_file.write('namespace WebKit {\n')

[abarth-chromium, 2013/10/25 04:04:02]

It's still hard-coded to use the WebKit namespace.  The other scripts are used
in WebCore, so we'll likely need to parameterize the script.

Also, should we be using jinja2 templates instead of manually building these
strings?

[Nils Barth (inactive), 2013/10/25 05:03:57]

Got it, made |namespace| into a parameter.
If we want to keep generating these files, yes, definitely Jinja2 would be
better!
(I understood you were suggesting we might want to simply replace
these scripts with something else?)

+        for variable_name, content in names_and_contents:
+            size = len(content)
+            header_file.write('extern const char %s[%d];\n' % (variable_name, size))
+        header_file.write('}\n')
+        if flag:
+            header_file.write('#endif\n')
+
+
+def write_cpp_file(cpp_file_name, flag, names_and_contents, header_file_name):
+    with open(cpp_file_name, 'w') as cpp_file:
+        cpp_file.write('#include "config.h"\n')
+        cpp_file.write('#include "%s"\n' % os.path.basename(header_file_name))
+        if flag:
+            cpp_file.write('#if ' + flag + '\n')
+        cpp_file.write('namespace WebKit {\n')
+        for variable_name, content in names_and_contents:
+            cpp_file.write(cpp_constant_string(variable_name, content))
+        cpp_file.write('}\n')
+        if flag:
+            cpp_file.write('#endif\n')
+
+
+def cpp_constant_string(variable_name, content):
+    output = []
+    size = len(content)
+    output.append('const char %s[%d] = {\n' % (variable_name, size))
+    for index in range(size):
+        char_code = ord(content[index])
+        if char_code < 128:
+            output.append('%d' % char_code)
+        else:
+            output.append(r"'\x%02x'" % char_code)
+        output.append(',' if index != len(content) - 1 else '};\n')
+        if index % 20 == 19:
+            output.append('\n')
+    output.append('\n')
+    return ''.join(output)
+
+
 def main():
     parser = OptionParser()
-    parser.add_option("--out-h", dest="out_header")
-    parser.add_option("--out-cpp", dest="out_cpp")
-    parser.add_option("--condition", dest="flag")
+    parser.add_option('--out-h', dest='out_header')
+    parser.add_option('--out-cpp', dest='out_cpp')
+    parser.add_option('--condition', dest='flag')
     (options, args) = parser.parse_args()
     if len(args) < 1:
         parser.error("Need one or more input files")
     if not options.out_header:
         parser.error("Need to specify --out-h=filename")
     if not options.out_cpp:
         parser.error("Need to specify --out-cpp=filename")
 
     if options.flag:
-        options.flag = options.flag.replace(" AND ", " && ")
-        options.flag = options.flag.replace(" OR ", " || ")
+        options.flag = options.flag.replace(' AND ', ' && ')
+        options.flag = options.flag.replace(' OR ', ' || ')
 
-    header_file = open(options.out_header, "w")
-    if options.flag:
-        header_file.write("#if " + options.flag + "\n")
-    header_file.write("namespace WebKit {\n")
+    names_and_contents = [process_file(file_name) for file_name in args]
 
-    cpp_file = open(options.out_cpp, "w")
-    cpp_file.write("#include \"config.h\"\n")
-    cpp_file.write("#include \"" + os.path.basename(options.out_header) + "\"\n")
-    if options.flag:
-        cpp_file.write("#if " + options.flag + "\n")
-    cpp_file.write("namespace WebKit {\n")
+    if options.out_header:
+        write_header_file(options.out_header, options.flag, names_and_contents)
+    write_cpp_file(options.out_cpp, options.flag, names_and_contents, options.out_header)
 
-    for file_name in args:
-        (variable_name, content) = make_variable_name_and_read(file_name)
-        content = strip_whitespace_and_comments(file_name, content)
-        size = len(content)
-        header_file.write("extern const char %s[%d];\n" % (variable_name, size))
-        cpp_file.write("const char %s[%d] = {\n" % (variable_name, size))
-        for index in range(size):
-            char_code = ord(content[index])
-            if char_code < 128:
-                cpp_file.write("%d" % char_code)
-            else:
-                cpp_file.write("'\\x%02x'" % char_code)
-            cpp_file.write("," if index != len(content) - 1 else "};\n")
-            if index % 20 == 19:
-                cpp_file.write("\n")
-        cpp_file.write("\n")
-
-    header_file.write("}\n")
-    if options.flag:
-        header_file.write("#endif\n")
-    header_file.close()
-
-    cpp_file.write("}\n")
-    if options.flag:
-        cpp_file.write("#endif\n")
-    cpp_file.close()
 
 
 if __name__ == "__main__":
     main()