Initial implementation for Quick Text Search

[andreamah]

Related to #189964

Initial implementation of text search in quick pick.

Some todos:

• find some way to make results load faster, either by streaming results in and/or showing open editor results first.
• fix tooltip on filenames to show directory structure.
• a shortcut to jump between sections/files.
• potentially showing some type of tree structure.
• potentially show preview editor when you're on a result.

[roblourens]

I'm not quite following this, you take the searchResult from the quick access' SearchModel but then that SearchModel creates a new SearchResult for itself?

[andreamah]

yes, since the picker will be closed after doing this, the model no longer needs the search result. The life cycle of a search result is a bit strange, in that a search result just clears and re-populates when a search happens instead of disposing and re-creating. So a search model seems to always make the assumption that a search result exists, and in this case when I'm de-referencing a search result to transfer to another search model, the original search model still needs one.

I agree that this is very strange. If I wanted to move a search result's data from one model to another, how would you expect this to happen? Since the original model doesn't need the results anymore, I was thinking that this process of literally moving the searchresult would be fine, although it seems sketchy. Alternatively, I could rebuild the FolderMatch, FileMatch, Match, etc. that was in the previous searchmodel- but I was thinking that it'd be more efficient to literally transfer it over? I'm open for suggestions, since I don't know what's best practice.

[roblourens]

I think I get it, so you are reusing the SearchResult object, then creating a new one on the search model to make sure that the result isn't referenced by two models.

Since the original model doesn't need the results anymore, I was thinking that this process of literally moving the searchresult would be fine, although it seems sketchy

If it can be really obvious that this is the case- like if you can transfer the SearchResult to the view immediately before disposing the search model, or something like that- then I think that would be fine. But if that's not the case then here are a couple ideas that I think would be easier to follow than this

• Can the whole SearchModel be used in the view instead of transferring SearchResult between models?
• add a transferSearchResult method to SearchModel, so that the details of how this works are kept inside the model, so that you don't have to know to call instantiateNewSearchResult from outside
• Add a clone() method to SearchResult and call that to get a copy and send that to the view

[andreamah]

If it can be really obvious that this is the case- like if you can transfer the SearchResult to the view immediately before disposing the search model, or something like that

The thing with this is that the SearchModel that originally housed the SearchResult still exists after its result gets moved out. That's why I create a new searchresult. This model is reused when the pick is used again.

Can the whole SearchModel be used in the view instead of transferring SearchResult between models?

I actually had this in an earlier version of this code, but I found it strange that the a whole SearchModel was being moved to be the SearchView's viewmodel. I think the code in ISearchWorkbenchService suggests that things that use it (such as the searchView) should follow a singleton approach, and I think that makes sense to me given the naming of SearchModel.

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

Lines 2310 to 2316 in e7fa6c4

	export const ISearchWorkbenchService = createDecorator<ISearchWorkbenchService>('searchWorkbenchService');
	export interface ISearchWorkbenchService {
	readonly _serviceBrand: undefined;
	readonly searchModel: SearchModel;
	}

add a transferSearchResult method to SearchModel, so that the details of how this works are kept inside the model, so that you don't have to know to call instantiateNewSearchResult from outside

This could probably work. Like public transferSearchResult(dest: SearchModel) that would do the work here:

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

Lines 336 to 337 in e7ad565

	this.viewModel.searchResult = searchModel.searchResult;
	searchModel.instantiateNewSearchResult();

Add a clone() method to SearchResult and call that to get a copy and send that to the view

I also thought of this one, and my rationale behind not using it is because the SearchResult to be cloned would no longer be used anyways, so it'd be more performant to just move the original instead of clone it.

[TylerLeonhardt]

were you doing the FastAndSlow picks later?

[andreamah]

Yup! There's some extra work with getting sync and async results from the search service, so I decided to merge it in after this.