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

WIP: integration of Language Server Protocol #172

Closed
wants to merge 1 commit into from

Conversation

vgramer
Copy link
Member

@vgramer vgramer commented Apr 9, 2024

Description

PoC for integration Regal, which is actually pretty straight forward (just one class to implement)
Capture d’écran 2024-04-09 à 07 39 00

TODO:

  • make this integration only for paid IntelliJ-based IDEs ( i have to check, but i think it's possible)
  • do not hardcode path of regal, would be nice to have something like sdk version
  • add settings to customize regal configuration?
  • Is it possible to add some quick-fix action for the problems reported by Regal? (e.g. opa fmt)

Fixes #171

Special notes for your reviewer

LSP api in IDEA is still under dev and unstable.

Signed-off-by: Vincent Gramer <[email protected]>
@anderseknert
Copy link
Member

This is awesome Vincent! 👏 Can't wait to try it out. @charlieegan3 took a look at some options here, so he'll reach out for a sync. But like you said, this clearly looks like it's a very simple addition from an integration POV.

add settings to customize regal configuration?

Perhaps we could integrate that in the UI later, via the LSP or not. In the meantime, users can always set their Regal config file.

Is it possible to add some quick-fix action for the problems reported by Regal? (e.g. opa fmt)

Yes, this is also covered by the language server protocol in the form of code actions and commands. We'll have a few basic actions (like opa-fmt) included in the next release, with more to follow. We'll mostly do this for rules in the style category though — for other rules there is a point in having the user read the docs and learn something :)

@charlieegan3
Copy link
Contributor

Hi Vincent! This is amazing, thanks for your work on this.

make this integration only for paid IntelliJ-based IDEs

Interesting, is this something that needs to be done before we can list the plugin in the IntelliJ store? If we must do that, is it worth also investigating using another option such as https://github.com/ballerina-platform/lsp4intellij? I was having a look over https://github.com/zzehring/intellij-jsonnet earlier in the week and it seemed like it could be good inspiration.

Mixed feelings about this, as I feel like even if the pace of the first-class support for LSP in IntelliJ is unstable, it feels like it'll inevitably by the route to go in future. I am also unsure how many users of the community IntelliJ flavours there are who use this plugin.

do not hardcode path of regal, would be nice to have something like sdk version

Might we be able to determine it from looking up in the PATH? and just quietly not run it if it's missing? In the vscode ext, we have an option to install Regal, but that's not required here I think. Best would be to prompt users to install it once if it's missing.

add settings to customize regal configuration?

While there is some functionality in the vscode client and Regal LSP to handle updates to regal config live, this is something I think we'll need to revisit. See this recent issue: StyraInc/regal#626. For now, or a first version, perhaps let's send the contents of .regal/config.yaml to the server too and we can come back to it later.

Is it possible to add some quick-fix action for the problems reported by Regal? (e.g. opa fmt)

This is something we're working on in the vscode client at the moment - it's proving a little trickier than expected!

Great work! 👏

@anderseknert
Copy link
Member

Have you had any time to look into this further, @vgramer? We've added a lot of features to the language server in just the last few weeks, including the "fix" feature for linter rules that you asked about. Would love to see this in IntelliJ sometime soon!

@angelozerr
Copy link

You are using lsp support from jetbrains which is available only for ultimate.

If you want to support ij community please try https://github.com/redhat-developer/lsp4ij which provides a free lsp support for ij

@anderseknert
Copy link
Member

@angelozerr yeah, we're coming back to this later this month, and will evaluate the different options then.

@angelozerr
Copy link

Thanks @anderseknert to consider LSP4IJ

You can find documentation at https://github.com/redhat-developer/lsp4ij/blob/main/docs%2FDeveloperGuide.md

Dont hesitate to create issues if you need more help

@anderseknert
Copy link
Member

Thanks @angelozerr!

You've clearly spent some time on those docs, and that's always a good sign 👍

Since you're asking for feedback... :)

One thing that I would appreciate is a comparison to the other solutions available, like the Balerina lib for example. I'm sure you had good reasons to create your library despite those having been around for some time — but it's not apparent from anywhere I can find what those reasons were, or why you'd recommend one over the other. Also, a bullet list of supported LSP features would be helpful.

Also, the telemetry thing... I'm not even in the "telemetry bad" camp.. but it seems odd to me to have telemetry sent by a library. Even if you provide a way for users to opt out, they'll be opting out of something from Redhat, and that is most certainly something they had no idea about where it came from to begin with. Is there a way to disable this in the integration already? I like Redhat, and I don't mind returning something back for the favor of using a good library, but that's not a concern I would want to delegate to end-users. But what do I know, perhaps I've just misunderstood how that works :)

@angelozerr
Copy link

You've clearly spent some time on those docs, and that's always a good sign 👍
Since you're asking for feedback... :)

Thanks!

;One thing that I would appreciate is a comparison to the other solutions available, like the Balerina lib for example

Ye sure I will explain you why we did that later since it is little long to explain.

Also, a bullet list of supported LSP features would be helpful.

Please read
https://github.com/redhat-developer/lsp4ij/blob/main/docs%2FLSPSupport.md

Also, the telemetry thing... I'm not even in the "telemetry bad" camp.. but it seems odd to me to have telemetry sent by a library. Even if you provide a way for users to opt out, they'll be opting out of something from Redhat, and that is most certainly something they had no idea about where it came from to begin with. Is there a way to disable this in the integration already? I like Redhat, and I don't mind returning something back for the favor of using a good library, but that's not a concern I would want to delegate to end-users. But what do I know, perhaps I've just misunderstood how that works :)

@fbricon could you please give an answer

@fbricon
Copy link

fbricon commented Jun 9, 2024

@anderseknert TL;DR: if your plugin depends on LSP4IJ, it won't force Red Hat telemetry collection onto your users.

First of all, LSP4IJ is not simply a library, but a plugin, providing user-facing features, such as the invaluable LSP Consoles view.

As explained in the readme: "If the Red Hat Telemetry plugin is installed, the LSP4IJ plugin will collect anonymous usage data"
The Red Hat Telemetry plugin is an optional dependency, so it won't be installed by default when installing LSP4IJ. It's only a hard dependency for more Red Hat-centric plugins (Quarkus, OpenShift). So in case it's installed, then, provided one opted-in for that, we'll collect the aforementioned anonymous usage data.

@fbricon
Copy link

fbricon commented Jun 9, 2024

BTW, you can try LSP4IJ out by simply creating a new Language Server definition from the UI, no need to write code/ develop anything

@angelozerr
Copy link

BTW, you can try LSP4IJ out by simply creating a new Language Server definition from the UI, no need to write code/ develop anything

Indeed @anderseknert I suggest that you start to register your ls with user defined language server to avoid developping an IJ plugin and evaluate LSP4IJ quickly. You will need just to fill the start command and mappings and that's it !

@anderseknert
Copy link
Member

Sounds great! Thanks to you both 👍 Will definitely give it a go when we come back to this.

@angelozerr
Copy link

angelozerr commented Jun 21, 2024

@anderseknert please note that we have released LSP4IJ 0.0.2 which provides syntax coloration inside Markdown, diagnostic tags support, etc see https://github.com/redhat-developer/lsp4ij/milestone/2?closed=1

We have published too the article Meet LSP4IJ, a new LSP Client for JetBrains-based IDEs and the section Why LSP4IJ? explains why we have created LSP4IJ.

Hope you will like it and hope you will be interested to try LSP4IJ.

@anderseknert
Copy link
Member

Very nice @angelozerr! Yeah, definitely leaning towards using LSP4IJ. We have a few other projects to finish first, but I'll let you know when we get there.

@angelozerr
Copy link

@anderseknert just for toir information lsp4ij start to be more and more stable and there are more and more plugins which are baszd on, see https://github.com/redhat-developer/lsp4ij?tab=readme-ov-file#who-is-using-lsp4ij

@anderseknert
Copy link
Member

@angelozerr 👍 we're planning to get to this November/December :)

@angelozerr
Copy link

Great! Dont hesitate to ping me if you have any questions about LSP4IJ.

@anderseknert
Copy link
Member

Will do! Closing this in the meantime as we're unlikely to want to use the IntelliJ solution for this.

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.

Regal language server integration (linting and language features)
5 participants