Merge pull request #3 from getdownandcode/feat

fix: filename quoting, nested globs, and small display bugs

Author: getdownandcode

.gitignore

@@ -66,6 +66,9 @@ Thumbs.db
 *.log
 *.tmp
 
+# Agent tooling droppings (not part of this project)
+.agents/
+
 #md files
 plan.md
 

src/gitmate/cli.py

@@ -125,7 +125,9 @@ def config_show() -> None:
     table.add_row("config file", str(config_mod.config_path()))
     table.add_row("model", cfg.model)
     table.add_row("commit_style", cfg.commit_style)
-    table.add_row("budget_cap_usd", str(cfg.budget_cap_usd) if cfg.budget_cap_usd else "none")
+    table.add_row(
+        "budget_cap_usd", str(cfg.budget_cap_usd) if cfg.budget_cap_usd is not None else "none"
+    )
     table.add_row("ignore_globs", ", ".join(cfg.ignore_globs) if cfg.ignore_globs else "none")
     table.add_row("api_key", "set" if key_set else "not set")
     console.print(table)

src/gitmate/diff_extractor.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import os
 import subprocess
 from collections.abc import Iterable
 from dataclasses import dataclass
@@ -72,11 +73,15 @@ def run(self, args: list[str], cwd: Path) -> str:
                 text=True,
                 timeout=self._timeout,
                 check=False,
+                # Deterministic English errors regardless of host locale.
+                env={**os.environ, "LC_ALL": "C"},
             )
         except FileNotFoundError as exc:
             raise GitCommandError("git binary not found on PATH.") from exc
         except subprocess.TimeoutExpired as exc:
             raise GitCommandError(f"git {' '.join(args)} timed out.") from exc
+        except UnicodeDecodeError as exc:
+            raise GitCommandError(f"git {' '.join(args)} output was not valid UTF-8.") from exc
         if proc.returncode != 0:
             err = proc.stderr.strip()
             if "not a git repository" in err:
@@ -85,17 +90,25 @@ def run(self, args: list[str], cwd: Path) -> str:
         return proc.stdout
 
 
+def _dir_match(posix: str, dirname: str) -> bool:
+    """True when dirname is the path itself or any parent directory in it."""
+    return posix == dirname or posix.startswith(dirname + "/") or f"/{dirname}/" in f"/{posix}"
+
+
 def matches_any(path: str, patterns: Iterable[str]) -> bool:
-    """Gitignore-flavored match: dir prefixes, bare basenames, else fnmatch."""
+    """Gitignore-flavored match: dir prefixes, bare basenames, else fnmatch.
+
+    Directory patterns (`dist/`, `dist/*`) match at any depth, so monorepo
+    `packages/ui/dist/` output is filtered just like root-level `dist/`.
+    """
     posix = PurePosixPath(path).as_posix()
     name = PurePosixPath(path).name
     for pat in patterns:
         if pat.endswith("/*"):
-            prefix = pat[:-1]
-            if posix == prefix[:-1] or posix.startswith(prefix):
+            if _dir_match(posix, pat[:-2]):
                 return True
         elif pat.endswith("/"):
-            if posix == pat[:-1] or posix.startswith(pat):
+            if _dir_match(posix, pat[:-1]):
                 return True
         elif "/" not in pat:
             if fnmatchcase(name, pat):
@@ -174,18 +187,91 @@ def _parse_name_status(output: str) -> dict[str, tuple[FileStatus, str | None]]:
             if len(parts) != 3:
                 raise GitCommandError(f"cannot parse rename/copy row: {line!r}.")
             _, old, new = parts
-            statuses[new] = (status, old)
+            statuses[_decode_quoted(new)] = (status, _decode_quoted(old))
         else:
             if len(parts) != 2:
                 raise GitCommandError(f"cannot parse status row: {line!r}.")
-            statuses[parts[1]] = (status, None)
+            statuses[_decode_quoted(parts[1])] = (status, None)
     return statuses
 
 
