Skip to content

Commit 11507bb

Browse files
authored
Code tab: right-click Open path:N jumps to file at that line (#891)
**The Code tab's right-click menu gains an `Open <path>:<line>` entry.** Reviewing a diff and wanting the full file at the same line was previously two steps — copy `path:N`, switch to browse mode, paste-navigate. Now it's one click; the menu item dispatches through the same pipeline a terminal-link click does. This is **Phase 0 of #881's redo** — small user-visible win on top of a seam Phase 1 (line-anchored comments) will reuse. The structural review behind the redo is in #881's body and replaces #879. ### Before / after ``` Diff view: alpha beta ← right-click line 2 gamma Before: [Copy path] [Copy notes.txt:2] After: [Copy path] [Copy notes.txt:2] [Open notes.txt:2] └──┐ one click ▼ browse mode, file open, line 2 highlighted ``` ### The seam The terminal-link click pipeline used to require a **paired write** at every call site: ```ts // Terminal.tsx (before) rightPanel.openCodeBrowser(); // ① uncollapse + tab + browse-mode requestCodeOpen({ ref, repoRoot, cwd, targetMode: "browse" }); // ② pending // Comment: "must fire in the same DOM-event tick or resetKey clears // what pendingCodeOpen is about to set." ``` The ordering hazard sat at the call site as documented discipline, which meant every new producer (right-click _Open_, future _Comment on_) inherited the same wall-of-text rationale. Phase 0 collapses both writes into one function: ```ts // Terminal.tsx (after) openInCodeTab({ ref, repoRoot, cwd, targetMode: "browse" }); ``` `openInCodeTab` (replacing `codeNavigation.ts`) encapsulates both writes; `useRightPanel.openCodeBrowser()` is generalized to `openCodeAt(mode: CodeTabView)` so a Phase 1 comment authored while reviewing a diff can re-open in that diff mode rather than always browse. ### What landed | Layer | Change | | --- | --- | | Producer-facing | `openInCodeTab(req)` — single call replaces the paired-write pattern at every site | | Panel state | `useRightPanel.openCodeAt(mode)` — atomic three-field preferences patch parameterized on the target sub-mode; skips the patch when already at target | | Menu data | `CodeContextMenuItem` becomes a discriminated union (`kind: "copy"` vs `kind: "action"`); `handleItem` dispatches via `ts-pattern.exhaustive()` | | Selection hook | `useLineSelection` gains `onOpen?: Accessor<((ref) => void) \| undefined>`; when present and a range is selected, an _Open path:N_ action item is emitted | | Code tab wiring | The diff-view `CodeMenuFrame` passes `onOpen` that routes through `openInCodeTab`. The browse-view path deliberately omits it — the file is already on screen at line precision | _The `Accessor<…>` shape on `onOpen` mirrors `initialRange` — buildItems runs at menu-open time, so a host whose prop arrives late in the SolidJS lifecycle still flips the item in._ ### Test coverage - `code-tab.feature` — new scenario: right-click in diff view, click _Open notes.txt:2_, assert mode flipped to browse + file selected + line 2 highlighted - `code-tab.feature` — updated multi-file diff scenario to assert the three-item menu (`Copy path | Copy file-b.txt:1 | Open file-b.txt:1`) - `file-ref-link.feature` — terminal-link clicks now flow through `openInCodeTab`; all three pre-existing scenarios still pass 43/43 scenarios, 503/503 steps locally. ### Try it locally ```sh nix run github:juspay/kolu/open-in-code-tab ``` _Generated by [`/do`](https://github.com/srid/agency) on Claude Code (model `claude-opus-4-7`)._
1 parent 3e7623b commit 11507bb

14 files changed

Lines changed: 422 additions & 162 deletions

File tree

packages/client/src/right-panel/CodeMenuFrame.tsx

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
CodeContextMenu,
1212
type CodeContextMenuController,
1313
} from "../ui/CodeContextMenu";
14+
import type { LineRef } from "../ui/lineRef";
1415
import { type LineSelection, useLineSelection } from "../ui/useLineSelection";
1516

