Test: New Shapes for FindTextInFiles and FindFiles

[andreamah]

Refs #59924, #48674

• macOS @paulacamargo25
• windows @Tyriar
• windows @alexr00

Complexity: 4
Roles: Developer, Engineering Manager

Create Issue

---

Intro / History

There has been a lot of work going into finalizing 4 search APIs. Here is a little table that sorta explains how they're related, plus the main function calls from each. Essentially, for file and text search, we have to finalize APIs for

1. Leveraging whatever providers (ie: ripgrep) that vscode has access to in order to find text and files (finders).
2. Given that a user is using API or the UI to find text or files, being able to provide results (ie: if someone had a virtual file system, this helps with giving accurate results) (consumers)

findFiles2 is meant to replace the existing finalized API workspace.findFiles. So it will be renamed to findFiles then.

EDIT: it will be added as an overload of the current findFiles call, where the old call signature will be deprecated (but still supported).

Right now, we are focusing on the APIs on the first row, as their API shapes are quite stable for the most part.

Since this is gonna be a huge change for any first-party extensions that are using these proposed APIs, I've split out the new shape in new files that end in New. For example, the new shape for findTextInFiles is in findTextInFilesNew.

Set up

⚠ Get the latest insiders! ⚠

Create an extension that can use the following proposed APIs:

• findFiles2New
• findTextInFilesNew
• TextSearchProviderNew (needed for some of the types that findTextInFiles uses).

Testing

Now, try the APIs out!

• https://github.com/microsoft/vscode/blob/main/src/vscode-dts/vscode.proposed.findTextInFilesNew.d.ts
• https://github.com/microsoft/vscode/blob/main/src/vscode-dts/vscode.proposed.findFiles2New.d.ts

Check:

• That basic functionality works, considering the known bugs listed below.
• That the API shape makes sense to you and that the documentation makes sense.
• That there isn't anything missing from the API that you think is critical.
• Any suggestions for API shape to make it clearer

Known bugs that I plan to fix!

• Listing any more than 1 GlobPattern in the following will not work. This is because there was no previous support for this, and I sort of scrapped together the connection for these two APIs in one day so that people could test. 😅
  • The filePattern arg for findFiles2New.
  • The excludes field in FindFiles2OptionsNew
  • The includes field in FindTextInFilesOptionsNew
  • The excludes field in FindTextInFilesOptionsNew
• If you set a baseUri for the following GlobPatterns, it will not be recognized. This is for a similar reason- this was a bug carried from the previous implementation.
  • The excludes field in FindTextInFilesOptionsNew
  • The excludes field in FindFiles2OptionsNew

Thanks for testing!! ❤ Please message me if you have any questions about the API!

PS: This TPI was made rather last-minute... I really hope I don't regret this... 😬

[alexr00]

@andreamah this felt like a big complexity 5 TPI to me. I tested for a couple hours but definitely didn't test this exhaustively. I would suggest that for testing this in the future that you break it into 2 smaller TPIs, one for each main API!

[Tyriar]

findFiles2 is meant to replace the existing finalized API workspace.findFiles. So it will be renamed to findFiles then.

It won't replace as that would be breaking would it? Is it going to be a just add an extra overload, similar to createTerminal which has several?

[andreamah]

@andreamah this felt like a big complexity 5 TPI to me. I tested for a couple hours but definitely didn't test this exhaustively. I would suggest that for testing this in the future that you break it into 2 smaller TPIs, one for each main API!

oops, thanks for the feedback! I meant for this to be an opportunity to take a first look at the APIs, but I def underestimated the time that it'd take to get used to the shapes as someone looking at it for the first time.

[andreamah]

findFiles2 is meant to replace the existing finalized API workspace.findFiles. So it will be renamed to findFiles then.

It won't replace as that would be breaking would it? Is it going to be a just add an extra overload, similar to createTerminal which has several?

it'd be an added overload, where the old call type would be deprecated.

[Tyriar]

@andreamah why would the old call be deprecated? Wouldn't it just stay there and remain supported?

[andreamah]

The new way would be the advised way of using it. We could also leave it, although it has confusing exclude configurations

[Tyriar]

@andreamah we have a pretty strict non breaking API policy, I think the only time we broke API was to get multi-root workspaces working properly and that involved emailing extension authors. We should be able to update the description and maybe even the parameter name without breaking though which could help clarify and say what the old one is an alias for in the new one.

[andreamah]

yeah, we can just support the new overload as just an alternative way of calling then. It would be nice if we could 'recommend' the new one though