-def _strip_git_prefix(path: str) -> str:
-    """Strip git's a//b/ prefixes and optional C-style quoting."""
+def _split_header_fallback(rest: str) -> tuple[str | None, str | None]:
+    """Split `a/X b/Y` across quoted/unquoted combinations on either side.
+
+    Our git flags pin the prefixes, so Y always begins `b/` or `"b/`, and
+    rfind lets the true boundary win over interior lookalikes. Rename/copy
+    metadata and ---/+++ re-keying backstop anything stranger still.
+    """
+    if rest.endswith('"'):
+        idx = rest.rfind(' "b/')
+        if idx == -1:
+            return None, None
+        return _strip_git_prefix(rest[:idx]), _strip_git_prefix(rest[idx + 1 :])
+    idx = rest.rfind(" b/")
+    if idx < 2:
+        return None, None
+    return _strip_git_prefix(rest[:idx]), _strip_git_prefix(rest[idx + 1 :])
+
+
+def _unescape_c_quoting(text: str) -> str:
+    """Decode git's C-style path escapes (`\\\\`, `\\"`, `\\n`, `\\t`, octal).
+
+    Only meaningful on paths git quoted; unquoted output (e.g. `-z --numstat`)
+    is never munged and must pass through untouched. Octal runs decode as
+    UTF-8 bytes because git escapes non-ASCII per byte, not per character.
+    """
+    out: list[str] = []
+    raw = bytearray()
+
+    def flush_octal() -> None:
+        if raw:
+            out.append(bytes(raw).decode("utf-8", errors="surrogateescape"))
+            raw.clear()
+
+    i, n = 0, len(text)
+    while i < n:
+        ch = text[i]
+        if ch != "\\" or i + 1 >= n:
+            flush_octal()
+            out.append(ch)
+            i += 1
+            continue
+        nxt = text[i + 1]
+        if nxt in "01234567":
+            digits = text[i + 1 : i + 4]
+            length = 0
+            while length < 3 and length < len(digits) and digits[length] in "01234567":
+                length += 1
+            raw.extend(int(digits[:length], 8).to_bytes(1, "big"))
+            i += 1 + length
+            continue
+        flush_octal()
+        if nxt == "n":
+            out.append("\n")
+        elif nxt == "t":
+            out.append("\t")
+        elif nxt == "\\":
+            out.append("\\")
+        elif nxt == '"':
+            out.append('"')
+        else:
+            out.append(ch)
+            out.append(nxt)
+        i += 2
+    flush_octal()
+    return "".join(out)
+
+
+def _decode_quoted(path: str) -> str:
+    """Remove git's C-style quoting when present; bare paths pass through."""
     if len(path) >= 2 and path.startswith('"') and path.endswith('"'):
-        path = path[1:-1]
+        return _unescape_c_quoting(path[1:-1])
+    return path
+
+
+def _strip_git_prefix(path: str) -> str:
+    """Strip git's a//b/ prefixes, unescaping C-style quoting when present."""
+    path = _decode_quoted(path)
     if path.startswith(("a/", "b/")):
         return path[2:]
     return path
@@ -202,11 +288,13 @@ def _parse_patch(output: str) -> dict[str, str]:
     def flush() -> None:
         nonlocal current, minus
         if current is not None or minus is not None:
+            # Rename/copy lines carry bare paths (never a//b/ prefixed), so
+            # decode quoting only; stripping would eat real a//b/ directories.
             for cl in chunk:
                 if cl.startswith(("rename to ", "copy to ")):
-                    current = _strip_git_prefix(cl.split(" ", 2)[2].rstrip("\n"))
+                    current = _decode_quoted(cl.split(" ", 2)[2].rstrip("\n"))
                 elif cl.startswith(("rename from ", "copy from ")):
-                    minus = _strip_git_prefix(cl.split(" ", 2)[2].rstrip("\n"))
+                    minus = _decode_quoted(cl.split(" ", 2)[2].rstrip("\n"))
             key = current if current is not None and current != "/dev/null" else minus
             if key is not None:
                 if key not in chunks:
@@ -224,13 +312,9 @@ def flush() -> None:
                     minus = _strip_git_prefix(rest[2:mid])
                     current = _strip_git_prefix(rest[mid + 3 :])
                 else:
-                    halves = rest.rsplit(" ", 1)
-                    minus = _strip_git_prefix(halves[0].strip()) if len(halves) == 2 else None
-                    current = _strip_git_prefix(halves[1].strip()) if len(halves) == 2 else None
+                    minus, current = _split_header_fallback(rest)
             else:
-                halves = rest.rsplit(" ", 1)
-                minus = _strip_git_prefix(halves[0].strip()) if len(halves) == 2 else None
-                current = _strip_git_prefix(halves[1].strip()) if len(halves) == 2 else None
+                minus, current = _split_header_fallback(rest)
             chunk = [line]
         elif current is None and minus is None:
             continue
@@ -248,6 +332,7 @@ def flush() -> None:
                     for prev in text.splitlines():
                         if prev.startswith("--- "):
                             key = _strip_git_prefix(prev[4:].strip())
+                            break
                 else:
                     key = plus
                 break

tests/test_cli.py

@@ -58,6 +58,16 @@ def test_config_set_field_persists(tmp_config_dir: Path) -> None:
     assert load_config().model == "my-model"
 
 
+def test_config_show_displays_zero_budget(
+    tmp_config_dir: Path, mem_store: InMemorySecretStore
+) -> None:
+    result = runner.invoke(cli.app, ["config", "set", "budget_cap_usd", "0"])
+    assert result.exit_code == 0
+    result = runner.invoke(cli.app, ["config", "show"])
+    assert result.exit_code == 0
+    assert "0.0" in result.output
+
+
 def test_config_set_unknown_field_fails(tmp_config_dir: Path) -> None:
     result = runner.invoke(cli.app, ["config", "set", "nope", "x"])
     assert result.exit_code != 0

tests/test_diff_extractor.py

@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import os
 import subprocess
 from pathlib import Path
 
@@ -88,6 +89,9 @@ def test_matcher_rules() -> None:
     assert matches_any("a/b/c.min.js", ["*.min.js"])
     assert not matches_any("src/app.py", ["dist/*", "*.min.js"])
     assert matches_any("out/log.txt", ["out/"])
