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

[pseyfert]

Summary

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.)

[pseyfert]

added comments

[MyDeveloperDay]

I guess this isn't something we could test with lit right?

Can you explain how this goes wrong, I've not seen it myself and I use the vim plugin

[pseyfert]

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.

[MyDeveloperDay]

LGTM

clang/tools/clang-format/clang-format.py

def get_buffer(encoding):
   if platform.python_version_tuple()[0] == '3':
     return vim.current.buffer
   return [ line.decode(encoding) for line in vim.current.buffer ]
 
 def main():
   # 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':
     lines = ['-lines', vim.eval('l:lines')]
   elif vim.eval('exists("l:formatdiff")') == '1':
     with open(vim.current.buffer.name, 'r') as f:
       ondisk = f.read().splitlines();
     sequence = difflib.SequenceMatcher(None, ondisk, vim.current.buffer)
def main():
   if not stdout:
     print(
         'No output from clang-format (crashed?).\n'
         'Please report to bugs.llvm.org.'
     )
   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':
         vim.current.buffer[op[1]:op[2]] = lines[op[3]:op[4]]
     if output.get('IncompleteFormat'):
       print('clang-format: incomplete (syntax errors)')
     vim.command('goto %d' % (output['Cursor'] + 1))
 
 main()