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

Hide explorer entries based on git ignore file #149967

Merged
merged 7 commits into from May 20, 2022
Merged

Conversation

lramos15
Copy link
Member

This PR fixes #38878

Parses the .gitignore file and hides the entries in the explorer based on the .gitignore file. Works with nested .gitignore files as well

@lramos15 lramos15 self-assigned this May 19, 2022
@lramos15 lramos15 added this to the May 2022 milestone May 19, 2022

for (const folder of this.contextService.getWorkspace().folders) {
const folderQuery = { folder: folder.uri };
let searchResults = (await this.searchService.fileSearch({ type: QueryType.File, folderQueries: [folderQuery], includePattern: { '**/.gitignore': true }, })).results;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing this a bit via rg CLI I'm reaffirmed in not liking the search approach.

In a repo like VS Code it burns 3.5 seconds of kernel time and takes 5 seconds to return at all:

rg --files **/.gitignore  0.28s user 3.45s system 75% cpu 4.966 total

That's quite a lot of CPU for something running on startup, and it has potential to make the OS kernel unresponsive to other requests. It's much worse when I try to open my Developer folder:

rg --files **/.gitignore  3.81s user 54.43s system 62% cpu 1:33.84 total

A full minute of time spent in the kernel, and ignored files won't hide until at least a minute and a half after you open the application. Opening my root burned almost 2 minutes of system time and took 3 minutes to return, a non-starter IMO.

Also of note: when the .gitignore file has been ignored itself (weird, but possible), rg won't find it at all, and we wont be able to read any of its contents.

I propose we exploit the fact that we will never need to use .gitignore files that the explorer hasn't already seen in some way and let the explorer service tell us where the .gitignore files it knows about are.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the insightful deep dive.

Two possibilities

  • Process gitIgnore files as they come in via the filter call when we receive a stat call of an explorer item.
  • Enrich the explorerService to have knowledge of visited gitignore files

Let me know your thoughts on a solution

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps give the each root ExplorerItem a seenIgnoreFiles array of URI's and an OnDidChangeSeenIgnoreFiles event which you maintain in the addChild and removeChild methods? Then in the FilesFilter you'd pretty much have the exact logic you have now, but with updateWorkspaceIgnoreFiles pulling the list of ignore files from the roots' seenIgnoreFiles arrays rather than running a file search.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Process gitIgnore files as they come in via the filter call when we receive a stat call of an explorer item.

This sounds like a bit of a hack on one hand but pretty reasonable on the other. It'd be nice to keep knowledege of gitignore files localized to the file filter and this would do that. I think it's worth pursuing.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented. Not as hacky feeling as I thought, actually appears quite clean. Largest downside is the following scenario
Folder with

  • test.js
  • foo.js
  • .gitignore

If the .gitignore includes *.js you will see the .js files flash when the node is expanded and then hide as the .gitignore is processed. A possible idea here is to cache .gitignore across reloads

@JacksonKearl
Copy link
Contributor

As discussed, this should be behind a (default false) setting.

@JacksonKearl
Copy link
Contributor

We should also probably never exclude .gitignore files themselves based on this, as Kai brought up.

@JacksonKearl
Copy link
Contributor

IgnoreFile util might not work on Windows, it was made for web where the paths are always reasonable. Checking...

@lramos15 lramos15 enabled auto-merge (squash) May 20, 2022 19:33
@JacksonKearl
Copy link
Contributor

Haven't been able to get a Windows build up to test on the VM. We can merge now and test in the product build.

@lramos15 lramos15 merged commit da180bc into main May 20, 2022
@lramos15 lramos15 deleted the lramos15/anxious-lynx branch May 20, 2022 20:24
@ariofrio
Copy link

This seems to be broken for some gitignore patterns: https://github.com/ariofrio/vscode-explorer-exclude-gitignore/blob/main/.gitignore

@github-actions github-actions bot locked and limited conversation to collaborators Jul 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use .gitignore to hide files in explorer
3 participants