Merge pull request #2485 from stefanhaller/interactive-rebase-improvements

pkg/commands/git_commands/commit_loader.go

@@ -113,7 +113,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod
 	// chances are we have as many commits as last time so we'll set the capacity to be the old length
 	result := make([]*models.Commit, 0, len(commits))
 	for i, commit := range commits {
-		if commit.Status != "rebasing" { // removing the existing rebase commits so we can add the refreshed ones
+		if !commit.IsTODO() { // removing the existing rebase commits so we can add the refreshed ones
 			result = append(result, commits[i:]...)
 			break
 		}

pkg/commands/git_commands/rebase.go

@@ -34,7 +34,7 @@ func NewRebaseCommands(
 }
 
 func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, message string) error {
-	if index == 0 {
+	if models.IsHeadCommit(commits, index) {
 		// we've selected the top commit so no rebase is required
 		return self.commit.RewordLastCommit(message)
 	}

pkg/commands/models/commit.go

@@ -65,3 +65,7 @@ func (c *Commit) IsMerge() bool {
 func (c *Commit) IsTODO() bool {
 	return c.Action != ""
 }
+
+func IsHeadCommit(commits []*Commit, index int) bool {
+	return !commits[index].IsTODO() && (index == 0 || commits[index-1].IsTODO())
+}

pkg/gui/controllers.go

@@ -30,6 +30,7 @@ func (gui *Gui) resetControllers() {
 	getSavedCommitMessage := func() string {
 		return gui.State.savedCommitMessage
 	}
+	gpgHelper := helpers.NewGpgHelper(helperCommon, gui.os, gui.git)
 	gui.helpers = &helpers.Helpers{
 		Refs:           refsHelper,
 		Host:           helpers.NewHostHelper(helperCommon, gui.git),
@@ -39,7 +40,7 @@ func (gui *Gui) resetControllers() {
 		Files:          helpers.NewFilesHelper(helperCommon, gui.git, osCommand),
 		WorkingTree:    helpers.NewWorkingTreeHelper(helperCommon, gui.git, gui.State.Contexts, refsHelper, model, setCommitMessage, getSavedCommitMessage),
 		Tags:           helpers.NewTagsHelper(helperCommon, gui.git),
-		GPG:            helpers.NewGpgHelper(helperCommon, gui.os, gui.git),
+		GPG:            gpgHelper,
 		MergeAndRebase: rebaseHelper,
 		MergeConflicts: helpers.NewMergeConflictsHelper(helperCommon, gui.State.Contexts, gui.git),
 		CherryPick: helpers.NewCherryPickHelper(
@@ -49,7 +50,8 @@ func (gui *Gui) resetControllers() {
 			func() *cherrypicking.CherryPicking { return gui.State.Modes.CherryPicking },
 			rebaseHelper,
 		),
-		Upstream: helpers.NewUpstreamHelper(helperCommon, model, suggestionsHelper.GetRemoteBranchesSuggestionsFunc),
+		Upstream:    helpers.NewUpstreamHelper(helperCommon, model, suggestionsHelper.GetRemoteBranchesSuggestionsFunc),
+		AmendHelper: helpers.NewAmendHelper(helperCommon, gui.git, gpgHelper),
 	}
 
 	gui.CustomCommandsClient = custom_commands.NewClient(

pkg/gui/controllers/files_controller.go

@@ -567,15 +567,7 @@ func (self *FilesController) handleAmendCommitPress() error {
 		return self.c.ErrorMsg(self.c.Tr.NoCommitToAmend)
 	}
 
-	return self.c.Confirm(types.ConfirmOpts{
-		Title:  self.c.Tr.AmendLastCommitTitle,
-		Prompt: self.c.Tr.SureToAmend,
-		HandleConfirm: func() error {
-			cmdObj := self.git.Commit.AmendHeadCmdObj()
-			self.c.LogAction(self.c.Tr.Actions.AmendCommit)
-			return self.helpers.GPG.WithGpgHandling(cmdObj, self.c.Tr.AmendingStatus, nil)
-		},
-	})
+	return self.helpers.AmendHelper.AmendHead()
 }
 
 func (self *FilesController) handleStatusFilterPressed() error {

pkg/gui/controllers/helpers/amend_helper.go

@@ -0,0 +1,36 @@
+package helpers
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/commands"
+	"github.com/jesseduffield/lazygit/pkg/gui/types"
+)
+
+type AmendHelper struct {
+	c   *types.HelperCommon
+	git *commands.GitCommand
+	gpg *GpgHelper
+}
+
+func NewAmendHelper(
+	c *types.HelperCommon,
+	git *commands.GitCommand,
+	gpg *GpgHelper,
+) *AmendHelper {
+	return &AmendHelper{
+		c:   c,
+		git: git,
+		gpg: gpg,
+	}
+}
+
+func (self *AmendHelper) AmendHead() error {
+	return self.c.Confirm(types.ConfirmOpts{
+		Title:  self.c.Tr.AmendLastCommitTitle,
+		Prompt: self.c.Tr.SureToAmend,
+		HandleConfirm: func() error {
+			cmdObj := self.git.Commit.AmendHeadCmdObj()
+			self.c.LogAction(self.c.Tr.Actions.AmendCommit)
+			return self.gpg.WithGpgHandling(cmdObj, self.c.Tr.AmendingStatus, nil)
+		},
+	})
+}

pkg/gui/controllers/helpers/helpers.go

@@ -14,6 +14,7 @@ type Helpers struct {
 	PatchBuilding  *PatchBuildingHelper
 	GPG            *GpgHelper
 	Upstream       *UpstreamHelper
+	AmendHelper    *AmendHelper
 }
 
 func NewStubHelpers() *Helpers {
@@ -31,5 +32,6 @@ func NewStubHelpers() *Helpers {
 		PatchBuilding:  &PatchBuildingHelper{},
 		GPG:            &GpgHelper{},
 		Upstream:       &UpstreamHelper{},
+		AmendHelper:    &AmendHelper{},
 	}
 }

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"
@@ -229,7 +230,7 @@ func (self *LocalCommitsController) reword(commit *models.Commit) error {
 func (self *LocalCommitsController) doRewordEditor() error {
 	self.c.LogAction(self.c.Tr.Actions.RewordCommit)
 
-	if self.context().GetSelectedLineIdx() == 0 {
+	if self.isHeadCommit() {
 		return self.c.RunSubprocessAndRefresh(self.os.Cmd.New("git commit --allow-empty --amend --only"))
 	}
 
@@ -326,7 +327,15 @@ func (self *LocalCommitsController) interactiveRebase(action string) error {
 // commit meaning you are trying to edit the todo file rather than actually
 // 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.Status != "rebasing" {
+	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
 	}
 
@@ -364,8 +373,8 @@ func (self *LocalCommitsController) moveDown(commit *models.Commit) error {
 		return nil
 	}
 
-	if commit.Status == "rebasing" {
-		if commits[index+1].Status != "rebasing" {
+	if commit.IsTODO() {
+		if !commits[index+1].IsTODO() {
 			return 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)
@@ -399,7 +412,7 @@ func (self *LocalCommitsController) moveUp(commit *models.Commit) error {
 		return nil
 	}
 
-	if commit.Status == "rebasing" {
+	if commit.IsTODO() {
 		// logging directly here because MoveTodoDown doesn't have enough information
 		// to provide a useful log
 		self.c.LogAction(self.c.Tr.Actions.MoveCommitUp)
@@ -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)
@@ -428,6 +445,17 @@ func (self *LocalCommitsController) moveUp(commit *models.Commit) error {
 }
 
 func (self *LocalCommitsController) amendTo(commit *models.Commit) error {
+	if self.isHeadCommit() {
+		if err := self.helpers.AmendHelper.AmendHead(); err != nil {
+			return err
+		}
+		return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC})
+	}
+
+	if self.git.Status.WorkingTreeState() != enums.REBASE_MODE_NONE {
+		return self.c.ErrorMsg(self.c.Tr.AlreadyRebasing)
+	}
+
 	return self.c.Confirm(types.ConfirmOpts{
 		Title:  self.c.Tr.AmendCommitTitle,
 		Prompt: self.c.Tr.AmendCommitPrompt,
@@ -728,3 +756,7 @@ func (self *LocalCommitsController) context() *context.LocalCommitsContext {
 func (self *LocalCommitsController) paste() error {
 	return self.helpers.CherryPick.Paste()
 }
+
+func (self *LocalCommitsController) isHeadCommit() bool {
+	return models.IsHeadCommit(self.model.Commits, self.context().GetSelectedLineIdx())
+}

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",

pkg/integration/tests/commit/amend.go

@@ -0,0 +1,41 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var Amend = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Amends the last commit from the files panel",
+	ExtraCmdArgs: "",
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateFileAndAdd("myfile", "myfile content\n")
+		shell.Commit("first commit")
+		shell.UpdateFileAndAdd("myfile", "myfile content\nmore content\n")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Lines(
+				Contains("first commit"),
+			)
+
+		t.Views().Files().
+			Focus().
+			Press(keys.Commits.AmendToCommit)
+
+		t.ExpectPopup().Confirmation().Title(
+			Equals("Amend Last Commit")).
+			Content(Contains("Are you sure you want to amend last commit?")).
+			Confirm()
+
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("first commit"),
+			)
+
+		t.Views().Main().Content(Contains("+myfile content").Contains("+more content"))
+	},
+})

pkg/integration/tests/interactive_rebase/amend_head_commit_during_rebase.go

@@ -0,0 +1,59 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var AmendHeadCommitDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Amends the current head commit from the commits panel during a rebase.",
+	ExtraCmdArgs: "",
+	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"),
+				Contains("<-- 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.Views().Commits().
+			Focus().
+			Press(keys.Commits.AmendToCommit).
+			Tap(func() {
+				t.ExpectPopup().Confirmation().
+					Title(Equals("Amend Last Commit")).
+					Content(Contains("Are you sure you want to amend last commit?")).
+					Confirm()
+			}).
+			Lines(
+				Contains("commit 03"),
+				Contains("<-- YOU ARE HERE --- commit 02").IsSelected(),
+				Contains("commit 01"),
+			)
+
+		t.Views().Main().
+			Content(Contains("fixup content"))
+	},
+})

pkg/integration/tests/interactive_rebase/amend_merge.go

@@ -42,8 +42,8 @@ var AmendMerge = NewIntegrationTest(NewIntegrationTestArgs{
 			Press(keys.Commits.AmendToCommit)
 
 		t.ExpectPopup().Confirmation().
-			Title(Equals("Amend Commit")).
-			Content(Contains("Are you sure you want to amend this commit with your staged files?")).
+			Title(Equals("Amend Last Commit")).
+			Content(Contains("Are you sure you want to amend last commit?")).
 			Confirm()
 
 		// assuring we haven't added a brand new commit

pkg/integration/tests/interactive_rebase/amend_non_head_commit_during_rebase.go

@@ -0,0 +1,43 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var AmendNonHeadCommitDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Tries to amend a commit that is not the head while already rebasing, resulting in an error message",
+	ExtraCmdArgs: "",
+	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"),
+				Contains("<-- YOU ARE HERE --- commit 02"),
+				Contains("commit 01"),
+			)
+
+		for _, commit := range []string{"commit 01", "commit 03"} {
+			t.Views().Commits().
+				NavigateToLine(Contains(commit)).
+				Press(keys.Commits.AmendToCommit)
+
+			t.ExpectPopup().Alert().
+				Title(Equals("Error")).
+				Content(Contains("Can't perform this action during a rebase")).
+				Confirm()
+		}
+	},
+})

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()
+	},
+})

pkg/integration/tests/interactive_rebase/reword_you_are_here_commit.go

@@ -0,0 +1,47 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var RewordYouAreHereCommit = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Rewords the current HEAD commit in an interactive rebase",
+	ExtraCmdArgs: "",
+	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").IsSelected(),
+				Contains("commit 02"),
+				Contains("commit 01"),
+			).
+			NavigateToLine(Contains("commit 02")).
+			Press(keys.Universal.Edit).
+			Lines(
+				Contains("commit 03"),
+				Contains("<-- YOU ARE HERE --- commit 02").IsSelected(),
+				Contains("commit 01"),
+			).
+			Press(keys.Commits.RenameCommit).
+			Tap(func() {
+				t.ExpectPopup().Prompt().
+					Title(Equals("reword commit")).
+					InitialText(Equals("commit 02")).
+					Clear().
+					Type("renamed 02").
+					Confirm()
+			}).
+			Lines(
+				Contains("commit 03"),
+				Contains("<-- YOU ARE HERE --- renamed 02").IsSelected(),
+				Contains("commit 01"),
+			)
+	},
+})

pkg/integration/tests/interactive_rebase/reword_you_are_here_commit_with_editor.go

@@ -0,0 +1,47 @@
+package interactive_rebase
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var RewordYouAreHereCommitWithEditor = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Rewords the current HEAD commit in an interactive rebase with editor",
+	ExtraCmdArgs: "",
+	Skip:         false,
+	SetupConfig: func(config *config.AppConfig) {
+	},
+	SetupRepo: func(shell *Shell) {
+		shell.
+			CreateNCommits(3).
+			SetConfig("core.editor", "sh -c 'echo renamed 02 >.git/COMMIT_EDITMSG'")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("commit 03").IsSelected(),
+				Contains("commit 02"),
+				Contains("commit 01"),
+			).
+			NavigateToLine(Contains("commit 02")).
+			Press(keys.Universal.Edit).
+			Lines(
+				Contains("commit 03"),
+				Contains("<-- YOU ARE HERE --- commit 02").IsSelected(),
+				Contains("commit 01"),
+			).
+			Press(keys.Commits.RenameCommitWithEditor).
+			Tap(func() {
+				t.ExpectPopup().Confirmation().
+					Title(Equals("Reword in editor")).
+					Content(Contains("Are you sure you want to reword this commit in your editor?")).
+					Confirm()
+			}).
+			Lines(
+				Contains("commit 03"),
+				Contains("<-- YOU ARE HERE --- renamed 02").IsSelected(),
+				Contains("commit 01"),
+			)
+	},
+})

