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

Files watcher exclude - do not ignore changes to the node_modules folder itself. #125801

Merged
merged 1 commit into from
Jun 16, 2021

Conversation

Aaaaash
Copy link
Contributor

@Aaaaash Aaaaash commented Jun 9, 2021

This PR fixes #125886

Glob online tester

node_modules-watcger

@Aaaaash
Copy link
Contributor Author

Aaaaash commented Jun 9, 2021

same issue

@Aaaaash
Copy link
Contributor Author

Aaaaash commented Jun 9, 2021

Friendly ping @bpasero

@bpasero
Copy link
Member

bpasero commented Jun 9, 2021

What issue does this address?

@bpasero bpasero added the info-needed Issue requires more information from poster label Jun 9, 2021
@Aaaaash
Copy link
Contributor Author

Aaaaash commented Jun 10, 2021

updated

@bpasero bpasero removed the info-needed Issue requires more information from poster label Jun 10, 2021
@bpasero bpasero added this to the Backlog milestone Jun 10, 2021
@@ -239,7 +239,7 @@ configurationRegistry.registerConfiguration({
},
'files.watcherExclude': {
'type': 'object',
'default': isWindows /* https://github.com/microsoft/vscode/issues/23954 */ ? { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/*/**': true, '**/.hg/store/**': true } : { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/**': true, '**/.hg/store/**': true },
'default': isWindows /* https://github.com/microsoft/vscode/issues/23954 */ ? { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/*/**': true, '**/.hg/store/**': true } : { '**/.git/objects/**': true, '**/.git/subtree-cache/**': true, '**/node_modules/**/*': true, '**/.hg/store/**': true },
Copy link
Member

Choose a reason for hiding this comment

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

@Aaaaash any reason we wouldn't just pick the exact same pattern as on Windows? From my testing, the advantage of '**/node_modules/*/**' over '**/node_modules/**/*' seems to be that you would see changes to top level folders in the node_modules folder, but nothing deeper. So essentially the explorer refreshes nicely on operations such as yarn add <module>.

With your suggested pattern, I only see node_modules folder appearing and hiding, but not changes inside the folder.

Copy link
Member

Choose a reason for hiding this comment

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

Ping @Aaaaash

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the late reply, you are right, so we can use the same pattern on Windows and Linux/macOS ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think so, can you update the PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@Aaaaash Aaaaash force-pushed the fix/file-excludes-default-value branch from d8e7cc0 to 814c156 Compare June 16, 2021 05:57
@bpasero bpasero modified the milestones: Backlog, June 2021 Jun 16, 2021
@bpasero
Copy link
Member

bpasero commented Jun 16, 2021

Thanks 👍

@bpasero bpasero merged commit 237a750 into microsoft:main Jun 16, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2021
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.

macOS: Changes to node_modules folder are ignored
2 participants