llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-format Author: eoineoineoin <details> <summary>Changes</summary> Today, git-clang-format will only format lines which have been modified. However, in some cases, that's not sufficient to get a "clean" file which would be unmodified by running `clang-format` manually. I've got a minimal repro using the default clang-format rules. Setup a new git repository and create a commit with an empty file: ``` mkdir /tmp/bla cd /tmp/bla git init touch t.cpp git add t.cpp git commit -m "V1" ``` Add a line to that file containing a comment: ``` echo "int x = 0; // Comment describing x" > t.cpp git add t.cpp git clang-format --staged # Reports "clang-format did not modify any files" git commit -m "V2" ``` There's nothing wrong with this file yet, and it's working as expected. Problem arises when we add a new line with a comment which is not aligned with the comment we've already added: ``` echo "int longerThanX = 0; // Should cause above to become aligned" >> t.cpp git add t.cpp git clang-format --staged # Reports "clang-format did not modify any files" ``` git-clang-format didn't change the staged file here as it's only inspecting the modified lines, but according to the default formatting rules, this new line actually has an effect on the preceding line. i.e., if you run `clang-format t.cpp`, you'll see that whitespace is added before the "Comment describing x" to align with the "Should cause..." It's not limited to _just_ the preceding line (e.g. in the case where we have multiple lines like the first with comments which become unaligned,) so can't just add more lines of context to the diff. This PR just adds an option to format the whole file, instead of just the modified sections. In accordance with the contribution guide, this is my first-time contribution so I don't have permission to add a reviewer; pinging @<!-- -->owenca @<!-- -->HazardyKnusperkeks - don't an explicit maintainers list for this directory, but you've reviewed changes to this file lately - apologies for the ping if I'm incorrect here. --- Full diff: https://github.com/llvm/llvm-project/pull/204336.diff 1 Files Affected: - (modified) clang/tools/clang-format/git-clang-format (+21-4) ``````````diff diff --git a/clang/tools/clang-format/git-clang-format b/clang/tools/clang-format/git-clang-format index d79b57e7f6e10..f630cd4f8f28c 100755 --- a/clang/tools/clang-format/git-clang-format +++ b/clang/tools/clang-format/git-clang-format @@ -190,6 +190,11 @@ def main(): action="store_true", help="format lines in the stage instead of the working dir", ) + p.add_argument( + "--whole-file", + action="store_true", + help="format whole file instead of only modified lines", + ) p.add_argument( "--style", default=config.get("clangformat.style", None), @@ -256,7 +261,7 @@ def main(): opts.binary = os.path.abspath(opts.binary) changed_lines = compute_diff_and_extract_lines( - commits, files, opts.staged, opts.diff_from_common_commit + commits, files, opts.staged, opts.whole_file, opts.diff_from_common_commit ) if opts.verbose >= 1: ignored_files = set(changed_lines) @@ -410,10 +415,10 @@ def get_object_type(value): return convert_string(stdout.strip()) -def compute_diff_and_extract_lines(commits, files, staged, diff_common_commit): +def compute_diff_and_extract_lines(commits, files, staged, whole_file, diff_common_commit): """Calls compute_diff() followed by extract_lines().""" diff_process = compute_diff(commits, files, staged, diff_common_commit) - changed_lines = extract_lines(diff_process.stdout) + changed_lines = extract_lines(diff_process.stdout, whole_file) diff_process.stdout.close() diff_process.wait() if diff_process.returncode != 0: @@ -446,7 +451,7 @@ def compute_diff(commits, files, staged, diff_common_commit): return p -def extract_lines(patch_file): +def extract_lines(patch_file, whole_file): """Extract the changed lines in `patch_file`. The return value is a dictionary mapping filename to a list of (start_line, @@ -456,11 +461,23 @@ def extract_lines(patch_file): zero lines of context. The return value is a dict mapping filename to a list of line `Range`s.""" matches = {} + + if whole_file: + # Find the repository root, in case this is run in a subdirectory + cmd = ['git', 'rev-parse', '--show-toplevel'] + p = subprocess.Popen(cmd, stdout=subprocess.PIPE) + repo_root = convert_string(p.stdout.read()).strip() + for line in patch_file: line = convert_string(line) match = re.search(r"^\+\+\+\ [^/]+/(.*)", line) if match: filename = match.group(1).rstrip("\r\n\t") + if whole_file and filename not in matches: + full_path = os.path.join(repo_root, filename) + num_lines = sum(1 for _ in open(full_path)) + matches.setdefault(filename, []).append(Range(1, num_lines)) + continue match = re.search(r"^@@ -[0-9,]+ \+(\d+)(,(\d+))?", line) if match: start_line = int(match.group(1)) `````````` </details> https://github.com/llvm/llvm-project/pull/204336 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
