Remove/evolve FileIndexProvider

[roblourens]

Master issue to track stabilizing the FileSearchProvider extension API...

Forked from #47058

• Remove FileIndexProvider
• Get shared fuzzy matching code into some npm module to share between vscode and Live Share

[roblourens]

Instead of copying the fuzzy matching support code into a new repo, we can publish it to an npm module out of the main vscode repo, similar to what we do with monaco.

[gjsjohnmurray]

I'd like to understand the plan for the proposed FileIndexProvider API. In #47058 (comment) @roblourens flagged up the cost of URI-ifying the filenames before returning, and asked if we could return strings of relative paths instead. The response from @jrieken was simply

No - the API uses Uri

Since this is still only a proposed API I'm unclear why that aspect couldn't be changed. Or maybe it's that the caller would still have to URI-ify the results, and perhaps it's no cheaper there than if it was done in the EH?

[jrieken]

We will not ship that API because of the inherent problems it has. It's easy to blame on uris (which means blaming the use of objects) but the concept of collecting all filenames (or uris) and sending them in a single batch is problematic. With uris (objects) that might happen sooner but no matter how low-level the abstract is chosen, given the right amount of data this will be a kamikaze API.

[gjsjohnmurray]

Thanks for the response. A get-all-in-one-batch will still happen when workspace.findFiles is called and an extension implements FileSearchProvider, correct?

[jrieken]

Yeah, that will happen but on the when the query is *, no result limit is set, and all (also the default) search excludes are nuked. In that (rare?) case all files of a working will be send around but it also gives us the chance to bail out, e.g. like a-query-result-too-large-error. We don't wanna make this case the API default of working with the system.

[gjsjohnmurray]

That all makes sense. Thank you for explaining.

[marcellourbani]

I wanted to use FileSearchProvider for a remote fs extension I'm working on (for SAP systems, which are not actual remote filesystems but close enough to work).
Will it be possible to exploit this new API instead? BTW, fileindexprovider wouldn't really work in my case as it's very expensive to get a file index from the server.

I did get a "file search" working, but it's rather poorly integrated as I don't want to use unreleased APIs

[jrieken]

@marcellourbani No, this API proposal will go away. For your scenario you wanna implement a FileSystemProvider and optionally the up coming search provider APIs

[gjsjohnmurray]

@jrieken Just to clarify my understanding - FileIndexProvider will go away, and FileSearchProvider will transition from an experimental API to a standard one. Correct?

[jrieken]

yes

[marcellourbani]

@marcellourbani No, this API proposal will go away. For your scenario you wanna implement a FileSystemProvider and optionally the up coming search provider APIs

Thank you.
I have FileSystemProvider working already, looking forward to the search provider. Now it's very annoying as code occasionally complains not having a search provider but I can't provide one (well, I can but then I couldn't publish the whole thing)

Will have to wait the upcoming API or stay where it is in a context menu entry

[roblourens]

As part of this we also have to reintroduce the cacheKey/clearCache() to the FileSearchProvider. I was thinking that we had resolved this already, but no, I remember we got rid of it when moving to the FileIndexProvider...

 	export interface FileSearchProvider {
                 // ...

		clearCache?(cacheKey: string): void;
	}

        export interface FileSearchOptions extends SearchOptions {
		// ...

		cacheKey?: string;
	}

Originally when we discussed this, we considered making the lifecycle of the request more explicit with the concept of a search "session" which is initialized, searches, and is disposed to clear the cache. I think that's overkill for this API. Let's keep this simpler option. I think that most search providers besides ones that we are involved with (liveshare) will ignore the lifecycle and cache anyway.

[jrieken]

The clearCache-function is a little ugly IMO. How about a rich type in the options bag, maybe a cancellation token or something similar. Like so

export interface FileSearchOptions extends SearchOptions {
  // a token that is being reused per session. will fire cancellation
  // when the session isn't needed anymore
  session: CancellationToken;
}

Then, as an extension I can use that token to manage my caches.

[roblourens]

That's a good idea