+    assert matches_any("packages/ui/dist/bundle.js", ["dist/*"])
+    assert matches_any("packages/ui/dist/bundle.js", ["dist/"])
+    assert not matches_any("src/distraction.py", ["dist/*"])
 
 
 def test_branch_comparison(git_repo: Path) -> None:
@@ -178,3 +182,79 @@ def test_pure_rename_with_spaces(git_repo: Path) -> None:
 def test_copy_status_row() -> None:
     statuses = _parse_name_status("C100\told.py\tnew.py\n")
     assert statuses["new.py"] == ("added", "old.py")
+
+
+def test_nested_monorepo_dist_filtered(git_repo: Path) -> None:
+    stage_file(git_repo, "packages/ui/dist/bundle.js", b"var x;\n")
+    stage_file(git_repo, "src/ok.py", b"ok\n")
+    (diff,) = DiffExtractor(git_repo).staged()
+    assert diff.path == "src/ok.py"
+
+
+def test_deleted_file_with_dash_comments_keeps_patch(git_repo: Path) -> None:
+    commit_file(git_repo, "query.sql", "SELECT 1;\n-- a comment\n", "add sql")
+    subprocess.run(["git", "rm", "query.sql"], cwd=git_repo, check=True)
+    (diff,) = DiffExtractor(git_repo).staged()
+    assert (diff.path, diff.status) == ("query.sql", "deleted")
+    assert "diff --git a/query.sql b/query.sql" in diff.patch_text
+    assert "-- a comment" in diff.patch_text
+
+
+def test_backslash_filename_single_entry(git_repo: Path) -> None:
+    # A literal backslash is always C-quoted by git, even with quotepath=false;
+    # the entry must align with the raw -z numstat key, not duplicate.
+    stage_file(git_repo, "back\\slash.py", b"x = 1\n")
+    (diff,) = DiffExtractor(git_repo).staged()
+    assert diff.path == "back\\slash.py"
+    assert (diff.status, diff.additions) == ("added", 1)
+    assert "+x = 1" in diff.patch_text
+
+
+def test_double_quote_filename_single_entry(git_repo: Path) -> None:
+    stage_file(git_repo, 'we"ird.py', b"x = 1\n")
+    (diff,) = DiffExtractor(git_repo).staged()
+    assert diff.path == 'we"ird.py'
+    assert diff.additions == 1
+
+
+def test_non_utf8_git_output_clean_error(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    # macOS refuses to create non-UTF-8 filenames, so fake the git binary
+    # itself emitting invalid bytes instead.
+    bindir = tmp_path / "bin"
+    bindir.mkdir()
+    fake = bindir / "git"
+    fake.write_bytes(b"#!/bin/sh\nprintf 'x\\377y'\n")
+    fake.chmod(0o755)
+    monkeypatch.setenv("PATH", f"{bindir}{os.pathsep}{os.environ['PATH']}")
+    with pytest.raises(GitCommandError):
+        DiffExtractor(tmp_path).staged()
+
+
+def test_rename_plain_to_quoted_keeps_patch(git_repo: Path) -> None:
+    commit_file(git_repo, "plain.txt", "line 1\n", "add plain")
+    subprocess.run(["git", "mv", "plain.txt", 'weird"name.txt'], cwd=git_repo, check=True)
+    (git_repo / 'weird"name.txt').write_text("line 1\nline 2\n", encoding="utf-8")
+    subprocess.run(["git", "add", "-A"], cwd=git_repo, check=True)
+    (diff,) = DiffExtractor(git_repo).staged()
+    assert (diff.path, diff.old_path, diff.status) == ('weird"name.txt', "plain.txt", "renamed")
+    assert "+line 2" in diff.patch_text
+
+
+def test_rename_quoted_to_plain_keeps_patch(git_repo: Path) -> None:
+    commit_file(git_repo, 'weird"name.txt', "line 1\n", "add weird")
+    subprocess.run(["git", "mv", 'weird"name.txt', "plain.txt"], cwd=git_repo, check=True)
+    (git_repo / "plain.txt").write_text("line 1\nline 2\n", encoding="utf-8")
+    subprocess.run(["git", "add", "-A"], cwd=git_repo, check=True)
+    (diff,) = DiffExtractor(git_repo).staged()
+    assert (diff.path, diff.old_path, diff.status) == ("plain.txt", 'weird"name.txt', "renamed")
+    assert "+line 2" in diff.patch_text
+
+
+def test_pure_rename_into_a_dir_keeps_patch(git_repo: Path) -> None:
+    commit_file(git_repo, "old.py", "line 1\n", "add old")
+    (git_repo / "a").mkdir()
+    subprocess.run(["git", "mv", "old.py", "a/foo.py"], cwd=git_repo, check=True)
+    subprocess.run(["git", "add", "-A"], cwd=git_repo, check=True)
+    (diff,) = DiffExtractor(git_repo).staged()
+    assert (diff.path, diff.old_path, diff.status) == ("a/foo.py", "old.py", "renamed")
+    assert "rename to a/foo.py" in diff.patch_text