When no results in search editor it throws an error

[svipas]

Does this issue occur when all extensions are disabled?: Yes/No

• VS Code Version: 1.95.1
• OS Version: 15.1 (24B83)

Steps to Reproduce:

1. Open Search Editor
2. Search for non existing word for e.g. helllllooooo
3. Use keyboard shortcut or run Search: Focus Next Search Result
4. You will see the error

What we need is when clause which could say that there are results in search editor, I tried to use hasSearchResult but it wasn't working.

[aayyob]

Hi @roblourens

My partner, @janjar, and I are students currently working on a software engineering course project at the University of Michigan, and we’re interested in contributing to the VSCode open-source project. We’ve reviewed issue #233200 and feel that it aligns well with our coursework and skills. We’d love the opportunity to work on this bug and submit a solution.

If possible, could you assign the issue to us? We’ll be following the project’s contribution guidelines closely and reaching out if we need any clarification on the requirements. We’re committed to maintaining high code quality, following VSCode’s standards, and ensuring our solution meets the expected functionality and quality assurance processes. We would also like to know the intended functionality.

Thanks very much for considering our request. We’re looking forward to contributing to the project!

[roblourens]

Sure, feel free to submit a PR. "Focus Next Search Result" is here: https://github.com/microsoft/vscode/blob/76b576a24feb0093fbdc36c1fb11643e97de2ab9/src/vs/workbench/contrib/search/browser/searchActionsNav.ts#L370 so that's a good place to start debugging

[janajar]

Hi @roblourens

We’ve investigated this issue further and identified a potential cause in the registerAction2 method. It appears the FocusNextSearchResult function is not executing as intended because of the precondition in the registerAction2 setup.

Currently, the precondition uses an OR condition between the context expressions that check if the window is focused and if search results are available. However, this should be an AND condition to ensure FocusNextSearchResult only activates when both conditions are met—i.e., the window is focused and there are search results available.

Here’s an example of where this could be adjusted:

registerAction2(class FocusNextSearchResultAction extends Action2 {
    constructor() {
        super({
            id: Constants.SearchCommandIds.FocusNextSearchResultActionId,
            title: nls.localize2('FocusNextSearchResult.label', "Focus Next Search Result"),
            keybinding: [{
                primary: KeyCode.F4,
                weight: KeybindingWeight.WorkbenchContrib,
            }],
            category,
            f1: true,
                         // Updated precondition
            precondition: ContextKeyExpr.and(Constants.SearchContext.HasSearchResults, SearchEditorConstants.InSearchEditor),
        });
    }

    override async run(accessor: ServicesAccessor): Promise<any> {
        return await focusNextSearchResult(accessor);
    }
});

With this change, FocusNextSearchResult should only trigger when the search editor has focus and there are actual search results to navigate through. This same issue is also affecting the FocusPreviousSearchResult functionality which can be fixed in the same exact manner. We're happy to submit a PR if this approach looks correct.

[roblourens]

This 'or' is ok- the action actually does two things, it navigates search results in the search side bar, or it navigates results in the search editor when in a search editor.

So in this scenario, I think the action should still execute, run should still be called, but the code that runs should be fixed to not throw an error when there are no results. Probably when there are no results it should just not do anything.

[codewithunknown]

Hello @svipas,

Could you please help me reproduce this issue? If possible, any screenshots or screen recordings would be greatly appreciated.

The steps I tried include searching and clicking on the "Focus Next Result" shortcut.

Additionally, while working on this, @roblourens, I noticed that we can't assign Ctrl + Alt + T or Ctrl + Alt + O as shortcuts. Is this intentional or unintentional?

[janajar]

Hi @codewithunknown, My partner and I are about to put a pull request in for this issue. We are just waiting for @roblourens feedback before we are pushing it. If you have any advice/feedback as well send me a message.

[svipas]

@codewithunknown It is not Search Editor it is Search. Those 2 are different. You need to open Search Editor.

[janajar]

Hi @roblourens, @aayyob and I have found a fix that works locally on our end. It uses the SearchEditorInput match range attribute to check if there are any search results before actually executing the command. As you specified earlier, we put this in the focusNextSearchResult and focusPreviousSearchResult functions so it actually runs. Here is our code:

if (input instanceof SearchEditorInput) {
    if (!input.getMatchRanges().length) {
        return;
    }
    // cast as we cannot import SearchEditor as a value b/c cyclic dependency.
    return (editorService.activeEditorPane as SearchEditor).focusPreviousResult();
}

If you think this looks good, then I will submit a pull request. Also, would you like a test case in order to ensure this bug stays fixed in all future versions? Appreciate all the help!

[roblourens]

Seems like that would work. But that still means that the focusPreviousResult fails in this case, right? And it could be called from somewhere else, now or in the future. Would it make sense to put the fix closer to the exact line that fails?

[janajar]

Hi @roblourens, We took your feedback and put the fix closer to the issue. The issue mainly occurs when the setSelection method receives an undefined matchRange, so we simply added a null check in the iterateThroughMatches method. When you get the chance please review the PR. Appreciate all the help!