Repository: jesseduffield/lazygit (language: go)

Issue:
Too easy to accidentally abort a rebase by hitting esc when there are conflicts
When a rebase stops with conflicts, there's the prompt `Conflicts! To abort press 'esc', otherwise press 'enter'`. It's too easy to accidentally hit escape here and abort a rebase, which can be super annoying when you already spent a long time resolving conflicts in earlier commits.

It's especially a problem because at the prompt `all merge conflicts resolved. Continue?` I usually do hit escape.

I don't have a great idea what to do about it; but a simple, additional `Are you sure you want to abort the rebase?` prompt would do, I guess.


Discussion / hints:
A solution could be as simple as this:

```diff
diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go
index 0e67fd7ea..4a7fd0442 100644
--- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go
+++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go
@@ -145,7 +145,7 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error {
 				return self.c.PushContext(self.c.Contexts().Files)
 			},
 			HandleClose: func() error {
-				return self.genericMergeCommand(REBASE_OPTION_ABORT)
+				return self.AbortMergeOrRebaseWithConfirm()
 			},
 		})
 	} else {
```

Any opinions @jesseduffield?
I agree that the current UX is bad. Looking for `HandleClose` in the codebase this really is the only place where we perform an action upon hitting escape in a popup.

As a rule, if 'I don't care' is a permitted response to a popup, the user should be able to hit escape and no action should be performed (with focus returning to the original context)

We should have a menu with the following two options (and no cancel button because it's not really appropriate)
* v - View conflicts
* a - Abort

Choosing 'View conflicts' will focus the files context, choosing 'Abort' will abort, and hitting escape will return focus to the original context.

This UX is backwards compatible in terms of viewing the conflicts (you're hitting enter either way) but not in terms of aborting (because escape now does something different). That's not a big deal though: if somebody habitually hits escape they can still abort by pressing 'm'.

Also, with this new UX you'll also be able to abort with a single keypress.

What do you think?
I like the suggestion, I'll see if I can find the time to work on a PR.

Fix patch:
diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go
index 0e67fd7ea7a..21ab44201ff 100644
--- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go
+++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go
@@ -138,15 +138,26 @@ func (self *MergeAndRebaseHelper) CheckMergeOrRebase(result error) error {
 		// assume in this case that we're already done
 		return nil
 	} else if isMergeConflictErr(result.Error()) {
-		return self.c.Confirm(types.ConfirmOpts{
-			Title:  self.c.Tr.FoundConflictsTitle,
-			Prompt: self.c.Tr.FoundConflicts,
-			HandleConfirm: func() error {
-				return self.c.PushContext(self.c.Contexts().Files)
-			},
-			HandleClose: func() error {
-				return self.genericMergeCommand(REBASE_OPTION_ABORT)
+		mode := self.workingTreeStateNoun()
+		return self.c.Menu(types.CreateMenuOptions{
+			Title: self.c.Tr.FoundConflictsTitle,
+			Items: []*types.MenuItem{
+				{
+					Label: self.c.Tr.ViewConflictsMenuItem,
+					OnPress: func() error {
+						return self.c.PushContext(self.c.Contexts().Files)
+					},
+					Key: 'v',
+				},
+				{
+					Label: fmt.Sprintf(self.c.Tr.AbortMenuItem, mode),
+					OnPress: func() error {
+						return self.genericMergeCommand(REBASE_OPTION_ABORT)
+					},
+					Key: 'a',
+				},
 			},
+			HideCancel: true,
 		})
 	} else {
 		return self.c.ErrorMsg(result.Error())
diff --git a/pkg/i18n/chinese.go b/pkg/i18n/chinese.go
index 8352017620a..52e92559f35 100644
--- a/pkg/i18n/chinese.go
+++ b/pkg/i18n/chinese.go
@@ -175,7 +175,6 @@ func chineseTranslationSet() TranslationSet {
 		ReturnToFilesPanel:                  `返回文件面板`,
 		FastForward:                         `从上游快进此分支`,
 		Fetching:                            "抓取并快进 {{.from}} -> {{.to}} ...",
-		FoundConflicts:                      "冲突！需要中止操作请按‘esc’, 否则按‘enter’",
 		FoundConflictsTitle:                 "自动合并失败",
 		ViewMergeRebaseOptions:              "查看 合并/变基 选项",
 		NotMergingOrRebasing:                "您目前既不进行变基也不进行合并",
diff --git a/pkg/i18n/dutch.go b/pkg/i18n/dutch.go
index 0be8cd2aaed..bd0a15b8e1c 100644
--- a/pkg/i18n/dutch.go
+++ b/pkg/i18n/dutch.go
@@ -140,8 +140,7 @@ func dutchTranslationSet() TranslationSet {
 		ReturnToFilesPanel:                  `Ga terug naar het bestanden paneel`,
 		FastForward:                         `Fast-forward deze branch vanaf zijn upstream`,
 		Fetching:                            "Fetching en fast-forwarding {{.from}} -> {{.to}} ...",
-		FoundConflicts:                      "Conflicten!, Om af te breken druk 'esc', anders druk op 'enter'",
-		FoundConflictsTitle:                 "Auto-merge mislukt",
+		FoundConflictsTitle:                 "Conflicten!",
 		ViewMergeRebaseOptions:              "Bekijk merge/rebase opties",
 		NotMergingOrRebasing:                "Je bent momenteel niet aan het rebasen of mergen",
 		RecentRepos:                         "Recente repositories",
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index 2a954d57d7f..22a00b91c83 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -181,8 +181,9 @@ type TranslationSet struct {
 	ReturnToFilesPanel                  string
 	FastForward                         string
 	Fetching                            string
-	FoundConflicts                      string
 	FoundConflictsTitle                 string
+	ViewConflictsMenuItem               string
+	AbortMenuItem                       string
 	PickHunk                            string
 	PickAllHunks                        string
 	ViewMergeRebaseOptions              string
@@ -867,8 +868,9 @@ func EnglishTranslationSet() TranslationSet {
 		ReturnToFilesPanel:                  `Return to files panel`,
 		FastForward:                         `Fast-forward this branch from its upstream`,
 		Fetching:                            "Fetching and fast-forwarding {{.from}} -> {{.to}} ...",
-		FoundConflicts:                      "Conflicts! To abort press 'esc', otherwise press 'enter'",
-		FoundConflictsTitle:                 "Auto-merge failed",
+		FoundConflictsTitle:                 "Conflicts!",
+		ViewConflictsMenuItem:               "View conflicts",
+		AbortMenuItem:                       "Abort the %s",
 		ViewMergeRebaseOptions:              "View merge/rebase options",
 		NotMergingOrRebasing:                "You are currently neither rebasing nor merging",
 		AlreadyRebasing:                     "Can't perform this action during a rebase",
diff --git a/pkg/i18n/korean.go b/pkg/i18n/korean.go
index 0f0a1f2c242..1c469ed68b0 100644
--- a/pkg/i18n/korean.go
+++ b/pkg/i18n/korean.go
@@ -176,7 +176,6 @@ func koreanTranslationSet() TranslationSet {
 		ReturnToFilesPanel:                  `파일 목록으로 돌아가기`,
 		FastForward:                         `Fast-forward this branch from its upstream`,
 		Fetching:                            "Fetching and fast-forwarding {{.from}} -> {{.to}} ...",
-		FoundConflicts:                      "Conflicts! To abort press 'esc', otherwise press 'enter'",
 		FoundConflictsTitle:                 "Auto-merge failed",
 		ViewMergeRebaseOptions:              "View merge/rebase options",
 		NotMergingOrRebasing:                "You are currently neither rebasing nor merging",
diff --git a/pkg/i18n/polish.go b/pkg/i18n/polish.go
index 729902cb471..5ac6b8a3a57 100644
--- a/pkg/i18n/polish.go
+++ b/pkg/i18n/polish.go
@@ -125,8 +125,7 @@ func polishTranslationSet() TranslationSet {
 		MoveUpCommit:                        "Przenieś commit 1 w górę",
 		EditCommit:                          "Edytuj commit",
 		AmendToCommit:                       "Popraw commit zmianami z poczekalni",
-		FoundConflicts:                      "Konflikty! Wciśnij 'esc' żeby przerwać, w przeciwnym razie wciśnij 'enter'",
-		FoundConflictsTitle:                 "Automatyczne scalenie nie powiodło się",
+		FoundConflictsTitle:                 "Konflikty!",
 		ViewMergeRebaseOptions:              "Widok scalenia/opcje zmiany bazy",
 		NotMergingOrRebasing:                "W tej chwili nie scalasz ani nie zmieniasz bazy",
 		RecentRepos:                         "Ostatnie repozytoria",
diff --git a/pkg/i18n/traditional_chinese.go b/pkg/i18n/traditional_chinese.go
index c9f036cb9f4..c51ae97b119 100644
--- a/pkg/i18n/traditional_chinese.go
+++ b/pkg/i18n/traditional_chinese.go
@@ -240,7 +240,6 @@ func traditionalChineseTranslationSet() TranslationSet {
 		ReturnToFilesPanel:                  `返回檔案面板`,
 		FastForward:                         `從上游快進此分支`,
 		Fetching:                            "{{.from}} -> {{.to}} 的擷取和快進中...",
-		FoundConflicts:                      "衝突！按 'esc' 退出，否則按 'enter'",
 		FoundConflictsTitle:                 "自動合併失敗",
 		ViewMergeRebaseOptions:              "查看合併/變基選項",
 		NotMergingOrRebasing:                "你當前既不在變基也不在合併中",
diff --git a/pkg/integration/components/common.go b/pkg/integration/components/common.go
index 0f2e3b8714c..fe81438eef7 100644
--- a/pkg/integration/components/common.go
+++ b/pkg/integration/components/common.go
@@ -19,9 +19,9 @@ func (self *Common) ContinueRebase() {
 }
 
 func (self *Common) AcknowledgeConflicts() {
-	self.t.ExpectPopup().Confirmation().
-		Title(Equals("Auto-merge failed")).
-		Content(Contains("Conflicts!")).
+	self.t.ExpectPopup().Menu().
+		Title(Equals("Conflicts!")).
+		Select(Contains("View conflicts")).
 		Confirm()
 }
 


Test patch:
diff --git a/pkg/integration/tests/branch/rebase_abort_on_conflict.go b/pkg/integration/tests/branch/rebase_abort_on_conflict.go
new file mode 100644
index 00000000000..4eba7762716
--- /dev/null
+++ b/pkg/integration/tests/branch/rebase_abort_on_conflict.go
@@ -0,0 +1,49 @@
+package branch
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+	"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
+)
+
+var RebaseAbortOnConflict = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Rebase onto another branch, abort when there are conflicts.",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shared.MergeConflictsSetup(shell)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().TopLines(
+			Contains("first change"),
+			Contains("original"),
+		)
+
+		t.Views().Branches().
+			Focus().
+			Lines(
+				Contains("first-change-branch"),
+				Contains("second-change-branch"),
+				Contains("original-branch"),
+			).
+			SelectNextItem().
+			Press(keys.Branches.RebaseBranch)
+
+		t.ExpectPopup().Menu().
+			Title(Equals("Rebase 'first-change-branch' onto 'second-change-branch'")).
+			Select(Contains("Simple rebase")).
+			Confirm()
+
+		t.ExpectPopup().Menu().
+			Title(Equals("Conflicts!")).
+			Select(Contains("Abort the rebase")).
+			Confirm()
+
+		t.Views().Branches().
+			IsFocused()
+
+		t.Views().Files().
+			IsEmpty()
+	},
+})
diff --git a/pkg/integration/tests/branch/rebase_cancel_on_conflict.go b/pkg/integration/tests/branch/rebase_cancel_on_conflict.go
new file mode 100644
index 00000000000..23b7661b239
--- /dev/null
+++ b/pkg/integration/tests/branch/rebase_cancel_on_conflict.go
@@ -0,0 +1,51 @@
+package branch
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+	"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
+)
+
+var RebaseCancelOnConflict = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Rebase onto another branch, cancel when there are conflicts.",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shared.MergeConflictsSetup(shell)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().TopLines(
+			Contains("first change"),
+			Contains("original"),
+		)
+
+		t.Views().Branches().
+			Focus().
+			Lines(
+				Contains("first-change-branch"),
+				Contains("second-change-branch"),
+				Contains("original-branch"),
+			).
+			SelectNextItem().
+			Press(keys.Branches.RebaseBranch)
+
+		t.ExpectPopup().Menu().
+			Title(Equals("Rebase 'first-change-branch' onto 'second-change-branch'")).
+			Select(Contains("Simple rebase")).
+			Confirm()
+
+		t.ExpectPopup().Menu().
+			Title(Equals("Conflicts!")).
+			Select(Contains("Abort the rebase")).
+			Cancel()
+
+		t.Views().Branches().
+			IsFocused()
+
+		t.Views().Files().
+			Lines(
+				Contains("UU file"),
+			)
+	},
+})
diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go
index 50377f10157..2a16df041ce 100644
--- a/pkg/integration/tests/test_list.go
+++ b/pkg/integration/tests/test_list.go
@@ -36,7 +36,9 @@ var tests = []*components.IntegrationTest{
 	branch.DetachedHead,
 	branch.OpenWithCliArg,
 	branch.Rebase,
+	branch.RebaseAbortOnConflict,
 	branch.RebaseAndDrop,
+	branch.RebaseCancelOnConflict,
 	branch.RebaseDoesNotAutosquash,
 	branch.Reset,
 	branch.ResetUpstream,