From c757063264d5386a67a22d88f6283dbbf1d09223 Mon Sep 17 00:00:00 2001
From: Stefan Haller <stefan@haller-berlin.de>
Date: Sat, 4 Mar 2023 18:11:17 +0100
Subject: [PATCH] Better error message when trying to edit or move a non-todo
 commit during rebase

Previously we would have tried to do the rebase, resulting in a long and
somewhat cryptic error message from git; now we check ourselves and show a less
intimidating message.
---
 .../controllers/local_commits_controller.go   | 17 +++++++++
 pkg/i18n/english.go                           |  2 +
 .../edit_non_todo_commit_during_rebase.go     | 37 +++++++++++++++++++
 pkg/integration/tests/test_list.go            |  1 +
 4 files changed, 57 insertions(+)
 create mode 100644 pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go

diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go
index 904dbfd33..348e0e73f 100644
--- a/pkg/gui/controllers/local_commits_controller.go
+++ b/pkg/gui/controllers/local_commits_controller.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/jesseduffield/lazygit/pkg/commands/models"
+	"github.com/jesseduffield/lazygit/pkg/commands/types/enums"
 	"github.com/jesseduffield/lazygit/pkg/gui/context"
 	"github.com/jesseduffield/lazygit/pkg/gui/types"
 	"github.com/jesseduffield/lazygit/pkg/utils"
@@ -327,6 +328,14 @@ func (self *LocalCommitsController) interactiveRebase(action string) error {
 // begin a rebase. It then updates the todo file with that action
 func (self *LocalCommitsController) handleMidRebaseCommand(action string, commit *models.Commit) (bool, error) {
 	if !commit.IsTODO() {
+		if self.git.Status.WorkingTreeState() != enums.REBASE_MODE_NONE {
+			// If we are in a rebase, the only action that is allowed for
+			// non-todo commits is rewording the current head commit
+			if !(action == "reword" && self.isHeadCommit()) {
+				return true, self.c.ErrorMsg(self.c.Tr.AlreadyRebasing)
+			}
+		}
+
 		return false, nil
 	}
 
@@ -383,6 +392,10 @@ func (self *LocalCommitsController) moveDown(commit *models.Commit) error {
 		})
 	}
 
+	if self.git.Status.WorkingTreeState() != enums.REBASE_MODE_NONE {
+		return self.c.ErrorMsg(self.c.Tr.AlreadyRebasing)
+	}
+
 	return self.c.WithWaitingStatus(self.c.Tr.MovingStatus, func() error {
 		self.c.LogAction(self.c.Tr.Actions.MoveCommitDown)
 		err := self.git.Rebase.MoveCommitDown(self.model.Commits, index)
@@ -417,6 +430,10 @@ func (self *LocalCommitsController) moveUp(commit *models.Commit) error {
 		})
 	}
 
+	if self.git.Status.WorkingTreeState() != enums.REBASE_MODE_NONE {
+		return self.c.ErrorMsg(self.c.Tr.AlreadyRebasing)
+	}
+
 	return self.c.WithWaitingStatus(self.c.Tr.MovingStatus, func() error {
 		self.c.LogAction(self.c.Tr.Actions.MoveCommitUp)
 		err := self.git.Rebase.MoveCommitDown(self.model.Commits, index-1)
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index ed89592e8..86ba0451c 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -190,6 +190,7 @@ type TranslationSet struct {
 	PickAllHunks                        string
 	ViewMergeRebaseOptions              string
 	NotMergingOrRebasing                string
+	AlreadyRebasing                     string
 	RecentRepos                         string
 	MergeOptionsTitle                   string
 	RebaseOptionsTitle                  string
@@ -839,6 +840,7 @@ func EnglishTranslationSet() TranslationSet {
 		PickAllHunks:                        "pick all hunks",
 		ViewMergeRebaseOptions:              "view merge/rebase options",
 		NotMergingOrRebasing:                "You are currently neither rebasing nor merging",
+		AlreadyRebasing:                     "Can't perform this action during a rebase",
 		RecentRepos:                         "recent repositories",
 		MergeOptionsTitle:                   "Merge Options",
 		RebaseOptionsTitle:                  "Rebase Options",
diff --git a/pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go b/pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go
new file mode 100644
index 000000000..22b127d36
--- /dev/null
+++ b/pkg/integration/tests/interactive_rebase/edit_non_todo_commit_during_rebase.go
@@ -0,0 +1,37 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var EditNonTodoCommitDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Tries to edit a non-todo commit while already rebasing, resulting in an error message",
+	ExtraCmdArgs: "",
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			CreateNCommits(2)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("commit 02").IsSelected(),
+				Contains("commit 01"),
+			).
+			Press(keys.Universal.Edit).
+			Lines(
+				Contains("<-- YOU ARE HERE --- commit 02"),
+				Contains("commit 01"),
+			).
+			NavigateToLine(Contains("commit 01")).
+			Press(keys.Universal.Edit)
+
+		t.ExpectPopup().Alert().
+			Title(Equals("Error")).
+			Content(Contains("Can't perform this action during a rebase")).
+			Confirm()
+	},
+})
diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go
index f0ec4b78c..ec8860057 100644
--- a/pkg/integration/tests/test_list.go
+++ b/pkg/integration/tests/test_list.go
@@ -83,6 +83,7 @@ var tests = []*components.IntegrationTest{
 	interactive_rebase.AmendFirstCommit,
 	interactive_rebase.AmendMerge,
 	interactive_rebase.EditFirstCommit,
+	interactive_rebase.EditNonTodoCommitDuringRebase,
 	interactive_rebase.FixupFirstCommit,
 	interactive_rebase.FixupSecondCommit,
 	interactive_rebase.Move,