stevearc / dressing.nvim

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

vim.ui.input: Distinguish Cancellation (interrupt) from empty string input #72

Closed wookayin closed 1 year ago

wookayin commented 1 year ago

Describe the bug

Two cases:

should be distinguished. In my opinion, (1) should not invoke the callback function, or (2) should pass the empty string ("") to the callback, not nil. We can choose and decide which behavior to accept, perhaps depending on how neovim does (neovim/neovim#18144).

System information

-- Paste your call to require("dressing").setup(...) in here
dressing.setup {
}

To Reproduce Steps to reproduce the behavior:

vim.ui.input({}, function(new_value) print(vim.inspect(new_value)) end)

Currently both of (1) and (2) prints nil.

Screenshots

Additional context

The built-in vim.ui.input also has similar issues. https://github.com/neovim/neovim/issues/18144

wookayin commented 1 year ago

It is not documented somewhere, but there is a cancelreturn option:

vim.ui.input(
  { cancelreturn = "some_canceled_value" },
  function(new_value) print(vim.inspect(new_value)) end
)

which can distinguish cancellation (cancelreturn) and entering empty string normally (the argument evaluates to nil). I suggest that for the case (2), it should pass "" instead of nil.

stevearc commented 1 year ago

The entire usefulness of the vim.ui.* functions is that they provide a single unified interface that users can replace with whatever implementation they want. If any of the implementations deviate from the base interface it makes it harder for users to swap implementations, and it makes it harder for plugin authors to know what features they can rely on.

I will be keeping dressing 100% API compatible with the core Neovim implementation. If it changes (as in the linked issue you mentioned), then I'll change dressing to match it.

Dressing does currently support cancelreturn, since it is technically part of the behavior of the core vim.ui.input even though it's not documented. If you find any inconsistencies with how it functions vs core, please file a bug.

wookayin commented 1 year ago

From neovim 0.9.0, the behavior of vim.ui.input() upon <Esc> will be changed to enter of the empty input string (https://github.com/neovim/neovim/pull/20883). FYI, the correct behavior upon <Ctrl-c> (although not documented) is to NOT invoke the on_confirm callback function.

Would you want to add some branch in dressing's ui.input() depending on neovim version? Or to correct the behavior for older neovim versions as well to conform with the documented behavior (but the behavior was buggy)?

stevearc commented 1 year ago

Nice job getting the changes into core! I think it makes the most sense to keep dressing in sync with the most up-to-date behavior of the API, regardless of what version of Neovim the user is running. I've made the change in 202bcf6bdb05ad833b2b2a869399a06699dd8b63