diff --git a/packages/app/e2e/browser/workspace-content-search-dependency.spec.ts b/packages/app/e2e/browser/workspace-content-search-dependency.spec.ts index cad874c72e..c52e7ef017 100644 --- a/packages/app/e2e/browser/workspace-content-search-dependency.spec.ts +++ b/packages/app/e2e/browser/workspace-content-search-dependency.spec.ts @@ -5,6 +5,10 @@ import path from "node:path"; import { test as base, expect } from "../support/fixtures"; import { prepareContentSearch } from "../support/helpers/workspace-content-search"; +// The missing-rg fixture builds a PATH from `which` and POSIX symlinks. Windows needs its own +// PATH construction, which belongs to the CI gate rather than to this journey. +base.skip(process.platform === "win32", "the missing-ripgrep PATH fixture is POSIX-shaped"); + const test = base.extend<{}, { searchHostPath: string }>({ searchHostPath: [ async ({ browserName }, provide) => { diff --git a/packages/app/e2e/browser/workspace-content-search.spec.ts b/packages/app/e2e/browser/workspace-content-search.spec.ts index 4fbe9f76e3..02c73b7b74 100644 --- a/packages/app/e2e/browser/workspace-content-search.spec.ts +++ b/packages/app/e2e/browser/workspace-content-search.spec.ts @@ -144,9 +144,24 @@ test("an empty result says which files the host never opened", async ({ page, wi await reportSkippedLargeFiles(page, await withWorkspace()); }); +// "a\b.txt" is one file on POSIX and a nested path on Windows, so the fixture only exists here. +// Windows separator handling is covered by resolveWorkspaceFilePaths and toWorkspaceRelativePath. +test.skip( + process.platform === "win32", + "a file name containing a backslash is not a valid Windows fixture", +); test("a file name containing a backslash opens itself, not a same-named nested path", async ({ page, withWorkspace, }) => { await openLiteralBackslashFile(page, await withWorkspace()); }); + +import { reopenSameOccurrenceAfterMovingAway } from "../support/helpers/workspace-content-search"; + +test("opening the same occurrence again navigates the pane it is already showing", async ({ + page, + withWorkspace, +}) => { + await reopenSameOccurrenceAfterMovingAway(page, await withWorkspace()); +}); diff --git a/packages/app/e2e/support/helpers/workspace-content-search.ts b/packages/app/e2e/support/helpers/workspace-content-search.ts index e3671bfd62..a0d4c3131d 100644 --- a/packages/app/e2e/support/helpers/workspace-content-search.ts +++ b/packages/app/e2e/support/helpers/workspace-content-search.ts @@ -190,3 +190,54 @@ export async function openLiteralBackslashFile(page: Page, workspace: CreatedWor await expect(source).not.toContainText("THIS IS THE OTHER FILE"); await expect(page.getByRole("status").filter({ hasText: "File changed" })).toHaveCount(0); } + +/** + * Opening the same occurrence twice has to navigate the pane both times. The tab target is + * deliberately identity-stable, so the second activation looks like a no-op unless it asks the + * file-opening owner to navigate again. + */ +export async function reopenSameOccurrenceAfterMovingAway(page: Page, workspace: CreatedWorkspace) { + const lines = Array.from( + { length: 1200 }, + (_value, index) => `export const filler${index} = ${index};`, + ); + lines.push('const marker = "REOPEN_TARGET";'); + await writeFile(path.join(workspace.repoPath, "reopen.ts"), `${lines.join("\n")}\n`); + await workspace.navigateTo(); + + const scroller = page.getByTestId("file-source-editor").locator(".cm-scroller"); + const offset = () => scroller.evaluate((element) => element.scrollTop); + + await openReopenTarget(page); + const revealed = await offset(); + expect(revealed).toBeGreaterThan(0); + await expect( + page.getByTestId("file-source-editor").locator(".cm-selectionBackground"), + ).toBeInViewport(); + + // Move the caret to the top of the same file, inside the same editor instance. + await page.getByTestId("file-source-editor").locator(".cm-content").click(); + await page.keyboard.press("ControlOrMeta+Home"); + await expect.poll(offset).toBeLessThan(revealed / 2); + + await openReopenTarget(page); + await expect + .poll(offset, { message: "reopening the same occurrence must navigate the pane again" }) + .toBeGreaterThan(revealed / 2); + await expect( + page.getByTestId("file-source-editor").locator(".cm-selectionBackground"), + ).toBeInViewport(); + // The editor instance is reused, so its history survives the second navigation. + await expect(page.getByTestId("file-source-editor")).toContainText("REOPEN_TARGET"); +} + +async function openReopenTarget(page: Page) { + await page.keyboard.press("Meta+Shift+F"); + const panel = page.getByTestId("command-center-panel"); + await page + .getByRole("textbox", { name: "Search saved file contents...", exact: true }) + .fill("REOPEN_TARGET"); + await expect(panel.getByRole("button", { name: /reopen\.ts:1201:/ })).toBeVisible(); + await page.keyboard.press("Enter"); + await expect(panel).toBeHidden(); +} diff --git a/packages/app/src/command-center/workspace-content-search/index.tsx b/packages/app/src/command-center/workspace-content-search/index.tsx index 554fb6596c..c294532772 100644 --- a/packages/app/src/command-center/workspace-content-search/index.tsx +++ b/packages/app/src/command-center/workspace-content-search/index.tsx @@ -11,12 +11,12 @@ import { useWorkspaceDirectory } from "@/stores/session-store-hooks"; import { useSessionStore } from "@/stores/session-store"; import { useWorkspaceLayoutStore } from "@/stores/workspace-layout-store"; import { usePanelStore } from "@/stores/panel-store"; -import { createWorkspaceFileTabTarget } from "@/workspace/file-open"; import { buildWorkspaceTabPersistenceKey } from "@/workspace-tabs/model"; +import { openWorkspaceFileInFocusedPane } from "@/screens/workspace/workspace-file-open-command"; import { clearCommandCenterFocusRestoreElement } from "@/utils/command-center-focus-restore"; import { WorkspaceFileSource } from "@/file-pane/source"; import { formatFileSize } from "@/utils/format-file-size"; -import { describeWorkspaceFilePath } from "../workspace-file-search-model"; +import { describeResultPath } from "./internal/result-path"; import { WorkspaceContentSearchModel, type ContentSearchSnapshot, @@ -104,16 +104,17 @@ function ContentSearch({ transport, open(location) { clearCommandCenterFocusRestoreElement(); - const workspaceKey = buildWorkspaceTabPersistenceKey({ serverId, workspaceId }); - if (!workspaceKey) return; const layout = useWorkspaceLayoutStore.getState(); - const tabId = layout.openTab({ - workspaceKey, - target: createWorkspaceFileTabTarget(location), - intent: "reveal", + openWorkspaceFileInFocusedPane({ + location, + persistenceKey: buildWorkspaceTabPersistenceKey({ serverId, workspaceId }), + closeExplorerAfterOpen: true, + showMobileAgent: usePanelStore.getState().showMobileAgent, + openWorkspaceTabInFocusedPane: (workspaceKey, target, placement) => + layout.openTab({ workspaceKey, target, intent: "reveal", placement }), + focusWorkspaceTab: layout.focusTab, + requestFileNavigation: layout.requestFileNavigation, }); - if (tabId) layout.focusTab(workspaceKey, tabId); - usePanelStore.getState().showMobileAgent(); closeRef.current(); }, }), @@ -230,7 +231,7 @@ function ResultRow({ [selected], ); const line = useMemo(() => describeMatchLine(item), [item]); - const file = useMemo(() => describeWorkspaceFilePath(item.path), [item.path]); + const file = useMemo(() => describeResultPath(item.path), [item.path]); return ( {line.match} {line.after} - - + {/* The file name and its coordinates are pinned; only the parent context may shrink, so a + deep path loses its middle rather than the identity the reader is scanning for. */} + + {file.directory ? ( + + {file.directory} + + ) : null} + {file.name}:{item.line}:{item.columnStart} - {file.directory ? {file.directory} : null} - + ); } @@ -346,13 +353,13 @@ function ContentPreview({ ); return ( - {match ? ( + {/* On desktop the selected row already names the file and Enter opens it, so the preview + carries no header. The compact flow pages away from the list and still needs both. */} + {compact && match ? ( - {compact ? ( - - ) : null} + {match.path} @@ -391,8 +398,9 @@ function ContentPreview({ ); } -// Matches the Command Center's own two-line result row. -const RESULT_ROW_HEIGHT = 56; +// Denser than the Command Center's own two-line row: these results are scanned, not read, and a +// search wants as many occurrences on screen as stay legible. +const RESULT_ROW_HEIGHT = 44; // Characters of the matched line kept ahead of the match itself. const SNIPPET_LEAD = 24; @@ -413,22 +421,37 @@ const styles = StyleSheet.create((theme) => ({ height: RESULT_ROW_HEIGHT, justifyContent: "center", paddingHorizontal: theme.spacing[4], - paddingVertical: theme.spacing[2], + paddingVertical: theme.spacing[1], }, hovered: { backgroundColor: theme.colors.surface1 }, selected: { backgroundColor: theme.colors.surface2 }, - // The matched line leads, in the code type scale; the location is its subtitle. + // Three levels: the match is what the reader is looking for, the rest of the line is the + // context it sits in, and the location is context for that. snippet: { - color: theme.colors.foreground, + color: theme.colors.foregroundMuted, fontSize: theme.fontSize.code, - lineHeight: 18, + lineHeight: 17, fontFamily: theme.fontFamily.mono, }, - match: { backgroundColor: theme.colors.terminal.selectionBackground }, - // The file name and coordinates lead so that truncation eats the directory, not the identity. - location: { fontSize: theme.fontSize.sm, lineHeight: 16 }, - locationFile: { color: theme.colors.foreground }, - locationDirectory: { color: theme.colors.foregroundMuted }, + match: { + color: theme.colors.foreground, + fontWeight: "600", + backgroundColor: theme.colors.terminal.selectionBackground, + }, + location: { flexDirection: "row", alignItems: "baseline" }, + locationFile: { + flexShrink: 0, + color: theme.colors.foregroundMuted, + fontSize: theme.fontSize.sm, + lineHeight: 15, + }, + locationDirectory: { + flexShrink: 1, + minWidth: 0, + color: theme.colors.foregroundExtraMuted, + fontSize: theme.fontSize.sm, + lineHeight: 15, + }, preview: { flex: 1, minWidth: 0, minHeight: 0 }, previewHeader: { flexDirection: "row", diff --git a/packages/app/src/command-center/workspace-content-search/internal/result-path.test.ts b/packages/app/src/command-center/workspace-content-search/internal/result-path.test.ts new file mode 100644 index 0000000000..f13261230d --- /dev/null +++ b/packages/app/src/command-center/workspace-content-search/internal/result-path.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { describeResultPath } from "./result-path"; + +describe("describeResultPath", () => { + it("keeps a root file whole", () => { + expect(describeResultPath("README.md")).toEqual({ directory: "", name: "README.md" }); + }); + + it("keeps a shallow path whole", () => { + expect(describeResultPath("docs/architecture.md")).toEqual({ + directory: "docs/", + name: "architecture.md", + }); + }); + + it("elides the head of a deep path and keeps the distinguishing parent", () => { + // These two rows are both "index.tsx"; the parents are the only thing telling them apart. + expect( + describeResultPath("packages/app/src/command-center/workspace-content-search/index.tsx"), + ).toEqual({ directory: "…/workspace-content-search/", name: "index.tsx" }); + expect(describeResultPath("packages/app/src/file-pane/source/index.tsx")).toEqual({ + directory: "…/source/", + name: "index.tsx", + }); + }); + + it("keeps a literal backslash in a file name out of the directory", () => { + expect(describeResultPath("a\\b.txt")).toEqual({ directory: "", name: "a\\b.txt" }); + }); +}); diff --git a/packages/app/src/command-center/workspace-content-search/internal/result-path.ts b/packages/app/src/command-center/workspace-content-search/internal/result-path.ts new file mode 100644 index 0000000000..c7cb8cef8e --- /dev/null +++ b/packages/app/src/command-center/workspace-content-search/internal/result-path.ts @@ -0,0 +1,25 @@ +/** + * How a result row names its file. + * + * A flat list of occurrences is full of repeated base names — four `index.tsx` rows tell the + * reader nothing. The nearest directories are what distinguish them, and they are also what an + * end-truncated path throws away first. So the row keeps the file name and the closest parents, + * and marks the elided head with a leading ellipsis. + */ +export interface ResultPathParts { + /** Directory context immediately above the file, already elided at the head when deeper. */ + directory: string; + /** File name, which is never truncated away. */ + name: string; +} + +const PARENTS_KEPT = 1; + +export function describeResultPath(path: string): ResultPathParts { + const segments = path.split("/"); + const name = segments.pop() ?? path; + if (segments.length === 0) return { directory: "", name }; + const kept = segments.slice(-PARENTS_KEPT); + const elided = segments.length > kept.length; + return { directory: `${elided ? "…/" : ""}${kept.join("/")}/`, name }; +} diff --git a/packages/app/src/command-center/workspace-file-search.ts b/packages/app/src/command-center/workspace-file-search.ts index 1008604bec..2971b611ec 100644 --- a/packages/app/src/command-center/workspace-file-search.ts +++ b/packages/app/src/command-center/workspace-file-search.ts @@ -1,5 +1,5 @@ import { useCallback, useEffect, useMemo, useState } from "react"; -import { openWorkspaceFileFromExplorer } from "@/screens/workspace/workspace-file-open-command"; +import { openWorkspaceFileInFocusedPane } from "@/screens/workspace/workspace-file-open-command"; import { useActiveWorkspaceSelection } from "@/stores/navigation-active-workspace-store"; import { usePanelStore } from "@/stores/panel-store"; import { useSessionStore } from "@/stores/session-store"; @@ -128,8 +128,8 @@ export function useWorkspaceFileSearch(input: { enabled: boolean; query: string (path: string) => { if (!serverId || !workspaceId) return; clearCommandCenterFocusRestoreElement(); - openWorkspaceFileFromExplorer({ - filePath: path, + openWorkspaceFileInFocusedPane({ + location: { path }, persistenceKey: buildWorkspaceTabPersistenceKey({ serverId, workspaceId }), closeExplorerAfterOpen: true, showMobileAgent: usePanelStore.getState().showMobileAgent, @@ -141,6 +141,7 @@ export function useWorkspaceFileSearch(input: { enabled: boolean; query: string placement, }), focusWorkspaceTab: useWorkspaceLayoutStore.getState().focusTab, + requestFileNavigation: useWorkspaceLayoutStore.getState().requestFileNavigation, }); }, [serverId, workspaceId], diff --git a/packages/app/src/components/compact-explorer-sidebar-host.tsx b/packages/app/src/components/compact-explorer-sidebar-host.tsx index 2dae4b9042..6018b89888 100644 --- a/packages/app/src/components/compact-explorer-sidebar-host.tsx +++ b/packages/app/src/components/compact-explorer-sidebar-host.tsx @@ -14,7 +14,7 @@ import { usePanelStore } from "@/stores/panel-store"; import { useWorkspaceLayoutStore } from "@/stores/workspace-layout-store"; import type { WorkspaceTabTarget } from "@/workspace-tabs/model"; import { useWorkspaceCheckoutStatus } from "@/screens/workspace/use-workspace-checkout-status"; -import { openWorkspaceFileFromExplorer } from "@/screens/workspace/workspace-file-open-command"; +import { openWorkspaceFileInFocusedPane } from "@/screens/workspace/workspace-file-open-command"; import { isWeb } from "@/constants/platform"; import { DiffDocumentWorkspaceCacheProvider } from "@/git/diff-document/workspace-cache"; import { @@ -119,6 +119,7 @@ export function CompactExplorerSidebarHost({ [openTab], ); const focusWorkspaceTab = useWorkspaceLayoutStore((state) => state.focusTab); + const requestFileNavigation = useWorkspaceLayoutStore((state) => state.requestFileNavigation); const handleOpenExplorer = useCallback(() => { if (!model?.workspaceRoot) { @@ -136,16 +137,24 @@ export function CompactExplorerSidebarHost({ if (!model) { return; } - openWorkspaceFileFromExplorer({ - filePath, + openWorkspaceFileInFocusedPane({ + location: { path: filePath }, persistenceKey: model.persistenceKey, closeExplorerAfterOpen: presentation === "overlay", showMobileAgent, openWorkspaceTabInFocusedPane, focusWorkspaceTab, + requestFileNavigation, }); }, - [focusWorkspaceTab, model, openWorkspaceTabInFocusedPane, presentation, showMobileAgent], + [ + focusWorkspaceTab, + model, + openWorkspaceTabInFocusedPane, + presentation, + requestFileNavigation, + showMobileAgent, + ], ); const handleContainerLayout = useCallback((event: LayoutChangeEvent) => { diff --git a/packages/app/src/file-pane/source/row-layout.test.ts b/packages/app/src/file-pane/source/row-layout.test.ts deleted file mode 100644 index e1b12ddb1f..0000000000 --- a/packages/app/src/file-pane/source/row-layout.test.ts +++ /dev/null @@ -1,63 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { sourceRowOffsets, visualLineCount } from "./row-layout"; - -// Measured on a Pixel-class Android device at the default code size: one visual line is 17.5234dp -// after the device rounds it, and 49 monospace characters fit beside the gutter. -const VISUAL_LINE_HEIGHT = 17.5234375; -const COLUMNS = 49; - -// An ordinary TypeScript line, 119 characters, of the kind that fills a real source file. -const ORDINARY_LINE = - "export function handlerNumber1(input: Record, options: { retries: number }): Promise { /* 1 */ }"; - -describe("visualLineCount", () => { - it("keeps a line that fits on one visual line", () => { - expect(visualLineCount("const a = 1;", COLUMNS)).toBe(1); - expect(visualLineCount("", COLUMNS)).toBe(1); - }); - - it("counts the visual lines an ordinary source line occupies", () => { - expect(visualLineCount(ORDINARY_LINE, COLUMNS)).toBe(3); - }); - - it("breaks early at a space, where dividing by the column count would not", () => { - // A larger code size narrows the columns. Character division says four lines; the platform - // cannot split "handlerNumber1(input:" across a space that is not there, so it needs five. - expect(Math.ceil(ORDINARY_LINE.length / 33)).toBe(4); - expect(visualLineCount(ORDINARY_LINE, 33)).toBe(5); - }); - - it("breaks a single word that is wider than the line", () => { - expect(visualLineCount("x".repeat(COLUMNS * 3), COLUMNS)).toBe(3); - }); - - it("assumes one visual line until the column width is measured", () => { - expect(visualLineCount(ORDINARY_LINE, 0)).toBe(1); - }); -}); - -describe("sourceRowOffsets", () => { - it("stacks rows by the visual lines they really occupy", () => { - const offsets = sourceRowOffsets([{ text: "const a = 1;" }, { text: ORDINARY_LINE }], { - columns: COLUMNS, - visualLineHeight: VISUAL_LINE_HEIGHT, - }); - expect(Array.from(offsets)).toEqual([0, VISUAL_LINE_HEIGHT, VISUAL_LINE_HEIGHT * 4]); - }); - - it("puts a match after a thousand ordinary lines where it is really drawn", () => { - // The regression: 1,200 ordinary lines, match on 1,201. Claiming one visual line per row - // reported 1200 * 17.52 = 21,028, which is about line 1,100 on screen — the match was roughly - // 90 lines below the fold and never revealed. - const lines = [ - ...Array.from({ length: 1200 }, () => ({ text: ORDINARY_LINE })), - { text: 'const target = "needle";' }, - ]; - const offsets = sourceRowOffsets(lines, { - columns: COLUMNS, - visualLineHeight: VISUAL_LINE_HEIGHT, - }); - expect(offsets[1200]).toBeCloseTo(1200 * 3 * VISUAL_LINE_HEIGHT, 5); - expect(offsets[1200]).toBeGreaterThan(1200 * VISUAL_LINE_HEIGHT * 2.9); - }); -}); diff --git a/packages/app/src/file-pane/source/row-layout.ts b/packages/app/src/file-pane/source/row-layout.ts deleted file mode 100644 index 9e3294b5e9..0000000000 --- a/packages/app/src/file-pane/source/row-layout.ts +++ /dev/null @@ -1,55 +0,0 @@ -/** - * Where each source row sits in the list. - * - * A row is one visual line of code tall, and a line too long for the available columns wraps onto - * more of them. The virtualized list is told these offsets so it can jump straight to a match - * thousands of lines in; if they claim one visual line per row, an ordinary source file — whose - * lines routinely wrap on a phone — puts the match hundreds of lines off screen. - */ -export interface SourceRowGeometry { - /** Monospace characters that fit beside the gutter; 0 before the first measurement. */ - columns: number; - /** Height of one visual line, already rounded the way the device rounds it. */ - visualLineHeight: number; -} - -/** - * Visual lines a line of code occupies, wrapped the way the platform wraps it: greedily at spaces, - * and mid-word only for a word too long to fit a line of its own. Dividing the character count by - * the column count instead under-counts every line that has to break early at a space, and the - * error compounds over a long file. One line until the column width has been measured. - */ -export function visualLineCount(text: string, columns: number): number { - if (columns <= 0) return 1; - let rows = 1; - let used = 0; - for (const word of text.split(" ")) { - const gap = used === 0 ? 0 : 1; - if (used + gap + word.length <= columns) { - used += gap + word.length; - continue; - } - if (used > 0) rows += 1; - if (word.length > columns) { - rows += Math.ceil(word.length / columns) - 1; - used = word.length % columns || columns; - continue; - } - used = word.length; - } - return rows; -} - -/** Cumulative top edge of every row, with one extra entry for the end of the last row. */ -export function sourceRowOffsets( - lines: ReadonlyArray<{ text: string }>, - geometry: SourceRowGeometry, -): Float64Array { - const stops = new Float64Array(lines.length + 1); - for (let index = 0; index < lines.length; index += 1) { - stops[index + 1] = - stops[index] + - visualLineCount(lines[index].text, geometry.columns) * geometry.visualLineHeight; - } - return stops; -} diff --git a/packages/app/src/file-pane/source/view.native.tsx b/packages/app/src/file-pane/source/view.native.tsx index 571cc195d7..3ef40c93d6 100644 --- a/packages/app/src/file-pane/source/view.native.tsx +++ b/packages/app/src/file-pane/source/view.native.tsx @@ -1,13 +1,12 @@ import { useTranslation } from "react-i18next"; -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { FlatList, PixelRatio, Text, View, type LayoutChangeEvent } from "react-native"; +import { useCallback, useEffect, useMemo, useRef } from "react"; +import { FlatList, Text, View } from "react-native"; import { StyleSheet } from "react-native-unistyles"; import { highlightCode, type HighlightToken } from "@getpaseo/highlight"; import { syntaxTokenStyleFor } from "@/styles/syntax-token-styles"; import type { WorkspaceFileLocation } from "@/workspace/file-open"; import type { EditorVisualTheme } from "../editor/extensions.web"; import { selectSourcePresentation } from "./presentation"; -import { sourceRowOffsets } from "./row-layout"; interface FileSourceViewProps { content: string; @@ -22,8 +21,6 @@ interface FileSourceViewProps { interface SourceLine { number: number; tokens: HighlightToken[]; - /** The line's own text, which decides how many visual lines its row occupies. */ - text: string; } export function FileSourceView({ @@ -32,7 +29,6 @@ export function FileSourceView({ location, navigationRevision, size, - theme, tooLargeMessage, }: FileSourceViewProps) { const presentation = selectSourcePresentation({ size, platform: "native" }); @@ -50,7 +46,6 @@ export function FileSourceView({ location={location} navigationRevision={navigationRevision} presentation={presentation} - codeFontSize={theme.codeFontSize} /> ); } @@ -61,10 +56,8 @@ function VirtualizedSource({ location, navigationRevision, presentation, - codeFontSize, }: Omit & { presentation: "highlighted" | "plain"; - codeFontSize: number; }) { const { t } = useTranslation(); const listRef = useRef>(null); @@ -74,57 +67,19 @@ function VirtualizedSource({ return highlightCode(source, filename).map((tokens, index) => ({ number: index + 1, tokens, - text: tokens.map((token) => token.text).join(""), })); - return source.split("\n").map((text, index) => ({ - number: index + 1, - tokens: [{ text, style: null }], - text, - })); + return source + .split("\n") + .map((text, index) => ({ number: index + 1, tokens: [{ text, style: null }] })); }, [content, filename, presentation]); - // Jumping to a match thousands of lines in only works if the list is told the truth about row - // heights. A row is one visual line at the reader's own code size, rounded the way the device - // rounds it, times however many visual lines the text wraps onto. Assuming one line per row put - // a match in an ordinary source file hundreds of lines off screen, because ordinary lines wrap. - const [columnWidth, setColumnWidth] = useState(0); - const [listWidth, setListWidth] = useState(0); - const visualLineHeight = PixelRatio.roundToNearestPixel(sourceRowHeight(codeFontSize)); - const columns = - columnWidth > 0 && listWidth > 0 - ? Math.max(1, Math.floor((listWidth - GUTTER_WIDTH) / columnWidth)) - : 0; - const offsets = useMemo( - () => sourceRowOffsets(lines, { columns, visualLineHeight }), - [columns, lines, visualLineHeight], - ); - const itemLayout = useCallback( - (_data: ArrayLike | null | undefined, index: number) => ({ - length: offsets[index + 1] - offsets[index], - offset: offsets[index], - index, - }), - [offsets], - ); - const measureColumn = useCallback( - (event: LayoutChangeEvent) => - setColumnWidth(event.nativeEvent.layout.width / COLUMN_PROBE.length), - [], - ); - const measureList = useCallback( - (event: LayoutChangeEvent) => setListWidth(event.nativeEvent.layout.width), - [], - ); useEffect(() => { if (!location.lineStart) return; listRef.current?.scrollToIndex({ index: Math.min(location.lineStart - 1, lines.length - 1), animated: false, - // Near the top with a little context above, rather than centred: centring is measured - // against the list's own box, which inside a sheet is taller than the part on screen. - viewPosition: 0, - viewOffset: visualLineHeight * CONTEXT_LINES_ABOVE_MATCH, + viewPosition: 0.5, }); - }, [lines.length, location.lineStart, navigationRevision, offsets, visualLineHeight]); + }, [lines.length, location.lineStart, navigationRevision]); const selectedLine = lines[(location.lineStart ?? 1) - 1]?.tokens .map((token) => token.text) .join(""); @@ -143,27 +98,22 @@ function VirtualizedSource({ [changed, location], ); return ( - - - {COLUMN_PROBE} - + {changed ? ( {t("shell.commandCenter.contentChanged")} ) : null} - {columns > 0 ? ( - - ) : null} + ); } @@ -196,17 +146,10 @@ function sourceLineKey(line: SourceLine): string { return String(line.number); } -/** One visual line of code; the stylesheet below builds its row from the same ratio. */ -function sourceRowHeight(codeFontSize: number): number { - return codeFontSize * SOURCE_LINE_RATIO; +function sourceLineLayout(_data: ArrayLike | null | undefined, index: number) { + return { length: 20, offset: index * 20, index }; } -const SOURCE_LINE_RATIO = 1.45; -const GUTTER_WIDTH = 56; -/** Measured once to learn how wide one monospace character is at the reader's code size. */ -const COLUMN_PROBE = "0".repeat(10); -const CONTEXT_LINES_ABOVE_MATCH = 2; - const styles = StyleSheet.create((theme) => ({ root: { flex: 1 }, notice: { @@ -222,25 +165,19 @@ const styles = StyleSheet.create((theme) => ({ padding: theme.spacing[4], }, unsupportedText: { color: theme.colors.foregroundMuted, textAlign: "center" }, - line: { flexDirection: "row", minHeight: sourceRowHeight(theme.fontSize.code) }, + line: { flexDirection: "row", minHeight: theme.fontSize.code * 1.45 }, gutter: { - width: GUTTER_WIDTH, + width: 56, paddingRight: theme.spacing[3], textAlign: "right", color: theme.colors.foregroundMuted, fontFamily: theme.fontFamily.mono, fontSize: theme.fontSize.code, }, - columnProbe: { - position: "absolute", - opacity: 0, - fontFamily: theme.fontFamily.mono, - fontSize: theme.fontSize.code, - }, text: { flex: 1, fontFamily: theme.fontFamily.mono, fontSize: theme.fontSize.code, - lineHeight: sourceRowHeight(theme.fontSize.code), + lineHeight: theme.fontSize.code * 1.45, }, })); diff --git a/packages/app/src/screens/workspace/workspace-file-open-command.test.ts b/packages/app/src/screens/workspace/workspace-file-open-command.test.ts index d1551c7a8c..90cc160865 100644 --- a/packages/app/src/screens/workspace/workspace-file-open-command.test.ts +++ b/packages/app/src/screens/workspace/workspace-file-open-command.test.ts @@ -1,22 +1,24 @@ import { describe, expect, it, vi } from "vitest"; -import { openWorkspaceFileFromExplorer } from "@/screens/workspace/workspace-file-open-command"; +import { openWorkspaceFileInFocusedPane } from "@/screens/workspace/workspace-file-open-command"; +import type { WorkspaceFileLocation } from "@/workspace/file-open"; function createInput(closeExplorerAfterOpen: boolean) { return { - filePath: "src/app.tsx", + location: { path: "src/app.tsx" } as WorkspaceFileLocation, persistenceKey: "server:workspace", closeExplorerAfterOpen, showMobileAgent: vi.fn(), openWorkspaceTabInFocusedPane: vi.fn(() => "file-tab"), focusWorkspaceTab: vi.fn(), + requestFileNavigation: vi.fn(), }; } -describe("openWorkspaceFileFromExplorer", () => { +describe("openWorkspaceFileInFocusedPane", () => { it("closes the phone overlay after opening a file", () => { const input = createInput(true); - openWorkspaceFileFromExplorer(input); + openWorkspaceFileInFocusedPane(input); expect(input.showMobileAgent).toHaveBeenCalledOnce(); }); @@ -24,10 +26,29 @@ describe("openWorkspaceFileFromExplorer", () => { it("keeps the tablet dock open after opening a file", () => { const input = createInput(false); - openWorkspaceFileFromExplorer(input); + openWorkspaceFileInFocusedPane(input); expect(input.showMobileAgent).not.toHaveBeenCalled(); expect(input.openWorkspaceTabInFocusedPane).toHaveBeenCalledOnce(); expect(input.focusWorkspaceTab).toHaveBeenCalledWith("server:workspace", "file-tab"); }); + + it("asks for navigation every time, so reopening the same occurrence recentres its pane", () => { + const input = createInput(false); + input.location = { path: "src/app.tsx", lineStart: 1201, columnStart: 17 }; + + openWorkspaceFileInFocusedPane(input); + openWorkspaceFileInFocusedPane(input); + + expect(input.requestFileNavigation).toHaveBeenCalledTimes(2); + expect(input.requestFileNavigation).toHaveBeenLastCalledWith("file-tab"); + }); + + it("does not navigate when no tab could be opened", () => { + const input = { ...createInput(false), openWorkspaceTabInFocusedPane: vi.fn(() => null) }; + + openWorkspaceFileInFocusedPane(input); + + expect(input.requestFileNavigation).not.toHaveBeenCalled(); + }); }); diff --git a/packages/app/src/screens/workspace/workspace-file-open-command.ts b/packages/app/src/screens/workspace/workspace-file-open-command.ts index 0d990f3557..b024fdf016 100644 --- a/packages/app/src/screens/workspace/workspace-file-open-command.ts +++ b/packages/app/src/screens/workspace/workspace-file-open-command.ts @@ -1,6 +1,7 @@ import { createWorkspaceFileTabTarget, normalizeWorkspaceFileLocation, + type WorkspaceFileLocation, } from "@/workspace/file-open"; import { FOCUSED_PANE_PLACEMENT, @@ -8,8 +9,8 @@ import { } from "@/stores/workspace-layout-store"; import type { WorkspaceTabTarget } from "@/workspace-tabs/model"; -interface OpenWorkspaceFileFromExplorerInput { - filePath: string; +interface OpenWorkspaceFileInput { + location: WorkspaceFileLocation; persistenceKey: string | null; closeExplorerAfterOpen: boolean; showMobileAgent: () => void; @@ -19,16 +20,24 @@ interface OpenWorkspaceFileFromExplorerInput { placement?: WorkspaceTabPlacement, ) => string | null; focusWorkspaceTab: (workspaceKey: string, tabId: string) => void; + requestFileNavigation: (tabId: string) => void; } -export function openWorkspaceFileFromExplorer(input: OpenWorkspaceFileFromExplorerInput): void { +/** + * Opens a workspace file in the focused pane and navigates to it. + * + * The navigation request is the part callers keep forgetting: a file target is identity-stable, so + * reopening the same path — or the same occurrence — reuses its tab and would otherwise leave the + * pane wherever the reader had scrolled it. Every explicit activation asks for a reveal. + */ +export function openWorkspaceFileInFocusedPane(input: OpenWorkspaceFileInput): void { if (input.closeExplorerAfterOpen) { input.showMobileAgent(); } if (!input.persistenceKey) { return; } - const location = normalizeWorkspaceFileLocation({ path: input.filePath }); + const location = normalizeWorkspaceFileLocation(input.location); if (!location) { return; } @@ -37,7 +46,9 @@ export function openWorkspaceFileFromExplorer(input: OpenWorkspaceFileFromExplor createWorkspaceFileTabTarget(location), FOCUSED_PANE_PLACEMENT, ); - if (tabId) { - input.focusWorkspaceTab(input.persistenceKey, tabId); + if (!tabId) { + return; } + input.focusWorkspaceTab(input.persistenceKey, tabId); + input.requestFileNavigation(tabId); } diff --git a/packages/app/src/screens/workspace/workspace-screen.tsx b/packages/app/src/screens/workspace/workspace-screen.tsx index d9510f517f..e0fb014b6e 100644 --- a/packages/app/src/screens/workspace/workspace-screen.tsx +++ b/packages/app/src/screens/workspace/workspace-screen.tsx @@ -1647,17 +1647,13 @@ function WorkspaceScreenContent({ ) => openTab({ workspaceKey, target, intent: "reveal", parentTabId, placement }), [openTab], ); - // File targets stay identity-stable so the same path reuses its tab. Keep navigation - // requests separate so clicking an unchanged path:line can still recenter the pane. - const [fileNavigationRevisionByTabId, setFileNavigationRevisionByTabId] = useState< - Record - >({}); - const requestFileNavigation = useCallback((tabId: string) => { - setFileNavigationRevisionByTabId((current) => ({ - ...current, - [tabId]: (current[tabId] ?? 0) + 1, - })); - }, []); + // File targets stay identity-stable so the same path reuses its tab. Navigation requests live + // with the tabs in the layout store, so an opener outside this screen — the Command Center — + // asks for the same reveal through the same capability. + const fileNavigationRevisionByTabId = useWorkspaceLayoutStore( + (state) => state.fileNavigationRevisionByTabId, + ); + const requestFileNavigation = useWorkspaceLayoutStore((state) => state.requestFileNavigation); const focusWorkspacePane = useWorkspaceLayoutStore((state) => state.focusPane); const hasHydratedWorkspaces = useSessionStore( (state) => state.sessions[normalizedServerId]?.hasHydratedWorkspaces ?? false, diff --git a/packages/app/src/stores/workspace-layout-store.ts b/packages/app/src/stores/workspace-layout-store.ts index 9281950eaf..1d904a98df 100644 --- a/packages/app/src/stores/workspace-layout-store.ts +++ b/packages/app/src/stores/workspace-layout-store.ts @@ -110,6 +110,12 @@ interface WorkspaceLayoutStore { pendingAgentIdsByWorkspace: Record>; hiddenAgentIdsByWorkspace: Record>; focusRestorationByWorkspace: Record; + /** + * Bumped whenever a file tab is explicitly activated. File targets are identity-stable so the + * same path reuses its tab; without this an activation of an unchanged location would look like + * a no-op and leave the pane wherever the reader had scrolled it. + */ + fileNavigationRevisionByTabId: Record; explorerSidebarPaneIdByWorkspace: Record; sidePaneIdByWorkspace: Record; openTab: (input: OpenWorkspaceTabInput) => string | null; @@ -120,6 +126,7 @@ interface WorkspaceLayoutStore { ensureSidePane: (workspaceKey: string) => string | null; closeTab: (workspaceKey: string, tabId: string) => void; focusTab: (workspaceKey: string, tabId: string) => void; + requestFileNavigation: (tabId: string) => void; selectTabInPane: (workspaceKey: string, paneId: string, tabId: string) => void; replaceTab: ( workspaceKey: string, @@ -770,6 +777,7 @@ export function createWorkspaceLayoutStore( pendingAgentIdsByWorkspace: {}, hiddenAgentIdsByWorkspace: {}, focusRestorationByWorkspace: {}, + fileNavigationRevisionByTabId: {}, explorerSidebarPaneIdByWorkspace: {}, sidePaneIdByWorkspace: {}, openTab: (input) => { @@ -1043,6 +1051,16 @@ export function createWorkspaceLayoutStore( }; }); }, + requestFileNavigation: (tabId) => { + const normalizedTabId = trimNonEmpty(tabId); + if (!normalizedTabId) return; + set((state) => ({ + fileNavigationRevisionByTabId: { + ...state.fileNavigationRevisionByTabId, + [normalizedTabId]: (state.fileNavigationRevisionByTabId[normalizedTabId] ?? 0) + 1, + }, + })); + }, focusTab: (workspaceKey, tabId) => { const normalizedWorkspaceKey = trimNonEmpty(workspaceKey); const normalizedTabId = trimNonEmpty(tabId); diff --git a/packages/server/src/server/daemon-e2e/workspace-content-search.e2e.test.ts b/packages/server/src/server/daemon-e2e/workspace-content-search.e2e.test.ts index 9f134fb581..d0d79cd0e4 100644 --- a/packages/server/src/server/daemon-e2e/workspace-content-search.e2e.test.ts +++ b/packages/server/src/server/daemon-e2e/workspace-content-search.e2e.test.ts @@ -24,10 +24,13 @@ test("real client searches saved contents, cancels host work and can search agai ); controller.abort(); expect(await searching).toMatchObject({ status: "error", code: "cancelled" }); + // A complete empty result also reports the per-file ceiling the host applied, so the client + // can say which files it never opened rather than claiming it read the whole workspace. expect(await client.searchWorkspaceContent({ cwd, query: "missing" })).toEqual({ status: "ok", matches: [], limited: false, + maxFileBytes: 1_048_576, }); const preview = await client.readFile(cwd, "a.ts", undefined, 1_048_576); expect(new TextDecoder().decode(preview.bytes)).toBe("const original = 'NEEDLE';\n"); diff --git a/packages/server/src/server/path-utils.test.ts b/packages/server/src/server/path-utils.test.ts new file mode 100644 index 0000000000..4857a97288 --- /dev/null +++ b/packages/server/src/server/path-utils.test.ts @@ -0,0 +1,18 @@ +import { sep } from "node:path"; +import { expect, test } from "vitest"; +import { toWorkspaceRelativePath } from "./path-utils.js"; + +// Workspace-relative identities cross the wire with "/" whatever the host separator is, so a +// client never has to guess which backslash was a separator and which was a file name character. +test("renders a host-relative path with forward slashes", () => { + expect(toWorkspaceRelativePath(["src", "app.ts"].join(sep))).toBe("src/app.ts"); + expect(toWorkspaceRelativePath("app.ts")).toBe("app.ts"); +}); + +test.skipIf(sep === "\\")("leaves a POSIX file name containing a backslash intact", () => { + expect(toWorkspaceRelativePath("a\\b.txt")).toBe("a\\b.txt"); +}); + +test.skipIf(sep !== "\\")("converts every Windows separator", () => { + expect(toWorkspaceRelativePath("src\\nested\\app.ts")).toBe("src/nested/app.ts"); +}); diff --git a/packages/server/src/server/workspace-content-search/index.test.ts b/packages/server/src/server/workspace-content-search/index.test.ts index eadd61c1c9..6bb96a4450 100644 --- a/packages/server/src/server/workspace-content-search/index.test.ts +++ b/packages/server/src/server/workspace-content-search/index.test.ts @@ -173,9 +173,16 @@ test("states the per-file ceiling so a search that skipped an oversized file is ).toEqual([["small.txt"], 1_048_576]); }); -test("keeps a literal backslash in a saved file name exactly as the host reported it", async () => { - // A backslash is an ordinary character in a Unix file name, never a separator. - const cwd = await workspace({ "a\\b.txt": "needle" }); - const result = await searchWorkspaceContent({ cwd, query: "needle" }); - expect(result.status === "ok" && result.matches.map((match) => match.path)).toEqual(["a\\b.txt"]); -}); +// A backslash is an ordinary character in a Unix file name but a separator on Windows, so this +// file name only exists on POSIX. Windows separator handling is covered by toWorkspaceRelativePath, +// which runs on every platform. +test.skipIf(process.platform === "win32")( + "keeps a literal backslash in a saved file name exactly as the host reported it", + async () => { + const cwd = await workspace({ "a\\b.txt": "needle" }); + const result = await searchWorkspaceContent({ cwd, query: "needle" }); + expect(result.status === "ok" && result.matches.map((match) => match.path)).toEqual([ + "a\\b.txt", + ]); + }, +);