[PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit

[Luis Héctor Chávez via cfe-commits]

lhchavez added a comment.

I'll post a no-op change with arcanist to fix the paths (I shouldn't have tried to manually upload the diff), and then another one to fix the patch so that the script actually does what it advertises and add the comment explaining the reason for create_tree_from_commit().

I'll tackle the interface change in the change following that one, after some more experimentation.


================
Comment at: cfe/trunk/tools/clang-format/git-clang-format:93
@@ -92,1 +92,3 @@
                  help='default commit to use if none is specified'),
+  p.add_argument('--single-commit', action='store_true',
+                 help=('run clang-format on a single commit instead of against '
----------------
lodato wrote:
> nit: I find this flag confusing since it does not follow any git convention. Instead, I suggest making the interface similar to `git diff`: if two `<commit>`s are given, take the diff of those two trees to find the list of changed lines, then run clang-format on the second commit.
> 
> For example:
> 
> * `git clang-format --diff HEAD HEAD~` would tell you if HEAD was properly clang-formatted or not.
> * `git clang-format --diff 8bf003 ae9172` would tell you if any of the lines in ae9172 that differ from 8bf003 are not properly clang-formatted.
> 
>  operate in this new mode only if two `<commit>`s are given.  Then the interface would be, for example, `git clang-format abcd1234 abcd1234~`.
The thing I liked about using git-diff-tree is that you can pass a single commit and even if it's a merge-commit, it does the right thing. I'll try that idea out and allow more than two commits and see how it behaves.

================
Comment at: cfe/trunk/tools/clang-format/git-clang-format:323
@@ -312,1 +322,3 @@
 
+def create_tree_from_commit(commit, filenames):
+  """Create a new git tree with the given files from `commit`.
----------------
lodato wrote:
> Unless I'm mistaken, this function is unnecessary. There is no need to filter out files in the tree that do not match the pattern, since the filtering happens in `compute_diff()` (`cmd.extend(files)`).
The reason I added this is to prevent the generation of much larger temporary indices for large projects (like Chromium) vs. git-clang-format in normal mode. I can add a comment explaining that.


Repository:
  rL LLVM

https://reviews.llvm.org/D24319