Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,6 @@ export function useFileTree({
async (absolutePath: string): Promise<void> => {
if (!rootPath || !absolutePath.startsWith(rootPath)) return;

// Collect ancestor directories from rootPath down to the parent of the target
const ancestors: string[] = [];
let current = getParentPath(absolutePath);
while (current.length >= rootPath.length && current !== absolutePath) {
Expand All @@ -494,10 +493,14 @@ export function useFileTree({
current = getParentPath(current);
}

// Expand all ancestors and load their contents
for (const dir of ancestors) {
await expand(dir);
}

const entry = stateRef.current.entriesByPath.get(absolutePath);
if (entry?.kind === "directory") {
await expand(absolutePath);
}
},
[expand, rootPath],
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export class LinkDetectorAdapter implements ILinkProvider {
event: MouseEvent,
link: DetectedLink,
) => void,
private readonly _onHover?: (event: MouseEvent, link: DetectedLink) => void,
private readonly _onLeave?: () => void,
) {}

provideLinks(
Expand Down Expand Up @@ -192,6 +194,12 @@ export class LinkDetectorAdapter implements ILinkProvider {
activate: (event: MouseEvent) => {
this._onActivate?.(event, detected);
},
hover: (event: MouseEvent) => {
this._onHover?.(event, detected);
},
leave: () => {
this._onLeave?.();
},
});
}
}
Expand Down Expand Up @@ -233,6 +241,12 @@ export class LinkDetectorAdapter implements ILinkProvider {
activate: (event: MouseEvent) => {
this._onActivate?.(event, detected);
},
hover: (event: MouseEvent) => {
this._onHover?.(event, detected);
},
leave: () => {
this._onLeave?.();
},
});
}

Expand Down
11 changes: 11 additions & 0 deletions apps/desktop/src/renderer/lib/terminal/links/word-link-detector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ export class WordLinkDetector implements ILinkProvider {
event: MouseEvent,
resolvedPath: string,
) => void,
private readonly _onHover?: (
event: MouseEvent,
resolvedPath: string,
) => void,
private readonly _onLeave?: () => void,
) {
this._separatorRegex = buildSeparatorRegex(DEFAULT_WORD_SEPARATORS);
}
Expand Down Expand Up @@ -108,6 +113,12 @@ export class WordLinkDetector implements ILinkProvider {
activate: (event: MouseEvent) => {
this._onActivate?.(event, resolved.path);
},
hover: (event: MouseEvent) => {
this._onHover?.(event, resolved.path);
},
leave: () => {
this._onLeave?.();
},
});
}

Expand Down
38 changes: 34 additions & 4 deletions apps/desktop/src/renderer/lib/terminal/terminal-link-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,22 @@ import {
WordLinkDetector,
} from "./links";

export type LinkHoverInfo =
| { kind: "file"; isDirectory: boolean }
| { kind: "url" };

