[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag
shafik added a comment. Herald added a subscriber: wangpc. Herald added a project: All. Is this still relevant or can we close this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41145/new/ https://reviews.llvm.org/D41145 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag
lodato added a comment. Oops! Fixed. Thanks for catching this! https://reviews.llvm.org/D41145 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag
lodato updated this revision to Diff 126792.
lodato added a comment.
Re-add check for `len(commits) > 2`.
https://reviews.llvm.org/D41145
Files:
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -36,12 +36,30 @@
usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and , which defaults to HEAD. Changes are
-only applied to the working directory.
+Run clang-format on all modified lines in the working directory or in a given
+commit.
-If two commits are given (requires --diff), run clang-format on all lines in the
-second that differ from the first .
+Forms:
+
+git clang-format []
+Run clang-format on all lines in the working directory that differ from
+, which defaults to HEAD. Changes are written in-place.
+
+git clang-format [] --diff
+Same as first form, but print the diff instead of writing changes
+in-place.
+
+git clang-format [] --patch
+Same as first form, but interactively choose hunks to apply, a la `git
+add -p`.
+
+git clang-format --diff
+Run clang-format on all lines in that differ from .
+Requires --diff.
+
+In all of the forms above, ... can be used to limit what files are
+affected, using the same syntax as `git diff`. Use `--` to disambiguate between
+files and commits.
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -125,14 +143,11 @@
opts.verbose -= opts.quiet
del opts.quiet
- commits, files = interpret_args(opts.args, dash_dash, opts.commit)
- if len(commits) > 1:
-if not opts.diff:
- die('--diff is required 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)
+ source, dest, files = interpret_args(opts.args, dash_dash,
+ default_commit=opts.commit)
+ if isinstance(dest, Revision) and not opts.diff:
+die('--diff is required when two commits are given')
+ changed_lines = dest.compute_diff_from(source, files)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -152,17 +167,10 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- if len(commits) > 1:
-old_tree = commits[1]
-new_tree = run_clang_format_and_save_to_tree(changed_lines,
- revision=commits[1],
- binary=opts.binary,
- style=opts.style)
- 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)
+ old_tree = dest.create_tree(changed_lines)
+ new_tree = dest.run_clang_format_and_save_to_tree(changed_lines,
+binary=opts.binary,
+style=opts.style)
if opts.verbose >= 1:
print('old tree: %s' % old_tree)
print('new tree: %s' % new_tree)
@@ -172,6 +180,7 @@
elif opts.diff:
print_diff(old_tree, new_tree)
else:
+assert isinstance(dest, Workdir)
changed_files = apply_changes(old_tree, new_tree, force=opts.force,
patch_mode=opts.patch)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
@@ -199,15 +208,20 @@
def interpret_args(args, dash_dash, default_commit):
- """Interpret `args` as "[commits] [--] [files]" and return (commits, files).
+ """Interpret `args` as "[commits] [--] [files]" and return (src, dest, files).
It is assumed that "--" and everything that follows has been removed from
args and placed in `dash_dash`.
If "--" is present (i.e., `dash_dash` is non-empty), the arguments to its
left (if present) are taken as commits. Otherwise, the arguments are checked
from left to right if they are commits or files. If commits are not given,
- a list with `default_commit` is used."""
+ a list with `default_commit` is used.
+
+ Return value is `(src, dest, files)`, where `src` and `dest` are TreeLocation
+ objects and `files` is a (possibly empty) list of filenames.
+ """
+ # First, get the list
[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag
klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format:252 + assert len(commits) != 0 + if len(commits) >= 2: +return Revision(commits[0]), Revision(commits[1]), files Don't we want to die if len(commits) > 2? https://reviews.llvm.org/D41145 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag
lodato created this revision.
lodato added reviewers: djasper, klimek, Alexander-Shukaev.
The only user-visible change is rewriting of the --help message to make the
different modes more clear.
Internally, this is a significant refactoring to make the source and
destination of the diff to be more clear by using subclasses rather than
conditionals. The purpose is to support the next patch in this series, which
adds support for a --staged flag. But even if that does not go through, the
intent is for this code to be a readability improvement.
https://reviews.llvm.org/D41145
Files:
google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
Index: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
===
--- google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
+++ google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format
@@ -36,12 +36,30 @@
usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
desc = '''
-If zero or one commits are given, run clang-format on all lines that differ
-between the working directory and , which defaults to HEAD. Changes are
-only applied to the working directory.
+Run clang-format on all modified lines in the working directory or in a given
+commit.
-If two commits are given (requires --diff), run clang-format on all lines in the
-second that differ from the first .
+Forms:
+
+git clang-format []
+Run clang-format on all lines in the working directory that differ from
+, which defaults to HEAD. Changes are written in-place.
+
+git clang-format [] --diff
+Same as first form, but print the diff instead of writing changes
+in-place.
+
+git clang-format [] --patch
+Same as first form, but interactively choose hunks to apply, a la `git
+add -p`.
+
+git clang-format --diff
+Run clang-format on all lines in that differ from .
+Requires --diff.
+
+In all of the forms above, ... can be used to limit what files are
+affected, using the same syntax as `git diff`. Use `--` to disambiguate between
+files and commits.
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -125,14 +143,11 @@
opts.verbose -= opts.quiet
del opts.quiet
- commits, files = interpret_args(opts.args, dash_dash, opts.commit)
- if len(commits) > 1:
-if not opts.diff:
- die('--diff is required 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)
+ source, dest, files = interpret_args(opts.args, dash_dash,
+ default_commit=opts.commit)
+ if isinstance(dest, Revision) and not opts.diff:
+die('--diff is required when two commits are given')
+ changed_lines = dest.compute_diff_from(source, files)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -152,17 +167,10 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- if len(commits) > 1:
-old_tree = commits[1]
-new_tree = run_clang_format_and_save_to_tree(changed_lines,
- revision=commits[1],
- binary=opts.binary,
- style=opts.style)
- 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)
+ old_tree = dest.create_tree(changed_lines)
+ new_tree = dest.run_clang_format_and_save_to_tree(changed_lines,
+binary=opts.binary,
+style=opts.style)
if opts.verbose >= 1:
print('old tree: %s' % old_tree)
print('new tree: %s' % new_tree)
@@ -172,6 +180,7 @@
elif opts.diff:
print_diff(old_tree, new_tree)
else:
+assert isinstance(dest, Workdir)
changed_files = apply_changes(old_tree, new_tree, force=opts.force,
patch_mode=opts.patch)
if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1:
@@ -199,15 +208,20 @@
def interpret_args(args, dash_dash, default_commit):
- """Interpret `args` as "[commits] [--] [files]" and return (commits, files).
+ """Interpret `args` as "[commits] [--] [files]" and return (src, dest, files).
It is assumed that "--" and everything that follows has been removed from
args and placed in `dash_dash`.
If "--" is present (i.e., `dash_dash` is
