[mini.completion] Some completions result in extra period

[thatsmydoing]

Contributing guidelines

• I have read CONTRIBUTING.md
• I have read CODE_OF_CONDUCT.md
• I have updated 'mini.nvim' to latest version

Module(s)

mini.completion

Description

With tsserver LSP, doing a mini completion from a . and selecting a completion option injects an extra .. I don't think the issue is restricted to tsserver though.

This is different from typescript-language-server/typescript-language-server#711 since it happens even with the workaround of setting completionDisableFilterText.

When using the built-in vim.lsp.omnifunc, the issue does not occur.

I think neovim has some code that adjusts the completion start based on results here https://github.com/neovim/neovim/blob/f1b415b3abbcccb8b0d2aa1a41a45dd52de1a5ff/runtime/lua/vim/lsp.lua#L2145-L2159

Neovim version

0.9

Steps to reproduce

Minimal config

require('lspconfig').tsserver.setup({
  init_options = {
    completionDisableFilterText = true,
  },
})
require('mini.completion').setup({})

1. nvim -u minimal.lua
2. Open a typescript file
3. Type in window. and force completion with <C-space>
4. Select a suggestion from the dropdown

Expected behavior

I would expect the result would be

window.AbortController

Actual behavior

This results in

window..AbortController

[echasnovski]

Thanks for the issue!

'mini.completion' doesn't use filterText at all. Instead it tries to use textEdit.newText > insertText > label(in that order). My guess is thattypescript-language-serveralso provides.AbortController` in one of those.

There is also a way for users to customize items returned from LSP servers by providing custom lsp_completion.process_items function. My initial thought right now is that the best solution here would be for users to tweak returned items to never start with dot. If added to buffer-local config for TypeScript, it can be used only for this language.

---

If you are not comfortable with writing your own process_items, I'd gladly help. The thing is - I don't use TypeScript. So would you mind providing the sample of what LSP server returns as items? Here are the steps:

• Use the following code to set up 'mini.completion':

require('mini.completion').setup({
  lsp_completion = {
    process_items = function(items, base)
      _G.args = { items = items, base = base }
      return MiniCompletion.default_process_items(items, base)
    end,
  },
  -- There might be some other code from your setup
})

• Reproduce your problematic completion case.
• Immediately after accepting it, execute the following command: :lua vim.fn.append(0, vim.split(vim.inspect(_G.args), '\n')). This will convert _G.args into string and paste it at the top of current buffer.
• Copy the whole table (it is probably very big, couple of hundreds of lines) and paste here under spoiler. Like this:

<details><summary>Arguments in my problematic completion</summary>

```
<You table goes here>
```

</details>

[thatsmydoing]

I've reduced it to something with a single completion, my source file is

const test = { foo: 5 };
test.

and I execute completion at test. it results in test..foo

Bad completion

{
  base = "",
  items = { {
      data = {
        entryNames = { "foo" },
        file = "/home/thomas/test.ts",
        line = 2,
        offset = 6
      },
      kind = 5,
      label = "foo",
      sortText = "11",
      textEdit = {
        newText = ".foo",
        range = {
          ["end"] = {
            character = 5,
            line = 1
          },
          start = {
            character = 4,
            line = 1
          }
        }
      }
    } }
}

When completing from test.f it appropriately completes to test.foo

Good completion

{
  base = "f",
  items = { {
      data = {
        entryNames = { "foo" },
        file = "/home/thomas/test.ts",
        line = 2,
        offset = 7
      },
      kind = 5,
      label = "foo",
      sortText = "11"
    } }
}

From my understanding, the use of textEdit.newText is not exactly correct since it is an edit that has to be applied and the start is not necessarily the same as where the completion was started, hence the startbyte workarounds in neovim's lsp omnifunc. I suppose this is more a limitation with the builtin completion (omnifunc, etc). Ideally, it would apply the textEdit itself rather than just replacing it with word. It's a bit unfortunate since the additionalTextEdits works well enough but not the primary one.

I did try making a process_items that seems to work but I haven't extensively tested yet with

process_items = function(items, base)
  for k,v in ipairs(items) do
    v["textEdit"] = nil
  end
  return MiniCompletion.default_process_items(items, base)
end

[echasnovski]

Thanks for the follow-up! This was really helpful!

Yes, I vaguely remember that I compromised on just using plain textEdit.newText instead of adding some non-trivial heuristic to make it 100% correct. And yes, it is a result of a completion protocol used in built-in completefunc and omnifunc, as it only allows a string as candidate. Of course, there are CompleteDonePre and CompleteDone events to hook into and try to resolve this, but again it feels too complicated.

