fix: guard search navigation when no result is focused - #336515
Sanskar (sanskarIN) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Sibling actions remain unguarded, and the new method indentation needs correction.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes a Search view error when navigation runs without a focused result.
Changes:
- Adds an early return for empty focus.
- Uses the validated focused result for match navigation.
File summaries
| File | Summary |
|---|---|
src/vs/workbench/contrib/search/browser/searchActionsNav.ts |
Adds an empty-focus guard. Remaining issues: sibling actions still need equivalent guards (moderate), and the new method needs proper indentation (nit). |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite (auto)
Note
Copilot is running an experiment and ran this review at Lite.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if (!focus) { | ||
| return; | ||
| } |
| run(accessor: ServicesAccessor) { | ||
| const searchView = getSearchView(accessor.get(IViewsService)); | ||
| if (searchView) { | ||
| const tree: WorkbenchCompressibleAsyncDataTree<ISearchResult, RenderableMatch> = searchView.getControl(); | ||
| const viewer = searchView.getControl(); | ||
| const focus = tree.getFocus()[0]; | ||
|
|
||
| if (!focus) { | ||
| return; | ||
| } | ||
|
|
||
| if (isSearchTreeFolderMatch(focus)) { | ||
| viewer.toggleCollapsed(focus); | ||
| } else { | ||
| searchView.open(<FileMatchOrMatch>focus, false, false, true); | ||
| } | ||
| } | ||
| } | ||
| }); |
|
Sanskar (@sanskarIN) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
Summary
This PR fixes a focus-related issue in the Search view. The search navigation action assumes that a result is always focused and directly reads tree.getFocus()[0]. In some cases, such as during refreshes or when no result is active, the focus list can be empty. This causes invalid access to an undefined result.
Fix
The action now checks whether a focused item exists before continuing. If no result is currently focused, it exits early instead of trying to toggle or open an undefined element.
Files Changed
src/vs/workbench/contrib/search/browser/searchActionsNav.ts
Why This Matters
This prevents runtime errors and keeps the Search view stable during transient empty-focus states without altering normal behavior when a result is selected.