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

Switch to adapter pattern #453

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Switch to adapter pattern #453

merged 2 commits into from
Apr 26, 2024

Conversation

bioball
Copy link
Contributor

@bioball bioball commented Apr 25, 2024

This switches the LSP implementation to use the adapter pattern, deferring as much as possible to the ANTLR-generated classes.

It also gets rid of CstBuilder, instead moving diagnostic handling to the analyzers.

It's not complete but provides a skeleton to be filled in.

@translatenix
Copy link
Contributor

translatenix commented Apr 25, 2024

Have you considered having a separate repository for the language server, similar to apple/pkl-Intellij? A language server is a large project, quite different from the rest of apple/pkl, should be releasable on its own, and apple/pkl already has some modularity issues. I’m also worried that this will slow down the build and especially the IntelliJ experience, which is already barely tolerable on my (not so old) laptop.

@bioball
Copy link
Contributor Author

bioball commented Apr 25, 2024

We've thought about it. We're still a ways off from releasing the LSP. And, putting it here is useful for us to share things with the rest of Pkl. We'll see.

I’m also worried that this will slow down the build and especially the IntelliJ experience, which is already barely tolerable on my (not so old) laptop

Is this because of WSL? Might be better once we have Windows support available for you. I haven't noticed any perf issues on macOS yet.

@translatenix
Copy link
Contributor

Is this because of WSL?

WSL is definitely part of the problem. But Gradle IntelliJ projects are very resource intensive in general, especially when using Gradle Kotlin DSL.

@stackoverflow stackoverflow merged commit b3b2ac1 into apple:lsp-setup Apr 26, 2024
2 of 5 checks passed
stackoverflow pushed a commit that referenced this pull request Apr 26, 2024
@bioball bioball deleted the lsp-setup branch April 26, 2024 15:33
stackoverflow pushed a commit that referenced this pull request May 16, 2024
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

3 participants