feat: When creating a fixup commit on an existing fixup commit, offer to use the base commit

pkg/gui/controllers/helpers/fixup_helper.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"regexp"
+	"slices"
 	"strings"
 
 	"github.com/jesseduffield/generics/set"
@@ -407,6 +408,25 @@ func IsFixupCommit(subject string) (string, bool) {
 		}
 		return subject, true
 	}
-
 	return subject, false
 }
+
+// FindFixupBaseCommit will search commits (oldest first) to find a matching
+// commit for the given subject. It expects the subject to be already trimmed,
+// as if it were returned from [IsFixupCommit].
+//
+// It also returns whether the commit message has an exact match to the target base commit.
+// If no matches are found, it returns nil.
+func FindFixupBaseCommit(subject string, commits []*models.Commit) (model *models.Commit) {
+	for _, commit := range slices.Backward(commits) {
+		candidateSubject, _, _ := strings.Cut(commit.Name, "\n")
+
+		if strings.HasPrefix(commit.Hash(), subject) {
+			return commit
+		}
+		if strings.HasPrefix(candidateSubject, subject) {
+			return commit
+		}
+	}
+	return nil
+}

pkg/gui/controllers/helpers/fixup_helper_test.go

@@ -205,6 +205,100 @@ func TestFixupHelper_IsFixupCommit(t *testing.T) {
 	}
 }
 
+func TestFixupHelper_FindFixupBaseCommit(t *testing.T) {
+	hashPool := &utils.StringPool{}
+
+	type commitDesc struct {
+		Hash string
+		Name string
+	}
+
+	scenarios := []struct {
+		subject    string
+		commits    []commitDesc
+		targetHash string
+	}{
+		{
+			subject: "fixup! Simple feature",
+			commits: []commitDesc{
+				{Hash: "abc123", Name: "Simple feature"},
+			},
+			targetHash: "abc123",
+		},
+		{
+			subject: "fixup! abc123",
+			commits: []commitDesc{
+				{Hash: "abc123", Name: "Something else"},
+			},
+			targetHash: "abc123",
+		},
+		{
+			subject: "fixup! Partial match",
+			commits: []commitDesc{
+				{Hash: "def456", Name: "Partial match for this commit"},
+			},
+			targetHash: "def456",
+		},
+		{
+			subject: "fixup! Multiple matches",
+			commits: []commitDesc{
+				{Hash: "111111", Name: "Multiple matches"},
+				{Hash: "222222", Name: "Multiple matches"},
+			},
+			targetHash: "222222",
+		},
+		{
+			subject: "fixup! Multiline",
+			commits: []commitDesc{
+				{Hash: "ghi789", Name: "Multiline\n\nDetailed description here"},
+			},
+			targetHash: "ghi789",
+		},
+		{
+			subject: "fixup! No match",
+			commits: []commitDesc{
+				{Hash: "jkl012", Name: "Unrelated work"},
+			},
+			targetHash: "",
+		},
+		{
+			subject: "fixup! 7777",
+			commits: []commitDesc{
+				{Hash: "77778888", Name: "Match by partial hash"},
+			},
+			targetHash: "77778888",
+		},
+		{
+			subject: "fixup! Feature A",
+			commits: []commitDesc{
+				{Hash: "abc123", Name: "Feature A"},
+				{Hash: "def456", Name: "Unrelated"},
+				{Hash: "ghi789", Name: "Feature A"},
+			},
+			targetHash: "ghi789",
+		},
+	}
+
+	makeCommitFromDesc := func(desc commitDesc, _ int) *models.Commit {
+		return models.NewCommit(hashPool, models.NewCommitOpts{Hash: desc.Hash, Name: desc.Name})
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.subject, func(t *testing.T) {
+			trimmedSubject, isFixupCommit := IsFixupCommit(s.subject)
+			assert.Equal(t, true, isFixupCommit)
+
+			commits := lo.Map(s.commits, makeCommitFromDesc)
+			found := FindFixupBaseCommit(trimmedSubject, commits)
+			if found == nil {
+				assert.Equal(t, s.targetHash, "")
+			} else {
+				assert.Equal(t, s.targetHash, found.Hash())
+			}
+		})
+	}
+}
+
 func TestFixupHelper_removeFixupCommits(t *testing.T) {
 	hashPool := &utils.StringPool{}
 

pkg/gui/controllers/local_commits_controller.go

@@ -1,6 +1,8 @@
 package controllers
 
 import (
+	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/go-errors/errors"
@@ -969,13 +971,113 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err
 		}
 	}
 
