[PATCH] D15465: [git-clang-format]: New option to perform formatting against staged changes only

[Alexander Shukaev via Phabricator via cfe-commits]

Alexander-Shukaev updated this revision to Diff 112801.
Alexander-Shukaev added a comment.

Alright, so you got me excited about this task once again.  Now, I've just rebased to the latest `git-clang-format` and it has changed a bit.  Nevertheless, I've updated the previous patch accordingly and applied those changes which @lodato was proposing (except for getting rid of `run_git_ls_files_and_save_to_tree` as I'm still not sure whether this would be the same as `git write-tree`).  Anyway, just tested the exact scenario posted by @lodato, and indeed with the current patch it works as he expects.  Namely, there is no diff from unstaged tree interfering anymore.  So far so good, but as I said in my previous post, there is another side of the medal.  If you didn't get it, then read through it again and try, for example, the following amended @lodato's scenario:

  $ mkdir tmp
  $ cd tmp
  $ git init
  $ cat > foo.cc <<EOF
  int main() {
    int x = 1;
    return 0;
  }
  EOF
  $ git add foo.cc
  $ git commit -m 'initial commit'
  $ sed -i -e 's/1/\n2\n/g' foo.cc
  $ git add foo.cc
  $ rm foo.cc
  $ cat > foo.cc <<EOF
  int main() {
    printf("hello\n");
    printf("goodbye\n");
    return 0;
  }
  EOF

Notice the newlines around `2`.  If you now do `git commit`, those bad newlines in the staged tree will for sure get fixed but at the cost that you will lose your changes in unstaged tree without any notification.  That is as I said, they will merely be overwritten by Clang-Format.  There are several possibilities to improve the implementation from here that I can think of:

1. Introduce an explicit conflict in unstaged tree to have both previous unstaged changes and changes done by Clang-Format (to reformat the staged tree) for manual resolution by users;
2. Report an error with a diff of those unstaged lines which prevent Clang-Format from doing it's job due to conflict, which is technically speaking already the same as it was before that patch;

Finally, regardless of what would be the choice between these two options, I'd reuse the `--force` option to forcefully overwrite the unstaged tree with the changes done by Clang-Format, essentially what the current patch does already anyway.  What do you guys think?  Contributions and suggestions are very welcome;  let's have another round to get it merged upstream!


Repository:
  rL LLVM

https://reviews.llvm.org/D15465

Files:
  git-clang-format

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D15465.112801.patch
Type: text/x-patch
Size: 7691 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170826/7e69e8a5/attachment-0001.bin>