[PATCH] D38446: update comments in clang-format.py for python3 compatibility

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert added a comment.

ping?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D38446/new/

https://reviews.llvm.org/D38446



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D38446: update comments in clang-format.py for python3 compatibility

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert added a comment.

thanks for the ping. looking back at Code Reviews with Phabricator 
, the following applies to me: "If you 
do not have commit access, someone has to commit the change for you (with 
attribution)." so, yes I need help landing.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D38446/new/

https://reviews.llvm.org/D38446



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70664: update string comparison in clang-format.py

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert created this revision.
pseyfert added reviewers: MyDeveloperDay, klimek, llvm-commits, cfe-commits.
pseyfert added a project: clang-format.
Herald added a project: clang.

Python 3.8 introduces a SyntaxWarning about string comparisons with 'is'. This 
commit updates the string comparison in clang-format.py that is done with 'is 
not' to '!='. This should not break compatibility with older python versions 
(tested 3.4.9, 2.7.17, 2.7.5, 3.8.0).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70664

Files:
  clang/tools/clang-format/clang-format.py


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -132,7 +132,7 @@
 lines = lines[1:]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
-  if op[0] is not 'equal':
+  if op[0] != 'equal':
 vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
 if output.get('IncompleteFormat'):
   print('clang-format: incomplete (syntax errors)')


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -132,7 +132,7 @@
 lines = lines[1:]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
-  if op[0] is not 'equal':
+  if op[0] != 'equal':
 vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
 if output.get('IncompleteFormat'):
   print('clang-format: incomplete (syntax errors)')
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70664: update string comparison in clang-format.py

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert added a comment.

PS: I do not have commit access, so someone with the rights would need to land 
this for me. Thanks in advance.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70664/new/

https://reviews.llvm.org/D70664



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70864: update trailing newline treatment in clang-format.py

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert created this revision.
pseyfert added reviewers: klimek, MyDeveloperDay.
pseyfert added projects: clang-format, clang.

The current clang-format.py does not handle trailing newlines at the end of a 
file correctly.
Trailing empty lines get removed except one.
As far as I understand this is because clang-format gets fed from stdin and 
writes to stdout when called from clang-format.py.
In a "normal" file (with no trailing empty lines) the string that gets passed 
to clang-format does not contain a trailing '\n' after the '\n'.join from 
python.
The clang-format binary does not add a trailing newline to input from stdin, 
but (if there are multiple trailing '\n', all except one get removed).

When reading back this means that we see in python from a "normal" file a 
string with no trailing '\n'. From a file with (potentially multiple) empty 
line(s) at the end, we get a string with one trailing '\n' back in python. In 
the former case all is fine, in the latter case split('\n') makes one empty 
line at the end of the file out of the clang-format output. Desired would be 
instead that the **file** ends with a newline, but not with an empty line.

For the case that a user specifies a range to format (and wants to keep 
trailing empty lines) I did **not** try to fix this by simply removing all 
trailing newlines from the clang-format output. Instead, I add a '\n' to the 
unformatted file content (i.e. newline-terminate what is passed to 
clang-format) and then strip off the last newline from the output (which itself 
is now for sure the newline termination of the clang-format output).

(Should this get approved, I'll need someone to help me land this.)


https://reviews.llvm.org/D70864

Files:
  clang/tools/clang-format/clang-format.py


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -70,7 +70,7 @@
   # Get the current text.
   encoding = vim.eval("&encoding")
   buf = get_buffer(encoding)
-  text = '\n'.join(buf)
+  text = '\n'.join(buf) + '\n'
 
   # Determine range to format.
   if vim.eval('exists("l:lines")') == '1':
@@ -129,7 +129,7 @@
   else:
 lines = stdout.decode(encoding).split('\n')
 output = json.loads(lines[0])
-lines = lines[1:]
+lines = lines[1:-1]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
   if op[0] is not 'equal':


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -70,7 +70,7 @@
   # Get the current text.
   encoding = vim.eval("&encoding")
   buf = get_buffer(encoding)
-  text = '\n'.join(buf)
+  text = '\n'.join(buf) + '\n'
 
   # Determine range to format.
   if vim.eval('exists("l:lines")') == '1':
@@ -129,7 +129,7 @@
   else:
 lines = stdout.decode(encoding).split('\n')
 output = json.loads(lines[0])
-lines = lines[1:]
+lines = lines[1:-1]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
   if op[0] is not 'equal':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70864: update trailing newline treatment in clang-format.py

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert updated this revision to Diff 231568.
pseyfert added a comment.

added comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70864/new/

https://reviews.llvm.org/D70864

Files:
  clang/tools/clang-format/clang-format.py


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -70,7 +70,8 @@
   # Get the current text.
   encoding = vim.eval("&encoding")
   buf = get_buffer(encoding)
-  text = '\n'.join(buf)
+  # Join the buffer into a single string with a terminating newline
+  text = '\n'.join(buf) + '\n'
 
   # Determine range to format.
   if vim.eval('exists("l:lines")') == '1':
@@ -129,7 +130,10 @@
   else:
 lines = stdout.decode(encoding).split('\n')
 output = json.loads(lines[0])
-lines = lines[1:]
+# Strip off the trailing newline (added above).
+# This maintains trailing empty lines present in the buffer if
+# the -lines specification requests them to remain unchanged.
+lines = lines[1:-1]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
   if op[0] is not 'equal':


Index: clang/tools/clang-format/clang-format.py
===
--- clang/tools/clang-format/clang-format.py
+++ clang/tools/clang-format/clang-format.py
@@ -70,7 +70,8 @@
   # Get the current text.
   encoding = vim.eval("&encoding")
   buf = get_buffer(encoding)
-  text = '\n'.join(buf)
+  # Join the buffer into a single string with a terminating newline
+  text = '\n'.join(buf) + '\n'
 
   # Determine range to format.
   if vim.eval('exists("l:lines")') == '1':
@@ -129,7 +130,10 @@
   else:
 lines = stdout.decode(encoding).split('\n')
 output = json.loads(lines[0])
-lines = lines[1:]
+# Strip off the trailing newline (added above).
+# This maintains trailing empty lines present in the buffer if
+# the -lines specification requests them to remain unchanged.
+lines = lines[1:-1]
 sequence = difflib.SequenceMatcher(None, buf, lines)
 for op in reversed(sequence.get_opcodes()):
   if op[0] is not 'equal':
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70864: update trailing newline treatment in clang-format.py

[Paul Seyfert via Phabricator via cfe-commits]

pseyfert added a comment.

As a demonstrator I put this 
 
docker image together:

  docker run -it --rm 
gitlab-registry.cern.ch/pseyfert/vim-clang-format-docker:latest

open `vim Chi2PerDoF.h` and hit Ctrl+I. There is a blank line at the end of the 
file that does not get removed. If you run `clang-format-9 Chi2PerDoF.h` The 
empty line at the end of the file does get removed.
And you can try adding extra empty lines at the end of the file, running 
clang-format from vim (Ctrl+I) removes all but one empty line from the file. 
Running clang-format-9 from the command line removes all blank lines at the end 
of the file.

For what concerns testing with lit: no idea, I'm unfamiliar with the testing. I 
remember one can run vim macros also from the command line such that the 
reproducer wouldn't require user interaction, such that one can just run `vim 
-crazylookingoptions` and then diff the file against a reference. Haven't done 
that in years but if that would help I can look it up.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70864/new/

https://reviews.llvm.org/D70864



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits