feat(code-editor): improve definition navigation - #3468
qingqing-ux wants to merge 1 commit into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 003a7c199f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| closeTempTextDocuments() { | ||
| this.tempTextDocumentIds.splice(0) | ||
| if (!this.isDisposed) this.setActiveTextDocument(this.mainTextDocumentId) | ||
| private goToDefinition(target: TextDocumentPosition | TextDocumentRange, source?: TextDocumentPosition) { |
There was a problem hiding this comment.
The navigation stack is populated only by this private definition-specific path, but cross-document links in components/editor/spx-code-editor/CodeLink.vue:52-66 still call the public ui.open(...) directly. Since this commit also removes DocumentTabs, following a Console or Copilot code link to another document leaves no tab or back action for returning to the original document and view state; direct cross-document opens should participate in the new navigation mechanism as well.
Useful? React with 👍 / 👎.
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review summary
This PR replaces the multi-tab "temporary document" navigation with an inline definition peek preview plus a back-navigation stack. The design is clean: removed members (tempTextDocuments, mainTextDocument, closeTempTextDocuments, the DocumentTab/DocumentTabs components, and the now-unused textDocumentIdEq import) are fully cleaned up, hover.ts's getGoToDefinitionAction → getDefinitionActions rename is consistent, and the new behavior is well covered by hover.test.ts and code-editor-ui.test.ts.
Findings below are minor/maintainability. No security issues were found (interpolated URIs go through setAttribute, and the shared Monaco model is correctly view-only via readOnly).
A couple of low-confidence notes not attached inline:
navigationStackis unbounded — everygoToDefinitionpushes an entry (with a full Monaco view state) and onlygoBackpops. In a long session with many jumps and little back-navigation this grows without bound. A max-depth cap (drop oldest) would bound it.docs/develop/code-editor/base.tsstill documentsbuiltInCommandGoToDefinitionwith a single[TextDocumentPosition]arg (andspx.*prefix), and does not mention the newxgo.viewDefinition. The doc already predated thexgo.*implementation, so this is likely out of scope — flagging only if that doc is meant to track this contract.
| const previous = this.navigationStack.pop() | ||
| if (previous == null) return | ||
| this.setActiveTextDocument(previous.textDocument.id) | ||
| if (previous.viewState != null) this.editor.restoreViewState(previous.viewState) |
There was a problem hiding this comment.
[P2] goBack restores view state twice (redundant)
goBack calls setActiveTextDocument(previous.textDocument.id), which already restores that document's saved view state from viewStateMap. It then restores again from previous.viewState. Both values are the same source-document view state, so the second restoreViewState is redundant — and the same view state is persisted twice on every goToDefinition (once in the navigation stack, once in the WeakMap). Consider a single source of truth: either let setActiveTextDocument own view-state restoration (and drop viewState from CodeNavigationEntry), or bypass the map here. Also note: if saveViewState() ever returns null, goBack restores nothing, so the cursor won't return to the captured position (which is otherwise only used for the back-bar label). Falling back to open(previous.textDocument.id, previous.position) when viewState == null would keep label and restored location consistent.
| /> | ||
| <DefinitionPeek | ||
| v-if="uiRef.definitionPeek != null" | ||
| :key="`${uiRef.definitionPeek.textDocument.id.uri}:${uiRef.definitionPeek.range.start.line}:${uiRef.definitionPeek.range.start.column}`" |
There was a problem hiding this comment.
[P3] DefinitionPeek :key omits range.end
DefinitionPeek applies model/selection/reveal only once in handleEditorInit; it does not watch textDocument/range, so it only updates when the :key forces a full remount. The key is uri:range.start.line:range.start.column and omits range.end. Two "view definition" actions with the same start but a different end range would reuse the same editor instance and show a stale selection. Low likelihood, but either include the full range in the key or add a watch on the props so the component is self-consistent regardless of keying. (Note: the full remount also tears down and recreates the Monaco editor instance on every peek-target change, which is one of the heavier editor operations — a stable instance updated via setModel/setSelection/revealRange would avoid that cost.)
This branch was successfully deployed
1 participant
改动说明
优化代码定义的查看与跳转体验，让用户能够明确区分：
本次改动包括：
Fixes #3455
设计考量
1. 区分「查看定义」与「跳转到定义」
悬浮卡片提供两个独立入口：
用户可以根据当前目的选择临时阅读，或进入目标文档继续查看和编辑。
Peek 保持只读，因为它是临时阅读界面。代码编辑仍在主编辑器中完成，避免在临时面板内引入保存和二次确认等额外状态。
2. 明确表达「从哪里来」和「现在在哪里」
执行「跳转到定义」后，编辑器顶部显示导航条：
返回 Board · 第 4 行：说明跳转来源，同时提供返回入口；当前位置：Stage：说明主编辑器当前打开的文档。返回时恢复来源文档以及跳转前的 Monaco 编辑器视图状态，包括光标、选区和滚动位置。
连续执行多次定义跳转时，来源位置按栈保存，用户可以依次返回上一处代码位置。
3. 移除临时文档管理心智
原来的临时文档列表同时承担代码导航和文档管理职责，存在以下问题：
因此移除右侧临时文档列表及「全部关闭」，使用编辑器顶部的返回入口承接代码导航。右侧只保留原有缩放控制。
4. 让状态反馈贴近代码上下文
导航条属于整个编辑器的导航状态，因此横跨代码区和右侧工具栏上方，并保留对称的左右间距。
定义 Peek 显示在来源代码下方，并对 Monaco 编辑器边栏进行精简：
5. 中英文支持
所有新增文案均使用项目现有的国际化机制。
文档名称优先使用项目已有的本地化显示名称；用户自定义名称保持不变。
验收路径
使用 PR 前端预览中的 Match3 项目，进入
Board精灵的代码页面。场景一：不离开来源文档查看定义
getRowCol；Definition in Stage · Line 39；定义位于 Stage · 第 39 行；Board；场景二：跳转到定义并返回
getRowCol；Stage中的定义；返回 Board · 第 4 行；当前位置：Stage；返回 Board · 第 4 行；Board；场景三：连续跳转和逐级返回
场景四：中文界面
场景五：布局检查
验证结果
pnpm exec vitest --run src/components/xgo-code-editorpnpm run lintpnpm run type-checkpnpm run build