echasnovski / mini.nvim

Library of 40+ independent Lua modules improving overall Neovim (version 0.8 and higher) experience with minimal effort
MIT License
4.74k stars 179 forks source link

[mini.completion] Does not work with the Zk LSP server #826

Closed pkazmier closed 4 months ago

pkazmier commented 4 months ago

Contributing guidelines

Module(s)

mini.completion

Description

zk (command line tool) and zk-nvim (vim plug-in) provide a rich environment for note taking with neovim. The zk command is also an LSP server that supports completion items. This allows a user to type [[ to trigger a completion to select a note and insert a markdown link to it [title of note](path_to_note). This functionality works with nvim-cmp, but does not work with mini.completion. When the user types [[ nothing happens.

Neovim version

NVIM v0.9.5, Build type: Release, LuaJIT 2.1.1703358377

Steps to reproduce

Install zk and create a note that we'll use to trigger completion later:

  1. Download the zk binary for your system.
  2. zk init --no-input /tmp
  3. sed -i.bak 's/^link-format/#link-format/' /tmp/.zk/config.toml
  4. echo "A test note" | ZK_NOTEBOOK_DIR=/tmp zk new -i -t "Test" --print-path
Using this `minimal.lua` configuration, execute the following

```lua -- Clone 'mini.nvim' manually in a way that it gets managed by 'mini.deps' local path_package = vim.fn.stdpath("data") .. "/site/" local mini_path = path_package .. "pack/deps/start/mini.nvim" if not vim.loop.fs_stat(mini_path) then vim.cmd('echo "Installing `mini.nvim`" | redraw') local clone_cmd = { "git", "clone", "--filter=blob:none", "https://github.com/echasnovski/mini.nvim", mini_path, } vim.fn.system(clone_cmd) vim.cmd("packadd mini.nvim | helptags ALL") vim.cmd('echo "Installed `mini.nvim`" | redraw') end vim.o.showmode = false vim.o.termguicolors = true require('mini.hues').setup({ background = '#011A33', foreground = '#c0c8cb', accent = 'azure' }) require("mini.deps").setup({ path = { package = path_package } }) require("mini.pick").setup() require("mini.completion").setup( { lsp_completion = { process_items = function(items, base) local res = vim.tbl_filter(function(item) print(string.format("DEBUG: item=%s", vim.inspect(item))) local text = item.filterText or H.get_completion_word(item) print(string.format("DEBUG: text='%s' base='%s'", text, base)) return vim.startswith(text, base) -- and item.kind ~= 15 (zk notes are 18) end, items) table.sort(res, function(a, b) return (a.sortText or a.label) < (b.sortText or b.label) end) return res end, }, }) MiniDeps.add("neovim/nvim-lspconfig") require("lspconfig") MiniDeps.add("zk-org/zk-nvim") require("zk").setup({ picker = "minipick" }) ```

  1. ZK_NOTEBOOK_DIR=/tmp NVIM_APPNAME=minimal nvim -nu ./minimal.lua
  2. Select a note using :ZkNotes (notice the nice mini.pick integration 😄 )
  3. Confirm the zk LSP is running with :LspInfo
  4. Open a new line o, type [[ to trigger completion (it won't do anything), press ESC
  5. Type :messages to see the items from the LSP server (debug output from my process_items)

Expected behavior

I expect the completion menu to open with the list of notes like it does with nvim-cmp.

Actual behavior

Nothing happens because the item.filterText does not start with the base text of [[.

If I modify the item.filterText in my process_items function to prepend [[, then I can get the completion menu to appear. I can then use C-n/C-p to select an item to insert the proper markdown link.

          local res = vim.tbl_filter(function(item)
            -- Keep items which match the base and are not snippets
            item.filterText = "[[" .. item.filterText
            print(string.format("DEBUG: item=%s", vim.inspect(item)))
            local text = item.filterText or H.get_completion_word(item)
            print(string.format("DEBUG: text='%s' base='%s'", text, base))
            return vim.startswith(text, base) -- and item.kind ~= 15
          end, items)
          ...

But even with the completion menu open, I'm still not able to start typing letters to narrow down my selection.

pkazmier commented 4 months ago

I meant to include the debug output that shows what the Zk LSP server returns in case it saves you from having to reproduce using all the steps above:

DEBUG: item={
  data = "/Users/kaz/notes/meetings/agmo.md",
  filterText = "2024-04-15: Pete Kazmier 1:1 meetings/agmo.md",
  kind = 18,
  label = "2024-04-15: Pete Kazmier 1:1",
  textEdit = {
    newText = "[2024-04-15: Pete Kazmier 1:1](agmo)",
    range = {
      ["end"] = {
        character = 2,
        line = 20
      },
      start = {
        character = 0,
        line = 20
      }
    }
  }
}
DEBUG: text='2024-04-15: Pete Kazmier 1:1 meetings/agmo.md' base='[['
echasnovski commented 4 months ago

Thanks for the issue!

I meant to include the debug output that shows what the Zk LSP server returns in case it saves you from having to reproduce using all the steps above:

DEBUG: item={
  data = "/Users/kaz/notes/meetings/agmo.md",
  filterText = "2024-04-15: Pete Kazmier 1:1 meetings/agmo.md",
  kind = 18,
  label = "2024-04-15: Pete Kazmier 1:1",
  textEdit = {
    newText = "[2024-04-15: Pete Kazmier 1:1](agmo)",
    range = {
      ["end"] = {
        character = 2,
        line = 20
      },
      start = {
        character = 0,
        line = 20
      }
    }
  }
}
DEBUG: text='2024-04-15: Pete Kazmier 1:1 meetings/agmo.md' base='[['

Yes, this most certainly helps.

Based on this debug data, 'mini.completion' correctly follows LSP specification here (as far as I can interpret it). The relevant part is in Completion Request section and has the following:

Completion Item with text edits: in this mode the server tells the client that it actually knows what it is doing. If you create a completion item with a text edit at the current cursor position no word guessing takes place and no filtering should happen. This mode can be combined with a sort text and filter text to customize two things. If the text edit is a replace edit then the range denotes the word used for filtering. If the replace changes the text it most likely makes sense to specify a filter text to be used.

So the text in line 20 from column 0 to 2 (excluded) should be used as base for filtering, which in this case is [[. As filterText does not start with it, this item is filtered out.

One slight room for various interpretations here is what is meant by "replace edit" in "If the text edit is a replace edit ...". It can be understood (at least) either if textEdit actually replaces text or if textEdit implements insertReplaceEdit interface. I tend to understand it as the first case, because the second one seems mostly to be for a more complicated case when a client can ask for either insert or replace operation.

So all in all, I am inclined to conclude that this is an issue in LSP which should have filterText start with "[[" in this case.

Closing as "I think 'mini.completion' follows LSP spec here, while server - not really".

pkazmier commented 4 months ago

Thanks for your insights. Even when I modified the filter text to include the initial "[[", I only had partial success (see my first post). Any thoughts on that?

Also, I discovered that zk allows a lot of configuration of its LSP output:

https://github.com/zk-org/zk/blob/main/docs/config-lsp.md#completion

So, I'll try various permutations of those when I get home (running errands atm).

Hoping I can get this working as I've used mini.completion this week (disabling fallback due to lag) and I've been quite satisfied.

echasnovski commented 4 months ago

But even with the completion menu open, I'm still not able to start typing letters to narrow down my selection. Thanks for your insights. Even when I modified the filter text to include the initial "[[", I only had partial success (see my first post). Any thoughts on that?

That is because filterText is not used when computing items for built-in completion popup. That would need textEdit.newText to also be modified to start with "[[", although it would lead to not proper text edit. Not sure what is the best solution here, but my initial reaction is that this kind of completion items (when inserted text does not start with the currently typed text) will probably not have very good support in 'mini.completion'.

pkazmier commented 4 months ago

Thank you for investigating.

After a week of testing, I think it’s time to switch back to nvim-cmp. The two main issues for me include this one and having to disable fallback entirely due to the lag—too many compromises with the user experience for my use cases.

Time for me to go play with mini.deps now—the last of the minis that I’ve yet to try.

echasnovski commented 4 months ago

having to disable fallback entirely due to the lag

For me this usually starts being noticeable when there is a buffer with something like 10000 words for <C-n> to go through. But even then usually not a huge deal. But yeah, asynchronous search would have been better.