-
Notifications
You must be signed in to change notification settings - Fork 2
Ignore non python files #66
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
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! One comment to check on.
packages/pyright-internal/src/common/chokidarFileWatcherProvider.ts
Outdated
Show resolved
Hide resolved
watcherOptions.usePolling = false; | ||
} | ||
|
||
const excludes: string[] = ['**/node_modules/**', '**/__pycache__/**', '**/.*', '**/.*/**']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just use a negative match pattern? '!**.py'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the whole relative or absolute path is tested, not just filename so when the path is a directory and the path name does not end with .py, it will be ignored
|
||
// Check if any part of the path starts with a dot (hidden file/directory). | ||
// Exclude '.' and '..' to avoid matching the current/parent directory indicators. | ||
if (normalizedPath.split('/').some((part) => part.startsWith('.') && part !== '.' && part !== '..')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is already the normalized path, there are no . and .. segments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks fine to me. I did not test it.
Would you mind take a look at this one? #67 It should make the CI run again |
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Why
Previously, the LSP watches all the files within the given paths. We could get
ENOSPC: System limit for number of file watchers reached
error in a large codebase with multiple different programming language files.What changed
ignore
option of Chokidar is using Anymatch under the hood, which allows us to pass in aAnymatchFn
handler function for customizing our own file matching rules.Test plan
https://replit.com/t/replit/repls/ShouldNotWatchNonPython?replId=dfb12081-2bb1-49b6-8080-5961e0980de9#.replit
In this python repl, we have both the original version pyright-extended running and a version of this PR. And there are 2000 non python files created in the repl. We can see that the original pyright-extended LSP watches 2000 extra files compared to the other one.
