From 0622e93bd8f82f6caec6c53a3dd9f2cd292210ec Mon Sep 17 00:00:00 2001 From: Mohamed Abdeltawab Date: Sat, 25 Jul 2026 14:42:35 +0300 Subject: [PATCH] fix: handle Windows drive-letter paths in grep (ripgrep) output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_parse_rg` split ripgrep output on ":" positionally, so a Windows absolute path like `C:\ws\a.py:12:def f()` parsed as file="C", line=0, with the real line number swallowed into the matched text. Every grep match on Windows (when `rg` is on PATH) came back corrupted. Pass `--with-filename --null` so ripgrep always prints the path and NUL-separates it from `line:text`, then split the path off on the NUL byte — a byte that cannot appear in a path — before parsing. `--null` defeats the drive-letter colon; `--with-filename` keeps the format universal so a single-file target isn't silently dropped. Portable: harmless on Linux/macOS, correct everywhere. Adds a platform-independent regression test that feeds a NUL-delimited drive-letter line through `_parse_rg` directly, since CI runs on Linux without `rg` and never exercises this code path otherwise. Closes #17 --- coworker/tools/search.py | 33 +++++++++++++++++++++++---------- tests/test_code_tools.py | 20 +++++++++++++++++++- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/coworker/tools/search.py b/coworker/tools/search.py index ad489e1fa2..0a50984bf1 100644 --- a/coworker/tools/search.py +++ b/coworker/tools/search.py @@ -91,6 +91,14 @@ def grep( "--line-number", "--no-heading", "--color=never", + # Always print the filename (even for a single-file target) and + # NUL-separate it from `line:text`, so parsing never has to guess + # where the path ends — a Windows drive-letter colon (C:\ws\a.py), + # or a colon inside the matched text, would otherwise be taken as a + # field separator (issue #17). Without --with-filename a single-file + # search emits no path and no NUL, and the parser would drop it. + "--with-filename", + "--null", "--max-count", str(n), "-e", @@ -135,18 +143,23 @@ def _rel(path: str, root: Path) -> str: def _parse_rg(stdout: str, root: Path, n: int) -> dict[str, Any]: + # `rg --null` emits each match as `\0:`. Splitting the path off + # on the NUL byte first means the colon inside a Windows drive letter (C:\...) is + # never confused with the line/text separators — the old `split(":", 2)` parsed + # `C:\ws\a.py:12:def f()` as file="C", line="\ws\a.py", text="12:def f()". matches: list[dict[str, Any]] = [] for line in stdout.splitlines(): - parts = line.split(":", 2) - if len(parts) == 3: - f, ln, txt = parts - matches.append( - { - "file": _rel(f, root), - "line": int(ln) if ln.isdigit() else 0, - "text": txt[:300], - } - ) + path, sep, rest = line.partition("\0") + if not sep: + continue + ln, _, txt = rest.partition(":") + matches.append( + { + "file": _rel(path, root), + "line": int(ln) if ln.isdigit() else 0, + "text": txt[:300], + } + ) if len(matches) >= n: break return {"count": len(matches), "matches": matches} diff --git a/tests/test_code_tools.py b/tests/test_code_tools.py index 0edc83b2f1..cb9e14e94e 100644 --- a/tests/test_code_tools.py +++ b/tests/test_code_tools.py @@ -13,7 +13,7 @@ from coworker.tools.files import file_tools from coworker.tools.git import git_tools -from coworker.tools.search import _py_grep, search_tools +from coworker.tools.search import _parse_rg, _py_grep, search_tools from coworker.web.fetch import _html_to_text, make_web_fetch_tool @@ -41,6 +41,24 @@ def test_grep_finds_matches_and_respects_glob(tmp_path): assert only_py["matches"][0]["line"] == 1 +def test_parse_rg_handles_windows_drive_letter_paths(): + # `rg --null` emits `\0:`. A Windows absolute path leads with a + # drive-letter colon (C:\...); the old `split(":", 2)` mis-parsed it as file="C", + # line=0, text="12:...". Regression for issue #17 — asserts hold on every platform. + from pathlib import Path + + stdout = "C:\\ws\\a.py\x0012:def hello():\n" + out = _parse_rg(stdout, Path("C:\\ws"), 100) + assert out["count"] == 1 + m = out["matches"][0] + assert m["line"] == 12 + assert m["text"] == "def hello():" + # the whole path survives (not truncated to the drive letter "C"); .endswith + # stays platform-independent — on Linux _rel can't relativize a `C:\` path, so it + # returns the raw string, which still ends with the filename. + assert m["file"].endswith("a.py") + + def test_ripgrep_uses_the_same_ignored_dirs_as_the_python_fallback(tmp_path, monkeypatch): import coworker.tools.search as search