From 845c80721f8f0445b0abe39ee381a6feecf158a2 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 18 Jul 2020 19:41:13 +1000 Subject: [PATCH] Decouple escaping from quitting When a user is not entering text into a prompt, the 'q' key should immediately quit the application. On the other hand, the 'esc' key should cancel/close/go-back to the previous context. If we're at the surface level (nothing to cancel/close) and the user hits the escape key, the default behaviour is to close the app, however we now have a `quitOnTopLevelReturn` config key to override this. I actually think from the beginning we should have made this config option default to false rather than true which is the default this PR gives it, but I don't want to anger too many people familiar with the existing behaviour. --- docs/Config.md | 2 ++ docs/keybindings/Keybindings_en.md | 7 +++++-- docs/keybindings/Keybindings_nl.md | 7 +++++-- docs/keybindings/Keybindings_pl.md | 7 +++++-- pkg/config/app_config.go | 1 + pkg/gui/keybindings.go | 8 +------- pkg/gui/menu_panel.go | 2 +- pkg/gui/quitting.go | 8 ++++++++ pkg/gui/view_helpers.go | 3 ++- pkg/i18n/english.go | 3 +++ scripts/generate_cheatsheet.go | 2 +- 11 files changed, 34 insertions(+), 16 deletions(-) diff --git a/docs/Config.md b/docs/Config.md index cf6558746..c2c320f7c 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -50,6 +50,8 @@ Default path for the config file: days: 14 # how often an update is checked for reporting: 'undetermined' # one of: 'on' | 'off' | 'undetermined' confirmOnQuit: false + # determines whether hitting 'esc' will quit the application when there is nothing to cancel/close + quitOnTopLevelReturn: true keybinding: universal: quit: 'q' diff --git a/docs/keybindings/Keybindings_en.md b/docs/keybindings/Keybindings_en.md index ed8416305..c4ea18d97 100644 --- a/docs/keybindings/Keybindings_en.md +++ b/docs/keybindings/Keybindings_en.md @@ -16,6 +16,8 @@ +: next screen mode (normal/half/fullscreen) _: prev screen mode :: execute custom command + |: view scoping options + ctrl+e: open diff menu ## Branches Panel @@ -40,6 +42,7 @@ f: fast-forward this branch from its upstream g: view reset options R: rename branch + ctrl+o: copy branch name to clipboard ,: previous page .: next page <: scroll to top @@ -53,6 +56,7 @@ esc: return to remotes list g: view reset options space: checkout + n: new branch M: merge into currently checked out branch d: delete branch r: rebase checked-out branch onto this branch @@ -134,11 +138,11 @@ p: pick commit (when mid-rebase) t: revert commit c: copy commit (cherry-pick) + ctrl+o: copy commit SHA to clipboard C: copy commit range (cherry-pick) v: paste commits (cherry-pick) enter: view commit's files space: checkout commit - i: select commit to diff with another commit T: tag commit ctrl+r: reset cherry-picked (copied) commits selection ,: previous page @@ -246,7 +250,6 @@ 
   esc: close menu
-  q: close menu
   ,: previous page
   .: next page
   <: scroll to top
diff --git a/docs/keybindings/Keybindings_nl.md b/docs/keybindings/Keybindings_nl.md
index 9b26b0ce8..f3bd9b37d 100644
--- a/docs/keybindings/Keybindings_nl.md
+++ b/docs/keybindings/Keybindings_nl.md
@@ -16,6 +16,8 @@
   +: next screen mode (normal/half/fullscreen)
   _: prev screen mode
   :: voor aangepast commando uit
+  |: view scoping options
+  ctrl+e: open diff menu
## Branches Panel @@ -40,6 +42,7 @@ f: fast-forward this branch from its upstream g: bekijk reset opties R: rename branch + ctrl+o: copy branch name to clipboard ,: previous page .: next page <: scroll to top @@ -53,6 +56,7 @@ esc: return to remotes list g: bekijk reset opties space: uitchecken + n: nieuwe branch M: merge in met huidige checked out branch d: verwijder branch r: rebase branch @@ -134,11 +138,11 @@ p: pick commit (when mid-rebase) t: commit omgedaan maken c: kopiëer commit (cherry-pick) + ctrl+o: copy commit SHA to clipboard C: kopiëer commit reeks (cherry-pick) v: plak commits (cherry-pick) enter: bekijk gecommite bestanden space: checkout commit - i: select commit to diff with another commit T: tag commit ctrl+r: reset cherry-picked (copied) commits selection ,: previous page @@ -246,7 +250,6 @@ 
   esc: close menu
-  q: close menu
   ,: previous page
   .: next page
   <: scroll to top
diff --git a/docs/keybindings/Keybindings_pl.md b/docs/keybindings/Keybindings_pl.md
index 2fa2b8205..3ea71d5d0 100644
--- a/docs/keybindings/Keybindings_pl.md
+++ b/docs/keybindings/Keybindings_pl.md
@@ -16,6 +16,8 @@
   +: next screen mode (normal/half/fullscreen)
   _: prev screen mode
   :: execute custom command
+  |: view scoping options
+  ctrl+e: open diff menu
## Gałęzie Panel @@ -40,6 +42,7 @@ f: fast-forward this branch from its upstream g: view reset options R: rename branch + ctrl+o: copy branch name to clipboard ,: previous page .: next page <: scroll to top @@ -53,6 +56,7 @@ esc: return to remotes list g: view reset options space: przełącz + n: nowa gałąź M: scal do obecnej gałęzi d: usuń gałąź r: rebase branch @@ -134,11 +138,11 @@ p: pick commit (when mid-rebase) t: revert commit c: copy commit (cherry-pick) + ctrl+o: copy commit SHA to clipboard C: copy commit range (cherry-pick) v: paste commits (cherry-pick) enter: view commit's files space: checkout commit - i: select commit to diff with another commit T: tag commit ctrl+r: reset cherry-picked (copied) commits selection ,: previous page @@ -246,7 +250,6 @@ 
   esc: close menu
-  q: close menu
   ,: previous page
   .: next page
   <: scroll to top
diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go
index 0d1bad23d..a16fec67d 100644
--- a/pkg/config/app_config.go
+++ b/pkg/config/app_config.go
@@ -278,6 +278,7 @@ update:
 reporting: 'undetermined' # one of: 'on' | 'off' | 'undetermined'
 splashUpdatesIndex: 0
 confirmOnQuit: false
+quitOnTopLevelReturn: true
 keybinding:
   universal:
     quit: 'q'