+	var baseWithinRebase *models.Commit
+	var baseGlobal *models.Commit
+	if baseName, ok := helpers.IsFixupCommit(commit.Name); ok {
+		commits := self.c.Model().Commits
+		currentIdx := slices.Index(commits, commit)
+		commits = commits[currentIdx:]
+
+		_, rebaseStartIdx, err := self.findCommitForSquashFixupsInCurrentBranch()
+		if err != nil {
+			rebaseStartIdx = -1
+		}
+
+		if rebaseStartIdx >= 0 {
+			baseWithinRebase = helpers.FindFixupBaseCommit(baseName, commits[:rebaseStartIdx+1])
+		}
+
+		// Use a more strict criteria for finding a base commit candidate when searching
+		// all of the commits when searching all of the commits.
+		//
+		// Search newest -> oldest where the subject is an exact match only
+		baseWithinRebase = helpers.FindFixupBaseCommit(baseName, commits)
+		for _, c := range commits {
+			subject, _, _ := strings.Cut(c.Name, "\n")
+			if subject == baseName {
+				baseGlobal = c
+			}
+		}
+	}
+
+	withMaybeBase := func(f func(commit *models.Commit) error) func() error {
+		if baseWithinRebase == nil && baseGlobal == nil {
+			return func() error { return f(commit) }
+		}
+
+		// We are trying to fixup a fixup/amend/reword commit. Likely the user just
+		// wants to apply the action to the base commit.
+
+		var withinSummary string
+		withinDisabled := &types.DisabledReason{
+			Text:             self.c.Tr.NoBaseCommitsFound,
+			ShowErrorInPanel: true,
+		}
+		if baseWithinRebase != nil {
+			s, _, _ := strings.Cut(baseWithinRebase.Name, "\n")
+			withinSummary = fmt.Sprintf("%s %s",
+				style.FgYellow.Sprint(baseWithinRebase.ShortRefName()),
+				utils.TruncateWithEllipsis(s, 80),
+			)
+			withinDisabled = nil
+		}
+
+		var globalSummary string
+		globalDisabled := &types.DisabledReason{
+			Text:             self.c.Tr.NoBaseCommitsFound,
+			ShowErrorInPanel: true,
+		}
+		if baseGlobal != nil {
+			s, _, _ := strings.Cut(baseGlobal.Name, "\n")
+			globalSummary = fmt.Sprintf("%s %s",
+				style.FgYellow.Sprint(baseGlobal.ShortRefName()),
+				utils.TruncateWithEllipsis(s, 80),
+			)
+			globalDisabled = nil
+		}
+
+		summary, _, _ := strings.Cut(commit.Name, "\n")
+		summary = fmt.Sprintf("%s %s",
+			style.FgYellow.Sprint(commit.ShortRefName()),
+			utils.TruncateWithEllipsis(summary, 80),
+		)
+
+		return func() error {
+			return self.c.Menu(types.CreateMenuOptions{
+				Title: self.c.Tr.FixupMenu_SelectCommit,
+				Items: []*types.MenuItem{
+					{
+						Label:          self.c.Tr.FixupMenu_SelectBase,
+						Key:            'b',
+						OnPress:        func() error { return f(baseWithinRebase) },
+						Tooltip:        withinSummary,
+						DisabledReason: withinDisabled,
+					},
+					{
+						Label:          self.c.Tr.FixupMenu_SelectGlobalBase,
+						Key:            'g',
+						OnPress:        func() error { return f(baseGlobal) },
+						Tooltip:        globalSummary,
+						DisabledReason: globalDisabled,
+					},
+					{
+						Label:   self.c.Tr.FixUpMenu_SelectSelected,
+						Key:     's',
+						OnPress: func() error { return f(commit) },
+						Tooltip: summary,
+					},
+				},
+			})
+		}
+	}
+
 	return self.c.Menu(types.CreateMenuOptions{
 		Title: self.c.Tr.CreateFixupCommit,
 		Items: []*types.MenuItem{
 			{
 				Label: self.c.Tr.FixupMenu_Fixup,
 				Key:   'f',
-				OnPress: func() error {
+				OnPress: withMaybeBase(func(commit *models.Commit) error {
 					return self.c.Helpers().WorkingTree.WithEnsureCommittableFiles(func() error {
 						self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit)
 						return self.c.WithWaitingStatusSync(self.c.Tr.CreatingFixupCommitStatus, func() error {
@@ -992,18 +1094,18 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err
 							return nil
 						})
 					})
-				},
+				}),
 				DisabledReason: disabledReasonWhenFilesAreNeeded,
 				Tooltip:        self.c.Tr.FixupMenu_FixupTooltip,
 			},
 			{
 				Label: self.c.Tr.FixupMenu_AmendWithChanges,
 				Key:   'a',
-				OnPress: func() error {
+				OnPress: withMaybeBase(func(commit *models.Commit) error {
 					return self.c.Helpers().WorkingTree.WithEnsureCommittableFiles(func() error {
 						return self.createAmendCommit(commit, true)
 					})
-				},
+				}),
 				DisabledReason: disabledReasonWhenFilesAreNeeded,
 				Tooltip:        self.c.Tr.FixupMenu_AmendWithChangesTooltip,
 			},
@@ -1171,10 +1273,8 @@ func countSquashableCommitsAbove(commits []*models.Commit, selectedIdx int, reba
 		if baseSubject, isFixup := helpers.IsFixupCommit(commit.Name); isFixup {
 			// Then, for each commit after the fixup, up to and including the
 			// rebase start commit, see if we find the base commit
-			for _, baseCommit := range commits[i+1 : rebaseStartIdx+1] {
-				if strings.HasPrefix(baseCommit.Name, baseSubject) {
-					result++
-				}
+			if helpers.FindFixupBaseCommit(baseSubject, commits[i+1:rebaseStartIdx+1]) != nil {
+				result++
 			}
 		}
 	}

pkg/i18n/english.go

@@ -479,6 +479,10 @@ type TranslationSet struct {
 	FixupMenu_AmendWithChangesTooltip     string
 	FixupMenu_AmendWithoutChanges         string
 	FixupMenu_AmendWithoutChangesTooltip  string
+	FixupMenu_SelectCommit                string
+	FixupMenu_SelectBase                  string
+	FixupMenu_SelectGlobalBase            string
+	FixUpMenu_SelectSelected              string
 	SquashAboveCommitsTooltip             string
 	SquashCommitsAboveSelectedTooltip     string
 	SquashCommitsInCurrentBranchTooltip   string
@@ -1592,6 +1596,10 @@ func EnglishTranslationSet() *TranslationSet {
 		FixupMenu_AmendWithChangesTooltip:    "Lets you fixup another commit and also change its commit message.",
 		FixupMenu_AmendWithoutChanges:        "amend! commit without changes (pure reword)",
 		FixupMenu_AmendWithoutChangesTooltip: "Lets you change the commit message of another commit without changing its content.",
+		FixupMenu_SelectCommit:               "Select base commit",
+		FixupMenu_SelectBase:                 "base commit",
+		FixupMenu_SelectGlobalBase:           "base commit (searching all commits)",
+		FixUpMenu_SelectSelected:             "selected commit",
 		SquashAboveCommits:                   "Apply fixup commits",
 		SquashAboveCommitsTooltip:            `Squash all 'fixup!' commits, either above the selected commit, or all in current branch (autosquash).`,
 		SquashCommitsAboveSelectedTooltip:    `Squash all 'fixup!' commits above the selected commit (autosquash).`,

pkg/integration/tests/commit/create_fixup_commit_on_fixup_commit.go

@@ -0,0 +1,59 @@
+package commit
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var CreateFixupCommitOnFixupCommit = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Create a fixup commit on an existing fixup commit, verify that it prompts you to create it on the base commit",
+	ExtraCmdArgs: []string{},
+	Skip:         false,
+	GitVersion:   AtLeast("2.38.0"),
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.NewBranch("branch1")
+		shell.EmptyCommit("branch1 commit 1")
+		shell.EmptyCommit("branch1 commit 2")
+		shell.EmptyCommit("branch1 commit 3")
+		shell.NewBranch("branch2")
+		shell.EmptyCommit("branch2 commit 1")
+		shell.EmptyCommit("fixup! branch2 commit 1")
+		shell.CreateFileAndAdd("fixup-file", "fixup content")
+
+		shell.SetConfig("rebase.updateRefs", "true")
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			Lines(
+				Contains("CI ◯ fixup! branch2 commit 1"),
+				Contains("CI ◯ branch2 commit 1"),
+				Contains("CI ◯ * branch1 commit 3"),
+				Contains("CI ◯ branch1 commit 2"),
+				Contains("CI ◯ branch1 commit 1"),
+			).
+			NavigateToLine(Contains("fixup! branch2 commit 1")).
+			Press(keys.Commits.CreateFixupCommit).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("Create fixup commit")).
+					Select(Contains("fixup! commit")).
+					Confirm()
+			}).
+			Tap(func() {
+				t.ExpectPopup().Menu().
+					Title(Equals("Select base commit")).
+					Select(Equals("b base commit")).
+					Confirm()
+			}).
+			Lines(
+				Contains("CI ◯ fixup! branch2 commit 1"),
+				Contains("CI ◯ fixup! branch2 commit 1"),
+				Contains("CI ◯ branch2 commit 1"),
+				Contains("CI ◯ * branch1 commit 3"),
+				Contains("CI ◯ branch1 commit 2"),
+				Contains("CI ◯ branch1 commit 1"),
+			)
+	},
+})

pkg/integration/tests/test_list.go

@@ -123,6 +123,7 @@ var tests = []*components.IntegrationTest{
 	commit.CopyTagToClipboard,
 	commit.CreateAmendCommit,
 	commit.CreateFixupCommitInBranchStack,
+	commit.CreateFixupCommitOnFixupCommit,
 	commit.CreateTag,
 	commit.DisableCopyCommitMessageBody,
 	commit.DiscardOldFileChanges,