tools: Add various improvements for clang_format.py/githelper.py

2 files changed, 46 insertions, 23 deletions

diff --git a/tools/clang_format.py b/tools/clang_format.py
index 80aa419c2f..9c26c76a67 100755
--- a/tools/clang_format.py
+++ b/tools/clang_format.py
@@ -20,6 +20,7 @@ BREAK_BEFORE = 80
 
 
 def get_clang_format_config_with_columnlimit(rootdir, limit):
+    """Create a temporary config with ColumnLimit set to 80."""
     cpp_file = os.path.join(rootdir, "src/mixxx.cpp")
     proc = subprocess.run(
         ["clang-format", "--dump-config", cpp_file],
@@ -27,24 +28,32 @@ def get_clang_format_config_with_columnlimit(rootdir, limit):
         text=True,
     )
     proc.check_returncode()
-    return re.sub(r"(ColumnLimit:\s*)\d+", r"\g<1>{}".format(80), proc.stdout,)
+    return re.sub(
+        r"(ColumnLimit:\s*)\d+", r"\g<1>{}".format(BREAK_BEFORE), proc.stdout
+    )
 
 
-def run_clang_format_on_lines(rootdir, changed_file, assume_filename=None):
+def run_clang_format_on_lines(rootdir, file_to_format, stylepath=None):
     logger = logging.getLogger(__name__)
 
     line_arguments = [
-        "--lines={}:{}".format(start, end) for start, end in changed_file.lines
+        "--lines={}:{}".format(start, end)
+        for start, end in file_to_format.lines
     ]
     assert line_arguments
 
-    logger.info("Reformatting %s...", changed_file.filename)
-    filename = os.path.join(rootdir, changed_file.filename)
+    logger.info("Reformatting %s...", file_to_format.filename)
+    filename = os.path.join(rootdir, file_to_format.filename)
     cmd = [
         "clang-format",
         "--style=file",
+        # The --assume-filename argument sets the path for the .clang-format
+        # config file implcitly by assuming a different location of the file to
+        # format
         "--assume-filename={}".format(
-            assume_filename if assume_filename else filename
+            os.path.join(
+                stylepath if stylepath else rootdir, file_to_format.filename
+            )
         ),
         *line_arguments,
     ]
@@ -86,7 +95,7 @@ def main(argv: typing.Optional[typing.List[str]] = None) -> int:
     # Filter filenames
     rootdir = githelper.get_toplevel_path()
 
-    # First pass: Format added lines using clang-format
+    # First pass: Format added/changed lines using clang-format
     logger.info("First pass: Reformatting added/changed lines...")
     files_with_added_lines = githelper.get_changed_lines_grouped(
         from_ref=args.from_ref,
@@ -96,12 +105,12 @@ def main(argv: typing.Optional[typing.List[str]] = None) -> int:
     for changed_file in files_with_added_lines:
         run_clang_format_on_lines(rootdir, changed_file)
 
-    # Second pass: Wrap long added lines using clang-format
+    # Second pass: Wrap long added/changed lines using clang-format
     logger.info("Second pass: Breaking long added/changed lines...")
     files_with_long_added_lines = githelper.get_changed_lines_grouped(
         from_ref=args.from_ref,
         filter_lines=lambda line: line.added
-        and LINE_LENGTH_THRESHOLD < (len(line.text) - 1),
+        and len(line.text) > LINE_LENGTH_THRESHOLD,
         include_files=args.files,
     )
     config = get_clang_format_config_with_columnlimit(rootdir, BREAK_BEFORE)
@@ -112,11 +121,7 @@ def main(argv: typing.Optional[typing.List[str]] = None) -> int:
             configfp.write(config)
 
         for changed_file in files_with_long_added_lines:
-            run_clang_format_on_lines(
-                rootdir,
-                changed_file,
-                assume_filename=os.path.join(tempdir, changed_file.filename),
-            )
+            run_clang_format_on_lines(rootdir, changed_file, stylepath=tempdir)
     return 0
 
 
diff --git a/tools/githelper.py b/tools/githelper.py
index b7eb1656b1..29e9ee0a4e 100644
--- a/tools/githelper.py
+++ b/tools/githelper.py
@@ -26,41 +26,52 @@ def get_toplevel_path() -> str:
 def get_changed_lines(
     from_ref=None, filter_lines=None, include_files=None
 ) -> typing.Iterable[Line]:
-    """Inspect `git diff` output, yields changed lines."""
+    """Inspect `git diff-index` output, yields changed lines."""
 
     logger = logging.getLogger(__name__)
 
-    cmd = ["git", "diff", "--unified=0", from_ref if from_ref else "HEAD"]
+    cmd = [
+        "git",
+        "diff-index",
+        "--patch",
+        "--unified=0",
+        from_ref if from_ref else "HEAD",
+    ]
     if include_files:
         cmd.extend(["--", *include_files])
     logger.debug("Executing: %r", cmd)
     proc = subprocess.run(cmd, capture_output=True)
     proc.check_returncode()
     current_file = None
-    lines_left = 0
+    hunk_lines_left = 0
     for line in proc.stdout.decode(errors="replace").splitlines():
         match_file = re.match(r"^\+\+\+ b/(.*)$", line)
         if match_file:
+            # Current line contains a diff filename
+            assert hunk_lines_left == 0
             current_file = match_file.group(1)
-            lines_left = 0
             continue
 
         match_lineno = re.match(
             r"^@@ -(\d+(?:,\d+)?) \+([0-9]+(?:,[0-9]+)?) @@", line
         )
         if match_lineno:
+            # Current line contains a hunk header
+            assert current_file is not None
+            assert hunk_lines_left == 0
             start_removed, _, length_removed = match_lineno.group(1).partition(
                 ","
             )
             start_added, _, length_added = match_lineno.group(2).partition(",")
             lineno_removed = int(start_removed)
             lineno_added = int(start_added)
-            lines_left = int(length_removed) if length_removed else 1
-            lines_left += int(length_added) if length_added else 1
+            hunk_lines_left = int(length_removed) if length_removed else 1
+            hunk_lines_left += int(length_added) if length_added else 1
             continue
 
-        if lines_left and line:
-            lines_left -= 1
+        if hunk_lines_left and line:
+            # Current line contains an added/removed line
+            hunk_lines_left -= 1
 
             assert line[0] in ("+", "-")
             if line.startswith("+"):
@@ -80,11 +91,18 @@ def get_changed_lines(
             if filter_lines is None or filter_lines(lineobj):
                 yield lineobj
 
+        # If we reach this part, the line does not contain a diff filename or a
+        # hunk header and does not belog to a hunk. This means that this line
+        # will be ignored implicitly.
+
+    # Make sure we really parsed all lines from the last hunk
+    assert hunk_lines_left == 0
+
 
 def get_changed_lines_grouped(
     from_ref=None, filter_lines=None, include_files=None
 ) -> typing.Iterable[FileLines]:
-    """Inspect `git diff` output, yields changed lines grouped by file."""
+    """Inspect `git diff-index` output, yields lines grouped by file."""
 
     lines = get_changed_lines(from_ref, filter_lines, include_files)
     for filename, file_lines in itertools.groupby(