Saghen / blink.cmp

Performant, batteries-included completion plugin for Neovim
MIT License
1.1k stars 63 forks source link

Insertion after expansion of snippet starts at wrong place, when the completion has autoimport #94

Closed Rishabh672003 closed 1 week ago

Rishabh672003 commented 3 weeks ago

As seen in the video, the insert starts at the wrong place because the completion has auto-import the header file.

https://github.com/user-attachments/assets/173a472d-b3ed-4693-bd48-b81ae4832f5d

My cofig:

{
    highlight = {
      -- sets the fallback highlight groups to nvim-cmp's highlight groups
      -- useful for when your theme doesn't support blink.cmp
      -- will be removed in a future release, assuming themes add support
      use_nvim_cmp_as_default = true,
    },
    nerd_font_variant = "normal",

    -- experimental auto-brackets support

    -- experimental signature help support
    trigger = { signature_help = { enabled = true } },
    -- for keymap, all values may be string | string[]
    -- use an empty table to disable a keymap
    keymap = {
      show = "<C-space>",
      hide = "<C-e>",
      accept = { "<C-y>", "<CR>" },
      select_prev = { "<Up>", "<C-p>" },
      select_next = { "<Down>", "<C-n>" },

      show_documentation = "<C-space>",
      hide_documentation = "<C-space>",
      scroll_documentation_up = "<C-b>",
      scroll_documentation_down = "<C-f>",

      snippet_forward = "<Tab>",
      snippet_backward = "<S-Tab>",
    },

    accept = {
      create_undo_point = true,
      auto_brackets = {
        enabled = true,
        default_brackets = { "(", ")" },
        override_brackets_for_filetypes = {},
        -- Overrides the default blocked filetypes
        force_allow_filetypes = {},
        blocked_filetypes = {},
        -- Synchronously use the kind of the item to determine if brackets should be added
        kind_resolution = {
          enabled = true,
          blocked_filetypes = { "typescriptreact", "javascriptreact", "vue" },
        },
        -- Asynchronously use semantic token to determine if brackets should be added
        semantic_token_resolution = {
          enabled = true,
          blocked_filetypes = {},
          -- How long to wait for semantic tokens to return before assuming no brackets should be added
          timeout_ms = 400,
        },
      },
    },

    fuzzy = {
      -- frencency tracks the most recently/frequently used items and boosts the score of the item
      use_frecency = true,
      -- proximity bonus boosts the score of items with a value in the buffer
      use_proximity = true,
      max_items = 200,
      -- controls which sorts to use and in which order, these three are currently the only allowed options
      sorts = { "label", "kind", "score" },

      prebuiltBinaries = {
        -- Whether or not to automatically download a prebuilt binary from github. If this is set to `false`
        -- you will need to manually build the fuzzy binary dependencies by running `cargo build --release`
        download = true,
        -- When downloading a prebuilt binary force the downloader to resolve this version. If this is uset
        -- then the downloader will attempt to infer the version from the checked out git tag (if any).
        --
        -- Beware that if the FFI ABI changes while tracking main then this may result in blink breaking.
        forceVersion = nil,
      },
    },

    windows = {
      autocomplete = {
        min_width = 30,
        max_height = 15,
        border = "none",
        winhighlight = "Normal:BlinkCmpMenu,FloatBorder:BlinkCmpMenuBorder,CursorLine:BlinkCmpMenuSelection,Search:None",
        -- keep the cursor X lines away from the top/bottom of the window
        scrolloff = 2,
        -- which directions to show the window,
        -- falling back to the next direction when there's not enough space
        direction_priority = { "s", "n" },
        -- whether to preselect the first item in the completion list
        preselect = true,
        -- Controls how the completion items are rendered on the popup window
        -- 'simple' will render the item's kind icon the left alongside the label
        -- 'reversed' will render the label on the left and the kind icon + name on the right
        -- 'minimal' will render the label on the left and the kind name on the right
        -- 'function(blink.cmp.CompletionRenderContext): blink.cmp.Component[]' for custom rendering
        draw = "simple",
        -- Controls the cycling behavior when reaching the beginning or end of the completion list.
        cycle = {
          -- When `true`, calling `select_next` at the *bottom* of the completion list will select the *first* completion item.
          from_bottom = true,
          -- When `true`, calling `select_prev` at the *top* of the completion list will select the *last* completion item.
          from_top = true,
        },
      },
    },
  }
