Skip to content

Commit

Permalink
fix: #1533 (#1534)
Browse files Browse the repository at this point in the history
  • Loading branch information
@kulabun
kulabun committed May 1, 2023
1 parent 524cc0b commit 5547295
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lua/cmp/entry.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ entry.get_offset = function(self)
if range then
offset = self.context.cache:ensure('entry:' .. 'get_offset:' .. tostring(range.start.character), function()
for idx = range.start.character + 1, self.source_offset do
if not char.is_white(string.byte(self.context.cursor_line, idx)) then
local byte = string.byte(self.context.cursor_line, idx)
if byte ~= nil and not char.is_white(byte) then
return idx
end
end
return offset
return offset + 1
end)
end
else
Expand Down
33 changes: 33 additions & 0 deletions lua/cmp/entry_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,4 +302,37 @@ describe('entry', function()
assert.are.equal(e:get_vim_item(e:get_offset()).word, '__init__(self) -> None:')
assert.are.equal(e:get_filter_text(), '__init__')
end)

it('[#1533] keyword pattern that include whitespace', function()
local state = spec.state(' ', 3, 2)
local state_source = state.source()

state_source.get_keyword_pattern = function(_)
return '.'
end

state.input(' ')
local e = entry.new(state.manual(), state_source, {
filterText = "constructor() {\n ... st = 'test';\n ",
kind = 1,
label = "constructor() {\n ... st = 'test';\n }",
textEdit = {
newText = "constructor() {\n this.test = 'test';\n }",
range = {
["end"] = {
character = 2,
col = 3,
line = 2,
row = 3
},
start = {
character = 0,
line = 2
}
}
}
})
assert.are.equal(e:get_offset(), 3)
assert.are.equal(e:get_vim_item(e:get_offset()).word, 'constructor() {')
end)
end)

7 comments on commit 5547295

@shunlir
Copy link

@shunlir shunlir commented on 5547295 May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @kulabun , thanks for your contribution,
but it seems this commit has regression issue. In below screenshot, the first character after the dot is ignored when filtering the candidates:

image

image

@kulabun
Copy link
Contributor Author

@kulabun kulabun commented on 5547295 May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't reproduce this issue. Do you have a minimal reproducible config?

@kulabun
Copy link
Contributor Author

@kulabun kulabun commented on 5547295 May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the issue you are talking about is related. It's something happening before completion items list is retrieved, while my change only affects how completion item is being applied. You can see that the code is executed only on self:get_completion_item().textEdit is present and is not null.

@shunlir
Copy link

@shunlir shunlir commented on 5547295 May 2, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kulabun

The issue can be reproduced with this commit (reset to the prev commit doesn't reproduce the issue).
If you want, here is the code I used https://github.com/MaskRay/ccls/blob/master/src/main.cc#L119 , you need to generate the compile command database and link it to the source repo root.
Below is my minimal reproducible vim config:

local lazypath = vim.fn.stdpath("data") .. "/lazy/lazy.nvim"
if not vim.loop.fs_stat(lazypath) then
  vim.fn.system({
    "git",
    "clone",
    "--filter=blob:none",
    "https://github.com/folke/lazy.nvim.git",
    "--branch=stable", -- latest stable release
    lazypath,
  })
end
vim.opt.rtp:prepend(lazypath)

require("lazy").setup({
  { 'williamboman/mason.nvim', build = ':MasonUpdate' },
  { 'williamboman/mason-lspconfig.nvim' },
  { 'neovim/nvim-lspconfig' },
  {
    "hrsh7th/nvim-cmp",
    dependencies = {
      'hrsh7th/cmp-vsnip',
      'hrsh7th/vim-vsnip',
      'hrsh7th/cmp-buffer', -- source: buffer
      'hrsh7th/cmp-nvim-lsp', -- source: nvim_lsp
    },
    config = function()
      local cmp = require'cmp'

      cmp.setup({
        snippet = {
          -- REQUIRED - you must specify a snippet engine
          expand = function(args)
            vim.fn["vsnip#anonymous"](args.body) -- For `vsnip` users.
          end,
        },
        window = {},
        mapping = cmp.mapping.preset.insert({
          ['<C-b>'] = cmp.mapping.scroll_docs(-4),
          ['<C-f>'] = cmp.mapping.scroll_docs(4),
          ['<C-Space>'] = cmp.mapping.complete(),
          ['<C-e>'] = cmp.mapping.abort(),
          ['<CR>'] = cmp.mapping.confirm({ select = true }), -- Accept currently selected item. Set `select` to `false` to only confirm explicitly selected items.
        }),
        sources = cmp.config.sources({
          { name = 'nvim_lsp' },
          { name = 'vsnip' }, -- For vsnip users.
        }, {
          { name = 'buffer' },
        })
      })

      -- Set up lspconfig.
      local capabilities = require('cmp_nvim_lsp').default_capabilities()
      require'lspconfig'.clangd.setup{
        capabilities = capabilities
      }
    end
  },
})

@uga-rosa
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced with clangd. And this is caused by offset+1 at L70 in entry.lua (undoing just this line will fix it)
Why this change?

@hrsh7th
Copy link
Owner

@hrsh7th hrsh7th commented on 5547295 May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll revert +1 offset at the moment.

@kulabun
Copy link
Contributor Author

@kulabun kulabun commented on 5547295 May 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turned out the problem was in:

local byte = string.byte(self.context.cursor_line, idx)
if byte ~= nil and not char.is_white(byte) then

When the byte is null at idx index we we should return offset rather than offset + 1

Please sign in to comment.