Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
25 changes: 25 additions & 0 deletions frontend/app/element/errorboundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,28 @@ export class ErrorBoundary extends React.Component<
}
}
}

export class NullErrorBoundary extends React.Component<
{ children: React.ReactNode; debugName?: string },
{ hasError: boolean }
> {
constructor(props: { children: React.ReactNode; debugName?: string }) {
super(props);
this.state = { hasError: false };
}

static getDerivedStateFromError() {
return { hasError: true };
}

componentDidCatch(error: Error, info: React.ErrorInfo) {
console.log(`${this.props.debugName ?? "NullErrorBoundary"} error boundary caught error`, error, info);
}

render() {
if (this.state.hasError) {
return null;
}
return this.props.children;
}
}
33 changes: 15 additions & 18 deletions frontend/app/view/term/term-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -782,40 +782,37 @@ export class TermViewModel implements ViewModel {
},
});

let selectionURL: URL = null;
if (selection) {
try {
const trimmedSelection = selection.trim();
const url = new URL(trimmedSelection);
if (url.protocol.startsWith("http")) {
selectionURL = url;
}
} catch (e) {
// not a valid URL
}
}
menu.push({ type: "separator" });
}

if (selectionURL) {
menu.push({ type: "separator" });
const hoveredLinkUri = this.termRef.current?.hoveredLinkUri;
if (hoveredLinkUri) {
let hoveredURL: URL = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

CRITICAL: TypeScript type error - cannot assign null to type URL

The variable is declared as let hoveredURL: URL = null; but URL type cannot be null. This should be URL | null to allow null assignment.

Suggested change
let hoveredURL: URL = null;
let hoveredURL: URL | null = null;

Copy link
Member Author

Choose a reason for hiding this comment

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

We are not using strict mode for typescript, so null assignment is fine.

try {
hoveredURL = new URL(hoveredLinkUri);
} catch (e) {
// not a valid URL
}
if (hoveredURL) {
menu.push({
label: "Open URL (" + selectionURL.hostname + ")",
label: "Open URL (" + hoveredURL.hostname + ")",
click: () => {
Comment on lines +796 to 799
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

hoveredURL.hostname is empty for non-HTTP(S) URLs, producing "Open URL ()".

file://, ssh://, or other custom-scheme URIs have an empty hostname, resulting in a misleading label. Guard with a fallback.

🛠 Proposed fix
-                    label: "Open URL (" + hoveredURL.hostname + ")",
+                    label: "Open URL (" + (hoveredURL.hostname || hoveredURL.href) + ")",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (hoveredURL) {
menu.push({
label: "Open URL (" + selectionURL.hostname + ")",
label: "Open URL (" + hoveredURL.hostname + ")",
click: () => {
if (hoveredURL) {
menu.push({
label: "Open URL (" + (hoveredURL.hostname || hoveredURL.href) + ")",
click: () => {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/app/view/term/term-model.ts` around lines 796 - 799, The menu label
building uses hoveredURL.hostname which is empty for non-HTTP(S) schemes; update
the label in the menu.push call (the block that constructs "Open URL (" +
hoveredURL.hostname + ")") to fall back to a meaningful value such as
hoveredURL.href (or hoveredURL.protocol + hoveredURL.pathname) when
hoveredURL.hostname is falsy so the label becomes "Open URL (…)" with the actual
URI for file://, ssh://, and custom schemes.

createBlock({
meta: {
view: "web",
url: selectionURL.toString(),
url: hoveredURL.toString(),
},
});
},
});
menu.push({
label: "Open URL in External Browser",
click: () => {
getApi().openExternal(selectionURL.toString());
getApi().openExternal(hoveredURL.toString());
},
});
menu.push({ type: "separator" });
}
menu.push({ type: "separator" });
}

menu.push({
Expand Down
139 changes: 139 additions & 0 deletions frontend/app/view/term/term-tooltip.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
// Copyright 2025, Command Line Inc.
// SPDX-License-Identifier: Apache-2.0

import { PLATFORM, PlatformMacOS } from "@/util/platformutil";
import { FloatingPortal, VirtualElement, flip, offset, shift, useFloating } from "@floating-ui/react";
import * as React from "react";
import type { TermWrap } from "./termwrap";

// ── low-level primitive ──────────────────────────────────────────────────────

interface TermTooltipProps {
/** Screen-space mouse position (clientX/clientY). null means hidden. */
mousePos: { x: number; y: number } | null;
content: React.ReactNode;
}

/**
* A floating tooltip anchored to the current mouse position.
* Uses a floating-ui virtual element (via refs.setPositionReference) so no
* real DOM reference is required. Renders into a FloatingPortal.
*/
export const TermTooltip = React.memo(function TermTooltip({ mousePos, content }: TermTooltipProps) {
const isOpen = mousePos != null;

// Keep latest mousePos in a ref so the virtual element always reflects it.
const mousePosRef = React.useRef(mousePos);
mousePosRef.current = mousePos;

const { refs, floatingStyles } = useFloating({
open: isOpen,
placement: "top-start",
middleware: [offset({ mainAxis: 12, crossAxis: -20 }), flip(), shift({ padding: 0 })],
});

// Update the position reference whenever mousePos changes.
React.useLayoutEffect(() => {
if (!isOpen) {
return;
}
const virtualEl: VirtualElement = {
getBoundingClientRect() {
const pos = mousePosRef.current ?? { x: 0, y: 0 };
return new DOMRect(pos.x, pos.y, 0, 0);
},
};
refs.setPositionReference(virtualEl);
}, [isOpen, mousePos?.x, mousePos?.y]);
Copy link
Contributor

Choose a reason for hiding this comment

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

WARNING: Missing refs in dependency array

The useLayoutEffect calls refs.setPositionReference() but refs is not included in the dependency array. While refs from useFloating is typically stable, it's better to include it for correctness and to avoid potential stale closure issues.

Suggested change
}, [isOpen, mousePos?.x, mousePos?.y]);
}, [isOpen, mousePos?.x, mousePos?.y, refs]);


if (!isOpen) {
return null;
}

return (
<FloatingPortal>
<div
ref={refs.setFloating}
style={floatingStyles}
className="bg-zinc-800/70 rounded-md px-2 py-1 text-xs text-secondary shadow-xl z-50 pointer-events-none select-none"
>
{content}
</div>
</FloatingPortal>
);
});

// ── wired-up sub-component ───────────────────────────────────────────────────

const HoverDelayMs = 600;
const MaxHoverTimeMs = 2200;
const modKey = PLATFORM === PlatformMacOS ? "Cmd" : "Ctrl";

interface TermLinkTooltipProps {
/**
* The live TermWrap instance. Pass the instance directly (not a ref) so
* React re-runs the effect when it changes (e.g. on terminal recreate).
*/
termWrap: TermWrap | null;
}

/**
* Self-contained sub-component that subscribes to the termWrap link-hover
* callback and renders a tooltip after a short delay. Keeping state here
* prevents unnecessary re-renders of the parent TerminalView.
*/
export const TermLinkTooltip = React.memo(function TermLinkTooltip({ termWrap }: TermLinkTooltipProps) {
const [mousePos, setMousePos] = React.useState<{ x: number; y: number } | null>(null);
const timeoutRef = React.useRef<number | null>(null);
const maxTimeoutRef = React.useRef<number | null>(null);

const clearMaxTimeout = () => {
if (maxTimeoutRef.current != null) {
window.clearTimeout(maxTimeoutRef.current);
maxTimeoutRef.current = null;
}
};

React.useEffect(() => {
if (termWrap == null) {
return;
}

termWrap.onLinkHover = (uri: string | null, mouseX: number, mouseY: number) => {
if (timeoutRef.current != null) {
window.clearTimeout(timeoutRef.current);
timeoutRef.current = null;
}

if (uri == null) {
clearMaxTimeout();
setMousePos(null);
return;
}

// Show after a short delay so fast mouse movements don't flicker.
timeoutRef.current = window.setTimeout(() => {
timeoutRef.current = null;
setMousePos({ x: mouseX, y: mouseY });
// Auto-dismiss after MaxHoverTimeMs so the tooltip doesn't linger forever.
clearMaxTimeout();
maxTimeoutRef.current = window.setTimeout(() => {
maxTimeoutRef.current = null;
setMousePos(null);
}, MaxHoverTimeMs);
}, HoverDelayMs);
};

return () => {
termWrap.onLinkHover = null;
if (timeoutRef.current != null) {
window.clearTimeout(timeoutRef.current);
timeoutRef.current = null;
}
clearMaxTimeout();
setMousePos(null);
};
}, [termWrap]);

return <TermTooltip mousePos={mousePos} content={<span>{modKey}-click to open link</span>} />;
});
10 changes: 9 additions & 1 deletion frontend/app/view/term/term.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// Copyright 2025, Command Line Inc.
// SPDX-License-Identifier: Apache-2.0

import { Block, SubBlock } from "@/app/block/block";
import { SubBlock } from "@/app/block/block";
import type { BlockNodeModel } from "@/app/block/blocktypes";
import { NullErrorBoundary } from "@/app/element/errorboundary";
import { Search, useSearch } from "@/app/element/search";
import { ContextMenuModel } from "@/app/store/contextmenu";
import { useTabModel } from "@/app/store/tab-model";
Expand All @@ -18,6 +19,7 @@ import clsx from "clsx";
import debug from "debug";
import * as jotai from "jotai";
import * as React from "react";
import { TermLinkTooltip } from "./term-tooltip";
import { TermStickers } from "./termsticker";
import { TermThemeUpdater } from "./termtheme";
import { computeTheme } from "./termutil";
Expand Down Expand Up @@ -167,6 +169,7 @@ const TermToolbarVDomNode = ({ blockId, model }: TerminalViewProps) => {
const TerminalView = ({ blockId, model }: ViewComponentProps<TermViewModel>) => {
const viewRef = React.useRef<HTMLDivElement>(null);
const connectElemRef = React.useRef<HTMLDivElement>(null);
const [termWrapInst, setTermWrapInst] = React.useState<TermWrap | null>(null);
const [blockData] = WOS.useWaveObjectValue<Block>(WOS.makeORef("block", blockId));
const termSettingsAtom = getSettingsPrefixAtom("term");
const termSettings = jotai.useAtomValue(termSettingsAtom);
Expand Down Expand Up @@ -302,6 +305,7 @@ const TerminalView = ({ blockId, model }: ViewComponentProps<TermViewModel>) =>
);
(window as any).term = termWrap;
model.termRef.current = termWrap;
setTermWrapInst(termWrap);
const rszObs = new ResizeObserver(() => {
termWrap.handleResize_debounced();
});
Expand All @@ -319,6 +323,7 @@ const TerminalView = ({ blockId, model }: ViewComponentProps<TermViewModel>) =>
return () => {
termWrap.dispose();
rszObs.disconnect();
setTermWrapInst(null);
};
}, [blockId, termSettings, termFontSize, connFontFamily]);

Expand Down Expand Up @@ -390,6 +395,9 @@ const TerminalView = ({ blockId, model }: ViewComponentProps<TermViewModel>) =>
onPointerOver={onScrollbarHideObserver}
/>
</div>
<NullErrorBoundary debugName="TermLinkTooltip">
<TermLinkTooltip termWrap={termWrapInst} />
</NullErrorBoundary>
<Search {...searchProps} />
</div>
);
Expand Down
42 changes: 28 additions & 14 deletions frontend/app/view/term/termwrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ export class TermWrap {
shellIntegrationStatusAtom: jotai.PrimitiveAtom<ShellIntegrationStatus | null>;
lastCommandAtom: jotai.PrimitiveAtom<string | null>;
nodeModel: BlockNodeModel; // this can be null
hoveredLinkUri: string | null = null;
onLinkHover?: (uri: string | null, mouseX: number, mouseY: number) => void;

// IME composition state tracking
// Prevents duplicate input when switching input methods during composition (e.g., using Capslock)
Expand Down Expand Up @@ -132,21 +134,33 @@ export class TermWrap {
this.terminal.loadAddon(this.fitAddon);
this.terminal.loadAddon(this.serializeAddon);
this.terminal.loadAddon(
new WebLinksAddon((e, uri) => {
e.preventDefault();
switch (PLATFORM) {
case PlatformMacOS:
if (e.metaKey) {
fireAndForget(() => openLink(uri));
}
break;
default:
if (e.ctrlKey) {
fireAndForget(() => openLink(uri));
}
break;
new WebLinksAddon(
(e, uri) => {
e.preventDefault();
switch (PLATFORM) {
case PlatformMacOS:
if (e.metaKey) {
fireAndForget(() => openLink(uri));
}
break;
default:
if (e.ctrlKey) {
fireAndForget(() => openLink(uri));
}
break;
}
},
{
hover: (e, uri) => {
this.hoveredLinkUri = uri;
this.onLinkHover?.(uri, e.clientX, e.clientY);
},
leave: () => {
this.hoveredLinkUri = null;
this.onLinkHover?.(null, 0, 0);
},
}
})
)
);
if (WebGLSupported && waveOptions.useWebGl) {
const webglAddon = new WebglAddon();
Expand Down
Loading