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

feat(kotlin_language_server): Add storagePath to initilaztion options #2930

Merged
merged 3 commits into from
Dec 17, 2023

Conversation

Glyphack
Copy link
Contributor

@Glyphack Glyphack commented Dec 16, 2023

  • Add default value for storagePath to enable caching. This feature is important for Kotlin project because the LSP start takes too long because of indexing on each start. I'm not sure if this option fits the boundary or not. For me it's a sane default
  • Update docs

feature was added in fwcd/kotlin-language-server#337 to help with caching and speeding up the language server.

I did not find the feature documented in the LSP repo to link but it has been used in:

@Glyphack Glyphack force-pushed the kotlin-lsp-storage-path branch 5 times, most recently from 2c75cc6 to 390eeb0 Compare December 16, 2023 13:16
@@ -1,9 +1,8 @@
local util = require 'lspconfig.util'


Copy link
Contributor Author

Choose a reason for hiding this comment

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

The lint step was failing in this file, so I reformatted the code. Not related to my original change.

cmd = { 'kotlin-language-server' },
capabilities = [[
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This section was not rendered in docs.
image

@Glyphack Glyphack marked this pull request as ready for review December 16, 2023 13:23
@Glyphack Glyphack requested a review from glepnir as a code owner December 16, 2023 13:23
@Glyphack Glyphack force-pushed the kotlin-lsp-storage-path branch from 390eeb0 to b88fb45 Compare December 16, 2023 13:26
@@ -24,6 +24,9 @@ return {
filetypes = { 'kotlin' },
root_dir = util.root_pattern(unpack(root_files)),
cmd = { bin_name },
init_options = {
storagePath = vim.loop.os_homedir(),
Copy link
Member

Choose a reason for hiding this comment

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

why server will use home dir storage data ?

Copy link
Contributor Author

@Glyphack Glyphack Dec 16, 2023

Choose a reason for hiding this comment

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

I think using the project workspace is the right place for it but I could figure out a way to get the project root path here. The root_pattern returns a function. Do you know can I set the project root here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the context the cache file is a single sqlite DB file.

Copy link
Member

Choose a reason for hiding this comment

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

root_pattern return a function then pass current buffer name to it and invoke it again.

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 did it using vim.fn.expand('%:p:h').
First I tried to use vim.api.nvim_buf_get_name(0). But root_pattern expects a directory. So I used the fn.expand because it's the short & clear to read. I could not find a one liner in lua APIs to get the current directory.

@Glyphack Glyphack marked this pull request as draft December 16, 2023 18:00
@Glyphack Glyphack force-pushed the kotlin-lsp-storage-path branch 2 times, most recently from b4e1ba5 to ce5fa59 Compare December 16, 2023 18:35
@Glyphack Glyphack marked this pull request as ready for review December 16, 2023 18:36
@Glyphack Glyphack force-pushed the kotlin-lsp-storage-path branch from a04d618 to aba170e Compare December 17, 2023 08:57
@@ -24,6 +24,9 @@ return {
filetypes = { 'kotlin' },
root_dir = util.root_pattern(unpack(root_files)),
cmd = { bin_name },
init_options = {
storagePath = util.root_pattern(unpack(root_files))(vim.fn.expand('%:p:h')),
Copy link
Member

Choose a reason for hiding this comment

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

simply api.nvim_buf_get_name(0)

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 tried to use vim.api.nvim_buf_get_name(0). But root_pattern expects a directory.

When I used util.root_pattern(unpack(root_files))(vim.api.nvim_buf_get_name(0)) it was not resolving to the correct path somehow. I thought the root pattern function wants a directory(it was used like this in tests) so I changed it to pass the current directory.

@Glyphack Glyphack force-pushed the kotlin-lsp-storage-path branch from aba170e to a8fc471 Compare December 17, 2023 08:59
@Glyphack Glyphack requested a review from glepnir December 17, 2023 09:01
@Glyphack Glyphack force-pushed the kotlin-lsp-storage-path branch from a8fc471 to 6e5f020 Compare December 17, 2023 09:04
@glepnir glepnir merged commit 5b6fc6d into neovim:master Dec 17, 2023
9 checks passed
@Glyphack Glyphack deleted the kotlin-lsp-storage-path branch December 17, 2023 09:27
@Glyphack
Copy link
Contributor Author

Hi @glepnir I'm new to neovim APIs I was trying to figure out how to replace the (vim.fn.expand '%:p:h') with vim.api.nvim_buf_get_name(0) in the call to root_pattern. But I was not successful. Could you help me understand how can I do it?

@glepnir
Copy link
Member

glepnir commented Dec 17, 2023

init run before start lsp not in fast loop so it's fine use vim.fn, i will check it later :)

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