Saghen commented 1 week ago

This appears to be an issue with the built-in snippet expansion and text edit utils. Do you mind reporting this upstream and referencing this issue? Reproducible via:

vim.snippet.expand('test(${1:ing})')
vim.lsp.util.apply_text_edits({
  {
    range = { start = { line = 0, character = 0 }, ['end'] = { line = 0, character = 0 } },
    newText = 'hello world\n',
  },
}, vim.api.nvim_get_current_buf(), 'utf-16')
Rishabh672003 commented 1 week ago

@Saghen I have exams these days so really busy, will do it later when i have time

Rishabh672003 commented 1 week ago

this doesnt happen with the builtin snippets expansion of the latest nightly

https://github.com/user-attachments/assets/79f625c0-b5dc-4270-861e-4e9b158235a2

Rishabh672003 commented 1 week ago

@Saghen wait it still happens with nightly with blink.cmp, sorry if my previous comment was misleading nvim -v : NVIM v0.11.0-dev-1045+gb922b7d6d

https://github.com/user-attachments/assets/9f7233cd-c434-4ab0-802a-fc78101c6b24

doesnt happen with default <C-x><C-o> style snippets

https://github.com/user-attachments/assets/7e57803c-8fcd-48c3-80de-9ff395aacd1e

MariaSolOs commented 1 week ago

@Saghen I don't believe this is an issue with core. The problem here is that snippets should be expanded after text edits have been applied. I haven't debugged this plugin carefully but it seems that this accept function applies additional text edits after expanding the snippet, hence the bug.

I would suggest taking a look at core's on_complete_done function here for a reference on how to handle snippet expansion.

Saghen commented 1 week ago

Yep, we apply additional text edits after because often times they must resolved from the LSP. I plan to add prefetching but there will still be cases where we have to wait on the LSP. But in general, I'm not sure I understand why the order should matter here, especially since other plugins could be adding lines after expansion triggers. You can reproduce this issue by adding any additional line to the buffer after the snippet expansion triggers.

vim.snippet.expand('test(${1:ing})')
vim.api.nvim_buf_set_lines(0, 0, 0, false, { 'hello world' })

Deferring the function prevents the issue

vim.snippet.expand('test(${1:ing})')
vim.defer_fn(function() 
  vim.api.nvim_buf_set_lines(0, 0, 0, false, { 'hello world' }) 
end, 100)
MariaSolOs commented 1 week ago

Yep, we apply additional text edits after because often times they must resolved from the LSP.

Yeah I know how LSP completion works. If you look at the function that I linked, the built-in completion implementation also has support for resolving text edits later and it still expands the snippet afterwards.

Also note that both built-in completion and nvim-cmp (when using vim.snippet) don't have the original problem described here, and so if other completion plugins using vim.snippet don't have the bug, I think blink.cmp is the one doing something incorrect.

vim.snippet.expand('test(${1:ing})')
vim.api.nvim_buf_set_lines(0, 0, 0, false, { 'hello world' })

Regarding this example, I dare to say this is by design. Snippet expansion starts a snippet session that depends on cursor movement and text insertion to update the relevant tabstops. I wouldn't be surprised if there's a timing problem with nvim_buf_set_lines moving the cursor invalidating the state of the snippet session.

I'm happy to reopen the issue in Neovim and let other maintainers weigh in on whether this is something we should fix, but I believe that's a different problem and unrelated to the original bug in this issue.

Saghen commented 1 week ago

@MariaSolOs Didn't mean to be disrespectful, sorry if it came off that way

@Rishabh672003 Resolved in https://github.com/Saghen/blink.cmp/commit/3927128e712806c22c20487ef0a1ed885bfec292 by always resolving before applying and prefetching.

I'll continue watching that neovim issue as I'd still like to fallback to resolving lazily if possible

Rishabh672003 commented 1 week ago

last thing @Saghen can you make a new release please would make upgrading so much easier for me on mini.deps

MariaSolOs commented 1 week ago

@MariaSolOs Didn't mean to be disrespectful, sorry if it came off that way

Not at all. I also apologize if I sounded passive-aggressive (we're all passionate editor tinkerers 😅). I think this is a great plugin and I definitely want to make sure that the right code is fixed :)