Smarten workspace search from selection logic

[JacksonKearl]

I think this could get a bit more clever:

• still run the search automatically as before if search view is empty
• also maybe you could detect if I removed results and run the search if I left the results untouched?

Originally posted by @bpasero in #90962 (comment)

[roblourens]

still run the search automatically as before if search view is empty

This is only once per window session for me, and I don't necessarily "know" if the search view is empty.

also maybe you could detect if I removed results

This feels like an arbitrary condition to me. I think if we change the behavior for a bunch of individual special cases then it will just feel like the behavior is random and unpredictable.

[roblourens]

I wonder about having different commands/keybindings to "focus and search" vs "just focus". I think a big part of the problem is still that cmd+shift+F is overloaded

[roblourens]

Thinking about it more, maybe "always focus and search unless results were removed" is not that bad

[roblourens]

Also #88160 is relevant, about the inconsistency in when we grab the word under the cursor

[roblourens]

I think that issue should be considered along with this. If we want consistency in when the word under the cursor is taken, we have to either

A. Do that every time cmd+shift+F is pressed
B. Add a setting to control the behavior
C. Add a separate command/default keybinding to focus the search input and seed it with the word under the cursor

I don't like A or B because sometimes I want that and sometimes I don't. What would people feel about C? Could pick a keybinding like cmd+shift+alt+F or some unused cmd+shift+ and users can customize it if they want it to be the cmd+shift+F default.

Then, I would also say that selected text is always copied to the search input. Then in both of these cases, we would always trigger a search, unless the user has "dirtied" the result tree by removing results. Then they have to press enter to trigger the search.

Thoughts?

[akbyrd]

I think uncoupling "focus search" and "focus and seed search" is a good idea. That's probably slightly better than an option for the seed behavior.

As long as search.searchOnType remains an option, I think your suggestion will work for my use cases. I never want that feature and need to be able to disable it completely.

I can imagine other users wanting an option to allow search.searchOnType even in the presence of modified results. I think it would be good to leave that control in the user's hands. Perhaps the values for that option should be [always, unmodified, never].

To ensure I'm understanding correctly, the final result would be this

workbench.action.findInFiles
workbench.view.search
Focus the search panel without seeding from the cursor.
Seed from selection if present.
Respect search.searchOnType
Default bind to Ctrl+Shift+F

workbench.view.searchWithSeed
Focus the search panel seeding from the cursor.
Seed from selection if present.
Respect search.searchOnType
Default bind to something else or not set

search.searchOnType
Possibly expanded from boolean to [always, unmodified, never]

editor.find.seedSearchStringFromSelection
Deprecated?

That last part is concerning. I'm not sure how all this interacts with regular Ctrl+F searching. I'd assume we want the same range of capability and control.

[thernstig]

So apparently I wrote a duplicate #92477, but for all it is worth, since it is a regression from previous behavior and there is no way to config what happens if one runs Search: Find in Files (e.g. via ctrl+shift+F) when having text selected, at least make it configurable if you want to keep the new behaviour 😃

[roblourens]

I think we should put back the behavior for the keybinding with text selected, but leave the seach triggering off when you click the search icon. But still have to think about the other points above.

I wouldn't change the behavior for the editor find, but I forgot that search actually respects the editor.find.seedSearchStringFromSelection setting here too, so I have to think about that, thanks for the reminder @akbyrd

[roblourens]

Current thinking:

• Restore previous behavior of cmd+shift+F which I think is an ok default, except the search won't be triggered when a result has been removed
• The command "Focus on Search View" already exists for people who don't want the seed to trigger a search at all, they can rebind it if they want
  • Example:

{
    "key": "shift+cmd+f",
    "command": "workbench.view.search.focus"
}

