feat(coding-agent/tools): allowed .. aliases for line range selectors
- Extended line selector parsing in path-utils to accept `..` as a forgiving alias for `-`, with `..` normalized during chunk parsing. - Updated selector-matching regexes for file selectors and internal URL selectors to recognize alias-style ranges in single chunks and comma-separated lists. - Added tests covering `N..M`, `N..`, mixed separators, inverted-range errors, and path-splitting behavior with `:N..M` selectors and `foo:../bar.ts` paths.
This commit is contained in:
@@ -7,14 +7,22 @@ import { InternalUrlRouter, type LocalProtocolOptions } from "../internal-urls";
|
||||
import { ToolError } from "./tool-errors";
|
||||
|
||||
const UNICODE_SPACES = /[\u00A0\u2000-\u200A\u202F\u205F\u3000]/g;
|
||||
const FILE_LINE_RANGE_RE = /^(?:L?\d+(?:[-+]L?\d+|-)?(?:,L?\d+(?:[-+]L?\d+|-)?)*|raw|conflicts)$/i;
|
||||
const FILE_LINE_RANGE_ONLY_RE = /^L?\d+(?:[-+]L?\d+|-)?(?:,L?\d+(?:[-+]L?\d+|-)?)*$/i;
|
||||
// A single line-range chunk: `N`, `N-M`, `N+K`, or open-ended `N-`. `..` is
|
||||
// accepted everywhere `-` is, as a forgiving alias for Rust/Python-style ranges
|
||||
// (e.g. `2724..2727` == `2724-2727`, `2724..` == `2724-`); it is normalized to
|
||||
// `-` in parseLineRangeChunk. Keep this fragment and LINE_RANGE_CHUNK_RE in sync.
|
||||
const RANGE_CHUNK_SRC = String.raw`L?\d+(?:(?:[-+]|\.\.)L?\d+|-|\.\.)?`;
|
||||
const RANGE_LIST_SRC = `${RANGE_CHUNK_SRC}(?:,${RANGE_CHUNK_SRC})*`;
|
||||
const FILE_LINE_RANGE_RE = new RegExp(`^(?:${RANGE_LIST_SRC}|raw|conflicts)$`, "i");
|
||||
const FILE_LINE_RANGE_ONLY_RE = new RegExp(`^${RANGE_LIST_SRC}$`, "i");
|
||||
const FILE_RAW_ONLY_RE = /^raw$/i;
|
||||
// Permissive selector chunk for internal URLs — accepts well-formed selectors
|
||||
// plus common malformed shapes (e.g. `:-N`) so the read tool peels the entire
|
||||
// selector chain off before dispatching to a protocol handler.
|
||||
const INTERNAL_URL_SELECTOR_PART_RE =
|
||||
/^(?:raw|conflicts|L?\d+(?:[-+]L?\d+|-)?(?:,L?\d+(?:[-+]L?\d+|-)?)*|-\d+(?:[-+]\d+)?)$/i;
|
||||
const INTERNAL_URL_SELECTOR_PART_RE = new RegExp(
|
||||
String.raw`^(?:raw|conflicts|${RANGE_LIST_SRC}|-\d+(?:[-+]\d+)?)$`,
|
||||
"i",
|
||||
);
|
||||
// Schemes whose host grammar is identifier-shaped, so any trailing
|
||||
// `:<selector-chunk>` is unambiguously a read-tool selector. `mcp://` is
|
||||
// excluded because mcp resource URIs may legitimately contain colons.
|
||||
@@ -144,9 +152,9 @@ export interface LineRange {
|
||||
endLine: number | undefined;
|
||||
}
|
||||
|
||||
const LINE_RANGE_CHUNK_RE = /^L?(\d+)(?:([-+])L?(\d+)?)?$/i;
|
||||
const LINE_RANGE_CHUNK_RE = /^L?(\d+)(?:(\.\.|[-+])L?(\d+)?)?$/i;
|
||||
|
||||
/** Parse a single `N`, `N-M`, `N-`, or `N+K` chunk. Throws via {@link ToolError} on invalid bounds. */
|
||||
/** Parse a single `N`, `N-M`, `N-`, `N+K`, or `..`-aliased (`N..M`, `N..`) chunk. Throws via {@link ToolError} on invalid bounds. */
|
||||
export function parseLineRangeChunk(sel: string): LineRange | null {
|
||||
const lineMatch = LINE_RANGE_CHUNK_RE.exec(sel);
|
||||
if (!lineMatch) return null;
|
||||
@@ -154,7 +162,8 @@ export function parseLineRangeChunk(sel: string): LineRange | null {
|
||||
if (rawStart < 1) {
|
||||
throw new ToolError("Line selector 0 is invalid; lines are 1-indexed. Use :1.");
|
||||
}
|
||||
const sep = lineMatch[2];
|
||||
// `..` is a forgiving alias for `-` (e.g. `2724..2727` == `2724-2727`).
|
||||
const sep = lineMatch[2] === ".." ? "-" : lineMatch[2];
|
||||
const rhs = lineMatch[3] ? Number.parseInt(lineMatch[3], 10) : undefined;
|
||||
let rawEnd: number | undefined;
|
||||
if (sep === "+") {
|
||||
|
||||
@@ -129,6 +129,36 @@ describe("read tool multi-range selector", () => {
|
||||
expect(text).not.toContain("line 19");
|
||||
});
|
||||
|
||||
it("accepts `..` as a forgiving alias for `-`, producing identical output", async () => {
|
||||
const filePath = path.join(tmpDir, "numbered.txt");
|
||||
await fs.writeFile(filePath, makeNumberedContent(30));
|
||||
|
||||
const tool = new ReadTool(createSession(tmpDir));
|
||||
const dotdot = textOutput(await tool.execute("call-dotdot", { path: `${filePath}:3..5` }));
|
||||
const dash = textOutput(await tool.execute("call-dash", { path: `${filePath}:3-5` }));
|
||||
|
||||
expect(dotdot).toContain("line 3");
|
||||
expect(dotdot).toContain("line 5");
|
||||
// `..` must be a pure alias: byte-for-byte identical to the `-` form.
|
||||
expect(dotdot).toBe(dash);
|
||||
});
|
||||
|
||||
it("accepts `..` in multi-range selectors", async () => {
|
||||
const filePath = path.join(tmpDir, "numbered.txt");
|
||||
await fs.writeFile(filePath, makeNumberedContent(50));
|
||||
|
||||
const tool = new ReadTool(createSession(tmpDir));
|
||||
const result = await tool.execute("call-dotdot-multi", { path: `${filePath}:3..5,20..22` });
|
||||
const text = textOutput(result);
|
||||
|
||||
expect(text).toContain("line 3");
|
||||
expect(text).toContain("line 5");
|
||||
expect(text).toContain("line 20");
|
||||
expect(text).toContain("line 22");
|
||||
expect(text).not.toContain("line 10");
|
||||
expect(text).toContain("…");
|
||||
});
|
||||
|
||||
it("rejects multi-range selectors on directories", async () => {
|
||||
const tool = new ReadTool(createSession(tmpDir));
|
||||
await expect(tool.execute("call-dir", { path: `${tmpDir}:1-2,5-6` })).rejects.toThrow(
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { parseLineRangeChunk, parseLineRanges, splitPathAndSel } from "@oh-my-pi/pi-coding-agent/tools/path-utils";
|
||||
import { ToolError } from "@oh-my-pi/pi-coding-agent/tools/tool-errors";
|
||||
|
||||
describe("`..` range selector alias", () => {
|
||||
it("treats `N..M` as the inclusive range `N-M`", () => {
|
||||
expect(parseLineRangeChunk("2724..2727")).toEqual({ startLine: 2724, endLine: 2727 });
|
||||
// Same line count and bounds as the canonical dash form.
|
||||
expect(parseLineRangeChunk("2724..2727")).toEqual(parseLineRangeChunk("2724-2727"));
|
||||
});
|
||||
|
||||
it("treats trailing `N..` as open-ended, like `N-`", () => {
|
||||
expect(parseLineRangeChunk("301..")).toEqual({ startLine: 301, endLine: undefined });
|
||||
expect(parseLineRangeChunk("301..")).toEqual(parseLineRangeChunk("301-"));
|
||||
});
|
||||
|
||||
it("accepts `..` inside comma-separated multi-range selectors", () => {
|
||||
expect(parseLineRanges("3..5,20..22")).toEqual([
|
||||
{ startLine: 3, endLine: 5 },
|
||||
{ startLine: 20, endLine: 22 },
|
||||
]);
|
||||
});
|
||||
|
||||
it("allows mixing `..` and `-` separators across chunks", () => {
|
||||
expect(parseLineRanges("3-5,20..22")).toEqual([
|
||||
{ startLine: 3, endLine: 5 },
|
||||
{ startLine: 20, endLine: 22 },
|
||||
]);
|
||||
});
|
||||
|
||||
it("rejects inverted `..` ranges with the same guard as `-`", () => {
|
||||
expect(() => parseLineRangeChunk("2727..2724")).toThrow(ToolError);
|
||||
});
|
||||
|
||||
it("peels a trailing `:N..M` selector off the path", () => {
|
||||
expect(splitPathAndSel("packages/editor/src/Editor.ts:2724..2727")).toEqual({
|
||||
path: "packages/editor/src/Editor.ts",
|
||||
sel: "2724..2727",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not mistake a `..` path segment for a selector", () => {
|
||||
// No digits around the dots → still a plain path, not a range selector.
|
||||
expect(splitPathAndSel("foo:../bar.ts")).toEqual({ path: "foo:../bar.ts" });
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user