Conversation
Considering this is the official LSP implementation it will supersede `kotlin-language-server` in the long run. Release notes: https://github.com/Kotlin/kotlin-lsp/releases/tag/kotlin-lsp%2Fv0.253.10629
f651ecc to
873c9d4
Compare
873c9d4 to
f53a7bd
Compare
a1ecc4f to
604ab55
Compare
15e3819 to
80ef8b1
Compare
|
I've tried running it locally and had to prevent kotlin-lsp.sh from trying to chmod java binary with: |
6f76832 to
73ebcbf
Compare
|
The commits should be squashed to just:
|
a708f32 to
a730873
Compare
|
I've squashed the commits down to
I felt it a bad idea to squash the update commit down as well |
|
I don’t think a separate update commit is needed, since you are introducing a new package anyway. |
kotlin-lsp: removed unneccessary ls from installPhase The `ls -la` in question was only used in building this package and is no longer required. kotlin-lsp: correct license Originally had a als20 instead of asl20 kotlin-lsp: removed unused buildInputs; add pre-/postInstall kotlin-lsp: remove maven from binPath kotlin-lsp: update to v261.13587.0 kotlin-lsp: add review suggestions
a730873 to
8713d58
Compare
axelkar
left a comment
There was a problem hiding this comment.
See also my (parallel and more polished) PR here: #482845. Use it as inspiration as you wish.
Some notes from my implementation:
selectSystemhelper for better error messages- Reduced auto-patchelf dependencies by removing unused libraries in included JRE
- Test that
kotlin-lsp --helpworks (JVM classpath and libraries loaded at init work)
There was a problem hiding this comment.
This should fail on a mismatch, see #260776
| --replace-fail 'chmod +x "$LOCAL_JRE_PATH/bin/java"' '# chmod removed for NixOS' |
There was a problem hiding this comment.
Currently unfree, see https://github.com/Kotlin/kotlin-lsp/#source-code
There was a problem hiding this comment.
this path is different on darwin systems
chmod +x $out/lib/kotlin-lsp/jre/Contents/Home/bin/java
I'd suggest declaring similar to my PR
https://github.com/NixOS/nixpkgs/pull/492163/changes#diff-0317ed1f782055d57bad91c5c56282ba1e3f8e324f86a2fab5cb02882fc82dd3R29-R39
I've tested it on darwin with java_bin_path fix and it works fine
There was a problem hiding this comment.
Suggested find solution also LGTM #435169 (comment)
There was a problem hiding this comment.
Hello! I've opened an alternative PR #514623 that improves the package in a number of ways:
- fixing the license
- avoiding depending on X11/Wayland packages (the lsp is headless, it has no need for graphical stuff!)
- does not use the deprecated
kotlin-lsp.shlauncher, but properly wraps theintellij-serveras instructed in upstream project - adding support for
.sitarchives used for MacOS - adding version check to ensure the binary works in
installCheckphase
There was a problem hiding this comment.
Note that using the kotlin-lsp.sh script is deprecated!
https://github.com/Kotlin/kotlin-lsp/blob/7ab9944cd2ff8b1758faaad8082ab17dd1ba4507/scripts/kotlin-lsp.sh#L11
|
Closing in favor of #514623 |
This PR adds the kotlin-lsp package in the latest available version at the time of writing this.
Considering this is the official LSP implementation it will supersede
kotlin-language-serverin the long run.Release notes:
https://github.com/Kotlin/kotlin-lsp/releases/tag/kotlin-lsp%2Fv0.253.10629
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.