hrsh7th / cmp-nvim-lsp-signature-help

cmp-nvim-lsp-signature-help
638 stars 26 forks source link

CR blocked with custom mapping #13

Open mikehaertl opened 2 years ago

mikehaertl commented 2 years ago

Problem

I have a custom mapping of CR for nvim-cmp:

  cmp.setup({
    mapping = {
      -- ... other mappings

      ['<C-e>'] = cmp.mapping.abort(),
      ['<CR>'] = cmp.mapping.confirm({ select = false }),
    },
    sources = cmp.config.sources({
      -- ... other sources
      { name = 'nvim_lsp_signature_help' },
    }),

Unfortunately this blocks the CR key when the signature popup is visible. For example in this case I would like to insert a linebreak:

image

If I press enter, nothing happens. I have to press C-e to abort the popup before I can insert a linebreak.

Possible solution

My workaround in pseudo code would be something like this:

    mapping = {
      -- ... other mappings

      ['<CR>'] = function(fallback)
        if (cmp.visible() && menu_contains_only_items_from_signature_source()) then
            fallback()
        else
            cmp.mapping.confirm({ select = false }),
         end
     end
    },

I just don't know how to implement menu_contains_only_items_from_signature_source(). Would this work at all?

emmanueltouzery commented 2 years ago

I hit the same issue, and although I know basically nothing about cmp, I managed to cobble this together, also thanks to @mikehaertl's starting point:

    ['<CR>'] = cmp.mapping(function(fallback) 
      -- workaround for https://github.com/hrsh7th/cmp-nvim-lsp-signature-help/issues/13
      if cmp.get_selected_entry().source.name == 'nvim_lsp_signature_help' then
        fallback()
      else
        cmp.mapping.confirm {
          behavior = cmp.ConfirmBehavior.Replace,
          select = true,
        }(fallback)
      end
    end),

I'm pretty sure it's not "the right fix", but it seems to work well for me so far :)

mikehaertl commented 2 years ago

@emmanueltouzery Awesome, thanks. (Note to self: I really should study the docs more, get_selected_entry() is not that hard to find out).

I was confused about the cmp.mapping.confirm {...}(...) part but just learned that it's syntactic sugar for cmp.mapping.confirm({...})(...). But that still makes me wonder what that final (fallback) is for. Could you explain?

Also final (little) problem left: When you're on the next line the signature help is now gone and can't be brought back.

emmanueltouzery commented 2 years ago

i really can't help that much :) it could be all wrong. since we give cmp.mapping. confirm {} to <CR> and since we pass a function now instead (although wrapped in cmp.mapping(..)), I assumed that cmp.mapping.confirm{} is a function, so I tried to call it like that, and it worked.

Honestly I couldn't find dev docs to author cmp plugins. If you have a link, I'd be thankful. I discovered get_selected_entry() using good old print(vim.expand(..)) debugging :)

mikehaertl commented 2 years ago

There's something in the online help (:h cmp-develop): https://github.com/hrsh7th/nvim-cmp/blob/main/doc/cmp.txt#L649, but it only explains the basics.

Regarding the extra (fallback) call in cmp.mapping.confirm() (fallback): I'm not sure if that really makes sense. There's no return value documented for cmp.mapping.confirm() either. I've left it away and your solution still works fine.

mikehaertl commented 2 years ago

I still had some error messages of this form

E5108: Error executing lua [string ":lua"]:32: attempt to index a nil value

So we really need to check if the menu is visible and if an item is selcted at all. So my final version now looks like this:

['<CR>'] = function(fallback)
  -- Don't block <CR> if signature help is active
  -- https://github.com/hrsh7th/cmp-nvim-lsp-signature-help/issues/13
  if not cmp.visible() or not cmp.get_selected_entry() or cmp.get_selected_entry().source.name == 'nvim_lsp_signature_help' then
    fallback()
  else
    cmp.confirm({
      -- Replace word if completing in the middle of a word
      -- https://github.com/hrsh7th/nvim-cmp/issues/664
      behavior = cmp.ConfirmBehavior.Replace,
      -- Don't select first item on CR if nothing was selected
      select = false,
    })
  end
end

Weird: I had cmp.mapping.confirm() instead of cmp.confirm() (according to the docs both are the same) but then CR did not work anymore to confirm a selection.

emmanueltouzery commented 2 years ago

yes I just changed to:

-      if cmp.get_selected_entry().source.name == 'nvim_lsp_signature_help' then
+      if cmp.get_selected_entry() ~= nil and cmp.get_selected_entry().source.name == 'nvim_lsp_signature_help' then

to fix the same issue. but your version is presumably better, i don't know.

mikehaertl commented 2 years ago

You probably also want to check if the cmp menu is visible at all and if not use the normal CR behavior (i.e. fallback()). That's why I added the not cmp.visible().

EDIT: Ah, get_selected_entry() is probably nil if the menu is not visible so you don't need that extra check.

HawkinsT commented 2 years ago

Thanks for the fixes. I haven't tested @emmanueltouzery's solution, but @mikehaertl's works perfectly for me. Any chance of a PR for this?

ronisbr commented 3 months ago

Hi! I am facing the same problem and @mikehaertl solution works perfectly!