Skip to content

feat: add kotlin language support #80

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

Merged
merged 19 commits into from
Apr 3, 2025

Conversation

IDontHaveBrain
Copy link
Contributor

Add Kotlin Language Server Support

Document symbols
Go to definition
Find references
Hover information
Code completions

Tested on Windows-x64 platforms with the kotlin-language-server from the fwcd/kotlin-language-server repository.

@LakshyAAAgrawal
Copy link
Collaborator

Hi @IDontHaveBrain

Thank you so much for the PRs you have kindly created.

At the moment the unit tests are failing. This is due to some external issue in the unit tests, which I have fixed in the latest commit to main branch.
Could you please merge/rebase on top of main branch to get the unit tests in the CI working again, after which I will review and merge the PR.

Could you please do this for all your branches?

@IDontHaveBrain
Copy link
Contributor Author

IDontHaveBrain commented Mar 29, 2025

Thanks for the review.

  1. Updated to fetch the Java environment directly in setup_runtime_dependencies method.
  2. Refactored the error handling in the server shutdown sequence. Do you have any additional suggestions for improving this part?

Please review when you have time.

@LakshyAAAgrawal
Copy link
Collaborator

Hi @IDontHaveBrain, Thanks a lot for updating the PR. The changes looks great, can you just address the one comment I added above?

Copy link
Collaborator

@LakshyAAAgrawal LakshyAAAgrawal left a comment

Choose a reason for hiding this comment

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

A few nits.

BTW, the PR is amazing, and I believe it solves some long-standing issues with multilspy. Thank you. Will merge after this final round of discussion.

},
"jdk": {
"win-x64": {
"url": "https://github.com/adoptium/temurin21-binaries/releases/download/jdk-21.0.6%2B7/OpenJDK21U-jdk_x64_windows_hotspot_21.0.6_7.zip",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you say if any of these can be replaced by the repos and urls at

"url": "https://github.com/redhat-developer/vscode-java/releases/download/v1.23.0/[email protected]",

@LakshyAAAgrawal
Copy link
Collaborator

Also, add an entry to the README about Kotlin support in the table.

"java_path": "extension/jre/17.0.8.1-win32-x86_64/bin/java.exe"
},
"win-arm64": {
"url": "https://github.com/adoptium/temurin21-binaries/releases/download/jdk-21.0.6%2B7/OpenJDK21U-jdk_aarch64_windows_hotspot_21.0.6_7.zip",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this not available through redhat?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this not available through redhat?

It seems that Red Hat OpenJDK does not support Windows ARM64 binaries.
At least, I couldn't find any in my search.

Copy link
Collaborator

Choose a reason for hiding this comment

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

https://blogs.windows.com/windows-insider/2020/12/10/introducing-x64-emulation-in-preview-for-windows-10-on-arm-pcs-to-the-windows-insider-program/

win32-x64 binaries can run on win-arm64 I believe. Could you just refer to that binary for the win-arm64 platform as well? Is it possible for you to test it? I don't have a win-arm machine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I understand that GitHub Actions supports Windows ARM64 environment.
will test it later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It's upto you: We can either test it before merging, or we can temporarily disable the win-arm64, merge it, track an issue to enable win-arm64 testing and add it in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the win-arm64 section from the runtime_dependencies.json file.
It would be good to revisit this when we add cross-platform testing to our GitHub Actions workflows in the future.

Thank you for your kind review and assistance throughout this
PR process!

@LakshyAAAgrawal LakshyAAAgrawal merged commit b7e1b6f into microsoft:main Apr 3, 2025
4 checks passed
@LakshyAAAgrawal
Copy link
Collaborator

Thank you very much @IDontHaveBrain, this is a great PR!

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.

2 participants