Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions core/tools/implementations/grepSearch.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,40 @@
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";

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;

// 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) {
Expand Down
86 changes: 86 additions & 0 deletions core/tools/implementations/grepSearch.vitest.ts
Original file line number Diff line number Diff line change
@@ -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.");
});
22 changes: 20 additions & 2 deletions core/util/grepSearch.ts
Original file line number Diff line number Diff line change
@@ -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 <query> .

Also can truncate the output to a specified number of characters
*/
export function formatGrepSearchResults(
Expand Down Expand Up @@ -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++;
Expand Down
54 changes: 54 additions & 0 deletions core/util/grepSearch.vitest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()");
});
9 changes: 7 additions & 2 deletions extensions/vscode/src/VsCodeIde.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@

import { Range } from "core";
import { EXTENSION_NAME } from "core/util/constants";
import { DEFAULT_IGNORES, defaultIgnoresGlob } from "core/indexing/ignore";

Check warning on line 6 in extensions/vscode/src/VsCodeIde.ts

View workflow job for this annotation

GitHub Actions / vscode-checks

`core/indexing/ignore` import should occur before import of `core/util/constants`
import { GREP_RESULT_PATH_PREFIX_SOURCE } from "core/util/grepSearch";
import * as URI from "uri-js";
import * as vscode from "vscode";

Expand Down Expand Up @@ -154,7 +155,7 @@
case "error":
return showErrorMessage(message, "Show logs").then((selection) => {
if (selection === "Show logs") {
vscode.commands.executeCommand("workbench.action.toggleDevTools");

Check warning on line 158 in extensions/vscode/src/VsCodeIde.ts

View workflow job for this annotation

GitHub Actions / vscode-checks

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
}
});
case "info":
Expand Down Expand Up @@ -322,7 +323,7 @@
new vscode.Position(startLine, 0),
new vscode.Position(endLine, 0),
);
openEditorAndRevealRange(vscode.Uri.parse(fileUri), range).then(

Check warning on line 326 in extensions/vscode/src/VsCodeIde.ts

View workflow job for this annotation

GitHub Actions / vscode-checks

Promises must be awaited, end with a call to .catch, end with a call to .then with a rejection handler or be explicitly marked as ignored with the `void` operator
(editor) => {
// Select the lines
editor.selection = new vscode.Selection(
Expand Down Expand Up @@ -616,8 +617,12 @@
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 {
Expand Down
Loading