Cleanup public includes directory.

tools/generate_includes_cpp.py

+#!/usr/bin/python
+
+'''
+Copyright 2014 Google Inc.
+
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+'''

[borenet, 2014/11/13 15:13:34]

Prefer this format, but not sure it really matters:
#!/usr/bin/env python
# Copyright (c) 2014 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 os
+import optparse
+

[borenet, 2014/11/13 15:13:34]

Nit: 2 lines between top-level stuff

[djsollen, 2014/11/13 15:45:26]

Done.

+def is_ignored(full_path, ignore_list):
+  for ignore_path in ignore_list:
+    if full_path.endswith(ignore_path):

[borenet, 2014/11/13 15:13:34]

Aren't some ignore_paths directories?  Should this be "if
full_path.startswith(ignore_path)"?  Or even "if ignore_path in full_path"?

[djsollen, 2014/11/13 15:45:26]

Nope, we always want exact matches here. the find_header_files function will
short circuit the recursion when it matches to a directory so we'll never see
the files in those directories.

+      return True
+  return False
+
+def find_header_files(include_dirs, ignore_list):
+  """Return a list of all '.h' files in top_dir.
+
+  Args:
+      include_dirs: Paths to the directories within which to recursively search
+          for files ending in '.h'

[borenet, 2014/11/13 15:13:34]

Please document ignore_list as well.

[djsollen, 2014/11/13 15:45:26]

Done.

+
+  Returns:
+      A list of all the files inside include_dirs that end in '.h', relative to
+          their respective include_dir.
+  """
+  headers = []
+  for top_dir in include_dirs:
+    for filename in os.listdir(top_dir):
+      full_path = os.path.join(top_dir, filename)
+      if is_ignored(full_path, ignore_list):
+        #print "%s\n" % full_path

[borenet, 2014/11/13 15:13:34]

Should this stay or go?

[djsollen, 2014/11/13 15:45:26]

Done.

+        continue
+      elif os.path.isdir(full_path):
+        nested_headers = find_header_files([full_path], ignore_list)
+        for nested_header in nested_headers:
+          headers.append(os.path.join(filename, nested_header))

[borenet, 2014/11/13 15:13:34]

Optional: this would get faster if you made this function a generator.  Instead
of "headers.append(X)" you'd write "yield X".  

Then these three lines become:

for nested_header in find_header_files([full_path], ignore_list):
  yield os.path.join(filename, nested_header)

And instead of storing a pointer to the list in line 46, you'd just iterate
directly:

for header in find_header_files(include_dirs, ignore_list):

+      elif filename.endswith('.h'):
+        headers.append(filename)
+  return headers
+
+def GenerateIncludeCPP(output_file, include_dirs, ignore_list):
+  headers = find_header_files(include_dirs, ignore_list)
+
+  # Emit resulting source file.
+  output = open(os.path.join(os.getcwd(), output_file), "w+")
+  for header in headers:
+    output.write("#include <%s>\n" % header)
+  output.close()

[borenet, 2014/11/13 15:13:34]

Please use the "with" syntax:

with open(os.path.join(os.getcwd(), output_file), "w+") as output:
  for header in headers:
    output.write("#include <%s>\n" % header)

It's equivalent to using try/finally, so it's safer than explicitly closing
after the writes.

[djsollen, 2014/11/13 15:45:26]

Done.

+
+def main():
+  parser = optparse.OptionParser()
+  parser.add_option("--ignore", action="store", type="string", dest="ignore",
+                    help="file to write the processed sources array to.")
+  parser.set_usage("""generate_include_cpp out.cpp include_dir
+      out.cpp: C++ code to be generated.
+      include_dirs: directories to traverse for include files""")
+  (options, args) = parser.parse_args()
+
+  GenerateIncludeCPP(args[0], args[1:], options.ignore.split())
+
+
+if __name__ == "__main__":
+  main()