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

Do not include git files in search results #85915

Closed
alexdima opened this issue Dec 2, 2019 · 9 comments
Closed

Do not include git files in search results #85915

alexdima opened this issue Dec 2, 2019 · 9 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues search-replace Search and replace issues verified Verification succeeded
Milestone

Comments

@alexdima
Copy link
Member

alexdima commented Dec 2, 2019

This has started recently, and it is annoying for me:

  • when having many tabs opened, including diff editors
  • and I want to do a cross-cutting refactoring, like renaming something in multiple places which don't work via F2 (like API), I use the search viewlet.
  • the search viewlet has now multiple times misled me and I ended up in what appears to be the "old" version of the file via the git fs provider.
  • this is highly annoying, in this case this is clearly separated in the viewlet, but I've run into situations where this was the only result and it was no longer clearly separated.

image

@joaomoreno
Copy link
Member

joaomoreno commented Dec 2, 2019

Yup that's pretty ugly, I seen it too. This is a consequence of using a FileSystemProvider for git, instead of the TextDocumentContentProvider API. cc @eamodio @bpasero

I wonder if we can filter out search documents based on URI scheme, @roblourens?

@joaomoreno joaomoreno added this to the November 2019 milestone Dec 2, 2019
@joaomoreno joaomoreno added bug Issue identified by VS Code Team member as probable bug git GIT issues search-replace Search and replace issues labels Dec 2, 2019
@roblourens
Copy link
Member

Should I specifically exclude the gitfs scheme or is there something else that distinguishes these from other documents?

I could ignore docs with a scheme that doesn't match that of a workspace folder (but including untitled docs)?

@roblourens
Copy link
Member

roblourens commented Dec 2, 2019

I think excluding git docs specifically might be the best fix, I don't want to make FileSystemProviders less useful by excluding their docs from search all the time.

Or does it make any sense for a FileSystemProvider to declare whether it wants to be searched?

@alexdima
Copy link
Member Author

alexdima commented Dec 2, 2019

👍 IMHO the safest change so late is to exclude git specifically. The best would be for a FileSystemProvider to opt-in/opt-out of searching.

@joaomoreno
Copy link
Member

joaomoreno commented Dec 3, 2019

Agreed, thanks @roblourens!

cc @isidorn and @jrieken for FileSystemProvider API

@jrieken
Copy link
Member

jrieken commented Dec 3, 2019

I think this problem isn't exclusive to the FileSystemProvider API because there many sources for editor model. @roblourens How does this work for other resources, like any document from a TextDocumentContentProvider, things like inmemory resources, or RHS diff editor etc? Why aren't they searched?

@roblourens
Copy link
Member

I think that everything else is handled by

if (!this.fileService.canHandleResource(resource)) {
	return;
}

@jrieken
Copy link
Member

jrieken commented Dec 4, 2019

Yeah - that should catch most of it but I also see special handling for untitled files and the search-result mode. It looks like the search service is missing a concept of "ignored schemes" or phrased different, while it only has a search for provider for the file-scheme it seems to text-model-search in almost all other files. So, question is if the text-model-search should only be attempted for schemes for which a provider is registered?

@roblourens
Copy link
Member

So, question is if the text-model-search should only be attempted for schemes for which a provider is registered?

You mean a search provider? Maybe. I don't know what all the different usecases of fs providers are and how many we want to search vs not search. I think we should probably search everything by default, exclude some things like this, and figure out what people complain about.

@JacksonKearl JacksonKearl added the verified Verification succeeded label Dec 5, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug git GIT issues search-replace Search and replace issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants