Repository navigation
Fix Cmd+click links not working inside tmux sessions - #2869
TomerCohen95 wants to merge 2 commits into
Conversation
Update ghostty submodule to allow Ctrl/Super modifier to bypass mouse capture for link detection, making URLs clickable in tmux.
|
@TomercoW is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughUpdated the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes Cmd+click link detection inside tmux sessions by extending the existing Shift modifier bypass in Ghostty's
Confidence Score: 3/5Not safe to merge until the submodule SHA is confirmed to exist in manaflow-ai/ghostty main and docs/ghostty-fork.md is updated One P1 finding: the submodule pointer may reference a commit only in TomerCohen95/ghostty, violating the documented workflow that requires the SHA to be on manaflow-ai/ghostty main before updating the parent pointer. If merged as-is, git submodule update would fail for all contributors. The underlying Surface.zig fix looks technically sound, but the submodule workflow issue must be resolved first. ghostty (submodule pointer) and docs/ghostty-fork.md (missing new entry) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant macOS
participant Ghostty as Ghostty (Surface.zig)
participant tmux
participant Shell
Note over tmux,Shell: tmux mouse mode ON (mouse_event != .none)
User->>macOS: Hold Cmd key
macOS->>Ghostty: keyEvent(Cmd)
Note over Ghostty: Before fix: clear over_link<br/>After fix: refresh link state<br/>(Cmd bypasses mouse capture)
User->>macOS: Move mouse over URL
macOS->>Ghostty: cursorPosCallback(x, y, mods.super=true)
Note over Ghostty: Before fix: skip link detection<br/>(mouse_event != .none blocks it)<br/>After fix: Cmd bypasses → detect link
Ghostty->>Ghostty: over_link = true, set hyperlink_hover dirty
Ghostty->>User: URL underlines (cursor → pointer)
User->>macOS: Cmd+click URL
macOS->>Ghostty: mouseButtonCallback(Cmd+click)
Ghostty->>Shell: open URL in browser
User->>macOS: Move mouse away
macOS->>Ghostty: cursorPosCallback(x2, y2)
Note over Ghostty: After fix: set hyperlink_hover dirty<br/>→ renderer clears underline immediately
Ghostty->>User: Underline clears
Reviews (1): Last reviewed commit: "Fix Cmd+click links not working inside t..." | Re-trigger Greptile |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -1 +1 @@ | |||
| Subproject commit 3b684a085d40ec79e8a0ae863a4f2b48ed4dba74 | |||
| Subproject commit f915b291c8fc547f1a73fe2f32a9dc4d6d8cb385 | |||
There was a problem hiding this comment.
manaflow-ai/ghostty
The PR description links the Ghostty changes to TomerCohen95/ghostty on branch fix/tmux-link-click, not to manaflow-ai/ghostty main. CLAUDE.md and docs/ghostty-fork.md both require that the submodule commit be pushed to manaflow-ai/ghostty and reachable from origin/main before the parent repo pointer is updated:
Ghostty changes must be committed in the
ghosttysubmodule and pushed to themanaflow-ai/ghosttyfork. Never commit on a detached HEAD or temporary branch — the commit will be orphaned and lost. Verify with:cd ghostty && git merge-base --is-ancestor HEAD origin/main
If SHA f915b291 is only in TomerCohen95/ghostty and hasn't been pushed to manaflow-ai/ghostty main, then after merging, anyone who runs git submodule update will get a fetch error because the configured submodule URL (manaflow-ai/ghostty) doesn't contain the commit.
| @@ -1 +1 @@ | |||
| Subproject commit 3b684a085d40ec79e8a0ae863a4f2b48ed4dba74 | |||
| Subproject commit f915b291c8fc547f1a73fe2f32a9dc4d6d8cb385 | |||
There was a problem hiding this comment.
docs/ghostty-fork.md not updated
The fork update checklist in docs/ghostty-fork.md (step 3) requires updating that file with a change summary and any conflict notes for every new fork patch. This PR adds a new entry to the fork (tmux Cmd+click link detection bypass in src/Surface.zig) but docs/ghostty-fork.md is unchanged. Future rebases on upstream won't know to preserve or re-apply the three Surface.zig hunks without this entry.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Document the new Surface.zig changes for Cmd+click link detection through mouse capture, and add merge conflict notes for the affected code paths.
Addressing review feedback@greptile-apps — thanks for the review. Two items addressed:
CI failures — All from |
|
Both concerns are well-addressed:
From a review standpoint, both issues are resolved. This is ready for maintainer merge once they apply commit Tip: You can customize Greptile's behavior for this repo with |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/ghostty-fork.md (1)
137-137: Consider adding explicit submodule-main verification at the “patches 1–9” status line.Small doc hardening: include a one-liner check so future updates avoid detached-head or unpushed-submodule-pointer mistakes.
Suggested doc tweak
-Fork main now carries patches 1–9 plus later upstream merges; +Fork main now carries patches 1–9 plus later upstream merges +(verify before bumping parent SHA: `cd ghostty && git merge-base --is-ancestor HEAD origin/main`); the current cmux pin is the head listed above.Based on learnings: "When modifying a submodule (ghostty, vendor/bonsplit, etc.), always push the submodule commit to its remote
mainbranch BEFORE committing the updated pointer in the parent repo... Verify with:cd <submodule> && git merge-base --is-ancestor HEAD origin/main".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ghostty-fork.md` at line 137, Add a one-line submodule-main verification next to the “Fork main now carries patches 1–9” status line: instruct maintainers to cd into the submodule (e.g., ghostty or vendor/bonsplit) and run a git merge-base --is-ancestor check to ensure the submodule HEAD is an ancestor of origin/main (i.e., the submodule commit was pushed to main) before committing the updated pointer in the parent repo, so contributors avoid detached-head or unpushed-submodule-pointer mistakes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/ghostty-fork.md`:
- Line 137: Add a one-line submodule-main verification next to the “Fork main
now carries patches 1–9” status line: instruct maintainers to cd into the
submodule (e.g., ghostty or vendor/bonsplit) and run a git merge-base
--is-ancestor check to ensure the submodule HEAD is an ancestor of origin/main
(i.e., the submodule commit was pushed to main) before committing the updated
pointer in the parent repo, so contributors avoid detached-head or
unpushed-submodule-pointer mistakes.
|
Hi, just wanted to add my voice here — I'm running into this exact issue and this fix would be a huge quality-of-life improvement for Claude Code users in cmux. The PR looks solid and the review concerns seem to have been addressed. Would it be possible to get this merged? Really appreciate the work put into this! |
|
Thanks for this! Cmd-click opens links under mouse reporting, tmux included landed on main in #5406. You opened this first, so you got there first. Closing since main covers it now. |
4 participants
Summary
set -g mouse on)mouse_event != .none), Ghostty's link hover detection was completely disabled —over_linkwas never set totrue, so click handling never triggeredSurface.zig:hyperlink_hoverdirty flag when leaving a link so the renderer removes the underline immediately (this was a pre-existing bug also visible outside tmux)Closes #2896
Linked submodule change
fix/tmux-link-clicksrc/Surface.zig(+11, -4)Testing
./scripts/reload.sh --tag local-dev --launchset -g mouse on:Demo Video
N/A — text-only terminal behavior change
Review Trigger (Copy/Paste as PR comment)
Checklist