Refactor make-file-arrays.py to ease reuse

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

Index: Source/web/scripts/make-file-arrays.py
diff --git a/Source/web/scripts/make-file-arrays.py b/Source/web/scripts/make-file-arrays.py
index cfb41f303ef462e1333fff4adb875bbdbf201066..f7882f1f4f29690f48e6639cd42b7de3072ca900 100755
--- a/Source/web/scripts/make-file-arrays.py
+++ b/Source/web/scripts/make-file-arrays.py
@@ -36,50 +36,100 @@ 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")
@@ -89,47 +139,15 @@ def main():
         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")
-
-    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()
+    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)
 
-    cpp_file.write("}\n")
-    if options.flag:
-        cpp_file.write("#endif\n")
-    cpp_file.close()
 
 
 if __name__ == "__main__":