pkg/integration/tests/test_list.go

@@ -44,6 +44,7 @@ var tests = []*components.IntegrationTest{
 	branch.Suggestions,
 	cherry_pick.CherryPick,
 	cherry_pick.CherryPickConflicts,
+	commit.Amend,
 	commit.Commit,
 	commit.CommitMultiline,
 	commit.CreateTag,
@@ -81,8 +82,11 @@ var tests = []*components.IntegrationTest{
 	filter_by_path.SelectFile,
 	filter_by_path.TypeFile,
 	interactive_rebase.AmendFirstCommit,
+	interactive_rebase.AmendHeadCommitDuringRebase,
 	interactive_rebase.AmendMerge,
+	interactive_rebase.AmendNonHeadCommitDuringRebase,
 	interactive_rebase.EditFirstCommit,
+	interactive_rebase.EditNonTodoCommitDuringRebase,
 	interactive_rebase.FixupFirstCommit,
 	interactive_rebase.FixupSecondCommit,
 	interactive_rebase.Move,
@@ -90,6 +94,8 @@ var tests = []*components.IntegrationTest{
 	interactive_rebase.Rebase,
 	interactive_rebase.RewordFirstCommit,
 	interactive_rebase.RewordLastCommit,
+	interactive_rebase.RewordYouAreHereCommit,
+	interactive_rebase.RewordYouAreHereCommitWithEditor,
 	interactive_rebase.SquashDownFirstCommit,
 	interactive_rebase.SquashDownSecondCommit,
 	interactive_rebase.SquashFixupsAboveFirstCommit,

test/default_test_config/config.yml

@@ -1,6 +1,8 @@
 # This config is used in our integration tests. If we want to modify this for a specific test, you can do so in the SetupConfig function
 
 disableStartupPopups: true
+promptToReturnFromSubprocess: false
+
 gui:
   theme:
     activeBorderColor: