Disable Copy Breadcrumbs Path without symbols#318042
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new context key that tracks whether the current breadcrumbs contain symbol (outline) items, and uses it to conditionally enable/show the “Copy Breadcrumbs Path” action.
Changes:
- Introduced
breadcrumbsHasSymbolscontext key and wired it into lifecycle/reset/hide paths. - Set
breadcrumbsHasSymbolsbased on the current breadcrumbs model elements. - Updated “Copy Breadcrumbs Path” command/menu conditions to require symbol breadcrumbs.
| }; | ||
| const items = model.getElements().map(element => element instanceof FileElement | ||
| const elements = model.getElements(); | ||
| this._ckBreadcrumbsHasSymbols.set(elements.some(element => element instanceof OutlineElement2)); |
There was a problem hiding this comment.
Thanks, good catch. I updated this in the latest push so breadcrumbsHasSymbols now uses the same classification as the rendering path: file breadcrumbs are FileElement, and everything else is treated as an outline/symbol breadcrumb.
| title: localize2('cmd.copyPath', "Copy Breadcrumbs Path"), | ||
| category: Categories.View, | ||
| precondition: BreadcrumbsControl.CK_BreadcrumbsVisible, | ||
| precondition: ContextKeyExpr.and(BreadcrumbsControl.CK_BreadcrumbsVisible, BreadcrumbsControl.CK_BreadcrumbsHasSymbols), |
There was a problem hiding this comment.
The linked issue is specifically about this command being available when it cannot copy a symbol breadcrumb path, and the maintainer direction was to disable it when there is no symbol selection. File/folder path copying is already covered by the neighboring Copy Path and Copy Relative Path actions, so this keeps the command scoped to its current symbol-breadcrumb behavior.
| group: '1_cutcopypaste', | ||
| order: 100, | ||
| when: BreadcrumbsControl.CK_BreadcrumbsPossible | ||
| when: ContextKeyExpr.and(BreadcrumbsControl.CK_BreadcrumbsPossible, BreadcrumbsControl.CK_BreadcrumbsHasSymbols) |
There was a problem hiding this comment.
Same rationale as above: this PR intentionally keeps Copy Breadcrumbs Path scoped to symbol breadcrumbs per #286240 and the maintainer comment there. File-only breadcrumbs should continue to use the existing Copy Path / Copy Relative Path actions instead of making this command silently no-op.
e4f966e to
9d350e6
Compare
Fixes #286240
This updates the breadcrumbs context keys so
Copy Breadcrumbs Pathis only enabled when the current breadcrumbs include symbol items. The command already no-ops when there are no symbol breadcrumbs; this makes the menu and command availability match that behavior.Testing:
npm ci --ignore-scriptsnpm run compile-check-ts-native -- --pretty falsenpx -y node@22.22.1 build/eslint.ts src/vs/workbench/browser/parts/editor/breadcrumbsControl.tsgit diff --checkNote: the default
npm run eslint -- src/vs/workbench/browser/parts/editor/breadcrumbsControl.tscommand failed under the machine's default Node v20.15.0 before linting because the repo currently expects Node 22. I reran the same lint entrypoint with Node 22.22.1.