From 881906436c84efcbe9a94dd02df5c9f39989e597 Mon Sep 17 00:00:00 2001 From: ZxlDragonDoctor <152901610+ZxlDragonDoctor@users.noreply.github.com> Date: Sun, 20 Sep 2026 19:24:38 +0800 Subject: [PATCH] fix(chat): preserve path:line refs through file open (#681) Chat tokens like src/a.ts:42 previously lost the line in parseFileRef before any opener ran. Keep the position on ChatPreviewTarget, thread it through useOpenChatFileRef, encode plugin locations as path#L{line}, and scroll the host file viewer when the bundled file-manager view is absent. --- apps/desktop/src/components/Markdown.tsx | 15 ++- .../src/components/workpanel/FilesTab.tsx | 26 +++- apps/desktop/src/hooks/use-preview-target.ts | 49 ++++++-- apps/desktop/src/lib/chat-links.ts | 112 ++++++++++++++++-- apps/desktop/src/lib/work-panel-tabs.ts | 23 +++- apps/desktop/src/stores/app-state.ts | 6 +- .../src/stores/slices/work-panel-slice.ts | 4 +- apps/desktop/test/chat-links.test.mjs | 39 ++++++ 8 files changed, 250 insertions(+), 24 deletions(-) diff --git a/apps/desktop/src/components/Markdown.tsx b/apps/desktop/src/components/Markdown.tsx index 9a03b7799..ae623d8ee 100644 --- a/apps/desktop/src/components/Markdown.tsx +++ b/apps/desktop/src/components/Markdown.tsx @@ -53,9 +53,12 @@ import { useAppStore } from "../stores/app-store"; import { useReferencedImageDataUrl } from "../lib/use-referenced-image-data-url"; import { useOpenChatFileRef } from "../hooks/use-preview-target"; import { + parseFileRefPosition, remarkChatFileLinks, resolvePreviewTarget, safeDecodeUri, + splitFileLocation, + stripLineRef, toWorkspaceRel, } from "../lib/chat-links"; import { @@ -477,7 +480,10 @@ function InlineCode({ title={target.kind === "file" ? fileTitle : urlTitle} onClick={() => target.kind === "file" - ? openFileRef(text ?? target.path, baseDir) + ? openFileRef(text ?? target.path, baseDir, undefined, { + line: target.line, + column: target.column, + }) : openHttpUrl(target.url) } > @@ -576,7 +582,12 @@ function Anchor({ const rel = toWorkspaceRel(safeDecodeUri(href), root, baseDir); if (rel) { e.preventDefault(); - openFileRef(rel, baseDir); + const decoded = safeDecodeUri(href); + const fromHash = splitFileLocation(decoded); + const fromColon = parseFileRefPosition(decoded); + const line = fromHash.line ?? fromColon?.line; + const column = fromHash.column ?? fromColon?.column; + openFileRef(stripLineRef(rel), baseDir, undefined, { line, column }); } }; return ( diff --git a/apps/desktop/src/components/workpanel/FilesTab.tsx b/apps/desktop/src/components/workpanel/FilesTab.tsx index 582f34bb4..fd4de7444 100644 --- a/apps/desktop/src/components/workpanel/FilesTab.tsx +++ b/apps/desktop/src/components/workpanel/FilesTab.tsx @@ -119,7 +119,7 @@ function HighlightedText({ path, content }: { path: string; content: string }) {
       {tokens
         ? tokens.tokens.map((row, i) => (
-            
+
{row.length === 0 ? "\n" : row.map((token, j) => ( @@ -130,7 +130,7 @@ function HighlightedText({ path, content }: { path: string; content: string }) {
)) : visible.split("\n").map((line, i) => ( -
+
{line || "\n"}
))} @@ -156,6 +156,7 @@ export function FilesTab() { const [selected, setSelected] = useState(null); const [file, setFile] = useState(null); const [fileError, setFileError] = useState(false); + const pendingLineRef = useRef(null); // Workspace switches reset all browsing state. Guarded so it only fires on // an actual root change: an unconditional [root] effect also runs on the @@ -241,8 +242,29 @@ export function FilesTab() { for (const dir of ancestors) void loadDir(dir); } void openFile(path, fileRequest.mimeType); + // Chat `path:line` refs land here when the bundled file view is absent; + // remember the line so the viewer can scroll once the content loads (#681). + pendingLineRef.current = fileRequest.line ?? null; }, [fileRequest, root, loadDir, openFile]); + useEffect(() => { + const line = pendingLineRef.current; + if (!line || !file) return; + pendingLineRef.current = null; + const id = `file-viewer-line-${line}`; + // Lines render 1-based; fall back to query by ordinal if ids are absent. + requestAnimationFrame(() => { + const node = document.getElementById(id); + if (node) { + node.scrollIntoView({ block: "center" }); + return; + } + const lines = document.querySelectorAll(".file-viewer-line"); + const target = lines[line - 1]; + if (target) target.scrollIntoView({ block: "center" }); + }); + }, [file]); + const renderDir = (rel: string, depth: number): React.ReactNode => { const state = dirs[rel]; if (!state) { diff --git a/apps/desktop/src/hooks/use-preview-target.ts b/apps/desktop/src/hooks/use-preview-target.ts index 313487cd6..32ba23f06 100644 --- a/apps/desktop/src/hooks/use-preview-target.ts +++ b/apps/desktop/src/hooks/use-preview-target.ts @@ -2,7 +2,13 @@ import { useCallback, useMemo } from "react"; import { useTranslation } from "react-i18next"; import { useAppStore } from "../stores/app-store"; import { api } from "../lib/api"; -import { isHtmlFilePath, toWorkspaceRel, type ChatPreviewTarget } from "../lib/chat-links"; +import { + fileLocationWithPosition, + isHtmlFilePath, + parseFileRefPosition, + toWorkspaceRel, + type ChatPreviewTarget, +} from "../lib/chat-links"; import { openHttpUrl } from "../lib/open-http-url"; import { FILE_MANAGER_PLUGIN_TAB, @@ -27,7 +33,12 @@ export function useOpenPreviewTarget() { const openFileRef = useOpenChatFileRef(); return useCallback( (target: ChatPreviewTarget) => - target.kind === "file" ? openFileRef(target.path) : openHttpUrl(target.url), + target.kind === "file" + ? openFileRef(target.path, undefined, undefined, { + line: target.line, + column: target.column, + }) + : openHttpUrl(target.url), [openFileRef], ); } @@ -37,6 +48,11 @@ function isDotRelative(path: string): boolean { return path.startsWith("./") || path.startsWith("../"); } +export type ChatFileRefPosition = { + line?: number; + column?: number; +}; + /** * Open a file reference the conversation mentioned. * @@ -54,6 +70,11 @@ function isDotRelative(path: string): boolean { * A workspace `.html` page in the primary folder stays with the side browser * (ADR 0163): it is a page to run, not a file to read. A reference that matches * nothing says so instead of opening an empty panel. + * + * Tokens such as `src/a.ts:42` carry a 1-based line (and optional column). + * Those are stripped before path resolution, then re-attached on the open + * request so the file view can jump to the cited line (#681). Plugin locations + * use `path#L{line}` (see `fileLocationWithPosition`). */ export function useOpenChatFileRef() { const { t } = useTranslation(); @@ -71,9 +92,17 @@ export function useOpenChatFileRef() { ); return useCallback( - (path: string, baseDir?: string, mimeType?: string) => { - const raw = String(path ?? "").trim(); + ( + path: string, + baseDir?: string, + mimeType?: string, + position?: ChatFileRefPosition, + ) => { + const parsed = parseFileRefPosition(path) ?? { path: path.trim() }; + const raw = String(parsed.path ?? "").trim(); if (!raw) return; + const line = position?.line ?? parsed.line; + const column = position?.column ?? parsed.column; // `./x` and `../x` are the one shape the caller resolves better than the // main process can: the base is the markdown file on screen, which only // the caller knows. Everything else is completed against the roots. @@ -104,13 +133,17 @@ export function useOpenChatFileRef() { return; } if (fileViewAvailable) { - openTab(fileManagerPluginTab(target)); + openTab( + fileManagerPluginTab( + fileLocationWithPosition(target, { line, column }), + ), + ); return; } - openFile(target, mimeType); + openFile(target, mimeType, { line, column }); return; } - openFile(match.absolutePath, mimeType); + openFile(match.absolutePath, mimeType, { line, column }); })(); }, [ @@ -124,4 +157,4 @@ export function useOpenChatFileRef() { workspacePath, ], ); -} +} \ No newline at end of file diff --git a/apps/desktop/src/lib/chat-links.ts b/apps/desktop/src/lib/chat-links.ts index 4cee3cccf..ed815a9aa 100644 --- a/apps/desktop/src/lib/chat-links.ts +++ b/apps/desktop/src/lib/chat-links.ts @@ -51,10 +51,67 @@ export function isHtmlFilePath(path: string): boolean { return /\.html?$/i.test(path); } -function stripLineRef(path: string): string { +export function stripLineRef(path: string): string { return path.replace(/:\d+(?::\d+)?$/, ""); } +export type FileRefPosition = { + path: string; + /** 1-based line from `path:line[:col]` refs; omitted when absent. */ + line?: number; + /** 1-based column when the token carried `path:line:col`. */ + column?: number; +}; + +const LINE_COL_SUFFIX_RE = /:(\d+)(?::(\d+))?$/; + +/** + * Split a chat `path:line[:col]` token into a file path plus optional + * 1-based position. `parseFileRef` keeps returning the path alone; this + * helper preserves the line the transcript named so openers can jump to it + * instead of landing at the top of the file (#681). + */ +export function parseFileRefPosition(text: string): FileRefPosition | null { + const path = parseFileRef(text); + if (!path) return null; + const raw = text.trim().replace(/^@/, ""); + const match = LINE_COL_SUFFIX_RE.exec(raw); + if (!match) return { path }; + const line = Number(match[1]); + if (!Number.isFinite(line) || line < 1) return { path }; + const column = match[2] === undefined ? undefined : Number(match[2]); + if (column !== undefined && (!Number.isFinite(column) || column < 1)) { + return { path, line }; + } + return column === undefined ? { path, line } : { path, line, column }; +} + +/** + * Encode a workspace path plus optional line for plugin-view location + * delivery (`piViewOpen` / `view:open`). Host and plugin agree on `#L{line}` + * so the path itself stays a normal relative path when no line is present. + */ +export function fileLocationWithPosition( + path: string, + position?: { line?: number; column?: number }, +): string { + if (!position?.line || position.line < 1) return path; + if (position.column && position.column >= 1) { + return `${path}#L${position.line}:C${position.column}`; + } + return `${path}#L${position.line}`; +} + +/** Inverse of `fileLocationWithPosition` for openers that receive a location. */ +export function splitFileLocation(location: string): FileRefPosition { + const hash = /#L(\d+)(?::C(\d+))?$/.exec(location); + if (!hash) return { path: location }; + const path = location.slice(0, hash.index); + const line = Number(hash[1]); + const column = hash[2] === undefined ? undefined : Number(hash[2]); + return column === undefined ? { path, line } : { path, line, column }; +} + function leafName(path: string): string { const normalized = path.replaceAll("\\", "/").replace(/\/+$/, ""); return normalized.slice(normalized.lastIndexOf("/") + 1) || path; @@ -182,8 +239,45 @@ export function toWorkspaceRel( } export type ChatPreviewTarget = - | { kind: "file"; path: string } - | { kind: "url"; url: string }; + | { kind: "url"; url: string } + | { + kind: "file"; + path: string; + /** 1-based line from `path:line` tokens; omitted when absent. */ + line?: number; + /** 1-based column when the token carried `path:line:col`. */ + column?: number; + }; + +function extractLineColumn(token: string): { + line?: number; + column?: number; +} { + const cleaned = token + .trim() + .replace(/^@/, "") + .replace(/^"|"$/g, ""); + const match = /:(\d+)(?::(\d+))?$/.exec(cleaned); + if (!match) return {}; + const line = Number(match[1]); + if (!Number.isFinite(line) || line < 1) return {}; + const column = match[2] === undefined ? undefined : Number(match[2]); + if (column !== undefined && (!Number.isFinite(column) || column < 1)) { + return { line }; + } + return column === undefined ? { line } : { line, column }; +} + +function fileTarget( + path: string, + line?: number, + column?: number, +): ChatPreviewTarget { + if (!line) return { kind: "file", path }; + return column + ? { kind: "file", path, line, column } + : { kind: "file", path, line }; +} /** Resolve one raw chat token into a previewable target, or null. */ export function resolvePreviewTarget( @@ -193,17 +287,21 @@ export function resolvePreviewTarget( ): ChatPreviewTarget | null { const trimmed = text.trim(); if (isHttpUrl(trimmed)) return { kind: "url", url: trimmed }; + const { line, column } = extractLineColumn(trimmed); const at = unwrapAtFileRef(trimmed); if (at) { // Scratch/attachment @refs stay absolute so fs/open can contain them. - if (at.startsWith("/")) return { kind: "file", path: at }; - const rel = toWorkspaceRel(at, root, baseDir); - return rel ? { kind: "file", path: rel } : null; + const rawPath = stripLineRef(at); + if (rawPath.startsWith("/")) return fileTarget(rawPath, line, column); + const rel = toWorkspaceRel(rawPath, root, baseDir); + if (!rel) return null; + return fileTarget(rel, line, column); } const file = parseFileRef(trimmed); if (!file) return null; const rel = toWorkspaceRel(file, root, baseDir); - return rel ? { kind: "file", path: rel } : null; + if (!rel) return null; + return fileTarget(rel, line, column); } /** Tool-call args → preview target (Read/Write/Edit paths, fetch URLs). */ diff --git a/apps/desktop/src/lib/work-panel-tabs.ts b/apps/desktop/src/lib/work-panel-tabs.ts index d38e56d26..b8879289f 100644 --- a/apps/desktop/src/lib/work-panel-tabs.ts +++ b/apps/desktop/src/lib/work-panel-tabs.ts @@ -12,6 +12,10 @@ export type WorkPanelTab = { location?: string; /** Stored attachment mimeType for extension-less `attachments/` images. */ mimeType?: string; + /** 1-based line from a chat `path:line` ref (#681); omitted when unknown. */ + line?: number; + /** 1-based column when the chat token carried `path:line:col`. */ + column?: number; }; export type WorkPanelTabsState = { @@ -19,9 +23,17 @@ export type WorkPanelTabsState = { activeTabId: string | null; }; +export type WorkPanelFileRequest = { + path: string; + seq: number; + mimeType?: string; + line?: number; + column?: number; +}; + export type WorkPanelContext = WorkPanelTabsState & { open: boolean; - fileRequest: { path: string; seq: number; mimeType?: string } | null; + fileRequest: WorkPanelFileRequest | null; }; let newWorkPanelTabSequence = 0; @@ -239,13 +251,20 @@ export function normalizeWorkPanelFilePath(path: string): string { return absolute ? `/${normalized}` : normalized; } -export function fileWorkPanelTab(path: string, mimeType?: string): WorkPanelTab { +export function fileWorkPanelTab( + path: string, + mimeType?: string, + position?: { line?: number; column?: number }, +): WorkPanelTab { const resource = normalizeWorkPanelFilePath(path); return { id: `file:${resource}`, kind: "file", resource, ...(mimeType ? { mimeType } : {}), + ...(position?.line + ? { line: position.line, column: position.column } + : {}), }; } diff --git a/apps/desktop/src/stores/app-state.ts b/apps/desktop/src/stores/app-state.ts index 093175da1..7969e9bc3 100644 --- a/apps/desktop/src/stores/app-state.ts +++ b/apps/desktop/src/stores/app-state.ts @@ -371,7 +371,11 @@ export type AppState = { /** Hide the visible panel while retaining its session-owned context. */ resetWorkPanelContext: () => void; setWorkPanelWidth: (width: number) => void; - openFileInWorkPanel: (path: string, mimeType?: string) => void; + openFileInWorkPanel: ( + path: string, + mimeType?: string, + position?: { line?: number; column?: number }, + ) => void; openUrlInWorkPanel: (url: string) => void; }; diff --git a/apps/desktop/src/stores/slices/work-panel-slice.ts b/apps/desktop/src/stores/slices/work-panel-slice.ts index 2fc5ae354..65dcd5902 100644 --- a/apps/desktop/src/stores/slices/work-panel-slice.ts +++ b/apps/desktop/src/stores/slices/work-panel-slice.ts @@ -356,8 +356,8 @@ export function createWorkPanelSlice({ saveWorkPanelWidth(get().workPanelWidth); }, - openFileInWorkPanel: (path, mimeType) => { - get().openWorkPanelTab(fileWorkPanelTab(path, mimeType)); + openFileInWorkPanel: (path, mimeType, position) => { + get().openWorkPanelTab(fileWorkPanelTab(path, mimeType, position)); }, openUrlInWorkPanel: (url) => { const hasBrowser = get().pluginViews.some( diff --git a/apps/desktop/test/chat-links.test.mjs b/apps/desktop/test/chat-links.test.mjs index 7352036a1..f2e6b8a92 100644 --- a/apps/desktop/test/chat-links.test.mjs +++ b/apps/desktop/test/chat-links.test.mjs @@ -2,13 +2,16 @@ import assert from "node:assert/strict"; import test from "node:test"; import { fileDirOf, + fileLocationWithPosition, getToolPreviewTarget, isHttpUrl, linkifyMdastTree, parseFileRef, + parseFileRefPosition, remarkChatFileLinks, resolvePreviewTarget, splitChatText, + splitFileLocation, toWorkspaceRel, } from "../src/lib/chat-links.ts"; @@ -25,6 +28,41 @@ test("parseFileRef accepts pathy tokens and strips line refs", () => { assert.equal(parseFileRef("../adr/0163.md"), "../adr/0163.md"); }); +test("parseFileRefPosition keeps the line the transcript named (#681)", () => { + assert.deepEqual(parseFileRefPosition("src/main.rs:42"), { + path: "src/main.rs", + line: 42, + }); + assert.deepEqual(parseFileRefPosition("src/main.rs:42:7"), { + path: "src/main.rs", + line: 42, + column: 7, + }); + assert.deepEqual(parseFileRefPosition("apps/desktop/src/App.tsx"), { + path: "apps/desktop/src/App.tsx", + }); + assert.equal(parseFileRefPosition("hello world"), null); +}); + +test("fileLocationWithPosition encodes #L for plugin open locations (#681)", () => { + assert.equal(fileLocationWithPosition("src/a.ts"), "src/a.ts"); + assert.equal(fileLocationWithPosition("src/a.ts", { line: 10 }), "src/a.ts#L10"); + assert.equal( + fileLocationWithPosition("src/a.ts", { line: 10, column: 3 }), + "src/a.ts#L10:C3", + ); + assert.deepEqual(splitFileLocation("src/a.ts#L10"), { + path: "src/a.ts", + line: 10, + }); + assert.deepEqual(splitFileLocation("src/a.ts#L10:C3"), { + path: "src/a.ts", + line: 10, + column: 3, + }); + assert.deepEqual(splitFileLocation("src/a.ts"), { path: "src/a.ts" }); +}); + test("parseFileRef accepts bare names only with known extensions", () => { assert.equal(parseFileRef("README.md"), "README.md"); assert.equal(parseFileRef("package.json"), "package.json"); @@ -91,6 +129,7 @@ test("resolvePreviewTarget classifies urls and workspace files", () => { assert.deepEqual(resolvePreviewTarget("src/a.ts:10", ROOT), { kind: "file", path: "src/a.ts", + line: 10, }); assert.deepEqual(resolvePreviewTarget("./README.md", ROOT, "docs"), { kind: "file",