1617
export type CodeMenuFrameProps = {
@@ -25,19 +26,57 @@ export type CodeMenuFrameProps = {
2526
* controller so a terminal `path:line` click drives both the
2627
* Pierre highlight AND the right-click menu's "Copy path:N" item. */
2728
initialSelectedLines?: SelectedLineRange | null;
29+
/** When provided, adds an "Open <path>:<line>" entry to the context
30+
* menu that dispatches the selected ref to the host (typically a
31+
* call to `openInCodeTab`). Omit for viewers where "open" is a
32+
* no-op (the file is already on screen at line precision). */
33+
onOpen?: (ref: LineRef) => void;
2834
};
2935

36+
/** Walk the contextmenu event's composed path (which pierces Pierre's
37+
* open shadow DOM, where `event.target` would otherwise be retargeted
38+
* to the shadow host) and return the line number from the first
39+
* element carrying `data-column-number`. Returns null when the
40+
* right-click landed outside any gutter line — empty area, scrollbar,
41+
* decoration row — so the host can skip opening a menu entirely. */
42+
function lineFromContextMenu(event: MouseEvent): number | null {
43+
for (const node of event.composedPath()) {
44+
if (!(node instanceof Element)) continue;
45+
const raw = node.getAttribute("data-column-number");
46+
if (raw === null) continue;
47+
const n = Number(raw);
48+
if (Number.isFinite(n) && n >= 1) return n;
49+
}
50+
return null;
51+
}
52+
3053
export const CodeMenuFrame: Component<CodeMenuFrameProps> = (props) => {
3154
let menuCtrl: CodeContextMenuController | undefined;
3255
const selection = useLineSelection(() => props.path, {
3356
initialRange: () => props.initialSelectedLines,
57+
onOpen: () => props.onOpen,
3458
});
3559
return (
3660
<div
3761
// Attach contextmenu via addEventListener so the host div doesn't
3862
// carry interactive JSX props — the inner Pierre canvas is the
3963
// actual interactive surface; the host is layout only.
40-
ref={(el) => el.addEventListener("contextmenu", (e) => menuCtrl?.open(e))}
64+
ref={(el) =>
65+
el.addEventListener("contextmenu", (e) => {
66+
// Right-click on a gutter line is the single entry point for
67+
// the context menu: it both selects the line and opens the
68+
// menu in one gesture. Right-clicks elsewhere (whitespace,
69+
// scrollbar, decoration row) clear the range and produce no
70+
// menu — `buildItems` returns empty when no range is set,
71+
// so `menuCtrl.open` short-circuits without preventing the
72+
// browser default.
73+
const line = lineFromContextMenu(e);
74+
selection.handleSelect(
75+
line === null ? null : { start: line, end: line },
76+
);
77+
menuCtrl?.open(e);
78+
})
79+
}
4180
class="h-full w-full"
4281
>
4382
{props.children(selection)}

packages/client/src/right-panel/CodeTab.tsx

Lines changed: 55 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -41,8 +41,12 @@ import {
4141
} from "../ui/pierreTheme";
4242
import { resolveLineRefPath } from "../ui/lineRef";
4343
import BrowseFileView from "./BrowseFileView";
44-
import { type CodeOpenRequest, pendingCodeOpen } from "./codeNavigation";
4544
import CodeMenuFrame from "./CodeMenuFrame";
45+
import {
46+
openInCodeTab,
47+
type OpenInCodeTabRequest,
48+
pendingOpen,
49+
} from "./openInCodeTab";
4650
import { projectFileTreeSearch } from "./fileSearch";
4751
import FileSearchInput from "./FileSearchInput";
4852
import ModeChipPicker, { type ModeOption } from "./ModeChipPicker";
@@ -176,57 +180,66 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => {
176180
// absolute path or `null`, never the empty string that would alias
177181
// null.
178182
const resetKey = createMemo(() => `${repoPath() ?? ""}::${view()}`);
183+
184+
/** The resetKey effect runs BEFORE the pendingOpen effect by
185+
* registration order. When a navigation request is about to land in
186+
* the new (repo, mode) — same repoRoot, target mode equals the
187+
* freshly-ticked `view()`, and the request hasn't already been
188+
* consumed by `handled()` — the resetKey effect must skip its clear,
189+
* or the pendingOpen effect would null what we're about to set.
190+
* This predicate names the cross-effect temporal coupling so the
191+
* guard isn't a wall of inline conjunctions a future editor has to
192+
* re-derive. The `batch()` in `openInCodeTab` ensures both writes
193+
* (`view` and `pendingOpen`) commit before either effect fires; the
194+
* registration-order discipline survives ABOVE that. */
195+
const isPendingOpenAboutToLand = (): boolean => {
196+
const req = pendingOpen();
197+
return (
198+
req !== null &&
199+
req.repoRoot === repoPath() &&
200+
req.targetMode === view() &&
201+
handled()?.request !== req
202+
);
203+
};
204+
179205
createEffect(
180206
on(
181207
resetKey,
182208
() => {
183209
setSearchQuery("");
184-
// Skip the selectedPath clear when an incoming request is
185-
// about to land in the new mode — the resetKey effect runs
186-
// before the pendingCodeOpen effect (registration order), and
187-
// an unconditional clear would null what we're about to set.
188-
// Reading `req.targetMode` (not `view()`) makes the guard
189-
// robust to user-driven mode flips that race the click.
190-
const req = pendingCodeOpen();
191-
if (
192-
req &&
193-
req.repoRoot === repoPath() &&
194-
req.targetMode === view() &&
195-
handled()?.request !== req
196-
) {
197-
return;
198-
}
210+
if (isPendingOpenAboutToLand()) return;
199211
setSelectedPath(null);
200212
},
201213
{ defer: true },
202214
),
203215
);
204216

205-
// Consume-once record for the latest pendingCodeOpen tick. Holds
206-
// the full request object (reference identity discriminates two
207-
// structurally-identical clicks — `requestCodeOpen` mints a fresh
217+
// Consume-once record for the latest pendingOpen tick. Holds the
218+
// full request object (reference identity discriminates two
219+
// structurally-identical clicks — `openInCodeTab` mints a fresh
208220
// object per call) alongside the resolved path. Storing the
209221
// request here lets `selectedRange` derive its value without
210222
// re-running `resolveLineRefPath` (single resolution site per
211223
// request) and lets `resetKey` know whether a pending request
212224
// has already been applied.
213225
const [handled, setHandled] = createSignal<{
214-
request: CodeOpenRequest;
226+
request: OpenInCodeTabRequest;
215227
resolvedPath: string | null;
216228
} | null>(null);
217229

218-
// Honor terminal file-ref clicks. The effect waits for the live
219-
// `fsListAll` stream to settle so resolution can validate against
220-
// a complete file list — otherwise a request fired during boot
221-
// would toast "not found" on a path that just hasn't been
222-
// enumerated yet. The terminal click handler is the sole site that
223-
// flips the panel to browse mode; this effect only sets
230+
// Honor every `openInCodeTab` request — terminal file-ref clicks,
231+
// right-click "Open path:N" entries, and any future producer. The
232+
// effect waits for the live `fsListAll` stream to settle so
233+
// resolution can validate against a complete file list — otherwise
234+
// a request fired during boot would toast "not found" on a path
235+
// that just hasn't been enumerated yet. `openInCodeTab` flips the
236+
// panel to browse mode itself; this effect only sets
224237
// `selectedPath`. The `resetKey` effect above guards against
225238
// clearing selectedPath when this effect is about to set it.
226239
createEffect(
227240
on(
228241
() => {
229-
const req = pendingCodeOpen();
242+
const req = pendingOpen();
230243
const paths = treePaths();
231244
const isPending = allPaths.pending();
232245
return { req, repo: repoPath(), paths, isPending };
@@ -263,7 +276,7 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => {
263276
//
264277
// No `equals` override: two clicks on the same `path:line` produce
265278
// structurally identical `{start, end}` but distinct request
266-
// objects (`requestCodeOpen` mints a fresh one per call), so the
279+
// objects (`openInCodeTab` mints a fresh one per call), so the
267280
// memo emits a fresh value on every click. Pierre's
268281
// `InteractionManager.setSelection` re-renders when the selection
269282
// is "dirty" — and tearing down the gutter (panel collapse,
@@ -275,7 +288,7 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => {
275288
start: number;
276289
end: number;
277290
} | null>(() => {
278-
const req = pendingCodeOpen();
291+
const req = pendingOpen();
279292
if (!req) return null;
280293
const h = handled();
281294
if (!h || h.request !== req || h.resolvedPath === null) return null;
@@ -538,7 +551,19 @@ const CodeTab: Component<{ meta: TerminalMetadata | null }> = (props) => {
538551
</Match>
539552
<Match when={diff()}>
540553
{(d) => (
541-
<CodeMenuFrame path={path}>
554+
<CodeMenuFrame
555+
path={path}
556+
onOpen={(ref) => {
557+
// Diff paths are repo-relative; cwd is irrelevant.
558+
const repo = repoPath();
559+
if (repo === null) return;
560+
openInCodeTab({
561+
ref,
562+
repoRoot: repo,
563+
targetMode: "browse",
564+
});
565+
}}
566+
>
542567
{(selection) => (
543568
// `<Virtualizer>` is the scroll container —
544569
// `<FileDiff>` consumes its context and

packages/client/src/right-panel/codeNavigation.ts

Lines changed: 0 additions & 39 deletions
This file was deleted.

packages/client/src/right-panel/fileSearch.ts

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
* the directories the wrapper should ensure are open so matches don't
1414
* hide behind a collapsed parent on first paint. */
1515

16+
import { ancestorDirectoryPaths } from "@kolu/solid-pierre";
17+
1618
type FileTreeSearchProjection = {
1719
projectedPaths: string[];
1820
expandedAncestors: string[];
@@ -39,21 +41,6 @@ function pathContainsTokensInOrder(
3941
return true;
4042
}
4143

42-
/** Pierre uses `getAncestorDirectoryPaths` internally to drive
43-
* expansion in `hide-non-matches` mode. Mirror that exact shape so
44-
* the wrapper's expansion request reaches every dir Pierre infers
45-
* from the projected paths. */
46-
function ancestorDirectoryPaths(path: string): string[] {
47-
const normalized = path.endsWith("/") ? path.slice(0, -1) : path;
48-
if (normalized.length === 0) return [];
49-
const segments = normalized.split("/");
50-
const out: string[] = [];
51-
for (let i = 1; i < segments.length; i += 1) {
52-
out.push(`${segments.slice(0, i).join("/")}/`);
53-
}
54-
return out;
55-
}
56-
5744
export function projectFileTreeSearch(
5845
paths: string[],
5946
query: string,
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/** Front door for "open this file:line in the Code tab". Every producer
2+
* — terminal-link click, right-click "Open path:N" context-menu entry,
3+
* future surfaces — calls `openInCodeTab(...)` instead of writing the
4+
* preferences patch and pending-request signal separately. The function
5+
* encapsulates the paired writes (panel-uncollapse + tab + browse-mode +
6+
* pending request) so the SolidJS effect-ordering invariant lives here,
7+
* not at every call site.
8+
*
9+
* Latest request wins; callers don't clear it. Each call mints a fresh
10+
* request object — two clicks on the same `path:line` are distinct by
11+
* reference, which is what lets `CodeTab` tell them apart even when
12+
* their `ref` content matches and re-paint the highlight. */
13+
14+
import type { CodeTabView } from "kolu-common/surface";
15+
import { batch, createSignal } from "solid-js";
16+
import type { LineRef } from "../ui/lineRef";
17+
import { useRightPanel } from "./useRightPanel";
18+
19+
export interface OpenInCodeTabRequest {
20+
/** Parsed `path:line[-end]` to navigate to. The path is interpreted
21+
* relative to `repoRoot` (or, when present, cwd-relative under
22+
* `repoRoot`) by `CodeTab` via `resolveLineRefPath`. */
23+
ref: LineRef;
24+
/** Per-terminal git repo root that `ref.path` is relative to (when
25+
* relative). Absolute paths beneath this root are also accepted —
26+
* the resolver normalizes both shapes. */
27+
repoRoot: string;
28+
/** Terminal cwd at the time of the request. Drives the "user typed
29+
* `bar.ts:42` while standing in a subdirectory of the repo" case;
30+
* undefined falls back to repo-relative interpretation only. */
31+
cwd?: string;
32+
/** Which Code-tab sub-mode the request expects to land in.
33+
* Producers that don't track an authoring mode pass `"browse"`. */
34+
targetMode: CodeTabView;
35+
}
36+
37+
// Module-level singleton. Right-panel state is a singleton in Kolu —
38+
// one panel, one CodeTab — and the navigation request is meant for
39+
// the unique consumer. If kolu ever mounts multiple CodeTab instances
40+
// (split panels, multi-window), this signal must move into a
41+
// SolidJS context or scope to a per-panel store, otherwise concurrent
42+
// consumers will race on each other's pending requests.
43+
const [pending, setPending] = createSignal<OpenInCodeTabRequest | null>(null);
44+
45+
export const pendingOpen = pending;
46+
47+
/** Open the right panel's Code tab at `req.targetMode` showing `req.ref`.
48+
* The two reactive writes (preferences patch + pending-request signal)
49+
* are wrapped in `batch()` so SolidJS defers dependent effects until
50+
* both have committed. Without the batch, the preferences optimistic
51+
* update ticks `view()` first, fires `CodeTab`'s `resetKey` effect
52+
* when `pendingOpen()` is still null, the guard fails, and the
53+
* selectedPath the user navigated to gets cleared. */
54+
export function openInCodeTab(req: OpenInCodeTabRequest): void {
55+
batch(() => {
56+
useRightPanel().openCodeAt(req.targetMode);
57+
setPending(req);
58+
});
59+
}

packages/client/src/right-panel/useRightPanel.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,26 @@ export function useRightPanel() {
4242
...(mode !== undefined && { codeMode: mode }),
4343
},
4444
}),
45-
/** Atomic "open the code browser at this file" — uncollapse the
46-
* panel, switch to Code, force browse mode. Single patch so the
47-
* UI ticks once instead of three times when callers need all
48-
* three transitions together. */
49-
openCodeBrowser: () =>
45+
/** Atomic "open the Code tab at `mode`" — uncollapse the panel,
46+
* switch to Code, set the requested sub-mode. Single preferences
47+
* patch so the UI ticks once instead of three times when callers
48+
* need all three transitions together. Skips the patch when the
49+
* panel is already in the target state (every diff→browse and
50+
* browse→browse `openInCodeTab` would otherwise round-trip a
51+
* three-field preferences write to the server). */
52+
openCodeAt: (mode: CodeTabView) => {
53+
const cur = rp();
54+
if (!cur.collapsed && cur.activeTab === "code" && cur.codeMode === mode) {
55+
return;
56+
}
5057
updatePreferences({
5158
rightPanel: {
5259
collapsed: false,
5360
activeTab: "code",
54-
codeMode: "browse",
61+
codeMode: mode,
5562
},
56-
}),
63+
});
64+
},
5765
/** Change the sub-mode within the Code tab. */
5866
setCodeMode: (mode: CodeTabView) =>
5967
updatePreferences({ rightPanel: { codeMode: mode } }),

0 commit comments

Comments
 (0)