• Introduce a setting to control seeding the search from the word under the cursor with no selection, to resolve Search seeding is inconsistent #88160
• Make the above command not seed search input at all
• Introduce a setting to disable seeding search when the search viewlet button is clicked (or disable this entirely as I just don't think it really makes sense)

[IllusionMH]

It seeds input but won't perform new search if results are "dirty".

BTW looks like there is inconsistency in marking results as "dirty" - removing file from result prevents next search to be performed immediately (requires Enter ), but removing one match from file that has several - allows immediate search.

[JacksonKearl]

So are you asking for a way to trigger a new search with a single keypress even when the prior search results were dirty?

[IllusionMH]

Yes, but I guess it should be available (probably as an option if feasible) only when "search.seedOnFocus": false.

Otherwise it will be back to state when searches were fast but previous searches were lost unintentionally.

[JacksonKearl]

I think the dirty case is rare enough that requiring an extra enter to run the search is okay, compared to the potential annoyance of configuring the setting by mistake and losing your modifications.

[bpasero]

@JacksonKearl I am lost what actually changed and how this new setting "search.seedOnFocus": true should work. Independent from how I configure it, using the keybinding to focus the search view seems to take from the editor selection and run the search.

I still actually would like a setting to have this behaviour:

• search view empty: Cmd+Shift+F takes selection from editor and runs search
• search view not-empty but results not modified: same as "search view empty"
• search view not-empty and some results removed: do nothing (neither change search query, nor run search).

[thernstig]

I prefer it always to search what I've selected in the search editor, no matter what already exists in the search view input field 😆

[bpasero]

@thernstig in my case I had a large search with thousands of results that I used like my todo list where I carefully collapsed some and removed others. In that case, replacing the list with a new search at random can really be frustrating.

[IllusionMH]

search view not-empty and some results removed: do nothing (neither change search query, nor run search).

Is it only when "search.seedOnFocus": true?

Because if it's applied when it false - deleting result will require manual copy/pasting actions for performing next search, even when I explicitly tell to perform action with shortcut. (Which will be as painful as Ctrl+P that's not seeded with selection)

[bpasero]

To be clear: I am only referring to the automated execution of search that seems to happen based on my editor selection and without further interaction once I focus the search viewlet. If you have a keybinding for running a search it should trigger no matter what.

[JacksonKearl]

To verify:

"search.seedOnFocus": true (old behaviour):

• Clicking on the search viewlet or running workbench.view.search.focus will cause the query to be replaced with the editor selection (no search is triggered)

"search.seedOnFocus": false (new, default):

• Clicking on the search viewlet or running workbench.view.search.focus will only focus the viewlet, it will not modify contents at all

"search.seedWithNearestWord": true:

• workbench.view.search will always replace the query with the word under cursor

"search.seedWithNearestWord": false:

• workbench.view.search will never replace the query with the word under cursor

@bpasero

• search view empty: Cmd+Shift+F takes selection from editor and runs search
  • default behaviour
• search view not-empty but results not modified: same as "search view empty"
  • default behaviour
• search view not-empty and some results removed: do nothing (neither change search query, nor run search).
  • default behaviour if there's no active selection
  • otherwise not possible currently, workbench.view.search will always place the active selection (if it exists) in the query box. However a new search will not be triggered. If we added a search.resultsModified context key you could potentially route to workbench.view.search.focus whenever that is true?

[bpasero]

search view not-empty but results not modified

I often see that search is not triggered when I invoke Cmd+Shift+F but I don't think I modified the search results.

What is the exact condition that needs to apply for the search to run from the selection in the editor, can you show me in code?

[JacksonKearl]

Yes, the core of the logic is here:

vscode/src/vs/workbench/contrib/search/browser/searchView.ts

Line 867 in 470bc08

updateTextFromSelection({ allowUnselectedWord = true, allowSearchOnType = true }): boolean {

[bpasero]

@JacksonKearl yeah I think I found the bug:

• do not configure anything special
• select something in editor and focus search
• search runs fine and shows results
• remove the search term from the input in search (actually you can also click on the clear icon)
• search results are cleared
• select something in editor and focus search

=> 🐛 search is NOT executed again!

In other words, it looks like once I cleared search results this feature is disabled from then on. This explains why often I noticed that it would not work for me anymore...

[roblourens]

Good catch, clearing the results triggers the "removed results" logic which it shouldn't in this case

[JacksonKearl]

@bpasero can you verify? Thanks!