-
Notifications
You must be signed in to change notification settings - Fork 228
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
feat: Add LSP stub #5197
feat: Add LSP stub #5197
Conversation
looks cool, would be great to build this out in time! to reduce dependencies in there meantime, could we add an optional |
Okay, I made the dependency optional. Yes it would be cool build this out in time. The next steps we need to do is to create |
Looks good! May need some refining on the tests outputs but good to go otherwise |
I think no more tests are needed for now because we shouldn't test the The test fails on some test-deps-min-version (verify Rust minimum version) thing. Can you merge this? |
OK, but we can't have It seems to additionally be failing because the completion snapshot files need updating: https://github.com/PRQL/prql/actions/runs/14046209187/job/39327515449?pr=5197 |
Okay, I updated shell completion snapshots. |
prqlc/prqlc/Cargo.toml
Outdated
@@ -22,12 +22,14 @@ cli = [ | |||
"color-eyre", | |||
"colorchoice-clap", | |||
"is-terminal", | |||
"lsp", |
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.
but lsp
shouldn't be in cli by default, then we're including features that introduce dependencies without any functionality
does that make sense? I think you can recognize this!
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.
(it's possible that fixing this undoes the changes to the tests, apologies if so — they should all reset with the standard test commands at least)
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.
Yeah, that makes sense.
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!
Adds a stub implementations of a basic Language Server Protocol (LSP) to
prqlc
. LSP clients can connect to it and disconnect from it.It use the
lsp-server
crate used by the official rust-analyzer.The LSP specification is at https://microsoft.github.io/language-server-protocol/specifications/specification-current