microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
161k stars 28.25k forks source link

Search API Work Summary #210692

Open andreamah opened 3 months ago

andreamah commented 3 months ago

I have some notes on what work is left to finalize some popular proposed search APIs. I will put them in this issue.

API Explanation

Background

There are currently three search APIs that are quite old and need to be finalized:

API Info

Blockers

FileSearchProvider

  1. Perf issue: for every URI that the extension sends, it must be serialized and deserialized again to be sent to the renderer process.

    • Is there a more efficient way?
      • Are simple path strings okay? But this is not a standard way with vscode APIs.
    • We should test that this is still an issue. Do some profiling.
  2. Uncertain element: does the caching work? There is a FileSearchOptions.session flag. We expect ext developers to have a map on their side that is used for caching. Here is an example.

    Map<FileSearchOptions.session, cacheEntry>
    - once the FileSearchOptions.session is cancelled, clear the map entry

    We should verify that something like this works.

  3. There is a limitation of one provider per scheme, and the default provider already covers the file scheme. What if the user wants to install a third-party provider for file scheme searching? What if there are multiple providers for a custom scheme? For example, what if they want to replace ripgrep as their search engine?

    • Ideally, people can set in their settings what they want to use as a search provider if they have more than one for a scheme.

TextSearchProvider

  1. Same as FileSearchProvider's first concern- even more so, since the result limit is 20 000.
  2. Same as FileSearchProvider's third concern.

FindTextInFiles

  1. Should be fine, just blocked by TextSearchProvider. This API shares types of TextSearchProvider, which means that we should make sure that those are sane before finalizing this

Extra Considerations

andreamah commented 3 months ago

After talking to @jrieken, some notes:

FileSearchProvider

  1. Ran a load test with >20 000 URIs serializing and deserializing; it seems to perform around <80ms.
  2. Should still test the cancellationTokens as a map, but using a WeakMap for this case should work just fine.
  3. We can maybe provide an option to disable ripgrep as a file search provider. People would disable this if they wanted to use a another searcher for the file scheme.

Extra: We should make sure that, if the cancellation token is cancelled, we don't listen to onProgress anymore and possibly log an error (like here). Is there any case where we should still let the caller contribute results?

As an extra next-step, we should try to look through some of the options of these APIs to see what seems ripgrep-specific and unnecessary.