Skip to content

vim style config: allow Q in pop-up inputs #818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

silmeth
Copy link
Contributor

@silmeth silmeth commented Jul 15, 2021

Change exit (which exits the app regardless of context) to Ctrl+C, and move Shift+Q to quit (which doesn’t cause quitting on upper-case Q insert in popup inputs, cf. #771).

  • make indendation consistent

@extrawurst
Copy link
Collaborator

Hi @silmeth thanks for your PR. I am no VIM user myself. So forgive me being cautious about this. could you explain to me why this was approached with yet another set of keys in another PR? see #789 ;)

@silmeth
Copy link
Contributor Author

silmeth commented Jul 15, 2021

From what I see the issue I’m having wasn’t approached in the other PR.

In vim you generally use :q for quitting (the : enters command mode in which you can enter the q command to quit). Ctrl+C and Esc can both be used to exit to ‘normal mode’ (the default mode of moving around in vim), so they both also have some kind of ‘exit/quit’ semantics (though they do not exit the program) – that’s why I think Ctrl+C is fine for exit.

The other PR tried to change the uppercase Q to lowercase q – which kinda makes sense (since you exit vim using :q, not :Q) – but it did not fix the issue of Q / q not being possible to enter in popup windows.

I didn’t change Q to q as I didn’t want to change too much the current key mapping (I’m fine with either Q or Shift+Q – I only want to fix the issue of quitting on Q in popup inputs.

@silmeth
Copy link
Contributor Author

silmeth commented Jul 15, 2021

Since quit was set to q, arguably just changing the exit line to Ctrl+C would be fine too (and would change only one line) – I wanted to keep the older behaviour of exiting on Shift+Q (which I think was in the config since its introduction).

@extrawurst
Copy link
Collaborator

extrawurst commented Jul 15, 2021

Since quit was set to q, arguably just changing the exit line to Ctrl+C would be fine too (and would change only one line) – I wanted to keep the older behaviour of exiting on Shift+Q (which I think was in the config since its introduction).

ok lets do that, lets only change the exit then..

what do you think @yanganto (you made the initial vi bindings, and capital Q was still your decision) ? now that we have the dedicated quit command, how do you feel about this change?

@yanganto
Copy link
Contributor

Yes, I use Shift + Q to quit Gitui at this moment, but I think the Esc to quit when non-pop window is also a good idea.

@extrawurst
Copy link
Collaborator

extrawurst commented Jul 15, 2021

Yes, I use Shift + Q to quit Gitui at this moment, but I think the Esc to quit when non-pop window is also a good idea.

actually the idea is: ctrl+c to exit (currently Q)
so the question is if you ignore you habit and imagine a new gitui user comes and expects VI bindings, would ctrl+c be appropriate for close gitui no matter where you are in the app (I assume so because usually that's exactly what ctrl+c does) and q as a regular quit when in no popup.

for your personal muscle memory you can still keep exit bound to Q

@silmeth
Copy link
Contributor Author

silmeth commented Jul 15, 2021

for your personal muscle memory you can still keep exit bound to Q

or, as initially proposed, rebind quit to Q instead (since exit as Q makes it impossible to input Q in popups which is the main issue with this binding)

@yanganto
Copy link
Contributor

Thanks for mention me. It is good for making the new users comfortable with much easier memory configure. Changing to ctrl + c is also good to me. :)

Change `exit` (which exits the app regardless of context) to <kbd>Ctrl</kbd>+<kbd>C</kbd> (which doesn’t cause quitting on upper-case `Q` insert in popup inputs, cf. gitui-org#771).
@yanganto
Copy link
Contributor

yanganto commented Jul 15, 2021

Rebinding quit to Q instead is good. The reason I love Shift + Q is that ctrl + c hurt my fingers, and Shift + Q does not hurt and is not easy to trigger without intention. And I can change my habit if people think another binding is much reasonable.

@yanganto
Copy link
Contributor

@silmeth Thanks for making this. 😄

@extrawurst
Copy link
Collaborator

@silmeth like I said, lets change exit to ctrl+c. we double checked, tig also exits on ctrl+c and q 👍

@extrawurst extrawurst merged commit 36e9f77 into gitui-org:master Jul 15, 2021
@extrawurst
Copy link
Collaborator

@silmeth thank you ❤️

@silmeth silmeth deleted the patch-1 branch July 15, 2021 14:34
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