Skip to content

Conversation

@mark2185
Copy link
Collaborator

@mark2185 mark2185 commented Aug 7, 2022

Inspired by #1111.

Adding n as an actual option for cancelling the prompt is TODO.

@jesseduffield
Copy link
Owner

I'm thinking we do the opposite: remove 'y' and stick to just enter/escape. That way there'll be less confusion when there's a menu that has a 'y' or 'n' keybinding against one of the options

@mark2185
Copy link
Collaborator Author

I'm thinking we do the opposite: remove 'y' and stick to just enter/escape. That way there'll be less confusion when there's a menu that has a 'y' or 'n' keybinding against one of the options

Maybe the confirmation menu should only ever have y, n, enter and escape as possible keymaps. Seems like a step back for it to clash with menu mappings, just like H for horiziontal scroll 😅

Also denoting Y as uppercase makes the default choice when pressing Enter, as is the unix way, I wouldn't steer away from having y/n (and maybe later a for always or something)

@jesseduffield
Copy link
Owner

Maybe the confirmation menu should only ever have y, n, enter and escape as possible keymaps. Seems like a step back for it to clash with menu mappings, just like H for horiziontal scroll 😅

I'm not sure what you mean here. To clarify I meant that a user who's used to using y/n on confirmation panels might think that you could use y to select a menu item and if y is actually tied to a particular menu item that's not currently selected, that could be confusing.

@jesseduffield
Copy link
Owner

At any rate @mark2185 do you reckon it's okay to say that y/n is allowed on confirmation prompts but in a menu you need to use enter to select an item and 'y' is allowed to be used as a keybinding for a specific menu item? I'd be okay with that.

@mark2185
Copy link
Collaborator Author

At any rate @mark2185 do you reckon it's okay to say that y/n is allowed on confirmation prompts but in a menu you need to use enter to select an item and 'y' is allowed to be used as a keybinding for a specific menu item? I'd be okay with that.

Yup, hit the nail on the head. I may have fumbled a bit with the all the confirmation and menu panels and whatnot panels, that's what I had in mind.

In short, I'm trying to say that I think the confirmation panels (the ones that are like Clippy asking you "Are you sure you want to quit?") should only have a handful of possible (but configurable) mappings:

  • confirm (y by default)
  • cancel (n by default)
  • enter (which would do the default for that specific confirmation window, which can be either y or n, but it would be denoted by uppercase since for some confirmation panels we might have N as the "default" choice (can't think of any, but thinking about the future here))
  • escape (which would always cancel)

@jesseduffield
Copy link
Owner

I've gone back and re-read the ticket and I'm thinking that we should actually not include the keys in the message itself. Rather, we should always phrase the confirmation as a question (e.g. 'would you like to force push?'), and we should always have the 'yes' answer mapped to the 'enter' key with the 'no' answer mapped to the 'esc' key. Then we can display the keys for doing things at the bottom left of the screen (as we currently do now).

My reasoning is that it's not hard to see what you need to do the first time and then you can use the muscle memory for all future times. This approach also allows for easier changes to keybindings down the line.

As for y/n, when we consider the above approach, the question is whether we want a second set of keybindings that do the exact same thing as enter/esc. Given 'n' apparently doesn't work, and given that 'y' isn't really advertised as being an available key, I'm back to thinking we should drop 'y'. Lemme know your thoughts @mark2185

@jesseduffield
Copy link
Owner

After thinking about it even more I think we should have the y/n keybinding for the confirmation panel but still only mention that we have that keybinding in the options view when the confirmation panel is shown

@mark2185
Copy link
Collaborator Author

After thinking about it even more I think we should have the y/n keybinding for the confirmation panel but still only mention that we have that keybinding in the options view when the confirmation panel is shown

What do you mean by options view when the confirmation panel is shown?

@jesseduffield
Copy link
Owner

I.e. the blue text at the bottom left of the screen

@mark2185 mark2185 closed this Mar 9, 2023
@mark2185 mark2185 deleted the feature-add-confirmation-to-messages branch March 14, 2023 05:55
@J053Fabi0
Copy link

J053Fabi0 commented May 8, 2023

I miss when I could click y or enter to confirm. With the latest updates it was removed, but I still find myself pressing y almost every time and being frustrated when I see it does nothing anymore.
Is there a way to set confirm to both y and enter? I tried remapping it with an array in the config file, but it gave an error.

@mark2185
Copy link
Collaborator Author

mark2185 commented May 8, 2023

We've decided that there is no need for multiple ways to confirm since they do the same thing.

How do you decide when you'd use y and when enter?

@J053Fabi0
Copy link

J053Fabi0 commented May 8, 2023

I use y for confirmation dialogs and enter for typing dialogs.
When I'm not writing, hitting y feels more convenient because the button is closer than enter, which I have to reach at the extreme of the keyboard with my pinkie.

@mark2185
Copy link
Collaborator Author

mark2185 commented May 8, 2023

Wouldn't remapping y to confirm then work for you?

Unless y mucks up the input windows then, but that sounds like it shouldn't happen.

@J053Fabi0
Copy link

J053Fabi0 commented May 8, 2023

Unless y mucks up the input windows

It does.

keybinding:
  universal:
    confirm: 'y'

@jesseduffield
Copy link
Owner

As it stands I don't think it's worth reintroducing the key binding. The enter key is a fairly universal way of confirming something. That said I'm happy to leave this issue open and see if enough other people want a way to get the old key binding back (please thumbs up the OP to signify that you want the change)

@jesseduffield
Copy link
Owner

Just realized I'm commenting on a PR haha. WELL, my point still stands: feel free to raise an issue for this and see if other people agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants