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

Add support for LSP4IJ #42

Merged
merged 10 commits into from
Jul 15, 2024
Merged

Add support for LSP4IJ #42

merged 10 commits into from
Jul 15, 2024

Conversation

InSyncWithFoo
Copy link
Owner

Fixes #38.

@InSyncWithFoo InSyncWithFoo added the help wanted Extra attention is needed label Jun 5, 2024
Copy link

github-actions bot commented Jun 5, 2024

Qodana Community for JVM

It seems all right 👌

No new problems were found according to the checks applied

💡 Qodana analysis was run in the pull request mode: only the changed files were checked

View the detailed Qodana report

To be able to view the detailed Qodana report, you can either:

To get *.log files or any other Qodana artifacts, run the action with upload-result option set to true,
so that the action will upload the files as the job artifacts:

      - name: 'Qodana Scan'
        uses: JetBrains/qodana-action@v2024.1.5
        with:
          upload-result: true
Contact Qodana team

Contact us at qodana-support@jetbrains.com

@InSyncWithFoo
Copy link
Owner Author

A few problems to be resolved:

  • The plugin needs to be able to:
    • Signal "do not start server" without throwing errors
    • Start and stop existing server(s) at will when the running mode is changed.
  • Configurations must be sent via workspace/didChangeConfiguration (see sister #1)

@InSyncWithFoo
Copy link
Owner Author

The verifier seems to be failing for whatever reason. That, too, needs to be taken care of.

@angelozerr
Copy link

Signal "do not start server" without throwing errors

I don't understand very well what you mean. You want disable language server?

Start and stop existing server(s) at will when the running mode is changed.

If I understand correctly you want to start / stop language server with a Java API? If it that I have a PR redhat-developer/lsp4ij#318 for that (you can read documentation from the PR)

Configurations must be sent via workspace/didChangeConfiguration

We did that in Intellij Quarkus, see https://github.com/redhat-developer/intellij-quarkus/blob/f69407114100adb185a4fac8360e9ad3f8460706/src/main/java/com/redhat/devtools/intellij/qute/lsp/QuteLanguageClient.java#L104

@InSyncWithFoo
Copy link
Owner Author

InSyncWithFoo commented Jun 6, 2024

You want disable language server?

That's correct. I would like to conditionally prevent it from being initialized.

If it that I have a PR redhat-developer/lsp4ij#318 for that (you can read documentation from the PR)

That PR seems to be what I'm looking for. Guess I'll have to wait until it is merged.

call triggerChangeConfiguration

Where am I supposed to call it? I need it to be sent just once right after initialization. QuteLanguageClient overrides profileChanged() from ProfileChangeAdapter, but what does that even do?

@angelozerr
Copy link

That's correct. I would like to conditionally prevent it from being initialized.

Ok, the PR redhat-developer/lsp4ij#318 manages that too. @fbricon we need to merge this PR ASAP because now we have 2 adaptors who need this support.

That PR seems to be what I'm looking for. Guess I'll have to wait until it is merged.

Great! Indeed we need to merge this PR and we plan to create a release as soon as this PR will be merged. See https://github.com/redhat-developer/lsp4ij/milestone/2

QuteLanguageClient overrides profileChanged() from ProfileChangeAdapter, but what does that even do?

The usecase with Qute is to track profile changed and send a didChangeConfiguration. In your case you want to call it when language server is started. Let me study how to do that.

@angelozerr
Copy link

Where am I supposed to call it?

See redhat-developer/lsp4ij#318 (comment)

@InSyncWithFoo
Copy link
Owner Author

This should be mergeable once #45 is merged.

@angelozerr
Copy link

@InSyncWithFoo once you will decide to adapt LSP4IJ could you please:

Thanks!

@jonashaag
Copy link

I've been trying this branch for a couple of days and it seems to be working great!

Copy from JetBrains/intellij-platform-plugin-template#438

Add missing property

Remove redundant block

Add `org.jetbrains.intellij.platform.buildFeature.useBinaryReleases = false`

Move `PythonCore` to `platformBundledPlugins`

Sync comment

Web link instead of app link

Unnecessary comment

Bump to v2.0.0-beta7

Re-add accidentally removed `systemProperty()` call

Manually specify the IDEs to verify against

Revert "Manually specify the IDEs to verify against"

This reverts commit ad49196.

Bump `org.jetbrains.kotlinx.kover` back to 0.8.1

Remove extraneous comment
@InSyncWithFoo InSyncWithFoo marked this pull request as ready for review July 13, 2024 19:42
@InSyncWithFoo InSyncWithFoo merged commit b67210b into master Jul 15, 2024
9 checks passed
@InSyncWithFoo InSyncWithFoo deleted the issue-38 branch July 15, 2024 00:48
github-actions bot pushed a commit that referenced this pull request Jul 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

What about using LSP4IJ?
3 participants