Repository navigation
Preview terminal links in the embedded browser #9374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
03e67b8
2a31ce6
9801df1
3ab61ec
676e822
789285b
52d6ec6
060726e
bd053bc
e773928
468e032
7e0a61d
d1146e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -170589,6 +170589,57 @@ | |
| } | ||
| } | ||
| }, | ||
| "settings.browser.terminalLinkPreviewDelay": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Terminal Link Preview Delay" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ターミナルリンクのプレビュー遅延" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.browser.terminalLinkPreviewDelay.milliseconds": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "%lld ms" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "%lldミリ秒" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.browser.terminalLinkPreviewDelay.subtitle": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "How long the pointer must rest on a terminal link before its live page preview appears." | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "ターミナルリンクにポインタを置いてからライブページのプレビューが表示されるまでの時間です。" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
|
Comment on lines
+170592
to
+170642
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
import json
from pathlib import Path
path = Path("Resources/Localizable.xcstrings")
catalog = json.loads(path.read_text())
strings = catalog["strings"]
keys = [
"settings.browser.terminalLinkPreviewDelay",
"settings.browser.terminalLinkPreviewDelay.milliseconds",
"settings.browser.terminalLinkPreviewDelay.subtitle",
"settings.search.alias.setting.browser.terminal-link-preview-delay",
]
catalog_locales = sorted({
locale
for entry in strings.values()
for locale in entry.get("localizations", {})
})
for key in keys:
missing = [
locale
for locale in catalog_locales
if locale not in strings[key].get("localizations", {})
]
if missing:
print(f"{key}: missing {', '.join(missing)}")
PYRepository: manaflow-ai/cmux Length of output: 740 Add missing localization entries for the new terminal preview keys.
🤖 Prompt for AI AgentsSource: Path instructions |
||
| "settings.browser.theme": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
|
|
@@ -188035,6 +188086,23 @@ | |
| } | ||
| } | ||
| }, | ||
| "settings.search.alias.setting.browser.terminal-link-preview-delay": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "browser.terminalLinkPreviewHoverDelayMilliseconds terminal url link hover preview dwell delay milliseconds thumbnail web page" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "browser.terminalLinkPreviewHoverDelayMilliseconds ターミナル URL リンク ホバー プレビュー 待機 遅延 ミリ秒 サムネイル ウェブページ" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "settings.search.alias.setting.browser.terminal-links": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4120,6 +4120,9 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| } else { | ||
| surface.reconcileAttachedWindowIfNeeded(for: self) | ||
| } | ||
| if let ghosttySurface = surface.surface { | ||
| ghostty_surface_set_unmodified_link_previews(ghosttySurface, true) | ||
| } | ||
| surface.setKeyboardCopyModeActive(keyboardCopyModeActive) | ||
| if !isAlreadyAttached { | ||
| updateSurfaceSize() | ||
|
|
@@ -6765,6 +6768,21 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations { | |
| lastKnownMousePointInView = point | ||
| } | ||
|
|
||
| /// Ghostty reports link hover asynchronously, so use the point captured | ||
| /// from the source mouse event instead of resampling the window afterward. | ||
| func terminalLinkPreviewAnchor(in previewView: NSView) -> NSPoint? { | ||
| guard let point = lastKnownMousePointInView, | ||
| let window, | ||
| previewView.window === window else { return nil } | ||
| return previewView.convert(point, from: self) | ||
| } | ||
|
|
||
| #if DEBUG | ||
| func setTerminalLinkPreviewPointerPointForTesting(_ point: NSPoint?) { | ||
| lastKnownMousePointInView = point | ||
| } | ||
| #endif | ||
|
|
||
| private func preferredPointerPoint(from eventPoint: NSPoint? = nil) -> NSPoint? { | ||
| if let eventPoint, pointIsUsableForWordResolution(eventPoint) { | ||
| lastKnownMousePointInView = eventPoint | ||
|
|
@@ -8231,6 +8249,7 @@ final class GhosttySurfaceScrollView: NSView { | |
| private let keyboardCopyModeBadgeIconView: NSImageView | ||
| private let keyboardCopyModeBadgeLabel: NSTextField | ||
| let linkHoverIndicatorView: TerminalLinkHoverIndicatorView | ||
| var terminalLinkPreviewController: TerminalLinkPreviewController? | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win Narrow the ownership of The property is an internal mutable optional, so any file in the module can replace it or set it to nil. The scroll view is the sole owner of this controller and initializes it once at Line 8503.
♻️ Proposed refactor- var terminalLinkPreviewController: TerminalLinkPreviewController?
+ let terminalLinkPreviewController: TerminalLinkPreviewController super.init(frame: .zero)
- terminalLinkPreviewController = TerminalLinkPreviewController(view: linkHoverIndicatorView)
+ terminalLinkPreviewController = TerminalLinkPreviewController(view: linkHoverIndicatorView)Then drop the Also applies to: 8503-8503 🤖 Prompt for AI Agents |
||
| private let imageTransferIndicatorContainerView: NSView | ||
| private let imageTransferIndicatorView: NSVisualEffectView | ||
| private let imageTransferIndicatorSpinner: NSProgressIndicator | ||
|
|
@@ -8496,6 +8515,7 @@ final class GhosttySurfaceScrollView: NSView { | |
| documentView.addSubview(surfaceView) | ||
|
|
||
| super.init(frame: .zero) | ||
| terminalLinkPreviewController = TerminalLinkPreviewController(view: linkHoverIndicatorView) | ||
| wantsLayer = true | ||
| layer?.masksToBounds = true | ||
|
|
||
|
|
@@ -8836,6 +8856,9 @@ final class GhosttySurfaceScrollView: NSView { | |
| observers.forEach { NotificationCenter.default.removeObserver($0) } | ||
| windowObservers.forEach { NotificationCenter.default.removeObserver($0) } | ||
| dropZoneOverlayView.removeFromSuperview() | ||
| MainActor.assumeIsolated { | ||
| terminalLinkPreviewController?.invalidate() | ||
| } | ||
| cancelFocusRequest() | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 50373
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 50372
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 33739
🏁 Script executed:
Repository: manaflow-ai/cmux
Length of output: 4878
Use one validated accessor for the preview delay.
JSONConfigStorerejects out-of-rangebrowser.terminalLinkPreviewHoverDelayMilliseconds, butDefaultsValueModel<Int>seedscurrentfrom the rawUserDefaultsvalue andBrowserSectionwrites it back through an unbounded store path. A stale or manually inserted value can appear in the settings UI, whileBrowserPanel.swiftrejects values outside200...2_000. Add the shared range clamp/rejection to the settings binding or normalize it at a single validated accessor.[low_effort_and_medium_reward]
🤖 Prompt for AI Agents