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

fix(lsp): add a document preload file system entry limit #18553

Merged
merged 6 commits into from
Apr 1, 2023

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Apr 1, 2023

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

return false;
}

// ignore cargo target directories for anyone using Deno with Rust
Copy link
Member Author

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.

Copy link
Member

@bartlomieju bartlomieju left a 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

Comment on lines +1679 to +1686
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,
);
Copy link
Member

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?

Copy link
Member Author

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.

@dsherret dsherret merged commit 0210c1c into denoland:main Apr 1, 2023
@dsherret dsherret deleted the fix_lsp_document_preload_limit branch April 1, 2023 19:10
dsherret added a commit that referenced this pull request Apr 1, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants