fix(file-tree): support copy path keybindings#4734
Merged
Conversation
Contributor
Walkthrough为文件树复制路径命令添加本地化标签，并在 file-tree 贡献中新增目标 URI 解析器以改进 COPY_PATH / COPY_RELATIVE_PATH 的目标解析与早期返回逻辑；同时补充单元测试验证绝对/相对路径与命令标签行为。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Cmd as "COPY_RELATIVE_PATH\n(Command)"
participant Explorer as "FileTreeModel\n(getExplorerTargetUri)"
participant PathUtil as "Path/URI Utils"
participant Clipboard as "IClipboardService\n(writeText)"
User->>Cmd: 触发命令 (可带 target arg)
Cmd->>Explorer: getExplorerTargetUri(arg)
Explorer-->>Cmd: 返回 targetUri 或 null
alt targetUri 存在
Cmd->>PathUtil: 处理 diff-scheme / 解析节点 / 计算(相对/绝对)路径
PathUtil-->>Cmd: 返回路径字符串
Cmd->>Clipboard: writeText(路径字符串)
Clipboard-->>User: 剪贴板写入成功
else 无 targetUri
Cmd-->>User: 早期返回（无操作）
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts`:
- Around line 161-167: The test is mocking IApplicationService but sets
backendOS to the actual host OS, causing COPY_PATH executor logic (which does
pathStr.slice(1) when backendOS === OperatingSystem.Windows) to produce
different clipboard text on Windows and fail the assertion; fix by forcing the
mock IApplicationService backendOS and getBackendOS to return a non-Windows
value (e.g., OperatingSystem.Linux or OperatingSystem.Mac) so the COPY_PATH code
path is deterministic in tests—update the mock for IApplicationService
(backendOS and getBackendOS) accordingly and keep mockClipboardService.writeText
expectation as '/userhome/test.ts'.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bca1fb23-1215-4d84-a188-e04d7ddf0dae
📒 Files selected for processing (3)
packages/core-browser/src/common/common.command.tspackages/file-tree-next/__tests__/browser/file-tree-contribution.test.tspackages/file-tree-next/src/browser/file-tree-contribution.ts
coffeedeveloper
force-pushed
the
fix/copy-path-keybindings
branch
from
ca2da58 to
42cefd9
Compare
coffeedeveloper
force-pushed
the
fix/copy-path-keybindings
branch
from
42cefd9 to
5af552d
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts (1)
161-167:⚠️ Potential issue | 🟡 Minor
backendOS使用宿主机 OS 会让 Windows runner 上的 copy-path 用例失败。
COPY_PATH执行器在backendOS === OperatingSystem.Windows时会执行pathStr.slice(1)，导致第 252 行断言'/userhome/test.ts'在 Windows CI 上变为'userhome/test.ts'而失败。建议在 mock 中固定为 Linux/Mac，使断言跨平台稳定。🛠️ 建议的修复
{ token: IApplicationService, useValue: { - backendOS: Promise.resolve(OS.type()), - getBackendOS: () => Promise.resolve(OS.type()), + backendOS: Promise.resolve(OS.Type.Linux), + getBackendOS: () => Promise.resolve(OS.Type.Linux), }, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts` around lines 161 - 167, The test mock uses the host OS (OS.type()) causing COPY_PATH executor logic to treat Windows differently; update the IApplicationService mock so backendOS and getBackendOS return a fixed non-Windows value (e.g. OperatingSystem.Linux or OperatingSystem.Darwin) instead of Promise.resolve(OS.type()), ensuring the COPY_PATH branch that does pathStr.slice(1) is not exercised and assertions for '/userhome/test.ts' are stable across CI.
🧹 Nitpick comments (1)
packages/file-tree-next/src/browser/file-tree-contribution.ts (1)
762-771: 可移除冗余的copyUri变量。重构后
copyUri仅是targetUri的别名，仅在第 767 行使用一次，可直接用targetUri，并将uriPath的初始化合并到 DIFF 分支之外，让两条分支结构对称，便于阅读。♻️ 建议的精简
const targetUri = this.getExplorerTargetUri(uri); if (!targetUri) { return; } - const copyUri: URI = targetUri; - let uriPath = copyUri.path.toString(); - if (targetUri.scheme === DIFF_SCHEME) { - const query = targetUri.getParsedQuery(); - uriPath = new URI(query.modified).path.toString(); - } + const uriPath = + targetUri.scheme === DIFF_SCHEME + ? new URI(targetUri.getParsedQuery().modified).path.toString() + : targetUri.path.toString();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/file-tree-next/src/browser/file-tree-contribution.ts` around lines 762 - 771, Remove the redundant copyUri alias and simplify uriPath initialization: use targetUri directly (from getExplorerTargetUri) instead of creating copyUri, initialize uriPath from targetUri.path.toString() and then override it inside the DIFF_SCHEME branch by parsing query.modified; update references in this block (targetUri, DIFF_SCHEME, uriPath) so the two branches remain symmetric and easier to read.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts`:
- Around line 161-167: The test mock uses the host OS (OS.type()) causing
COPY_PATH executor logic to treat Windows differently; update the
IApplicationService mock so backendOS and getBackendOS return a fixed
non-Windows value (e.g. OperatingSystem.Linux or OperatingSystem.Darwin) instead
of Promise.resolve(OS.type()), ensuring the COPY_PATH branch that does
pathStr.slice(1) is not exercised and assertions for '/userhome/test.ts' are
stable across CI.
---
Nitpick comments:
In `@packages/file-tree-next/src/browser/file-tree-contribution.ts`:
- Around line 762-771: Remove the redundant copyUri alias and simplify uriPath
initialization: use targetUri directly (from getExplorerTargetUri) instead of
creating copyUri, initialize uriPath from targetUri.path.toString() and then
override it inside the DIFF_SCHEME branch by parsing query.modified; update
references in this block (targetUri, DIFF_SCHEME, uriPath) so the two branches
remain symmetric and easier to read.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c54fc8b2-6597-40cd-b4fd-678ed7f1907e
📒 Files selected for processing (3)
packages/core-browser/src/common/common.command.tspackages/file-tree-next/__tests__/browser/file-tree-contribution.test.tspackages/file-tree-next/src/browser/file-tree-contribution.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core-browser/src/common/common.command.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts (2)
30-31: 为新增的 mock 变量补充类型声明。
mockClipboardService和mockFileTreeModelService没有类型注解，整个测试在any上跑，IDE 也无法在 mock 字段对不上FileTreeModelService真实接口时给出提示。一旦后续FileTreeModelService增减字段（例如增加新的 priority 来源），这里也不会有任何编译期反馈。♻️ 建议的小改动
- let mockClipboardService; - let mockFileTreeModelService; + let mockClipboardService: { writeText: jest.Mock }; + let mockFileTreeModelService: Partial<FileTreeModelService> & { + selectedFiles: Array<{ uri: URI }>; + focusedFile: { uri: URI } | undefined; + contextMenuFile: { uri: URI } | undefined; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts` around lines 30 - 31, 为 mockClipboardService 和 mockFileTreeModelService 补充明确类型注解：在文件顶部引入真实接口类型（例如 ClipboardService、FileTreeModelService），并将 mockClipboardService 标注为对应的 jest.Mocked<ClipboardService> 或 Partial<ClipboardService>，将 mockFileTreeModelService 标注为 jest.Mocked<FileTreeModelService> 或 Partial<FileTreeModelService>（或使用更精确的 MockedFunction/MockedObject 工具），以便在对 FileTreeModelService 的字段或方法进行模拟时获得编译期和 IDE 提示，并在需要时显式列出被模拟的方法签名以保持与真实接口同步。
234-270: 需要补充 copy path 命令的降级优先级测试覆盖。生产代码在
getExplorerTargetUri方法中实现了明确的优先级：activeUri → focusedFile → selectedFiles → contextMenuFile。当前测试用例仅覆盖selectedFiles分支，缺少对其他三个降级分支及其优先级关系的验证。后续若重构改动了优先级顺序或遗漏某个分支，现有用例无法捕获。建议至少补充以下场景的测试：
- 仅设置
focusedFile时，验证命令执行使用该文件路径- 同时设置
focusedFile与selectedFiles（指向不同 URI），验证 focusedFile 优先级更高- 仅设置
contextMenuFile时，验证命令执行使用该文件路径- 有
activeUri时优先于其他来源（若在当前 contribution 内实现此分支）🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts` around lines 234 - 270, Add unit tests for the copy path commands to cover all fallback branches of getExplorerTargetUri: create tests that (1) set only mockFileTreeModelService.focusedFile and verify filetree.copy.path and filetree.copy.relativepath write that focused URI to mockClipboardService, (2) set both mockFileTreeModelService.focusedFile and mockFileTreeModelService.selectedFiles with different URIs and assert focusedFile wins, (3) set only mockFileTreeModelService.contextMenuFile and assert that path is used, and (4) set an activeUri (via the mock editor service / active editor used by getExplorerTargetUri) and assert activeUri takes precedence over others; in each case registerCommands() and call commands.get('filetree.copy.path').execute() / 'filetree.copy.relativepath' and expect mockClipboardService.writeText to be called with the exact path or relative path accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/file-tree-next/__tests__/browser/file-tree-contribution.test.ts`:
- Around line 30-31: 为 mockClipboardService 和 mockFileTreeModelService
补充明确类型注解：在文件顶部引入真实接口类型（例如 ClipboardService、FileTreeModelService），并将
mockClipboardService 标注为对应的 jest.Mocked<ClipboardService> 或
Partial<ClipboardService>，将 mockFileTreeModelService 标注为
jest.Mocked<FileTreeModelService> 或 Partial<FileTreeModelService>（或使用更精确的
MockedFunction/MockedObject 工具），以便在对 FileTreeModelService 的字段或方法进行模拟时获得编译期和 IDE
提示，并在需要时显式列出被模拟的方法签名以保持与真实接口同步。
- Around line 234-270: Add unit tests for the copy path commands to cover all
fallback branches of getExplorerTargetUri: create tests that (1) set only
mockFileTreeModelService.focusedFile and verify filetree.copy.path and
filetree.copy.relativepath write that focused URI to mockClipboardService, (2)
set both mockFileTreeModelService.focusedFile and
mockFileTreeModelService.selectedFiles with different URIs and assert
focusedFile wins, (3) set only mockFileTreeModelService.contextMenuFile and
assert that path is used, and (4) set an activeUri (via the mock editor service
/ active editor used by getExplorerTargetUri) and assert activeUri takes
precedence over others; in each case registerCommands() and call
commands.get('filetree.copy.path').execute() / 'filetree.copy.relativepath' and
expect mockClipboardService.writeText to be called with the exact path or
relative path accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1cbf15e6-e998-40ed-b886-afd5c58da981
📒 Files selected for processing (3)
packages/core-browser/src/common/common.command.tspackages/file-tree-next/__tests__/browser/file-tree-contribution.test.tspackages/file-tree-next/src/browser/file-tree-contribution.ts
✅ Files skipped from review due to trivial changes (2)
- packages/core-browser/src/common/common.command.ts
- packages/file-tree-next/src/browser/file-tree-contribution.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4734 +/- ##
=======================================
Coverage 52.96% 52.97%
=======================================
Files 1686 1686
Lines 105031 105031
Branches 22830 22839 +9
=======================================
+ Hits 55634 55635 +1
+ Misses 41043 41042 -1
Partials 8354 8354
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
2 participants
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.
Types
Background
The explorer context menu provides
Copy PathandCopy Relative Path, but these commands depend on the menu-provided URI argument. When users configure custom shortcuts for these commands, the keybinding service invokes them without that URI, so nothing is copied.Copy Relative Pathalso lacked a command label, which prevented it from being exposed properly as a configurable command in the keybindings UI.Solution
filetree.copy.relativepathso users can configure a shortcut forCopy Relative Path.Changelog
fix(file-tree): support custom shortcuts forCopy Path.fix(file-tree): support custom shortcuts forCopy Relative Path.test(file-tree): add coverage for shortcut-triggered copy path commands.Summary by CodeRabbit
发布说明
新功能
测试