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

fix: Outdated completion item with mini.snippets #2126

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

abeldekat
Copy link

Fixes #2119

The issue as described by @xzbdmw has been debated in several other places:

.. triggers completion when it should not ..
beta testing mini.snippets: expanding snippets from lsp

@xzbdmw provided a clear explanation:

I'll try to explain more detail:
mini.snippets: insert mode -> type 'f' -> both mini.snippets and cmp detects TextChangedI -> cmp-nvim-lsp sends request -> lua_ls receives the buffer content, which is vim.schedule(ffn) -> mini.snippets clears the placeholder fn, it becomes vim.schedule(f) -> "fun" completion_item contains TextEdits that want to clear the fn , now what in fn's original place is now the right pair -> right pair gone, before snippets expanding. Neither mini.snippets or cmp is to blame here IMO, they all do what they supposed to do.

luasnip: select mode -> type 'f' -> nvim clears the placeholder, by the definition of select mode -> cmp-nvim-lsp sends request -> lua_ls receives the buffer content, which is vim.schedule(f) -> all good.

@hrsh7th

I considered solving the problem in cmp-nvim-lsp, introducing some kind of flag indicating whether the call to complete should be scheduled or not.

Then I saw that TextChangedI and TextChangedP do not yet use async.debounce_next_tick.

I think this solves the issue on a more appropriate level.

@abeldekat
Copy link
Author

It is possible to solve the problem in the user's config:

local function add_cmp()
  local group = vim.api.nvim_create_augroup("mini_snippets_nvim_cmp", { clear = true })

  -- NOTE: Firstly, make sure that nvim-cmp never provides completion items directly after snippet insert
  -- See https://github.com/abeldekat/cmp-mini-snippets/issues/6
  vim.api.nvim_create_autocmd("User", {
    group = group,
    pattern = "MiniSnippetsSessionStart",
    callback = function(_) require("cmp.config").set_onetime({ sources = {} }) end,
  })

  -- HACK: Secondly, it's now possible to prevent outdated completion items
  -- See https://github.com/hrsh7th/nvim-cmp/issues/2119
  local function make_complete_override(complete_fn)
    return function(self, params, callback)
      local override_fn = complete_fn
      if MiniSnippets.session.get(false) ~= nil then override_fn = vim.schedule_wrap(override_fn) end
      override_fn(self, params, callback)
    end
  end
  local function find_cmp_nvim_lsp(id)
    for _, source in ipairs(require("cmp").get_registered_sources()) do
      if source.id == id and source.name == "nvim_lsp" then return source.source end
    end
  end
  vim.api.nvim_create_autocmd("User", {
    group = group,
    pattern = "CmpRegisterSource",
    callback = function(ev)
      local cmp_nvim_lsp = find_cmp_nvim_lsp(ev.data.source_id)
      if cmp_nvim_lsp then
        local org_complete = cmp_nvim_lsp.complete
        cmp_nvim_lsp.complete = make_complete_override(org_complete)
      end
    end,
  })

  MiniDeps.add({ source = "hrsh7th/nvim-cmp", depends = { "hrsh7th/cmp-nvim-lsp", "hrsh7th/cmp-buffer" } })
  local cmp = require("cmp")
  require("cmp").setup({
    snippet = {
      expand = function(args) -- use mini.snippets to expand snippets from lsp
        local insert = MiniSnippets.config.expand.insert or MiniSnippets.default_insert
        insert({ body = args.body }) -- Insert at cursor
      end,
    },
    sources = cmp.config.sources({ { name = "nvim_lsp" }, { name = "buffer" } }),
    mapping = cmp.mapping.preset.insert(),
    completion = { completeopt = "menu,menuone,noinsert" },
  })
end

@hrsh7th
Copy link
Owner

hrsh7th commented Jan 19, 2025

In my career as a front-end developer, solving any kind of asynchronous timing problem with things like setTimeout or vim.schedule has always been an anti-pattern.

I think this problem should be solved by mini.snippets.

@echasnovski
Copy link

I think this problem should be solved by mini.snippets.

As far as I understand, there is no problem in 'mini.snippets' to solve. The way it works mostly served as a source to reproduce an issue in completion plugins, which is roughly as follows: there can be other text changes in between completion plugin reacting to 'TextChangedI' / 'TextChangedP' and async action they do as a result. This might lead to action being outdated as the buffer text state has already changed.

The 'mini.snippets' showed this issue because it can react on text change by removing tabstop placeholder immediately in the same tick (otherwise there will be flicker) inside 'TextChanged{I,P}' callback. Other snippet engines utilize Select mode here: its replacing effect is done internally by Neovim and is already in place for 'TextChanged{I,P}' events.

@hrsh7th
Copy link
Owner

hrsh7th commented Jan 19, 2025

I didn't know about that behavior of mini.snippets.

I'm guessing that mini.snippets changes the text during TextChangedI/P, which then causes TextChangedI/P to fire again and complete correctly, but let's look into it.

@echasnovski
Copy link

I'm guessing that mini.snippets changes the text during TextChangedI/P, which then causes TextChangedI/P to fire again and complete correctly, but let's look into it.

Yes, removing placeholder in 'mini.snippets' indeed triggers either TextChangedI or TextChangedP (after ones caused by the user typing a character which causes placeholder removal). They are not suppressed because there is a text change in Insert mode which those events are meant monitor.

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.

Outdated completion item with mini.snippets
3 participants