diff --git a/core/tools/implementations/grepSearch.ts b/core/tools/implementations/grepSearch.ts index 69ccf6aacc3..94bca2d62cf 100644 --- a/core/tools/implementations/grepSearch.ts +++ b/core/tools/implementations/grepSearch.ts @@ -1,7 +1,10 @@ import { ToolImpl } from "."; import { ContextItem } from "../.."; import { ContinueError, ContinueErrorReason } from "../../util/errors"; -import { formatGrepSearchResults } from "../../util/grepSearch"; +import { + formatGrepSearchResults, + GREP_RESULT_PATH_PREFIX_SOURCE, +} from "../../util/grepSearch"; import { prepareQueryForRipgrep } from "../../util/regexValidator"; import { getStringArg } from "../parseArgs"; @@ -9,13 +12,21 @@ const DEFAULT_GREP_SEARCH_RESULTS_LIMIT = 100; const DEFAULT_GREP_SEARCH_CHAR_LIMIT = 7500; // ~1500 tokens, will keep truncation simply for now function splitGrepResultsByFile(content: string): ContextItem[] { - const matches = [...content.matchAll(/^\.\/([^\n]+)$/gm)]; + // `.\path` on Windows, `./path` elsewhere — see isGrepResultPathLine. + const headingRegex = new RegExp( + `^${GREP_RESULT_PATH_PREFIX_SOURCE}([^\\n]+)$`, + "gm", + ); + const matches = [...content.matchAll(headingRegex)]; const contextItems: ContextItem[] = []; for (let i = 0; i < matches.length; i++) { const match = matches[i]; - const filepath = match[1]; + // Normalise separators: this becomes a `file` context-item URI, and those + // are glob-matched for rule application, where a backslash is an escape + // character rather than a separator. + const filepath = match[1].replace(/\\/g, "/"); const startIndex = match.index!; const endIndex = i < matches.length - 1 ? matches[i + 1].index! : content.length; @@ -23,7 +34,7 @@ function splitGrepResultsByFile(content: string): ContextItem[] { // Extract grepped content for this file const fileContent = content .substring(startIndex, endIndex) - .replace(/^\.\/[^\n]+\n/, "") // remove the line with file path + .replace(new RegExp(`^${GREP_RESULT_PATH_PREFIX_SOURCE}[^\\n]+\\n`), "") // remove the line with file path .trim(); if (fileContent) { diff --git a/core/tools/implementations/grepSearch.vitest.ts b/core/tools/implementations/grepSearch.vitest.ts new file mode 100644 index 00000000000..6aead0c8894 --- /dev/null +++ b/core/tools/implementations/grepSearch.vitest.ts @@ -0,0 +1,86 @@ +import { expect, test, vi } from "vitest"; + +import { ToolExtras } from "../.."; + +import { grepSearchImpl } from "./grepSearch"; + +function extrasReturning(results: string) { + return { + fetch: vi.fn() as any, + ide: { + getSearchResults: vi.fn().mockResolvedValue(results), + } as any, + } as unknown as ToolExtras; +} + +// ripgrep is run with `.` as the search root, so `--heading` echoes that root +// back using the platform separator: `./path` on POSIX, `.\path` on Windows. +const posixResults = "./src/calc.ts\n subtract(n) {\n return this;"; +const windowsResults = ".\\src\\calc.ts\n subtract(n) {\n return this;"; + +test("returns results for POSIX-style headings", async () => { + const result = await grepSearchImpl( + { query: "subtract" }, + extrasReturning(posixResults), + ); + + expect(result).toHaveLength(1); + expect(result[0].content).toContain("subtract(n) {"); +}); + +test("returns results for Windows-style headings", async () => { + // The reported bug: ripgrep found the match, but no heading was recognised, + // so numResults stayed 0 and the tool answered "no results" with the content + // sitting right there in its hand. + const result = await grepSearchImpl( + { query: "subtract" }, + extrasReturning(windowsResults), + ); + + expect(result).toHaveLength(1); + expect(result[0].content).not.toBe("The search returned no results."); + expect(result[0].content).toContain("subtract(n) {"); +}); + +test("splits Windows results per file and normalises the URI separators", async () => { + const result = await grepSearchImpl( + { query: "subtract", splitByFile: true }, + extrasReturning( + `${windowsResults}\n.\\test.py\n def subtract(self):\n pass`, + ), + ); + + expect(result).toHaveLength(2); + // Forward slashes, even though ripgrep reported backslashes: this value is a + // `file` context-item URI, and those get glob-matched to decide which rules + // apply — and in a glob a backslash escapes the next character rather than + // separating path segments, so `src\calc.ts` would match nothing. + expect(result[0].uri).toEqual({ type: "file", value: "src/calc.ts" }); + expect(result[1].uri).toEqual({ type: "file", value: "test.py" }); + // The heading line itself is stripped from each chunk's content. + expect(result[0].content).toBe("subtract(n) {\n return this;"); + expect(result[1].content).toBe("def subtract(self):\n pass"); +}); + +test("splits POSIX results per file", async () => { + const result = await grepSearchImpl( + { query: "subtract", splitByFile: true }, + extrasReturning( + `${posixResults}\n./test.py\n def subtract(self):\n pass`, + ), + ); + + expect(result).toHaveLength(2); + expect(result[0].uri).toEqual({ type: "file", value: "src/calc.ts" }); + expect(result[1].uri).toEqual({ type: "file", value: "test.py" }); +}); + +test("still reports genuinely empty searches as empty", async () => { + const result = await grepSearchImpl( + { query: "nothing" }, + extrasReturning(""), + ); + + expect(result).toHaveLength(1); + expect(result[0].content).toBe("The search returned no results."); +}); diff --git a/core/util/grepSearch.ts b/core/util/grepSearch.ts index be5b7ee082f..3c7767f3b70 100644 --- a/core/util/grepSearch.ts +++ b/core/util/grepSearch.ts @@ -1,8 +1,26 @@ +/* + ripgrep is invoked with `.` as the search root, and with `--heading` it echoes + that root back on the file-heading line using the platform separator: `./path` + on POSIX, but `.\path` on Windows. Every parser of this output has to accept + both, or Windows results are silently discarded — the content is all there, + but no heading is ever recognised, so the result count stays 0. +*/ +export function isGrepResultPathLine(line: string): boolean { + return line.startsWith("./") || line.startsWith(".\\"); +} + +/** + * Regex source for the same heading prefix, for callers that need it inside a + * larger pattern. Kept next to {@link isGrepResultPathLine} so the two cannot + * drift apart. + */ +export const GREP_RESULT_PATH_PREFIX_SOURCE = "\\.[\\\\/]"; + /* Formats the output of a grep search to reduce unnecessary indentation, lines, etc Assumes a command with these params ripgrep -i --ignore-file .continueignore --ignore-file .gitignore -C 2 --heading -m 100 -e . - + Also can truncate the output to a specified number of characters */ export function formatGrepSearchResults( @@ -57,7 +75,7 @@ export function formatGrepSearchResults( let resultLines: string[] = []; for (const line of results.split("\n").filter((l) => !!l)) { - if (line.startsWith("./") || line === "--") { + if (isGrepResultPathLine(line) || line === "--") { processResult(resultLines); // process previous result resultLines = [line]; numResults++; diff --git a/core/util/grepSearch.vitest.ts b/core/util/grepSearch.vitest.ts index df1a71efd82..bdde3e75518 100644 --- a/core/util/grepSearch.vitest.ts +++ b/core/util/grepSearch.vitest.ts @@ -215,3 +215,57 @@ test("decreases indentation when original is more than 2 spaces", () => { expect(result.formatted).toContain(" tooMuchIndent();"); expect(result.formatted).toContain(" }"); }); + +// ripgrep echoes the `.` search root back with the platform separator, so on +// Windows every heading arrives as `.\path\file` rather than `./path/file`. +// Headings that go unrecognised are not just mis-titled: processResult() only +// keeps lines that follow a heading, so the whole result set is dropped and +// numResults stays 0 — the caller reports "no results" with the matches in hand. +const sampleWindowsGrepOutput = `.\\program.cs + Console.WriteLine("Hello World!"); +-- + } + +.\\src\\test.kt + fun subtract(number: Double): Test { + result -= number`; + +test("formats Windows-style backslash headings", () => { + const result = formatGrepSearchResults(sampleWindowsGrepOutput); + + expect(result.numResults).toBe(3); + expect(result.formatted).toContain(".\\program.cs"); + expect(result.formatted).toContain(".\\src\\test.kt"); + expect(result.formatted).toContain('Console.WriteLine("Hello World!");'); + expect(result.formatted).toContain("fun subtract(number: Double): Test {"); +}); + +test("counts Windows headings so results are not reported as empty", () => { + // The reported symptom: content present, numResults 0, caller says + // "The search returned no results." + const result = formatGrepSearchResults(".\\file.ts\n const x = 1;"); + + expect(result.numResults).toBe(1); + expect(result.formatted).toBe(".\\file.ts\n const x = 1;"); +}); + +test("handles mixed separators in a single result set", () => { + // Multi-root workspaces concatenate one ripgrep run per directory, so a + // single string can carry both forms. + const input = "./posix.ts\n a();\n.\\windows.ts\n b();"; + const result = formatGrepSearchResults(input); + + expect(result.numResults).toBe(2); + expect(result.formatted).toContain("./posix.ts"); + expect(result.formatted).toContain(".\\windows.ts"); +}); + +test("does not treat a bare relative path as a heading", () => { + // Only `./` and `.\` are headings. A content line that merely starts with a + // dot must not open a new result. + const result = formatGrepSearchResults("./file.ts\n ...spread\n .method()"); + + expect(result.numResults).toBe(1); + expect(result.formatted).toContain(" ...spread"); + expect(result.formatted).toContain(" .method()"); +}); diff --git a/extensions/vscode/src/VsCodeIde.ts b/extensions/vscode/src/VsCodeIde.ts index 9770d318898..1f1ae5884ca 100644 --- a/extensions/vscode/src/VsCodeIde.ts +++ b/extensions/vscode/src/VsCodeIde.ts @@ -4,6 +4,7 @@ import { exec } from "node:child_process"; import { Range } from "core"; import { EXTENSION_NAME } from "core/util/constants"; import { DEFAULT_IGNORES, defaultIgnoresGlob } from "core/indexing/ignore"; +import { GREP_RESULT_PATH_PREFIX_SOURCE } from "core/util/grepSearch"; import * as URI from "uri-js"; import * as vscode from "vscode"; @@ -616,8 +617,12 @@ class VsCodeIde implements IDE { if (maxResults) { // In case of multiple workspaces, do max results per workspace and then truncate to maxResults // Will prioritize first workspace results, fine for now - // Results are separated by either ./ or -- - const matches = Array.from(allResults.matchAll(/(\n--|\n\.\/)/g)); + // Results are separated by either ./ or -- (.\ on Windows) + const matches = Array.from( + allResults.matchAll( + new RegExp(`(\\n--|\\n${GREP_RESULT_PATH_PREFIX_SOURCE})`, "g"), + ), + ); if (matches.length > maxResults) { return allResults.substring(0, matches[maxResults].index); } else {