fix(completion): stop completion when completion is exhausted

[famiu]

Problem: Currently, completion is not stopped when completion is exhausted (no matching completion items), which also means CompleteDone is not called either. Furthermore, CompleteDone is called when pressing a special character (Backspace, Space, Esc) instead.

Solution: Correctly stop completion and trigger CompleteDone when completion is exhausted.

[zeertzjq]

No tests?

[famiu]

No tests?

Added a regression test

[zeertzjq]

I don't think this change is correct. After this change, typing backspace after completion is exhausted can no longer resume completion. For example:

1. Run :set completeopt+=noinsert
2. Type i<C-X><C-V>
3. Type a
4. Type a
5. Type <BS>

Without this PR the <BS> resumes completion. With this PR it doesn't.

[famiu]

I don't think this change is correct. After this change, typing backspace after completion is exhausted can no longer resume completion. For example:

1. Run :set completeopt+=noinsert
2. Type i<C-X><C-V>
3. Type a
4. Type a
5. Type <BS>

Without this PR the <BS> resumes completion. With this PR it doesn't.

@mfussenegger thoughts?

[clason]

I guess we now know why exhaustion does not trigger CompleteDone...

[mfussenegger]

Why does <C-X><C-V> have this re-trigger on <BS> behavior, but complete() doesn't?

local fp = io.open("/tmp/nvim.log", "a+")

vim.o.completeopt = "menuone,noinsert"
vim.api.nvim_create_autocmd("CompleteDone", {
  callback = function()
    fp:write("called\n")
    fp:flush()
  end
})

vim.keymap.set("i", "<C-s>", function()
  vim.fn.complete(vim.fn.col("."), {"abcd", "abdc", "acbd"})
end)

With a<C-s>e<BS> this doesn't trigger again.
Could we make this change exclusive to complete() to not break <C-X><C-V>?

[famiu]

Why does <C-X><C-V> have this re-trigger on <BS> behavior, but complete() doesn't?

local fp = io.open("/tmp/nvim.log", "a+")

vim.o.completeopt = "menuone,noinsert"
vim.api.nvim_create_autocmd("CompleteDone", {
  callback = function()
    fp:write("called\n")
    fp:flush()
  end
})

vim.keymap.set("i", "<C-s>", function()
  vim.fn.complete(vim.fn.col("."), {"abcd", "abdc", "acbd"})
end)

With a<C-s>e<BS> this doesn't trigger again. Could we make this change exclusive to complete() to not break <C-X><C-V>?

I think it might be better to implement the <BS> retrigger logic instead, but let's leave the PR open for now if we decide that that's not worth doing

[famiu]

Why does <C-X><C-V> have this re-trigger on <BS> behavior, but complete() doesn't?

local fp = io.open("/tmp/nvim.log", "a+")

vim.o.completeopt = "menuone,noinsert"
vim.api.nvim_create_autocmd("CompleteDone", {
  callback = function()
    fp:write("called\n")
    fp:flush()
  end
})

vim.keymap.set("i", "<C-s>", function()
  vim.fn.complete(vim.fn.col("."), {"abcd", "abdc", "acbd"})
end)

With a<C-s>e<BS> this doesn't trigger again. Could we make this change exclusive to complete() to not break <C-X><C-V>?

@mfussenegger
Seems like things like completefunc and omnifunc can return a refresh key alongside the list of matches, which is how they can be refreshed (see |complete-functions|). There seems to be no equivalent to it for complete().

@justinmk Would it make sense to add something like nvim_complete() that takes in a Lua function which returns a list of matches, and allows additional options (like refresh, for example)?

[justinmk]

Would it make sense to add something like nvim_complete() that takes in a Lua function which returns a list of matches, and allows additional options (like refresh, for example)?

why do we need a new function, could it be a new arg on the existing complete() ? or perhaps nvim_complete_set() should be revisited, can it be extended or generalized to also cover this purpose?

[famiu]

Would it make sense to add something like nvim_complete() that takes in a Lua function which returns a list of matches, and allows additional options (like refresh, for example)?

why do we need a new function, could it be a new arg on the existing complete() ? or perhaps nvim_complete_set() should be revisited, can it be extended or generalized to also cover this purpose?

A new arg on complete might work, yes, but the reason I suggested a new API is that a completion provider might want its completion items list to be regenerated when refreshing completion, so taking in a callback instead of a list of items would be useful in that case complete() will always use completion items from the initial list, which means updating that list isn't possible. We could refresh completion based on the initial list, but I think something that can take a callback would be much more robust and useful