At the moment I'd suggest using custom process_items. In particular, I'd make these changes:

• Create ~/.config/nvim/after/ftplugin/typescript.lua.
• Add this buffer-local config to it:

vim.b.minicompletion_config = {
  lsp_completion = {
    process_items = function(items, base)
      -- Remove dots as prefix from `textEdit.newText` as it is used verbatim
      for _, item in ipairs(items) do
        local new_text = (item.textEdit or {}).newText
        if type(new_text) == 'string' then
          item.textEdit.newText = new_text:gsub('^%.+', '')
        end
      end

      return MiniCompletion.default_process_items(items, base)
    end,
  },
}

This way it will affect only 'typescript' filetype.

I'll think about resolving this issue more cleanly.

Edit: The issue should now be resolved in main branch.

[marcusandre]

@echasnovski Thanks for the head-tip. I ran into the same problem and originally wanted to report it on my own. I will try your proposed solution tomorrow morning and hand in some feedback. Cheers!

[marcusandre]

@echasnovski As promised, I tried your proposal and - for the sake of completeness - added files for typescript and typescriptreact. With that, I don't see the double .. appearing anymore. Thanks a lot!

[echasnovski]

@echasnovski As promised, I tried your proposal and - for the sake of completeness - added files for typescript and typescriptreact. With that, I don't see the double .. appearing anymore. Thanks a lot!

That is great to hear! Thanks for the feedback.

I still can't think of a better way to resolve this problem, so having a confirmation that it works is good.

[gbishop]

I see this issue with the builtin vim.lsp.omnifunc as well.

[echasnovski]

I think utilizing suggested lsp_completion.process_items is the way to go here. Couldn't come up with anything better here.

[thatsmydoing]

There's another few interesting cases that hits these limitations:

Optional Chaining

interface Foo { foo: number };
let v: Foo | undefined;

v.

Completes to

v.?.foo

It should complete to

v?.foo

Arguments

``` { base = "", items = { { data = { entryNames = { "foo" }, file = "/Users/thomas/test.ts", line = 4, offset = 3 }, kind = 5, label = "foo", sortText = "11", textEdit = { newText = "?.foo", range = { ["end"] = { character = 2, line = 3 }, start = { character = 1, line = 3 } } } } } } ```

Property Access

interface Foo { 'f-o': number };
let v: Foo;

v.

Completes to

v.["f-o"]

It should complete to

v["f-o"]

Arguments

``` { base = "", items = { { data = { entryNames = { "f-o" }, file = "/Users/thomas/test.ts", line = 4, offset = 3 }, kind = 5, label = "f-o", sortText = "11", textEdit = { newText = '["f-o"]', range = { ["end"] = { character = 2, line = 3 }, start = { character = 1, line = 3 } } } } } } ```

Interestingly, the builtin vim.lsp.omnifunc does not complete anything at all for either of these. I assume it filters these out since it can't actually complete them correctly.

If you complete from v.f the LSP does not give any results at all.

I don't think these can be worked around aside from removing them as candidates unfortunately.

[echasnovski]

@thatsmydoing, if you still happen to use 'mini.completion', would you mind please test driving the backlog-completion branch? It seems to fix both the "extra period" case and the behavior from your comment, but extra verifying on more real world cases would be great. Don't forget to remove an ad hoc fix from this comment before testing.

[thatsmydoing]

Cool! I gave it a go. The first thing that popped out is that I get these warnings as I type

E5108: Error executing lua ...lib/pack/nixpkgs/start/mini.nvim/lua/mini/completion.lua:1307: attempt to index field 'range' (a nil value)
stack traceback:
        ...lib/pack/nixpkgs/start/mini.nvim/lua/mini/completion.lua:1307: in function 'get_completion_start_server'
        ...lib/pack/nixpkgs/start/mini.nvim/lua/mini/completion.lua:1294: in function <...lib/pack/nixpkgs/start/mini.nvim/lua/mini/completion.lua:1289>

This is on revision 95d4115

But otherwise, it does seem to work better. The "extra period" case seems to be fixed on main. The optional chaining and property access now seem to match the built-in LSP omnifunc where it just doesn't show those completions.

In that sense, I guess it's fixed? A bit unfortunate though. I've actually since switched to nvim-cmp since I do use those completions relatively often, but I'd be eager to switch back to mini.completion if you ever get those cases fixed. Though I guess that's more dependent on neovim making omnifunc more powerful.