diff --git a/pkg/gui/keybindings.go b/pkg/gui/keybindings.go
index 8248e5e99..283daa779 100644
--- a/pkg/gui/keybindings.go
+++ b/pkg/gui/keybindings.go
@@ -217,7 +217,7 @@ func (gui *Gui) GetInitialKeybindings() []*Binding {
 			ViewName: "",
 			Key:      gui.getKey("universal.return"),
 			Modifier: gocui.ModNone,
-			Handler:  gui.handleQuit,
+			Handler:  gui.handleTopLevelReturn,
 		},
 		{
 			ViewName:    "",
@@ -848,12 +848,6 @@ func (gui *Gui) GetInitialKeybindings() []*Binding {
 			Handler:     gui.handleMenuClose,
 			Description: gui.Tr.SLocalize("closeMenu"),
 		},
-		{
-			ViewName:    "menu",
-			Key:         gui.getKey("universal.quit"),
-			Handler:     gui.handleMenuClose,
-			Description: gui.Tr.SLocalize("closeMenu"),
-		},
 		{
 			ViewName: "information",
 			Key:      gocui.MouseLeft,
diff --git a/pkg/gui/menu_panel.go b/pkg/gui/menu_panel.go
index ac0844216..9b2eefac5 100644
--- a/pkg/gui/menu_panel.go
+++ b/pkg/gui/menu_panel.go
@@ -25,7 +25,7 @@ func (gui *Gui) handleMenuSelect(g *gocui.Gui, v *gocui.View) error {
 
 func (gui *Gui) renderMenuOptions() error {
 	optionsMap := map[string]string{
-		fmt.Sprintf("%s/%s", gui.getKeyDisplay("universal.return"), gui.getKeyDisplay("universal.quit")):       gui.Tr.SLocalize("close"),
+		gui.getKeyDisplay("universal.return"): gui.Tr.SLocalize("close"),
 		fmt.Sprintf("%s %s", gui.getKeyDisplay("universal.prevItem"), gui.getKeyDisplay("universal.nextItem")): gui.Tr.SLocalize("navigate"),
 		gui.getKeyDisplay("universal.select"): gui.Tr.SLocalize("execute"),
 	}
diff --git a/pkg/gui/quitting.go b/pkg/gui/quitting.go
index f9ad00177..4a7bba23a 100644
--- a/pkg/gui/quitting.go
+++ b/pkg/gui/quitting.go
@@ -34,6 +34,14 @@ func (gui *Gui) handleQuit(g *gocui.Gui, v *gocui.View) error {
 	return gui.quit(v)
 }
 
+func (gui *Gui) handleTopLevelReturn(g *gocui.Gui, v *gocui.View) error {
+	if gui.Config.GetUserConfig().GetBool("quitOnTopLevelReturn") {
+		return gui.handleQuit(g, v)
+	}
+
+	return nil
+}
+
 func (gui *Gui) quit(v *gocui.View) error {
 	if gui.State.Updating {
 		return gui.createUpdateQuitConfirmation(gui.g, v)
diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go
index 429ab46b5..032d05002 100644
--- a/pkg/gui/view_helpers.go
+++ b/pkg/gui/view_helpers.go
@@ -513,7 +513,8 @@ func (gui *Gui) renderGlobalOptions() error {
 	return gui.renderOptionsMap(map[string]string{
 		fmt.Sprintf("%s/%s", gui.getKeyDisplay("universal.scrollUpMain"), gui.getKeyDisplay("universal.scrollDownMain")):                                                                                 gui.Tr.SLocalize("scroll"),
 		fmt.Sprintf("%s %s %s %s", gui.getKeyDisplay("universal.prevBlock"), gui.getKeyDisplay("universal.nextBlock"), gui.getKeyDisplay("universal.prevItem"), gui.getKeyDisplay("universal.nextItem")): gui.Tr.SLocalize("navigate"),
-		fmt.Sprintf("%s/%s", gui.getKeyDisplay("universal.return"), gui.getKeyDisplay("universal.quit")):                                                                                                 gui.Tr.SLocalize("close"),
+		gui.getKeyDisplay("universal.return"):     gui.Tr.SLocalize("cancel"),
+		gui.getKeyDisplay("universal.quit"):       gui.Tr.SLocalize("quit"),
 		gui.getKeyDisplay("universal.optionMenu"): gui.Tr.SLocalize("menu"),
 		"1-5": gui.Tr.SLocalize("jump"),
 	})
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index 27b3fa46f..d26790033 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -249,6 +249,9 @@ func addEnglish(i18nObject *i18n.Bundle) error {
 		}, &i18n.Message{
 			ID:    "close",
 			Other: "close",
+		}, &i18n.Message{
+			ID:    "quit",
+			Other: "quit",
 		}, &i18n.Message{
 			ID:    "SureResetThisCommit",
 			Other: "Are you sure you want to reset to this commit?",
diff --git a/scripts/generate_cheatsheet.go b/scripts/generate_cheatsheet.go
index 1cfa3aab6..e61a72dd2 100644
--- a/scripts/generate_cheatsheet.go
+++ b/scripts/generate_cheatsheet.go
@@ -31,7 +31,7 @@ func main() {
 
 	for _, lang := range langs {
 		os.Setenv("LC_ALL", lang)
-		mApp, _ := app.NewApp(mConfig)
+		mApp, _ := app.NewApp(mConfig, "")
 		file, err := os.Create(getProjectRoot() + "/docs/keybindings/Keybindings_" + lang + ".md")
 		if err != nil {
 			panic(err)