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