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

feat(cli): '--lsp' flag to start a basic LSP server #710

Closed
wants to merge 27 commits into from
Closed

feat(cli): '--lsp' flag to start a basic LSP server #710

wants to merge 27 commits into from

Conversation

tekumara
Copy link
Contributor

@tekumara tekumara commented Apr 9, 2023

Adds the --lsp flag which starts a basic LSP server running on stdin/out.

The initial capabilities of the server are minimal. It only provides diagnostics to identify typos.

Future PRs will include code actions (ie: quick fixes) and support for config files.

Resolves #263

@tekumara
Copy link
Contributor Author

tekumara commented Apr 9, 2023

This is my first real contribution in Rust, so I'm open to suggestions on best practice here.

@tekumara tekumara mentioned this pull request Apr 9, 2023
@@ -0,0 +1,2 @@


Copy link
Collaborator

Choose a reason for hiding this comment

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

Whats this file about?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, this was lingering around from a refactor and can be removed.

Comment on lines +93 to +95
/// Start the LSP server.
#[arg(long, group = "mode")]
pub(crate) lsp: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

In seeing this come in and thinking through this more, I'm not sure I want to take this on.

If nothing else, this is taking on a new technology stack that I'm not very familiar with (I don't even use LSPs myself) and I do not have the available focus to come up to speed on it at this time. This means I don't have the background to review this, to make sure this approach aligns well with the relevant technologies, and then to maintain this going forward. For this to be "community maintained", it would need to be distinct enough to make clear that this has a different compatibility and support level. I've been doing some of that in clap and my experience with it hasn't been great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No problem, and thanks for taking a look at this. At this initial point, the support offered would realistically just be me. Given that, would a typos-lsp crate within this repo provide enough of a distinction, or better to leave this out of this repo altogether? If the later, I'll probably end up moving this into tekumara/typos-vscode and may suggest small successive PRs as needed to make the integration work 🙏 I'll happily work with either option.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you for being understanding!

Let's start outside the repo so I can better observe how all of this works before integrating it into my processes.

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.

LSP Support?
2 participants