[PATCH] D30773: Make git-clang-format python 3 compatible

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added a comment.

Some nits from a Python3 hacker.



================
Comment at: tools/clang-format/git-clang-format:143
       for filename in ignored_files:
-        print '   ', filename
+        print('   ', filename)
     if changed_lines:
----------------
I find `print('template', tail)` surprising in Python3, but it could be because we don't use it in our local standards. I'd jump immediately to formatting to make this 2/3-compatible:

    print('    %s' % filename)

or in this case maybe just join the strings:

    print('    ' + filename)



================
Comment at: tools/clang-format/git-clang-format:323
   allowed_extensions = frozenset(allowed_extensions)
-  for filename in dictionary.keys():
+  for filename in list(dictionary.keys()):
     base_ext = filename.rsplit('.', 1)
----------------
This should not be necessary for iteration -- in Python3 it returns a generator instead of a list, but generators can be iterated.


================
Comment at: tools/clang-format/git-clang-format:491
     if unstaged_files:
-      print >>sys.stderr, ('The following files would be modified but '
-                           'have unstaged changes:')
-      print >>sys.stderr, unstaged_files
-      print >>sys.stderr, 'Please commit, stage, or stash them first.'
+      print(('The following files would be modified but '
+                           'have unstaged changes:'), file=sys.stderr)
----------------
No need for parentheses around the string


================
Comment at: tools/clang-format/git-clang-format:492
+      print(('The following files would be modified but '
+                           'have unstaged changes:'), file=sys.stderr)
+      print(unstaged_files, file=sys.stderr)
----------------
I wonder if `file=sys.stderr` works with Python 2.7's print statement. Have you tested this with 2.7?

You can use `__future__` to get the print function behavior in 2.7 as well:
http://stackoverflow.com/questions/32032697/how-to-use-from-future-import-print-function


================
Comment at: tools/clang-format/git-clang-format:512-515
+def to_string(bytes):
+    if isinstance(bytes, str):
+        return bytes
+    return to_bytes(bytes)
----------------
This looks wrong -- where does `to_bytes` come from?

I can never get my head around Python2's string/bytes philosophy. In Python3 you would do:

    # stdout and stderr are always `bytes`
    stdout = stdout.decode('utf-8')
    stderr = stderr.decode('utf-8')

(assuming the input *is* utf-8)

Not sure how that plays with Python2, but the current code doesn't look right.



https://reviews.llvm.org/D30773