Skip to content

grep tool: parse ripgrep output correctly on Windows drive paths - #27

Closed
Saidheerajgollu wants to merge 1 commit into
andrewyng:mainfrom
Saidheerajgollu:fix-rg-parse-windows-drive-paths
Closed

grep tool: parse ripgrep output correctly on Windows drive paths#27
Saidheerajgollu wants to merge 1 commit into
andrewyng:mainfrom
Saidheerajgollu:fix-rg-parse-windows-drive-paths

Conversation

@Saidheerajgollu

Copy link
Copy Markdown
Contributor

The bug

The grep tool invokes ripgrep with an absolute search base (cmd.append(str(base)) in coworker/tools/search.py), so rg echoes absolute paths in its path:line:text output. _parse_rg then splits each line with line.split(":", 2) — which on Windows breaks on the drive letter:

C:\ws\src\a.py:12:def hello():
└── file="C", line="\ws\src\a.py" (→ 0), text="12:def hello():"

Every match the agent sees on Windows has a one-letter filename, line 0, and the real line number glued to the front of the text — the tool is effectively unusable there, and the model can't cite or re-read anything it "found". POSIX is unaffected, which is why the existing tests (and everyday macOS use) never caught it.

The fix

Match lines are parsed with a regex whose path group tolerates an optional [A-Za-z]: drive prefix and whose line group requires digits:

_RG_LINE = re.compile(r"^(?P<file>(?:[A-Za-z]:)?[^:]*):(?P<line>\d+):(?P<text>.*)$")

POSIX paths and match text containing colons (URLs, timestamps) parse exactly as before; non-match lines are now skipped instead of misparsed, since the line group must be numeric.

Tests

Two new tests in tests/test_code_tools.py exercise _parse_rg directly (it's a pure function, so the Windows shape is testable from any OS): a POSIX line with colons in the match text, and a Windows drive-letter line asserting the file isn't truncated to C and the line number survives. tests/test_code_tools.py: 16 passed.

Made with Cursor

The grep tool parsed each rg match line with split(":", 2). rg is
invoked with an absolute search base, so on Windows every line comes
back as C:\ws\src\a.py:12:text - the split yields file="C",
line="\ws\src\a.py" (not a digit, coerced to 0), and the real line
number ends up glued to the front of the text. Every grep result on
Windows is garbage: unusable file names, line 0, wrong text.

Match lines are now parsed with a regex whose path group tolerates an
optional drive prefix; the line group requires digits, so stray output
lines are skipped instead of misparsed. POSIX output and matches whose
text contains colons parse exactly as before.

Co-authored-by: Cursor <cursoragent@cursor.com>
@Saidheerajgollu

Copy link
Copy Markdown
Contributor Author

Closing this one — #20 landed a few minutes earlier and takes an equivalent approach (anchor the parse on the numeric :line: field instead of a positional split). One thing worth carrying over from here regardless of which patch lands: a POSIX-side regression test asserting that colons inside the matched text (URLs, dict literals) still parse, since that's the other way a rewrite of this parser can silently regress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant