Re: [PATCH] D24319: clang-format: Add an option to git-clang-format to diff between to commits
lhchavez added a comment. Gentle ping. Is there anything else that needs addressing? Did I miss anything? https://reviews.llvm.org/D24319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24319: clang-format: Add an option to git-clang-format to diff between to commits
lhchavez updated this revision to Diff 72064.
lhchavez added a comment.
Addressed lovato's comments
https://reviews.llvm.org/D24319
Files:
tools/clang-format/git-clang-format
Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -32,12 +32,15 @@
import subprocess
import sys
-usage = 'git clang-format [OPTIONS] [] [--] [...]'
+usage = 'git clang-format [OPTIONS] [] [] [--] [...]'
desc = '''
-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.
+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.
+
+If two commits are given (requires --diff), run clang-format on all lines in the
+second that differ from the first .
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -121,8 +124,14 @@
opts.verbose -= opts.quiet
del opts.quiet
- commit, files = interpret_args(opts.args, dash_dash, opts.commit)
- changed_lines = compute_diff_and_extract_lines(commit, files)
+ 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)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -142,10 +151,17 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- 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)
+ 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)
if opts.verbose >= 1:
print 'old tree:', old_tree
print 'new tree:', new_tree
@@ -182,40 +198,41 @@
def interpret_args(args, dash_dash, default_commit):
- """Interpret `args` as "[commit] [--] [files...]" and return (commit, files).
+ """Interpret `args` as "[commits] [--] [files]" and return (commits, 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 argument to its
- left (if present) is taken as commit. Otherwise, the first argument is
- checked if it is a commit or a file. If commit is not given,
- `default_commit` is used."""
+ 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."""
if dash_dash:
if len(args) == 0:
- commit = default_commit
-elif len(args) > 1:
- die('at most one commit allowed; %d given' % len(args))
+ commits = [default_commit]
else:
- commit = args[0]
-object_type = get_object_type(commit)
-if object_type not in ('commit', 'tag'):
- if object_type is None:
-die("'%s' is not a commit" % commit)
- else:
-die("'%s' is a %s, but a commit was expected" % (commit, object_type))
+ commits = args
+for commit in commits:
+ object_type = get_object_type(commit)
+ if object_type not in ('commit', 'tag'):
+if object_type is None:
+ die("'%s' is not a commit" % commit)
+else:
+ die("'%s' is a %s, but a commit was expected" % (commit, object_type))
files = dash_dash[1:]
elif args:
-if disambiguate_revision(args[0]):
- commit = args[0]
- files = args[1:]
-else:
- commit = default_commit
- files = args
+commits = []
+while args:
+ if not disambiguate_revision(args[0]):
+break
+ commits.append(args.pop(0))
+if not commits:
+ commits = [default_commit]
+files = args
else:
-commit = def
Re: [PATCH] D24319: clang-format: Add an option to git-clang-format to diff between to commits
lhchavez marked 9 inline comments as done. lhchavez added a comment. Addressed all comments. PTAAL. https://reviews.llvm.org/D24319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
lhchavez created this revision.
lhchavez added a reviewer: djasper.
lhchavez added subscribers: srhines, cfe-commits.
lhchavez set the repository for this revision to rL LLVM.
lhchavez added a project: clang-c.
When building pre-upload hooks using git-clang-format, it is useful to limit
the scope to a single commit (instead of from a commit against the working
tree) to allow for less false positives in dependent commits.
This change adds a flag (--single-commit) to git-clang-format, which uses a
different strategy to diff (using git-diff-tree instead of git-diff-index) and
create the temporary working tree (using the git hashes of the objects present
in the specified commit instead of the ones found in the current working
directory).
Repository:
rL LLVM
https://reviews.llvm.org/D24319
Files:
cfe/trunk/tools/clang-format/git-clang-format
Index: cfe/trunk/tools/clang-format/git-clang-format
===
--- cfe/trunk/tools/clang-format/git-clang-format
+++ cfe/trunk/tools/clang-format/git-clang-format
@@ -35,9 +35,9 @@
usage = 'git clang-format [OPTIONS] [] [--] [...]'
desc = '''
-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 lines that differ between the working directory and
+ (which defaults to HEAD), or all lines that changed in a specific
+commit. Changes are only applied to the working directory.
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -90,6 +90,9 @@
p.add_argument('--commit',
default=config.get('clangformat.commit', 'HEAD'),
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 '
+ 'the working tree')),
p.add_argument('--diff', action='store_true',
help='print a diff instead of applying the changes')
p.add_argument('--extensions',
@@ -121,7 +124,8 @@
del opts.quiet
commit, files = interpret_args(opts.args, dash_dash, opts.commit)
- changed_lines = compute_diff_and_extract_lines(commit, files)
+ changed_lines = compute_diff_and_extract_lines(commit, files,
+ opts.single_commit)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -141,7 +145,10 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- old_tree = create_tree_from_workdir(changed_lines)
+ if opts.single_commit:
+old_tree = create_tree_from_commit(opts.commit, 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)
@@ -242,9 +249,9 @@
return stdout.strip()
-def compute_diff_and_extract_lines(commit, files):
+def compute_diff_and_extract_lines(commit, files, single_commit):
"""Calls compute_diff() followed by extract_lines()."""
- diff_process = compute_diff(commit, files)
+ diff_process = compute_diff(commit, files, single_commit)
changed_lines = extract_lines(diff_process.stdout)
diff_process.stdout.close()
diff_process.wait()
@@ -254,13 +261,16 @@
return changed_lines
-def compute_diff(commit, files):
+def compute_diff(commit, files, single_commit):
"""Return a subprocess object producing the diff from `commit`.
The return value's `stdin` file object will produce a patch with the
differences between the working directory and `commit`, filtered on `files`
(if non-empty). Zero context lines are used in the patch."""
- cmd = ['git', 'diff-index', '-p', '-U0', commit, '--']
+ git_tool = 'diff-index'
+ if single_commit:
+git_tool = 'diff-tree'
+ cmd = ['git', git_tool, '-p', '-U0', commit, '--']
cmd.extend(files)
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
p.stdin.close()
@@ -310,6 +320,29 @@
os.chdir(toplevel)
+def create_tree_from_commit(commit, filenames):
+ """Create a new git tree with the given files from `commit`.
+
+ Returns the object ID (SHA-1) of the created tree."""
+ def index_info_generator(lines, filenames):
+for line in lines:
+ match = re.match(r'^(\d+) blob ([0-9a-f]+)\t(.*)', line)
+ if not match:
+continue
+ mode, blob_id, filename = match.groups()
+ if filename not in filenames:
+continue
+ yield '%s %s\t%s' % (mode, blob_id, filename)
+ cmd = ['git', 'ls-tree', '-r', commit]
+ p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+ stdout = p.communicate()[0]
+
[PATCH] D24401: clang-format: Add Java detection to git-clang-format.
lhchavez created this revision. lhchavez added a reviewer: djasper. lhchavez added subscribers: cfe-commits, srhines. lhchavez set the repository for this revision to rL LLVM. This change adds "java" to the list of known extensions that clang-format supports. Repository: rL LLVM https://reviews.llvm.org/D24401 Files: cfe/trunk/tools/clang-format/git-clang-format Index: cfe/trunk/tools/clang-format/git-clang-format === --- cfe/trunk/tools/clang-format/git-clang-format +++ cfe/trunk/tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) Index: cfe/trunk/tools/clang-format/git-clang-format === --- cfe/trunk/tools/clang-format/git-clang-format +++ cfe/trunk/tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24401: clang-format: Add Java detection to git-clang-format.
lhchavez updated this revision to Diff 71075. lhchavez added a comment. Generated the diff properly this time, and it now shows full context. Repository: rL LLVM https://reviews.llvm.org/D24401 Files: cfe/trunk/tools/clang-format/git-clang-format Index: cfe/trunk/tools/clang-format/git-clang-format === --- cfe/trunk/tools/clang-format/git-clang-format +++ cfe/trunk/tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) Index: cfe/trunk/tools/clang-format/git-clang-format === --- cfe/trunk/tools/clang-format/git-clang-format +++ cfe/trunk/tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24401: clang-format: Add Java detection to git-clang-format.
lhchavez updated this revision to Diff 71087. lhchavez added a comment. Using arcanist to fix the paths as well https://reviews.llvm.org/D24401 Files: tools/clang-format/git-clang-format Index: tools/clang-format/git-clang-format === --- tools/clang-format/git-clang-format +++ tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) Index: tools/clang-format/git-clang-format === --- tools/clang-format/git-clang-format +++ tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
lhchavez updated this revision to Diff 71095.
lhchavez added a comment.
No-op update. Using arcanist to fix the diff and the paths.
https://reviews.llvm.org/D24319
Files:
tools/clang-format/git-clang-format
Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -35,9 +35,9 @@
usage = 'git clang-format [OPTIONS] [] [--] [...]'
desc = '''
-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 lines that differ between the working directory and
+ (which defaults to HEAD), or all lines that changed in a specific
+commit. Changes are only applied to the working directory.
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -90,6 +90,9 @@
p.add_argument('--commit',
default=config.get('clangformat.commit', 'HEAD'),
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 '
+ 'the working tree')),
p.add_argument('--diff', action='store_true',
help='print a diff instead of applying the changes')
p.add_argument('--extensions',
@@ -121,7 +124,8 @@
del opts.quiet
commit, files = interpret_args(opts.args, dash_dash, opts.commit)
- changed_lines = compute_diff_and_extract_lines(commit, files)
+ changed_lines = compute_diff_and_extract_lines(commit, files,
+ opts.single_commit)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -141,7 +145,10 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- old_tree = create_tree_from_workdir(changed_lines)
+ if opts.single_commit:
+old_tree = create_tree_from_commit(opts.commit, 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)
@@ -242,9 +249,9 @@
return stdout.strip()
-def compute_diff_and_extract_lines(commit, files):
+def compute_diff_and_extract_lines(commit, files, single_commit):
"""Calls compute_diff() followed by extract_lines()."""
- diff_process = compute_diff(commit, files)
+ diff_process = compute_diff(commit, files, single_commit)
changed_lines = extract_lines(diff_process.stdout)
diff_process.stdout.close()
diff_process.wait()
@@ -254,13 +261,16 @@
return changed_lines
-def compute_diff(commit, files):
+def compute_diff(commit, files, single_commit):
"""Return a subprocess object producing the diff from `commit`.
The return value's `stdin` file object will produce a patch with the
differences between the working directory and `commit`, filtered on `files`
(if non-empty). Zero context lines are used in the patch."""
- cmd = ['git', 'diff-index', '-p', '-U0', commit, '--']
+ git_tool = 'diff-index'
+ if single_commit:
+git_tool = 'diff-tree'
+ cmd = ['git', git_tool, '-p', '-U0', commit, '--']
cmd.extend(files)
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
p.stdin.close()
@@ -310,6 +320,29 @@
os.chdir(toplevel)
+def create_tree_from_commit(commit, filenames):
+ """Create a new git tree with the given files from `commit`.
+
+ Returns the object ID (SHA-1) of the created tree."""
+ def index_info_generator(lines, filenames):
+for line in lines:
+ match = re.match(r'^(\d+) blob ([0-9a-f]+)\t(.*)', line)
+ if not match:
+continue
+ mode, blob_id, filename = match.groups()
+ if filename not in filenames:
+continue
+ yield '%s %s\t%s' % (mode, blob_id, filename)
+ cmd = ['git', 'ls-tree', '-r', commit]
+ p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+ stdout = p.communicate()[0]
+ if p.returncode != 0:
+die('`%s` failed' % ' '.join(cmd))
+
+ return create_tree(index_info_generator(stdout.splitlines(), set(filenames)),
+ '--index-info')
+
+
def create_tree_from_workdir(filenames):
"""Create a new git tree with the given files from the working directory.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
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
> ``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 ``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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
lhchavez updated this revision to Diff 71097.
lhchavez added a comment.
Fix the script so it does what it advertises.
https://reviews.llvm.org/D24319
Files:
tools/clang-format/git-clang-format
Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -35,9 +35,9 @@
usage = 'git clang-format [OPTIONS] [] [--] [...]'
desc = '''
-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 lines that differ between the working directory and
+ (which defaults to HEAD), or all lines that changed in a specific
+commit. Changes are only applied to the working directory.
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -90,6 +90,9 @@
p.add_argument('--commit',
default=config.get('clangformat.commit', 'HEAD'),
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 '
+ 'the working tree')),
p.add_argument('--diff', action='store_true',
help='print a diff instead of applying the changes')
p.add_argument('--extensions',
@@ -121,7 +124,8 @@
del opts.quiet
commit, files = interpret_args(opts.args, dash_dash, opts.commit)
- changed_lines = compute_diff_and_extract_lines(commit, files)
+ changed_lines = compute_diff_and_extract_lines(commit, files,
+ opts.single_commit)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -141,7 +145,10 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- old_tree = create_tree_from_workdir(changed_lines)
+ if opts.single_commit:
+old_tree = create_tree_from_commit(commit, 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)
@@ -242,9 +249,9 @@
return stdout.strip()
-def compute_diff_and_extract_lines(commit, files):
+def compute_diff_and_extract_lines(commit, files, single_commit):
"""Calls compute_diff() followed by extract_lines()."""
- diff_process = compute_diff(commit, files)
+ diff_process = compute_diff(commit, files, single_commit)
changed_lines = extract_lines(diff_process.stdout)
diff_process.stdout.close()
diff_process.wait()
@@ -254,13 +261,16 @@
return changed_lines
-def compute_diff(commit, files):
+def compute_diff(commit, files, single_commit):
"""Return a subprocess object producing the diff from `commit`.
The return value's `stdin` file object will produce a patch with the
differences between the working directory and `commit`, filtered on `files`
(if non-empty). Zero context lines are used in the patch."""
- cmd = ['git', 'diff-index', '-p', '-U0', commit, '--']
+ git_tool = 'diff-index'
+ if single_commit:
+git_tool = 'diff-tree'
+ cmd = ['git', git_tool, '-p', '-U0', commit, '--']
cmd.extend(files)
p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE)
p.stdin.close()
@@ -310,6 +320,32 @@
os.chdir(toplevel)
+def create_tree_from_commit(commit, filenames):
+ """Create a new git tree with the given files from `commit`.
+
+ This reduces the size of the generated temporary index in case the project has
+ a large number of files.
+
+ Returns the object ID (SHA-1) of the created tree."""
+ def index_info_generator(lines, filenames):
+for line in lines:
+ match = re.match(r'^(\d+) blob ([0-9a-f]+)\t(.*)', line)
+ if not match:
+continue
+ mode, blob_id, filename = match.groups()
+ if filename not in filenames:
+continue
+ yield '%s %s\t%s' % (mode, blob_id, filename)
+ cmd = ['git', 'ls-tree', '-r', commit]
+ p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
+ stdout = p.communicate()[0]
+ if p.returncode != 0:
+die('`%s` failed' % ' '.join(cmd))
+
+ return create_tree(index_info_generator(stdout.splitlines(), set(filenames)),
+ '--index-info')
+
+
def create_tree_from_workdir(filenames):
"""Create a new git tree with the given files from the working directory.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
lhchavez updated this revision to Diff 71103.
lhchavez added a comment.
Using lodato's proposed interface. This patch:
- Accepts an arbitrary number of commits as arguments. Validation will be done
in main(), such that two commits are valid only when running in --diff mode.
- Allows diffing two commits against each other in addition to diffing against
the working directory (still using git diff-tree). The previous version didn't
really work with merge-commits anyways.
- Runs clang-format against the file in the working directory (if a single
commit is passed), or against the second commit.
https://reviews.llvm.org/D24319
Files:
tools/clang-format/git-clang-format
Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -35,9 +35,9 @@
usage = 'git clang-format [OPTIONS] [] [--] [...]'
desc = '''
-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 lines that differ between the working directory and
+ (which defaults to HEAD), or all lines that changed in a specific
+commit. Changes are only applied to the working directory.
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -120,8 +120,14 @@
opts.verbose -= opts.quiet
del opts.quiet
- commit, files = interpret_args(opts.args, dash_dash, opts.commit)
- changed_lines = compute_diff_and_extract_lines(commit, files)
+ commits, files = interpret_args(opts.args, dash_dash, opts.commit)
+ if not opts.diff:
+if len(commits) > 1:
+ die('at most one commit allowed; %d given' % len(commits))
+ else:
+if len(commits) > 2:
+ die('at most two commits allowed; %d given' % len(commits))
+ changed_lines = compute_diff_and_extract_lines(commits, files)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -141,10 +147,17 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- 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)
+ if len(commits) > 1:
+old_tree = create_tree_from_commit(commits[1], changed_lines)
+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)
if opts.verbose >= 1:
print 'old tree:', old_tree
print 'new tree:', new_tree
@@ -181,40 +194,42 @@
def interpret_args(args, dash_dash, default_commit):
- """Interpret `args` as "[commit] [--] [files...]" and return (commit, files).
+ """Interpret `args` as "[commits...] [--] [files...]" and return (commits,
+ 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 argument to its
- left (if present) is taken as commit. Otherwise, the first argument is
- checked if it is a commit or a file. If commit is not given,
- `default_commit` is used."""
+ 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."""
if dash_dash:
if len(args) == 0:
- commit = default_commit
-elif len(args) > 1:
- die('at most one commit allowed; %d given' % len(args))
+ commits = [default_commit]
else:
- commit = args[0]
-object_type = get_object_type(commit)
-if object_type not in ('commit', 'tag'):
- if object_type is None:
-die("'%s' is not a commit" % commit)
- else:
-die("'%s' is a %s, but a commit was expected" % (commit, object_type))
+ commits = args
+for commit in commits:
+ object_type = get_object_type(commit)
+ if object_type not in ('commit', 'tag'):
+if object_type is None:
+ die("'%s' is not a commit" % commit)
+else:
+ die("'%s' is a %s, but a commit was expected" % (commit, object_type))
files = dash_dash[1:]
elif args:
-if disambiguate
Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
lhchavez marked 3 inline comments as done.
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
'
lhchavez wrote:
> 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 ``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 ``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.
Nevermind, `git diff-tree` didn't work with merge-commits if a single commit is
passed. Changed to your proposal.
https://reviews.llvm.org/D24319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24401: clang-format: Add Java detection to git-clang-format.
lhchavez updated this revision to Diff 71104. lhchavez added a comment. Rebased to r281293 https://reviews.llvm.org/D24401 Files: tools/clang-format/git-clang-format Index: tools/clang-format/git-clang-format === --- tools/clang-format/git-clang-format +++ tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) Index: tools/clang-format/git-clang-format === --- tools/clang-format/git-clang-format +++ tools/clang-format/git-clang-format @@ -77,6 +77,7 @@ 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hpp', # C++ # Other languages that clang-format supports 'proto', 'protodevel', # Protocol Buffers + 'java', # Java 'js', # JavaScript 'ts', # TypeScript ]) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
lhchavez updated this revision to Diff 7.
lhchavez marked an inline comment as done.
lhchavez added a comment.
Got rid of create_tree_from_commit
https://reviews.llvm.org/D24319
Files:
tools/clang-format/git-clang-format
Index: tools/clang-format/git-clang-format
===
--- tools/clang-format/git-clang-format
+++ tools/clang-format/git-clang-format
@@ -35,9 +35,9 @@
usage = 'git clang-format [OPTIONS] [] [--] [...]'
desc = '''
-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 lines that differ between the working directory and
+ (which defaults to HEAD), or all lines that changed in a specific
+commit. Changes are only applied to the working directory.
The following git-config settings set the default of the corresponding option:
clangFormat.binary
@@ -121,8 +121,14 @@
opts.verbose -= opts.quiet
del opts.quiet
- commit, files = interpret_args(opts.args, dash_dash, opts.commit)
- changed_lines = compute_diff_and_extract_lines(commit, files)
+ commits, files = interpret_args(opts.args, dash_dash, opts.commit)
+ if not opts.diff:
+if len(commits) > 1:
+ die('at most one commit allowed; %d given' % len(commits))
+ else:
+if len(commits) > 2:
+ die('at most two commits allowed; %d given' % len(commits))
+ changed_lines = compute_diff_and_extract_lines(commits, files)
if opts.verbose >= 1:
ignored_files = set(changed_lines)
filter_by_extension(changed_lines, opts.extensions.lower().split(','))
@@ -142,10 +148,17 @@
# The computed diff outputs absolute paths, so we must cd before accessing
# those files.
cd_to_toplevel()
- 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)
+ if len(commits) > 1:
+old_tree = get_tree_from_commit(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)
if opts.verbose >= 1:
print 'old tree:', old_tree
print 'new tree:', new_tree
@@ -182,40 +195,42 @@
def interpret_args(args, dash_dash, default_commit):
- """Interpret `args` as "[commit] [--] [files...]" and return (commit, files).
+ """Interpret `args` as "[commits...] [--] [files...]" and return (commits,
+ 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 argument to its
- left (if present) is taken as commit. Otherwise, the first argument is
- checked if it is a commit or a file. If commit is not given,
- `default_commit` is used."""
+ 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."""
if dash_dash:
if len(args) == 0:
- commit = default_commit
-elif len(args) > 1:
- die('at most one commit allowed; %d given' % len(args))
+ commits = [default_commit]
else:
- commit = args[0]
-object_type = get_object_type(commit)
-if object_type not in ('commit', 'tag'):
- if object_type is None:
-die("'%s' is not a commit" % commit)
- else:
-die("'%s' is a %s, but a commit was expected" % (commit, object_type))
+ commits = args
+for commit in commits:
+ object_type = get_object_type(commit)
+ if object_type not in ('commit', 'tag'):
+if object_type is None:
+ die("'%s' is not a commit" % commit)
+else:
+ die("'%s' is a %s, but a commit was expected" % (commit, object_type))
files = dash_dash[1:]
elif args:
-if disambiguate_revision(args[0]):
- commit = args[0]
- files = args[1:]
-else:
- commit = default_commit
- files = args
+commits = []
+while args:
+ if not disambiguate_revision(args[0]):
+break
+ commits.append(args.pop(0))
+if not commits:
+ commits = [default_commit]
+files = args
else:
-commit = default_commit
+commits = [default_commit]
files = []
- return commit, files
+ return commits, files
Re: [PATCH] D24319: clang-format: Add a flag to limit git-clang-format's diff to a single commit
lhchavez marked 2 inline comments as done. 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`. lhchavez wrote: > 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. Wait, I understand what you mean now. I can just extract the tree from the commit and use that as diff base. Changed it to a simpler `git show` command. https://reviews.llvm.org/D24319 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
