five

In 67b8ef449c we changed the "edit" command to insert a "break" after the
selected commit, rather than setting the selected todo to "edit". The reason for
doing this was that it now works for merge commits too.

Back then, I claimed "In most cases the behavior is exactly the same as before."
Unfortunately that's not true, there are two reasons why the previous behavior
was better (both are demonstrated by tests earlier in this branch):
- when editing the last commit of a branch in the middle of a stack of branches,
  we are now missing the update-ref todo after it, which means that amending the
  commit breaks the stack
- it breaks auto-amending (see the added test earlier in this branch for an
  explanation)

For these reasons, we are going back to the previous approach of setting the
selected commit to "edit" whenever possible, i.e. unless it's a merge commit.

The only scenario where this could still be a problem is when you have a stack
of branches, and the last commit of one of the branches in the stack is a merge
commit, and you try to edit that. In my experience with stacked branches this is
very unlikely, in almost all cases my stacked branches are linear.

haha

@@ -0,0 +1 @@
+lolll

lol

@@ -0,0 +1 @@
+lol

pkg/commands/git_commands/rebase.go

@@ -145,11 +145,11 @@ func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, startIdx
 
 	baseHashOrRoot := getBaseHashOrRoot(commits, baseIndex)
 
-	changes := lo.Map(commits[startIdx:endIdx+1], func(commit *models.Commit, _ int) daemon.ChangeTodoAction {
+	changes := lo.FilterMap(commits[startIdx:endIdx+1], func(commit *models.Commit, _ int) (daemon.ChangeTodoAction, bool) {
 		return daemon.ChangeTodoAction{
 			Hash:      commit.Hash,
 			NewAction: action,
-		}
+		}, !commit.IsMerge()
 	})
 
 	self.os.LogCommand(logTodoChanges(changes), false)

pkg/gui/controllers/local_commits_controller.go

@@ -9,6 +9,7 @@ import (
 	"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/context/traits"
 	"github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers"
 	"github.com/jesseduffield/lazygit/pkg/gui/keybindings"
 	"github.com/jesseduffield/lazygit/pkg/gui/style"
@@ -115,7 +116,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
 		},
 		{
 			Key:     opts.GetKey(editCommitKey),
-			Handler: self.withItems(self.edit),
+			Handler: self.withItemsRange(self.edit),
 			GetDisabledReason: self.require(
 				self.itemRangeSelected(self.midRebaseCommandEnabled),
 			),
@@ -510,11 +511,25 @@ func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, start
 	return nil
 }
 
-func (self *LocalCommitsController) edit(selectedCommits []*models.Commit) error {
+func (self *LocalCommitsController) edit(selectedCommits []*models.Commit, startIdx int, endIdx int) error {
 	if self.isRebasing() {
 		return self.updateTodos(todo.Edit, selectedCommits)
 	}
 
+	commits := self.c.Model().Commits
+	if !commits[endIdx].IsMerge() {
+		selectionRangeAndMode := self.getSelectionRangeAndMode()
+		err := self.c.Git().Rebase.InteractiveRebase(commits, startIdx, endIdx, todo.Edit)
+		return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(
+			err,
+			types.RefreshOptions{
+				Mode: types.BLOCK_UI, Then: func() error {
+					self.restoreSelectionRangeAndMode(selectionRangeAndMode)
+					return nil
+				},
+			})
+	}
+
 	return self.startInteractiveRebaseWithEdit(selectedCommits)
 }
 
@@ -532,10 +547,7 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit(
 ) error {
 	return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func(gocui.Task) error {
 		self.c.LogAction(self.c.Tr.Actions.EditCommit)
-		selectedIdx, rangeStartIdx, rangeSelectMode := self.context().GetSelectionRangeAndMode()
-		commits := self.c.Model().Commits
-		selectedHash := commits[selectedIdx].Hash
-		rangeStartHash := commits[rangeStartIdx].Hash
+		selectionRangeAndMode := self.getSelectionRangeAndMode()
 		err := self.c.Git().Rebase.EditRebase(commitsToEdit[len(commitsToEdit)-1].Hash)
 		return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(
 			err,
@@ -554,23 +566,41 @@ func (self *LocalCommitsController) startInteractiveRebaseWithEdit(
 					}
 				}
 
-				// We need to select the same commit range again because after starting a rebase,
-				// new lines can be added for update-ref commands in the TODO file, due to
-				// stacked branches. So the selected commits may be in different positions in the list.
-				_, newSelectedIdx, ok1 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool {
-					return c.Hash == selectedHash
-				})
-				_, newRangeStartIdx, ok2 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool {
-					return c.Hash == rangeStartHash
-				})
-				if ok1 && ok2 {
-					self.context().SetSelectionRangeAndMode(newSelectedIdx, newRangeStartIdx, rangeSelectMode)
-				}
+				self.restoreSelectionRangeAndMode(selectionRangeAndMode)
 				return nil
 			}})
 	})
 }
 