/**
* Link handler callbacks for the v2 terminal.
*/
export interface TerminalLinkHandlers {
/** Called when a file path link is activated (Cmd/Ctrl+click). */
onFileLinkClick?: (event: MouseEvent, link: DetectedLink) => void;
/** Called when a URL link is activated. */
onUrlClick?: (url: string) => void;
onUrlClick?: (event: MouseEvent, url: string) => void;
/** Called when the mouse enters a detected link (file path or URL). */
onLinkHover?: (event: MouseEvent, info: LinkHoverInfo) => void;
/** Called when the mouse leaves a previously hovered link. */
onLinkLeave?: () => void;
/**
* Stat callback to validate file paths exist. Called via the host service
* which handles all path resolution (relative, tilde, etc.) server-side.
Expand Down Expand Up @@ -93,21 +101,39 @@ export class TerminalLinkManager {
this._resolver = new TerminalLinkResolver(handlers.stat);
}

const onLinkHover = handlers.onLinkHover;
const onLinkLeave = handlers.onLinkLeave;

// 1. File path detector (highest priority)
const detector = new LocalLinkDetector(this._resolver);
const adapter = new LinkDetectorAdapter(
this._terminal,
detector,
handlers.onFileLinkClick,
onLinkHover
? (event, link) =>
onLinkHover(event, {
kind: "file",
isDirectory: link.isDirectory,
})
: undefined,
onLinkLeave,
);
this._disposables.push(this._terminal.registerLinkProvider(adapter));

// 2. URL link provider (handles hard-wrapped URLs)
if (handlers.onUrlClick) {
const onUrlClick = handlers.onUrlClick;
const urlProvider = new UrlLinkProvider(this._terminal, (_event, uri) => {
onUrlClick(uri);
});
const urlProvider = new UrlLinkProvider(
this._terminal,
(event, uri) => {
onUrlClick(event, uri);
},
onLinkHover
? (event) => onLinkHover(event, { kind: "url" })
: undefined,
onLinkLeave,
);
this._disposables.push(this._terminal.registerLinkProvider(urlProvider));
}

Expand Down Expand Up @@ -135,6 +161,10 @@ export class TerminalLinkManager {
colEnd: undefined,
});
},
onLinkHover
? (event) => onLinkHover(event, { kind: "file", isDirectory: false })
: undefined,
onLinkLeave,
);
this._disposables.push(this._terminal.registerLinkProvider(wordDetector));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { ProgressAddon } from "@xterm/addon-progress";
import type { SearchAddon } from "@xterm/addon-search";
import type { TerminalAppearance } from "./appearance";
import {
type LinkHoverInfo,
type TerminalLinkHandlers,
TerminalLinkManager,
} from "./terminal-link-manager";
Expand Down Expand Up @@ -216,4 +217,4 @@ if (import.meta.hot) {
import.meta.hot.data.registry = terminalRuntimeRegistry;
}

export type { ConnectionState, TerminalLinkHandlers };
export type { ConnectionState, LinkHoverInfo, TerminalLinkHandlers };
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export {
type OpenInExternalEditorOptions,
useOpenInExternalEditor,
} from "./useOpenInExternalEditor";
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { toast } from "@superset/ui/sonner";
import { eq } from "@tanstack/db";
import { useLiveQuery } from "@tanstack/react-db";
import { useCallback } from "react";
import { electronTrpcClient } from "renderer/lib/trpc-client";
import { useCollections } from "renderer/routes/_authenticated/providers/CollectionsProvider";
import { useLocalHostService } from "renderer/routes/_authenticated/providers/LocalHostServiceProvider";

export interface OpenInExternalEditorOptions {
line?: number;
column?: number;
}

export function useOpenInExternalEditor(workspaceId: string) {
const collections = useCollections();
const { machineId } = useLocalHostService();
const { data: workspacesWithHost = [] } = useLiveQuery(
(q) =>
q
.from({ workspaces: collections.v2Workspaces })
.leftJoin({ hosts: collections.v2Hosts }, ({ workspaces, hosts }) =>
eq(workspaces.hostId, hosts.id),
)
.where(({ workspaces }) => eq(workspaces.id, workspaceId))
.select(({ hosts }) => ({
hostMachineId: hosts?.machineId ?? null,
})),
[collections, workspaceId],
);
const workspaceHost = workspacesWithHost[0];

return useCallback(
(path: string, opts?: OpenInExternalEditorOptions) => {
// Treat unloaded host data as non-local to avoid firing the mutation
// against a potentially remote workspace before locality is confirmed.
if (workspaceHost?.hostMachineId !== machineId) {
toast.error("Can't open remote workspace paths in an external editor");
return;
}
electronTrpcClient.external.openFileInEditor
.mutate({ path, line: opts?.line, column: opts?.column })
.catch((error) => {
console.error("Failed to open in external editor:", error);
toast.error("Failed to open in external editor");
});
},
[workspaceHost, machineId],
);
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import type { RendererContext } from "@superset/panes";
import { toast } from "@superset/ui/sonner";
import { workspaceTrpc } from "@superset/workspace-client";
import "@xterm/xterm/css/xterm.css";
import {
Expand All @@ -15,7 +14,9 @@ import {
terminalRuntimeRegistry,
} from "renderer/lib/terminal/terminal-runtime-registry";
import { electronTrpcClient } from "renderer/lib/trpc-client";
import { useOpenInExternalEditor } from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/useOpenInExternalEditor";
import type {
BrowserPaneData,
PaneViewerData,
TerminalPaneData,
} from "renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/types";
Expand All @@ -24,11 +25,15 @@ import { ScrollToBottomButton } from "renderer/screens/main/components/Workspace
import { TerminalSearch } from "renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/TerminalSearch";
import { useTheme } from "renderer/stores/theme";
import { resolveTerminalThemeType } from "renderer/stores/theme/utils";
import { LinkHoverTooltip } from "./components/LinkHoverTooltip";
import { useLinkHoverState } from "./hooks/useLinkHoverState";
import { useTerminalAppearance } from "./hooks/useTerminalAppearance";

interface TerminalPaneProps {
ctx: RendererContext<PaneViewerData>;
workspaceId: string;
onOpenFile: (path: string, openInNewTab?: boolean) => void;
onRevealPath: (path: string) => void;
}

function subscribeToState(terminalId: string) {
Expand All @@ -40,7 +45,18 @@ function getConnectionState(terminalId: string): ConnectionState {
return terminalRuntimeRegistry.getConnectionState(terminalId);
}

export function TerminalPane({ ctx, workspaceId }: TerminalPaneProps) {
export function TerminalPane({
ctx,
workspaceId,
onOpenFile,
onRevealPath,
}: TerminalPaneProps) {
const openInExternalEditor = useOpenInExternalEditor(workspaceId);
const {
hoveredLink,
onHover: onLinkHover,
onLeave: onLinkLeave,
} = useLinkHoverState();
const paneData = ctx.pane.data as TerminalPaneData;
const { terminalId } = paneData;
const containerRef = useRef<HTMLDivElement | null>(null);
Expand Down Expand Up @@ -141,27 +157,49 @@ export function TerminalPane({ ctx, workspaceId }: TerminalPaneProps) {
return null;
}
},
onFileLinkClick: (_event, link) => {
if (!_event.metaKey && !_event.ctrlKey) return;
_event.preventDefault();
electronTrpcClient.external.openFileInEditor
.mutate({
path: link.resolvedPath,
onFileLinkClick: (event, link) => {
if (!event.metaKey && !event.ctrlKey) return;
event.preventDefault();
if (event.shiftKey) {
openInExternalEditor(link.resolvedPath, {
line: link.row,
column: link.col,
})
.catch((error) => {
console.error("[v2 Terminal] Failed to open file:", error);
toast.error("Failed to open file in editor");
});
return;
}
if (link.isDirectory) {
onRevealPath(link.resolvedPath);
} else {
onOpenFile(link.resolvedPath);
}
},
Comment on lines +160 to 175
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify that terminalLinkBehavior is not referenced anywhere in the v2 terminal link path
rg -nP --type=ts --type=tsx -C2 'terminalLinkBehavior|getTerminalLinkBehavior'
# And confirm the settings router exposes it (sanity check)
rg -nP --type=ts -C2 'getTerminalLinkBehavior' -g '**/trpc/**'

Repository: superset-sh/superset

Length of output: 553


🏁 Script executed:

# First, find the exact file path and read the TerminalPane.tsx to see the full context
find . -name "TerminalPane.tsx" -type f | head -20

Repository: superset-sh/superset

Length of output: 213


🏁 Script executed:

# Read the TerminalPane.tsx file to understand the full implementation
# Specifically looking for getTerminalLinkBehavior calls and the onFileLinkClick handler
fd -e tsx "TerminalPane.tsx" --exec cat {} \;

Repository: superset-sh/superset

Length of output: 7397


🏁 Script executed:

# Also search for any references to getTerminalLinkBehavior or terminalLinkBehavior in the v2 workspace context
rg -t ts 'terminalLinkBehavior|getTerminalLinkBehavior' apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/ -C3

Repository: superset-sh/superset

Length of output: 46


Setting terminalLinkBehavior is never consulted — feature appears unimplemented.

The PR objective states that v2 terminal link clicks should honor the Settings → Terminal → Terminal file links preference by reading it via electronTrpcClient.settings.getTerminalLinkBehavior.query() on each click, with "file-viewer" as the default/error fallback. The onFileLinkClick handler (lines 162–177) never reads this setting: it uses a shift-modifier heuristic instead (shift ⇒ external, else ⇒ in-app). As a result, a user who has set Terminal file links to "external-editor" will still see files open in the FilePane on a plain Cmd/Ctrl-click — the opposite of what the setting requests.

Please clarify whether the setting-lookup approach was intentionally replaced with a shift modifier, or if the getTerminalLinkBehavior query is missing. If the latter, it should be called inside onFileLinkClick (async context), routing to onOpenFile vs onOpenExternal based on the resolved setting value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/hooks/usePaneRegistry/components/TerminalPane/TerminalPane.tsx
around lines 162 - 177, The onFileLinkClick handler currently ignores the
terminal link setting and uses a shift-key heuristic; update onFileLinkClick to
asynchronously call electronTrpcClient.settings.getTerminalLinkBehavior.query()
on each click (with fallback "file-viewer" on error) and then route: if behavior
=== "external-editor" or (event.shiftKey && behavior !== "file-viewer") call
onOpenExternalRef.current(link.resolvedPath, {line: link.row, column:
link.col}); if behavior === "file-viewer" open in-app via
onOpenFileRef.current(link.resolvedPath); keep the existing directory handling
with onRevealPathRef.current(link.resolvedPath); ensure errors default to
"file-viewer" and preserve event.preventDefault() semantics.

onUrlClick: (url) => {
electronTrpcClient.external.openUrl.mutate(url).catch((error) => {
console.error("[v2 Terminal] Failed to open URL:", url, error);
onUrlClick: (event, url) => {
if (event.shiftKey) {
electronTrpcClient.external.openUrl.mutate(url).catch((error) => {
console.error("[v2 Terminal] Failed to open URL:", url, error);
});
return;
}
ctx.store.getState().openPane({
pane: {
kind: "browser",
data: { url } satisfies BrowserPaneData,
},
});
},
onLinkHover,
onLinkLeave,
});
}, [terminalId, workspaceId]);
}, [
terminalId,
workspaceId,
ctx.store,
onOpenFile,
onRevealPath,
openInExternalEditor,
onLinkHover,
onLinkLeave,
]);

useHotkey(
"CLEAR_TERMINAL",
Expand Down Expand Up @@ -217,6 +255,7 @@ export function TerminalPane({ ctx, workspaceId }: TerminalPaneProps) {
<span>Disconnected</span>
</div>
)}
<LinkHoverTooltip hoveredLink={hoveredLink} />
</div>
);
}
Loading
Loading