Add confirmation window options

[omelette-watin]

Hey,

First off, big shoutout to everyone involved in this project – I love it !

I'm a bit more used to [Y]es or [N]o rather than [O]k or [C]ancel tho, so I thought it would be cool to make both the labels and key mappings configurable to suit different preferences.

I dont have a lot of experience in lua so i don't know if this PR respects good practices (especially regarding the mergeTables fn).

[jedrzejboczar]

Hi, I also wanted this feature, so I implemented it myself https://github.com/stevearc/oil.nvim/compare/master...jedrzejboczar:oil.nvim:jb/confirm-dialog-config and then I checked existing PRs just to realize that someone else has already created such a PR. So basically I am all for merging this PR :)

What's funny is that my implementation is almost identical, even the naming of config options is similar. The main difference that I would suggest is to use nowait = true for the mappings (e.g. I want to use y for "[Y]es", but it waits for yank movement). I also decided to have "special" cancel keys ({ "q", "<C-c>", "<Esc>" }) explicitly in config, but this doesn't seem too important.

I dont have a lot of experience in lua so i don't know if this PR respects good practices (especially regarding the mergeTables fn).

As these tables are Lua lists (only numeric keys starting from 1) it would be simpler to use vim.list_extend, e.g.

local cancel_keys = vim.list_extend({ "q", "<C-c>", "<Esc>" }, config.confirmation.cancel.keymaps)

Note that vim.list_extend modifies the first table in-place and then returns it.

[omelette-watin]

Thanks for your help @jedrzejboczar

i updated the pr to include nowait = true and vim.list_extend : good call!

I also decided to have "special" cancel keys ({ "q", "<C-c>", "<Esc>" }) explicitly in config, but this doesn't seem too important.

My original thought was that those mappings are "expected" to work to cancel an action or close a window but i think you are right on this.
The whole point of this pr is to fit every one preferences so it makes sense to make them configurable too. Maybe they should be behind a boolean flag tho, to avoid rewriting them on each config?

Something like

  confirmation = {
    confirm = {
      text = "[O]k",
      keys = { "O", "o" },
    },
    cancel = {
      text = "[C]ancel",
      keys = { "C", "c", },
      -- use "q", "<C-c>" and"<Esc>" cancel mappings
      use_defaults = true
    },
  },

Maybe not the best naming.

Let me know your thoughts on this

[stevearc]

I pulled this locally to poke around and here's my proposal: instead of making this configurable, what about just changing the defaults to Yes/No? I've pushed back on doing this before, but I'm willing to abandon my original reasoning and accept this as a reasonable default.

But once that is the default, is there any reason to customize it? I don't think most people would want to change it to something else. What do you think?

[omelette-watin]

Thanks for taking the time to test it. Allowing for configuration (like you did in #39beb36) is a bit better imo. But if you don't want the config options becoming heavier it's understandable and fully transitioning to yes/no makes sense then.

[stevearc]

@omelette-watin why do you think that allowing for configuration is better? Do you have a use case for different labels/keymaps?

[omelette-watin]

@stevearc in my opinion, giving option for configuration presents the advantage to avoid similar pull requests on the future for some other use cases that some users might have

The only drawback that i see is a bit heavier config.

But, as i said i’m really happy with your full transition to yes/no, so if you just to keep it like that it’s fine by me!

Anyway, thanks for taking the time to answer this pr and for this amazing plugin !

[jedrzejboczar]

I'd also vote for leaving configuration options, for the same reason - avoiding issues like this from users. While I don't think that it is very important to have configurable labels, I'd say it is good to have configurable keymaps. It would probably be best to have something like vim.ui.confirm in the core, similar to vim.ui.input/select to give users a standard interface for a binary decision that is selected with a single key press. There is vim.fn.confirm and if using noice.nvim it looks quite good (popup), but this still cannot be customized by the user. I would be also fine with hacking some autocmd to change the keymaps myself, but it seems not possible - the windows use ft=oil_preview and there is no confirm/cancel api to bind to.

But it's also completely fair if you just want to avoid more configuration options.

[stevearc]

I'm already a little unhappy with the amount of configuration options that oil has, and I really want to avoid adding more where it's not necessary. I don't think anyone will want the labels to change from the new values, and the keymaps can be adjusted on the user side, as explained in this comment on an earlier issue.

https://github.com/stevearc/oil.nvim/issues/114#issuecomment-1571332722

I'll merge as-is for now. If a strong need arises in the future to add configuration options, I'll reconsider at that time.

Thanks for the PR!