+type SelectionRangeAndMode struct {
+	selectedHash   string
+	rangeStartHash string
+	mode           traits.RangeSelectMode
+}
+
+func (self *LocalCommitsController) getSelectionRangeAndMode() SelectionRangeAndMode {
+	selectedIdx, rangeStartIdx, rangeSelectMode := self.context().GetSelectionRangeAndMode()
+	commits := self.c.Model().Commits
+	selectedHash := commits[selectedIdx].Hash
+	rangeStartHash := commits[rangeStartIdx].Hash
+	return SelectionRangeAndMode{selectedHash, rangeStartHash, rangeSelectMode}
+}
+
+func (self *LocalCommitsController) restoreSelectionRangeAndMode(selectionRangeAndMode SelectionRangeAndMode) {
+	// We need to select the same commit range again because after starting a rebase,
+	// new lines can be added for update-ref commands in the TODO file, due to
+	// stacked branches. So the selected commits may be in different positions in the list.
+	_, newSelectedIdx, ok1 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool {
+		return c.Hash == selectionRangeAndMode.selectedHash
+	})
+	_, newRangeStartIdx, ok2 := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool {
+		return c.Hash == selectionRangeAndMode.rangeStartHash
+	})
+	if ok1 && ok2 {
+		self.context().SetSelectionRangeAndMode(newSelectedIdx, newRangeStartIdx, selectionRangeAndMode.mode)
+	}
+}
+
 func (self *LocalCommitsController) findCommitForQuickStartInteractiveRebase() (*models.Commit, error) {
 	commit, index, ok := lo.FindIndexOf(self.c.Model().Commits, func(c *models.Commit) bool {
 		return c.IsMerge() || c.Status == models.StatusMerged

pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go

@@ -38,7 +38,6 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{
 			).
 			NavigateToLine(Contains("commit 02")).
 			Press(keys.Universal.Edit).
-			Focus().
 			Lines(
 				Contains("pick").Contains("CI commit 07"),
 				Contains("pick").Contains("CI commit 06"),

pkg/integration/tests/interactive_rebase/edit_and_auto_amend.go

@@ -0,0 +1,55 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var EditAndAutoAmend = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Edit a commit, make a change and stage it, then continue the rebase to auto-amend the commit",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			CreateNCommits(3)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("commit 03"),
+				Contains("commit 02"),
+				Contains("commit 01"),
+			).
+			NavigateToLine(Contains("commit 02")).
+			Press(keys.Universal.Edit).
+			Lines(
+				Contains("commit 03"),
+				MatchesRegexp("YOU ARE HERE.*commit 02").IsSelected(),
+				Contains("commit 01"),
+			)
+
+		t.Shell().CreateFile("fixup-file", "fixup content")
+		t.Views().Files().
+			Focus().
+			Press(keys.Files.RefreshFiles).
+			Lines(
+				Contains("??").Contains("fixup-file").IsSelected(),
+			).
+			PressPrimaryAction()
+
+		t.Common().ContinueRebase()
+
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("commit 03"),
+				Contains("commit 02").IsSelected(),
+				Contains("commit 01"),
+			)
+
+		t.Views().Main().
+			Content(Contains("fixup content"))
+	},
+})

pkg/integration/tests/interactive_rebase/edit_last_commit_of_stacked_branch.go

