-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
fix(lsp): add a document preload file system entry limit #18553
fix(lsp): add a document preload file system entry limit #18553
Conversation
return false; | ||
} | ||
|
||
// ignore cargo target directories for anyone using Deno with Rust |
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.
I think it's worth it to look for and handle these kind of scenarios.
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.
LGTM, thanks for fixing this
lsp_warn!( | ||
concat!( | ||
"Hit the language server document preload limit of {} file system entries. ", | ||
"You may want to use the \"deno.enablePaths\" configuration setting to only have Deno ", | ||
"partially enable a workspace." | ||
), | ||
self.limit, | ||
); |
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.
Maybe in vscode_deno
we could show a pop up if this message is printed?
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.
Let's iterate on this in the future. I'd have to refactor the code to get the lsp client here and perhaps some users might not like this message.
I was testing this out and it's badly needed. For now, it's not configurable and limited to 1,000 file system entries. Related to #18538
I was testing this out and it's badly needed. For now, it's not configurable and limited to 1,000 file system entries.
Related to #18538