bug: Extend LSP go-to-definition fallback to context menu and cmd+click#10102
Merged
Conversation
F12 already falls through to the language server when marimo cannot resolve a symbol; route the context menu and cmd+click entry points through the same fallback so external definitions work consistently. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Architecture diagram
sequenceDiagram
participant User
participant EdView as EditorView
participant GotoDef as goToDefinitionWithLspFallback
participant Marimo as goToDefinitionAtCursorPosition
participant LSPTrig as requestLspGoToDefinition
participant LSPHandler as LSP Keymap Handler
Note over User,LSPHandler: Go‑to‑definition flows (F12 / context menu / cmd+click)
alt F12 keybinding
User->>EdView: F12 keydown
EdView->>GotoDef: run callback
else Context menu item click
User->>CellActionsContextMenu: click "Go to Definition"
CellActionsContextMenu->>GotoDef: call with editorView
else Cmd+click on underlined symbol
User->>MetaUnderlineVariablePlugin: click
Note over MetaUnderlineVariablePlugin: Moves cursor to clicked position first
MetaUnderlineVariablePlugin->>GotoDef: call with editorView
end
GotoDef->>Marimo: try marimo's notebook resolver
Marimo->>EdView: check variables state, navigate if found
alt Marimo resolved symbol
Marimo-->>GotoDef: true
GotoDef-->>User: true (navigated)
else Marimo cannot resolve (e.g. external library)
Marimo-->>GotoDef: false
GotoDef->>LSPTrig: fallback to LSP
LSPTrig->>EdView: keyboard event (F12) + runScopeHandlers
EdView->>LSPHandler: trigger F12 keybinding
LSPHandler-->>User: navigate (if LSP responds)
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the existing “marimo resolver → LSP fallback” go-to-definition behavior beyond the F12 keybinding to additional entry points (context menu and Cmd/Ctrl+click) by introducing a shared helper that triggers the LSP handler when marimo can’t resolve a symbol.
Changes:
- Added
goToDefinitionWithLspFallback(andrequestLspGoToDefinition) to unify resolution logic and invoke LSP via CodeMirror scope handlers when needed. - Updated the underline (Cmd/Ctrl+click) plugin to dispatch the cursor selection before resolving, so the LSP uses the clicked position.
- Updated the cell context menu action to use the new fallback helper and added tests for the fallback behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/core/codemirror/go-to-definition/utils.ts | Adds LSP fallback helper and a function to programmatically request LSP go-to-definition via keymap handlers. |
| frontend/src/core/codemirror/go-to-definition/underline.ts | Changes Cmd/Ctrl+click handling to move cursor first and then resolve via the shared helper. |
| frontend/src/core/codemirror/go-to-definition/extension.ts | Switches underline click action to call goToDefinitionWithLspFallback. |
| frontend/src/core/codemirror/go-to-definition/tests/utils.test.ts | Adds tests covering fallback-to-LSP and “don’t call LSP when marimo resolves”. |
| frontend/src/components/editor/cell/cell-context-menu.tsx | Context menu “Go to Definition” now uses the shared LSP-fallback helper. |
Translate CodeMirror shortcut strings like Ctrl-F12 into synthetic keyboard events with proper modifier flags so customized hotkeys still trigger the language-server fallback. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Replace synthetic keyboard event parsing with a direct keymap lookup, which handles customized shortcuts without extra shortcut translation. Co-authored-by: Cursor <cursoragent@cursor.com>
Compare the configured go-to-definition hotkey against each binding's platform-specific key when invoking keymap handlers programmatically. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep keymap binding platform selection aligned with the platform used to resolve configured hotkeys from hotkeysAtom. Co-authored-by: Cursor <cursoragent@cursor.com>
mscolnick
approved these changes
Jul 8, 2026
Contributor
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.14-dev39 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request was authored by a coding agent.
📝 Summary
Follow-up to #10099 / #7135.
F12 go-to-definition already falls through to the language server when marimo cannot resolve a symbol. This PR extends that same fallback to the other entry points:
Adds a shared
goToDefinitionWithLspFallbackhelper that tries marimo's notebook resolver first, then triggers the LSP keybinding viarunScopeHandlers.📋 Pre-Review Checklist
✅ Merge Checklist
Made with Cursor