@@ -0,0 +1,74 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var EditLastCommitOfStackedBranch = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Edit and amend the last commit of a branch in a stack of branches, and ensure that it doesn't break the stack",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	GitVersion:   AtLeast("2.38.0"),
+	SetupConfig: func(config *config.AppConfig) {
+		config.GetUserConfig().Git.MainBranches = []string{"master"}
+		config.GetAppState().GitLogShowGraph = "never"
+	},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			CreateNCommits(1).
+			NewBranch("branch1").
+			CreateNCommitsStartingAt(2, 2).
+			NewBranch("branch2").
+			CreateNCommitsStartingAt(2, 4)
+
+		shell.SetConfig("rebase.updateRefs", "true")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("CI commit 05").IsSelected(),
+				Contains("CI commit 04"),
+				Contains("CI * commit 03"),
+				Contains("CI commit 02"),
+				Contains("CI commit 01"),
+			).
+			NavigateToLine(Contains("commit 03")).
+			Press(keys.Universal.Edit).
+			Lines(
+				Contains("pick").Contains("CI commit 05"),
+				Contains("pick").Contains("CI commit 04"),
+				Contains("update-ref").Contains("branch1"),
+				Contains("<-- YOU ARE HERE --- * commit 03").IsSelected(),
+				Contains("CI commit 02"),
+				Contains("CI commit 01"),
+			)
+
+		t.Shell().CreateFile("fixup-file", "fixup content")
+		t.Views().Files().
+			Focus().
+			Press(keys.Files.RefreshFiles).
+			Lines(
+				Contains("??").Contains("fixup-file").IsSelected(),
+			).
+			PressPrimaryAction().
+			Press(keys.Files.AmendLastCommit)
+		t.ExpectPopup().Confirmation().
+			Title(Equals("Amend last commit")).
+			Content(Contains("Are you sure you want to amend last commit?")).
+			Confirm()
+
+		t.Common().ContinueRebase()
+
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("CI commit 05"),
+				Contains("CI commit 04"),
+				Contains("CI * commit 03"),
+				Contains("CI commit 02"),
+				Contains("CI commit 01"),
+			)
+	},
+})

pkg/integration/tests/interactive_rebase/edit_range_select_down_to_merge_outside_rebase.go

@@ -0,0 +1,43 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+	"github.com/jesseduffield/lazygit/pkg/integration/tests/shared"
+)
+
+var EditRangeSelectDownToMergeOutsideRebase = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Select a range of commits (the last one being a merge commit) to edit outside of a rebase",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	GitVersion:   AtLeast("2.22.0"), // first version that supports the --rebase-merges option
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shared.CreateMergeCommit(shell)
+		shell.CreateNCommits(2)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			TopLines(
+				Contains("CI ◯ commit 02").IsSelected(),
+				Contains("CI ◯ commit 01"),
+				Contains("Merge branch 'second-change-branch' into first-change-branch"),
+			).
+			Press(keys.Universal.RangeSelectDown).
+			Press(keys.Universal.RangeSelectDown).
+			Press(keys.Universal.Edit).
+			Lines(
+				Contains("edit  CI commit 02").IsSelected(),
+				Contains("edit  CI commit 01").IsSelected(),
+				Contains("      CI ⏣─╮ <-- YOU ARE HERE --- Merge branch 'second-change-branch' into first-change-branch").IsSelected(),
+				Contains("      CI │ ◯ * second-change-branch unrelated change"),
+				Contains("      CI │ ◯ second change"),
+				Contains("      CI ◯ │ first change"),
+				Contains("      CI ◯─╯ * original"),
+				Contains("      CI ◯ three"),
+				Contains("      CI ◯ two"),
+				Contains("      CI ◯ one"),
+			)
+	},
+})

pkg/integration/tests/test_list.go

@@ -209,8 +209,11 @@ var tests = []*components.IntegrationTest{
 	interactive_rebase.DropCommitInCopiedBranchWithUpdateRef,
 	interactive_rebase.DropTodoCommitWithUpdateRef,
 	interactive_rebase.DropWithCustomCommentChar,
+	interactive_rebase.EditAndAutoAmend,
 	interactive_rebase.EditFirstCommit,
+	interactive_rebase.EditLastCommitOfStackedBranch,
 	interactive_rebase.EditNonTodoCommitDuringRebase,
+	interactive_rebase.EditRangeSelectDownToMergeOutsideRebase,
 	interactive_rebase.EditRangeSelectOutsideRebase,
 	interactive_rebase.EditTheConflCommit,
 	interactive_rebase.FixupFirstCommit,