Vscode: 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 https://github.com/microsoft/vscode/issues/90962#issuecomment-593309168_

[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 https://github.com/microsoft/vscode/issues/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 https://github.com/microsoft/vscode/issues/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:

• [x] 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
• [x] 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"
}

• [x] Introduce a setting to control seeding the search from the word under the cursor with no selection, to resolve https://github.com/microsoft/vscode/issues/88160
• [x] Make the above command _not_ seed search input at all
• [x] 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]

Is it possible to have setting that controls action when user clicks on tab or icon in actions bar?

Would be useful to clearly pick effect on click: show & search vs show. Would help to not re-run search and reset previous results if I just copy form files, but not remove results(to mark search dirty) or just start unnecessary searches if there is selection in file (I always use shortcut for this).

[roblourens]

I didn't change what happens if you click the search icon, that should still not trigger a search. I don't quite follow what you are asking for though.

[IllusionMH]

I'm asking for setting that will disable seeding search when search icon is is clicked. Because currently there is no way to disable seeding when view is shown.

With current implementation search is not triggered, but search field is seeded.

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

If second exists - why not trigger search right on Ctrl+Shift+F because currently second one seeds input?

[roblourens]

Got it. I agree, updated the above comment https://github.com/microsoft/vscode/issues/91901#issuecomment-602373065

[JacksonKearl]

Added configuration search.seedOnFocus which controls if clicking on the search icon or running "Focus on Search View" seed the search. This is currently false by default, as clicking on the search icon and running "Focus on Search View" I don't think would be expected to modify the contents of the search view.

This is a modification to existing behavior, so we're likely to see some issues about this. search.seedOnFocus: true restores existing behaviour.

[JacksonKearl]

Also added a new setting search.seedWithNearestWord that is consulted instead of checking the emptiness of the search input when deciding whether to use nearest word. Defaults to false, as that's the most common case previously (only true for the first search in a workspace or if you manually clear the input)

[roblourens]

Awesome thanks for working on this @JacksonKearl!

[IllusionMH]

Thanks for changes! Looks like "search.seedOnFocus": false what I was missing for a long time 👍

With search.seedOnFocus in place is it possible to have an option immediately trigger search when "search.seedOnFocus": false and user press Ctrl+Shift+F ?

[JacksonKearl]

@IllusionMH if Ctrl+Shift+F is bound to "workbench.view.search" it should seed with the selection. One thing is that this wont work if the search viewlet is open and you have the default when condition (!searchViewletVisible). Maybe that default when condition should be removed?

Edit: Never mind, I see that the default workbench.action.findInFiles binding should make this work. (I had that unset)

[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: https://github.com/microsoft/vscode/blob/470bc088b34e7ba47c26a54634539b822979adee/src/vs/workbench/contrib/search/browser/searchView.ts#L867

[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!