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
Index: git-clang-format
===================================================================
--- git-clang-format
+++ git-clang-format
@@ -108,6 +108,8 @@
help='select hunks interactively')
p.add_argument('-q', '--quiet', action='count', default=0,
help='print less information')
+ p.add_argument('--staged', '--cached', action='store_true',
+ help='consider only staged lines')
p.add_argument('--style',
default=config.get('clangformat.style', None),
help='passed to clang-format'),
@@ -129,10 +131,12 @@
if len(commits) > 1:
if not opts.diff:
die('--diff is required when two commits are given')
+ if opts.staged:
+ die('--staged is not allowed when two commits are given')
else:
if len(commits) > 2:
die('at most two commits allowed; %d given' % len(commits))
- changed_lines = compute_diff_and_extract_lines(commits, files)
+ changed_lines = compute_diff_and_extract_lines(commits, files, opts.staged)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -159,10 +163,14 @@
binary=opts.binary,
style=opts.style)
else:
- old_tree = create_tree_from_workdir(changed_lines)
+ if opts.staged:
+ old_tree = run_git_ls_files_and_save_to_tree(changed_lines)
+ else:
+ old_tree = create_tree_from_workdir(changed_lines)
new_tree = run_clang_format_and_save_to_tree(changed_lines,
binary=opts.binary,
- style=opts.style)
+ style=opts.style,
+ staged=opts.staged)
if opts.verbose >= 1:
print('old tree: %s' % old_tree)
print('new tree: %s' % new_tree)
@@ -261,9 +269,10 @@
return convert_string(stdout.strip())
-def compute_diff_and_extract_lines(commits, files):
+def compute_diff_and_extract_lines(commits, files, staged=False):
"""Calls compute_diff() followed by extract_lines()."""
- diff_process = compute_diff(commits, files)
+ assert not staged or len(commits) < 2
+ diff_process = compute_diff(commits, files, staged)
changed_lines = extract_lines(diff_process.stdout)
diff_process.stdout.close()
diff_process.wait()
@@ -273,17 +282,22 @@
return changed_lines
-def compute_diff(commits, files):
+def compute_diff(commits, files, staged=False):
"""Return a subprocess object producing the diff from `commits`.
The return value's `stdin` file object will produce a patch with the
differences between the working directory and the first commit if a single
one was specified, or the difference between both specified commits, filtered
on `files` (if non-empty). Zero context lines are used in the patch."""
- git_tool = 'diff-index'
+ assert not staged or len(commits) < 2
+ cmd = ['git']
if len(commits) > 1:
- git_tool = 'diff-tree'
- cmd = ['git', git_tool, '-p', '-U0'] + commits + ['--']
+ cmd.append('diff-tree')
+ else:
+ cmd.append('diff-index')
+ if staged:
+ cmd.append('--cached')
+ cmd.extend(['-p', '-U0'] + commits + ['--'])
cmd.extend(files)
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
p.stdin.close()
@@ -343,11 +357,43 @@
return create_tree(filenames, '--stdin')
+def run_git_ls_files_and_save_to_tree(changed_lines):
+ """Run git ls-files and save the result to a git tree.
+
+ Returns the object ID (SHA-1) of the created tree."""
+ index_path = os.environ.get('GIT_INDEX_FILE')
+ def iteritems(container):
+ try:
+ return container.iteritems() # Python 2
+ except AttributeError:
+ return container.items() # Python 3
+ def index_info_generator():
+ for filename, line_ranges in iteritems(changed_lines):
+ old_index_path = os.environ.get('GIT_INDEX_FILE')
+ if index_path is None:
+ del os.environ['GIT_INDEX_FILE']
+ else:
+ os.environ['GIT_INDEX_FILE'] = index_path
+ try:
+ mode, id, stage, filename = run('git', 'ls-files', '--stage', '--',
+ filename).split()
+ yield '%s %s\t%s' % (mode, id, filename)
+ finally:
+ if old_index_path is None:
+ del os.environ['GIT_INDEX_FILE']
+ else:
+ os.environ['GIT_INDEX_FILE'] = old_index_path
+ return create_tree(index_info_generator(), '--index-info')
+
+
def run_clang_format_and_save_to_tree(changed_lines, revision=None,
- binary='clang-format', style=None):
+ binary='clang-format', style=None,
+ staged=False):
"""Run clang-format on each file and save the result to a git tree.
Returns the object ID (SHA-1) of the created tree."""
+ assert not staged or revision is None
+ index_path = os.environ.get('GIT_INDEX_FILE')
def iteritems(container):
try:
return container.iteritems() # Python 2
@@ -368,11 +414,23 @@
# Adjust python3 octal format so that it matches what git expects
if mode.startswith('0o'):
mode = '0' + mode[2:]
- blob_id = clang_format_to_blob(filename, line_ranges,
- revision=revision,
- binary=binary,
- style=style)
- yield '%s %s\t%s' % (mode, blob_id, filename)
+ old_index_path = os.environ.get('GIT_INDEX_FILE')
+ if index_path is None:
+ del os.environ['GIT_INDEX_FILE']
+ else:
+ os.environ['GIT_INDEX_FILE'] = index_path
+ try:
+ blob_id = clang_format_to_blob(filename, line_ranges,
+ revision=revision,
+ binary=binary,
+ style=style,
+ staged=staged)
+ yield '%s %s\t%s' % (mode, blob_id, filename)
+ finally:
+ if old_index_path is None:
+ del os.environ['GIT_INDEX_FILE']
+ else:
+ os.environ['GIT_INDEX_FILE'] = old_index_path
return create_tree(index_info_generator(), '--index-info')
@@ -397,22 +455,24 @@
def clang_format_to_blob(filename, line_ranges, revision=None,
- binary='clang-format', style=None):
+ binary='clang-format', style=None, staged=False):
"""Run clang-format on the given file and save the result to a git blob.
Runs on the file in `revision` if not None, or on the file in the working
directory if `revision` is None.
Returns the object ID (SHA-1) of the created blob."""
+ assert not staged or revision is None
clang_format_cmd = [binary]
if style:
clang_format_cmd.extend(['-style='+style])
clang_format_cmd.extend([
'-lines=%s:%s' % (start_line, start_line+line_count-1)
for start_line, line_count in line_ranges])
- if revision:
+ if staged or revision:
clang_format_cmd.extend(['-assume-filename='+filename])
- git_show_cmd = ['git', 'cat-file', 'blob', '%s:%s' % (revision, filename)]
+ git_show_cmd = ['git', 'cat-file', 'blob', '%s:%s' % (revision or '',
+ filename)]
git_show = subprocess.Popen(git_show_cmd, stdin=subprocess.PIPE,
stdout=subprocess.PIPE)
git_show.stdin.close()
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
