[PATCH] D33944: git-clang-format: Add --cached option to format index
owenpan commandeered this revision. owenpan edited reviewers, added: kevinoid; removed: owenpan. owenpan added a comment. Herald added a project: All. We have `--cached` now. Herald added a comment. NOTE: Clang-Format Team Automated Review Comment It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an `NFC` or refactoring, adding documentation etc..) Add your unit tests in `clang/unittests/Format` and you can build with `ninja FormatTests`. We recommend using the `verifyFormat(xxx)` format of unit tests rather than `EXPECT_EQ` as this will ensure you change is tolerant to random whitespace changes (see FormatTest.cpp as an example) For situations where your change is altering the TokenAnnotator.cpp which can happen if you are trying to improve the annotation phase to ensure we are correctly identifying the type of a token, please add a token annotator test in `TokenAnnotatorTest.cpp` Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33944/new/ https://reviews.llvm.org/D33944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33944: git-clang-format: Add --cached option to format index
MyDeveloperDay added a comment. Is this revision still relavant, I find it annoying that I have to run git add again on any files that are already added but have been formatted. If you think this is useful and can rebase it perhaps we can go around the review cycle again with a fresh set of eyes Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33944/new/ https://reviews.llvm.org/D33944 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D33944: git-clang-format: Add --cached option to format index
kevinoid updated this revision to Diff 101799. kevinoid added a comment. Fixed handling of case when no formatting changes are made but `old_tree != new_tree` in `apply_changes`. https://reviews.llvm.org/D33944 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 @@ -92,6 +92,8 @@ p.add_argument('--binary', default=config.get('clangformat.binary', 'clang-format'), help='path to clang-format'), + p.add_argument('--cached', action='store_true', + help='format index instead of working directory'), p.add_argument('--commit', default=config.get('clangformat.commit', 'HEAD'), help='default commit to use if none is specified'), @@ -129,10 +131,12 @@ if len(commits) > 1: if not opts.diff: die('--diff is required when two commits are given') +if opts.cached: + die('--cached is not applicable 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.cached) if opts.verbose >= 1: ignored_files = set(changed_lines) filter_by_extension(changed_lines, opts.extensions.lower().split(',')) @@ -154,15 +158,17 @@ 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) +fmt_tree = commits[1] + elif opts.cached: +old_tree = run('git', 'write-tree') +fmt_tree = old_tree 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) +fmt_tree = None + new_tree = run_clang_format_and_save_to_tree(changed_lines, + revision=fmt_tree, + binary=opts.binary, + style=opts.style) if opts.verbose >= 1: print('old tree: %s' % old_tree) print('new tree: %s' % new_tree) @@ -173,7 +179,7 @@ print_diff(old_tree, new_tree) else: changed_files = apply_changes(old_tree, new_tree, force=opts.force, - patch_mode=opts.patch) + patch_mode=opts.patch, cached=opts.cached) if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1: print('changed files:') for filename in changed_files: @@ -261,9 +267,9 @@ return convert_string(stdout.strip()) -def compute_diff_and_extract_lines(commits, files): +def compute_diff_and_extract_lines(commits, files, cached=False): """Calls compute_diff() followed by extract_lines().""" - diff_process = compute_diff(commits, files) + diff_process = compute_diff(commits, files, cached) changed_lines = extract_lines(diff_process.stdout) diff_process.stdout.close() diff_process.wait() @@ -273,7 +279,7 @@ return changed_lines -def compute_diff(commits, files): +def compute_diff(commits, files, cached=False): """Return a subprocess object producing the diff from `commits`. The return value's `stdin` file object will produce a patch with the @@ -283,7 +289,11 @@ git_tool = 'diff-index' if len(commits) > 1: git_tool = 'diff-tree' - cmd = ['git', git_tool, '-p', '-U0'] + commits + ['--'] + cmd = ['git', git_tool, '-p', '-U0'] + if cached: +cmd.append('--cached') + cmd.extend(commits) + cmd.append('--') cmd.extend(files) p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE) p.stdin.close() @@ -487,23 +497,43 @@ '--']) -def apply_changes(old_tree, new_tree, force=False, patch_mode=False): - """Apply the changes in `new_tree` to the working directory. +def apply_changes(old_tree, new_tree, force=False, patch_mode=False, + cached=False): + """Apply the changes in `new_tree` to the working directory or index. Bails if there are local changes in those files and not `force`. If - `patch_mode`, runs `git checkout --patch` to select hunks interactively.""" + `patch_mode`, adds `--patch` option to select hunks interactively.""" changed_files = run('git', 'diff-tree', '--diff-filter=M', '-r', '-z', '--name-only', old_tree, new_tree).rstrip('\0').split('\0') + if changed_files == ['']:
[PATCH] D33944: git-clang-format: Add --cached option to format index
kevinoid created this revision. kevinoid added a project: clang-tools-extra. Add --cached option to git-clang-format which behaves analogously to the use of --cached for other git subcommands, by causing the operation to work against the index state rather than the working directory state. This can be particularly useful for hook scripts which need to check or change the formatting of the index state before commit. Repository: rL LLVM https://reviews.llvm.org/D33944 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 @@ -92,6 +92,8 @@ p.add_argument('--binary', default=config.get('clangformat.binary', 'clang-format'), help='path to clang-format'), + p.add_argument('--cached', action='store_true', + help='format index instead of working directory'), p.add_argument('--commit', default=config.get('clangformat.commit', 'HEAD'), help='default commit to use if none is specified'), @@ -129,10 +131,12 @@ if len(commits) > 1: if not opts.diff: die('--diff is required when two commits are given') +if opts.cached: + die('--cached is not applicable 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.cached) if opts.verbose >= 1: ignored_files = set(changed_lines) filter_by_extension(changed_lines, opts.extensions.lower().split(',')) @@ -154,15 +158,17 @@ 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) +fmt_tree = commits[1] + elif opts.cached: +old_tree = run('git', 'write-tree') +fmt_tree = old_tree 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) +fmt_tree = None + new_tree = run_clang_format_and_save_to_tree(changed_lines, + revision=fmt_tree, + binary=opts.binary, + style=opts.style) if opts.verbose >= 1: print('old tree: %s' % old_tree) print('new tree: %s' % new_tree) @@ -173,7 +179,7 @@ print_diff(old_tree, new_tree) else: changed_files = apply_changes(old_tree, new_tree, force=opts.force, - patch_mode=opts.patch) + patch_mode=opts.patch, cached=opts.cached) if (opts.verbose >= 0 and not opts.patch) or opts.verbose >= 1: print('changed files:') for filename in changed_files: @@ -261,9 +267,9 @@ return convert_string(stdout.strip()) -def compute_diff_and_extract_lines(commits, files): +def compute_diff_and_extract_lines(commits, files, cached=False): """Calls compute_diff() followed by extract_lines().""" - diff_process = compute_diff(commits, files) + diff_process = compute_diff(commits, files, cached) changed_lines = extract_lines(diff_process.stdout) diff_process.stdout.close() diff_process.wait() @@ -273,7 +279,7 @@ return changed_lines -def compute_diff(commits, files): +def compute_diff(commits, files, cached=False): """Return a subprocess object producing the diff from `commits`. The return value's `stdin` file object will produce a patch with the @@ -283,7 +289,11 @@ git_tool = 'diff-index' if len(commits) > 1: git_tool = 'diff-tree' - cmd = ['git', git_tool, '-p', '-U0'] + commits + ['--'] + cmd = ['git', git_tool, '-p', '-U0'] + if cached: +cmd.append('--cached') + cmd.extend(commits) + cmd.append('--') cmd.extend(files) p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE) p.stdin.close() @@ -487,23 +497,39 @@ '--']) -def apply_changes(old_tree, new_tree, force=False, patch_mode=False): - """Apply the changes in `new_tree` to the working directory. +def apply_changes(old_tree, new_tree, force=False, patch_mode=False, + cached=False): + """Apply the changes in `new_tree` to the working directory or index. Bails if there are local changes in those files and not `force`. If - `patch_mode`, runs `git checkout --patch` to select hunks interactively.""" + `patch_mode`,