[echasnovski]

Would you mind carving a reproducible example of when you get those errors as you type? All cases from this issue work as expected on backlog-completion branch without errors. And none should work on main.

[thatsmydoing]

Oh sorry, I realized I was still setting LSP capabilities based on nvim-cmp. After I unset it, everything does work without errors. The branch does indeed fix the extra period case (and is broken on main). The branch also matches built-in LSP omnifunc with regards to the other cases.

[echasnovski]

Oh sorry, I realized I was still setting LSP capabilities based on nvim-cmp. After I unset it, everything does work without errors. The branch does indeed fix the extra period case (and is broken on main). The branch also matches built-in LSP omnifunc with regards to the other cases.

That's very good news! Thanks for the follow up.
I'll test drive the backlog-completion branch for a (couple of) day(s) and then merge into main.

[gbishop]

Let me make sure I'm doing what is necessary. I'm using Lazy for package management. I edited my lua/pluging/mini.lua and added branch = "backlog-completion" to the configuration. I also commented out the content of my after/ftplugin/javascript.lua file to remove the hack previously required. I let Lazy install the version from the branch which seemed to go OK.

Now, I edit a javascript file and type

pageLoaded. and the completions have 2 dots instead of 1.

What am I missing?

[echasnovski]

Let me make sure I'm doing what is necessary. I'm using Lazy for package management. I edited my lua/pluging/mini.lua and added branch = "backlog-completion" to the configuration. I also commented out the content of my after/ftplugin/javascript.lua file to remove the hack previously required. I let Lazy install the version from the branch which seemed to go OK.

Now, I edit a javascript file and type

pageLoaded. and the completions have 2 dots instead of 1.

What am I missing?

Not sure, as I don't entirely know how 'lazy.nvim' operates. Here is the checklist I'd go through:

• Go directly to plugin source (I'd suspect at '~/.local/share/lazy/mini.nvim') and look at file 'lua/mini/completion.lua' if it is at correct revision. For example, it should contain this code at line 1303. If it does not, then revision is not correct. What version do you have in 'lazy.nvim' spec table? I believe having it version = false should be appropriate here.
• Make sure that 'mini.completion' is used for popup: either :echo &omnifunc or :echo &completefunc should show v:lua.MiniCompletion.completefunc_lsp.

If both of this is true, then I'd ask for you for a reproducible steps. I.e. which LSP server should I use (tsserver a.k.a. typescript-language-server, or something else?), what filename should I create and which text to input. I tried opening 'test.js' file with tsserver and typing pageLoaded. and it didn't show any LSP related completion (as for pageLoaded itself).

[gbishop]

In /home/gb/.local/share/nvim/lazy/mini.nvim which is a directory, I see that it was not on branch backlog-completion. Manually checking out backlog-completion makes it appear to work properly.

I'll have to look into my Lazy config to see why it isn't checking out the branch.

[gbishop]

My mistake... I told Lazy to Check but not to Update.

[echasnovski]

All examples from this issues should now work on the main branch. Thanks everyone for the early testing.

[GitMurf]

Is it expected that if I do vim.api. I will not get any completion?

When I do the force twostep completion mapping, it then just shows the fallback actually and no LSP items.

As opposed to if I do vim.api.n typing the first letter n then everything works as expected.

Please let me know if you are not able to reproduce. I have a pretty minimal setup outside of mini modules so shouldn't have anything effecting my setup to cause these issues.

[GitMurf]

And if I turn off fuzzy matching and just use the defaults and do NOT set lsp_completion.process_items then it works as expected (I ofcourse just won't get fuzzy matching).

If it helps, here is how I have the lsp_completion.process_items setup in my last comment when I am having the problems. Is this setup correctly?

lsp_completion = {
  process_items = function(items, base)
    return MiniFuzzy.process_lsp_items(items, base)
  end,
},

[GitMurf]

Ok as an FYI, here is my workaround for now. If I get zero results from fuzzy match for LSP then it just provides the default process items.

lsp_completion = {
  process_items = function(items, base)
    local origItemsCount = #items
    local fuzzyResults = MiniFuzzy.process_lsp_items(items, base)
    local fuzzyResultsCount = #fuzzyResults
    if fuzzyResultsCount == 0 and origItemsCount > 0 then
      return MiniCompletion.default_process_items(items, base)
    end
    return fuzzyResults
  end,
},