diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index dfde8365b17..f63a4f8943b 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/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 +} diff --git a/pkg/gui/controllers/helpers/fixup_helper_test.go b/pkg/gui/controllers/helpers/fixup_helper_test.go index 466e90087f2..8c671bfc637 100644 --- a/pkg/gui/controllers/helpers/fixup_helper_test.go +++ b/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{} diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 51b03137685..6d8b9dd7cfe 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/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++ } } } diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index ce87693a24d..a6945f43d12 100644 --- a/pkg/i18n/english.go +++ b/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).`, diff --git a/pkg/integration/tests/commit/create_fixup_commit_on_fixup_commit.go b/pkg/integration/tests/commit/create_fixup_commit_on_fixup_commit.go new file mode 100644 index 00000000000..c09ca154b76 --- /dev/null +++ b/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"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 4d9edaa70ab..d9fa7b25d15 100644 --- a/pkg/integration/tests/test_list.go +++ b/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,