Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

folderOptions on query, vs options #226814

Closed
roblourens opened this issue Aug 27, 2024 · 4 comments
Closed

folderOptions on query, vs options #226814

roblourens opened this issue Aug 27, 2024 · 4 comments
Assignees
Labels
info-needed Issue requires more information from poster

Comments

@roblourens
Copy link
Member

Testing #226669

It sort of feels like folderOptions should be on the "query", not on the options, since it just feels like a fundamental part of the query. I am searching this folder for this pattern, that's the most important thing. Then you have surroundingContext, previewOptions, etc which are additional "meta" flags that relate to how results are presented, etc.

@andreamah
Copy link
Contributor

I think of query as more of how the text itself is looked up (how you know that it's a 'match' when you see it in text), whereas the options are more like how to filter files and any extra limitations. I guess that this also simplifies some of the arguments for findTextInFiles, as it also uses the same query type as TextSearchProvider.

@andreamah
Copy link
Contributor

I guess that you could argue that the provider implementer will look at it as more like "given these files, I need to search using this criteria", so then thing like the include are really important. But I also could argue that the way that we have it right now with clear parallels also just makes the relationship between the APIs cleaner and easier to understand.

@roblourens
Copy link
Member Author

Yeah, I can see it either way.

@andreamah
Copy link
Contributor

I'm going to continue discussion about this what's in query vs options in #226738. Although it's not an identical convo, I should probably just collect feedback on how to re-shape on that thread.

@andreamah andreamah closed this as not planned Won't fix, can't repro, duplicate, stale Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

2 participants