stevearc / dressing.nvim

Neovim plugin to improve the default vim.ui interfaces
MIT License
1.69k stars 32 forks source link

[fzf-lua] latest commits in dressing might not be necessary anymore #142

Closed ibhagwan closed 5 months ago

ibhagwan commented 5 months ago

Hi @stevearc,

Context: https://github.com/ibhagwan/fzf-lua/issues/1014

It seems I really misconfigured dressing. Sorry for the trouble

After the above report I decided to test fzf-lua with dressing, to my surprise I saw the latest 2 commits (namely https://github.com/stevearc/dressing.nvim/commit/9f9b77315d559fa09850eb3db621ba6fce3d3d32) and this comment:

-- Because fzf-lua doesn't call the on_choice function if exited (e.g. with <C-c>), we need to
-- add an autocmd ourselves to make sure that happens.

This made no sense as I had written the code in the past in such a way that should call on_choice with nil when no item is selected.

Turns out, some time ago I added "dummy" actions (keybinds) for ctrl-c/esc (for better resume support) which in turn affected the code route I had setup before to call on_choice(nil, nil) when no item was selected. While I appreiate the innovative solution your commits :smile: I think it's better to also solve the original bug (on_choice must be called according to the docs):

Fixed in https://github.com/ibhagwan/fzf-lua/commit/5474df3a722b0efbeb9f47d0b2fdbce7d2ff258e, the new code flow (I can probablly eliminate the if not selected route, left for posterity):


    if not selected then
      -- with `actions.dummy_abort` this doesn't get called anymore
      -- as the action is configured as a valid fzf "accept" (thus
      -- `selected` isn't empty), see below comment for mor info
      on_choice(nil, nil)
    else
      o._on_choice_called = nil
      actions.act(o.actions, selected, o)
      if not o._on_choice_called then
        -- see  comment above, `on_choice` wasn't called, either
        -- "dummy_abort" (ctrl-c/esc) or (unlikely) the user setup
        -- additional binds that aren't for "accept". Not calling
        -- with nil (no action) can cause issues, for example with
        -- dressing.nvim (#1014)
        on_choice(nil, nil)
      end
    end
``
stevearc commented 5 months ago

That's great news! I was going to report this, but I noticed the behavior mid-week and just needed to get a fix in ASAP. Glad it was fixed upstream!

ibhagwan commented 5 months ago

That's great news! I was going to report this, but I noticed the behavior mid-week and just needed to get a fix in ASAP. Glad it was fixed upstream!

Ty @stevearc!