lodato added a subscriber: lodato.
lodato requested changes to this revision.
lodato added a reviewer: lodato.
lodato added a comment.
This revision now requires changes to proceed.
This does not work properly because it calls `clang-format` on the files in the
working directory, even if `--staged` is given. To fix, you need to somehow
pass in the version of the files in the index into `clang-format`. To do that,
I think you'd want to pass in the blob via stdin and add `-assume-filename=...`
to the command line.
Example:
$ 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/2/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
Now we have the situation where the stage and the working directory differ, but
both are clang-format-compatible and should produce no changes.
$ git diff --cached
diff --git c/foo.cc i/foo.cc
index cb7e0b0..0a5833c 100644
--- c/foo.cc
+++ i/foo.cc
@@ -1,4 +1,4 @@
int main() {
- int x = 1;
+ int x = 2;
return 0;
}
$ git diff
diff --git i/foo.cc w/foo.cc
index 0a5833c..b821f3e 100644
--- i/foo.cc
+++ w/foo.cc
@@ -1,4 +1,5 @@
int main() {
- int x = 2;
+ printf("hello\n");
+ printf("goodbye\n");
return 0;
}
However, your script produces the following, which is clearly incorrect:
$ git-clang-format --staged --diff
diff --git a/foo.cc b/foo.cc
index 0a5833c..b821f3e 100644
--- a/foo.cc
+++ b/foo.cc
@@ -1,4 +1,5 @@
int main() {
- int x = 2;
+ printf("hello\n");
+ printf("goodbye\n");
return 0;
}
================
Comment at: git-clang-format:104
@@ -103,1 +103,3 @@
help='select hunks interactively')
+ p.add_argument('-s', '--staged', action='store_true',
+ help='consider only staged lines')
----------------
Please move this down after `-q` so it stays sorted.
Also I would drop the `-s` since the this will be an uncommon option.
Finally, could you add `aliases=['cached']` to mirror the behavior of `git
diff`?
================
Comment at: git-clang-format:124
@@ -121,3 +123,3 @@
del opts.quiet
commit, files = interpret_args(opts.args, dash_dash, opts.commit)
----------------
This will work without `--diff` (otherwise it will try to apply changes in the
index to the working directory, which doesn't make sense), so could you please
add a check that `--staged` requires `--diff`?
================
Comment at: git-clang-format:250
@@ -244,3 +249,3 @@
-def compute_diff_and_extract_lines(commit, files):
+def compute_diff_and_extract_lines(commit, files, cached=False):
"""Calls compute_diff() followed by extract_lines()."""
----------------
I suggest sticking with the name `staged` throughout the code.
================
Comment at: git-clang-format:328
@@ -319,1 +327,3 @@
+def run_git_ls_files_and_save_to_tree(changed_lines):
+ """Run git ls-files and save the result to a git tree.
----------------
There is already a git command that does this, `git write-tree`, so you can
replace this entire function with a call to `run('git', 'write-tree')`.
Repository:
rL LLVM
http://reviews.llvm.org/D15465
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
