From b41bb8f845f3c9abc95c50bf32fe7eff127ce400 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 16 Jan 2026 08:33:33 +0100 Subject: [PATCH 1/5] Add a test that documents the current behavior Currently we get an annoying error message, but we'd like the base commit to be selected, disregarding the fixup. --- ...p_disregard_fixups_for_same_base_commit.go | 46 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 47 insertions(+) create mode 100644 pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go diff --git a/pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go b/pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go new file mode 100644 index 000000000..07f32126a --- /dev/null +++ b/pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go @@ -0,0 +1,46 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var FindBaseCommitForFixupDisregardFixupsForSameBaseCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Finds the base commit to create a fixup for, disregarding fixup commits for the same base commit", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + EmptyCommit("1st commit"). + NewBranch("mybranch"). + CreateFileAndAdd("file1", "1\n2\n3\n"). + Commit("2nd commit"). + UpdateFileAndAdd("file1", "1\n2\n3a\n"). + Commit("fixup! 2nd commit"). + EmptyCommit("3rd commit"). + UpdateFileAndAdd("file1", "1a\n2\n3b\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Lines( + Contains("3rd commit").IsSelected(), + Contains("fixup! 2nd commit"), + Contains("2nd commit"), + Contains("1st commit"), + ) + + t.Views().Files(). + Focus(). + Press(keys.Files.FindBaseCommitForFixup) + + t.ExpectPopup().Alert(). + Title(Equals("Error")). + Content( + MatchesRegexp("Multiple base commits found.*\n\n" + + ".*fixup! 2nd commit\n" + + ".*2nd commit"), + ). + Confirm() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 26d6d3030..faf35a601 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -130,6 +130,7 @@ var tests = []*components.IntegrationTest{ commit.DoNotShowBranchMarkerForHeadCommit, commit.FailHooksThenCommitNoHooks, commit.FindBaseCommitForFixup, + commit.FindBaseCommitForFixupDisregardFixupsForSameBaseCommit, commit.FindBaseCommitForFixupDisregardMainBranch, commit.FindBaseCommitForFixupOnlyAddedLines, commit.FindBaseCommitForFixupWarningForAddedLines, From 2ba3c1dc3fc0afbd5a6965bd19093e7ef980ecdb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 15 Jan 2026 20:05:33 +0100 Subject: [PATCH 2/5] Move isFixupCommit to FixupHelper --- pkg/gui/controllers/helpers/fixup_helper.go | 27 +++++++++++ .../controllers/helpers/fixup_helper_test.go | 46 +++++++++++++++++++ .../controllers/local_commits_controller.go | 29 +----------- .../local_commits_controller_test.go | 46 ------------------- 4 files changed, 74 insertions(+), 74 deletions(-) diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index 2e3b19e77..201749761 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -350,3 +350,30 @@ func (self *FixupHelper) findCommit(commits []*models.Commit, hash string) (*mod return commit.Hash() == hash }) } + +// Check whether the given subject line is the subject of a fixup commit, and +// returns (trimmedSubject, true) if so (where trimmedSubject is the subject +// with all fixup prefixes removed), or (subject, false) if not. +func IsFixupCommit(subject string) (string, bool) { + prefixes := []string{"fixup! ", "squash! ", "amend! "} + trimPrefix := func(s string) (string, bool) { + for _, prefix := range prefixes { + if trimmedSubject, ok := strings.CutPrefix(s, prefix); ok { + return trimmedSubject, true + } + } + return s, false + } + + if subject, wasTrimmed := trimPrefix(subject); wasTrimmed { + for { + // handle repeated prefixes like "fixup! amend! fixup! Subject" + if subject, wasTrimmed = trimPrefix(subject); !wasTrimmed { + break + } + } + return subject, true + } + + return subject, false +} diff --git a/pkg/gui/controllers/helpers/fixup_helper_test.go b/pkg/gui/controllers/helpers/fixup_helper_test.go index e50a4249f..e281a4c28 100644 --- a/pkg/gui/controllers/helpers/fixup_helper_test.go +++ b/pkg/gui/controllers/helpers/fixup_helper_test.go @@ -155,3 +155,49 @@ index 9ce8efb33..0632e41b0 100644 }) } } + +func TestFixupHelper_IsFixupCommit(t *testing.T) { + scenarios := []struct { + subject string + expectedTrimmedSubject string + expectedIsFixup bool + }{ + { + subject: "Bla", + expectedTrimmedSubject: "Bla", + expectedIsFixup: false, + }, + { + subject: "fixup Bla", + expectedTrimmedSubject: "fixup Bla", + expectedIsFixup: false, + }, + { + subject: "fixup! Bla", + expectedTrimmedSubject: "Bla", + expectedIsFixup: true, + }, + { + subject: "fixup! fixup! Bla", + expectedTrimmedSubject: "Bla", + expectedIsFixup: true, + }, + { + subject: "amend! squash! Bla", + expectedTrimmedSubject: "Bla", + expectedIsFixup: true, + }, + { + subject: "fixup!", + expectedTrimmedSubject: "fixup!", + expectedIsFixup: false, + }, + } + for _, s := range scenarios { + t.Run(s.subject, func(t *testing.T) { + trimmedSubject, isFixupCommit := IsFixupCommit(s.subject) + assert.Equal(t, s.expectedTrimmedSubject, trimmedSubject) + assert.Equal(t, s.expectedIsFixup, isFixupCommit) + }) + } +} diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 1091b9178..2c8932f33 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -1096,7 +1096,7 @@ func countSquashableCommitsAbove(commits []*models.Commit, selectedIdx int, reba // For each commit _above_ the selection, ... for i, commit := range commits[0:selectedIdx] { // ... see if it is a fixup commit, and get the base subject it applies to - if baseSubject, isFixup := isFixupCommit(commit.Name); isFixup { + 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] { @@ -1109,33 +1109,6 @@ func countSquashableCommitsAbove(commits []*models.Commit, selectedIdx int, reba return result } -// Check whether the given subject line is the subject of a fixup commit, and -// returns (trimmedSubject, true) if so (where trimmedSubject is the subject -// with all fixup prefixes removed), or (subject, false) if not. -func isFixupCommit(subject string) (string, bool) { - prefixes := []string{"fixup! ", "squash! ", "amend! "} - trimPrefix := func(s string) (string, bool) { - for _, prefix := range prefixes { - if trimmedSubject, ok := strings.CutPrefix(s, prefix); ok { - return trimmedSubject, true - } - } - return s, false - } - - if subject, wasTrimmed := trimPrefix(subject); wasTrimmed { - for { - // handle repeated prefixes like "fixup! amend! fixup! Subject" - if subject, wasTrimmed = trimPrefix(subject); !wasTrimmed { - break - } - } - return subject, true - } - - return subject, false -} - func (self *LocalCommitsController) createTag(commit *models.Commit) error { return self.c.Helpers().Tags.OpenCreateTagPrompt(commit.Hash(), func() {}) } diff --git a/pkg/gui/controllers/local_commits_controller_test.go b/pkg/gui/controllers/local_commits_controller_test.go index d425821d7..c5c5e7a5d 100644 --- a/pkg/gui/controllers/local_commits_controller_test.go +++ b/pkg/gui/controllers/local_commits_controller_test.go @@ -93,49 +93,3 @@ func Test_countSquashableCommitsAbove(t *testing.T) { }) } } - -func Test_isFixupCommit(t *testing.T) { - scenarios := []struct { - subject string - expectedTrimmedSubject string - expectedIsFixup bool - }{ - { - subject: "Bla", - expectedTrimmedSubject: "Bla", - expectedIsFixup: false, - }, - { - subject: "fixup Bla", - expectedTrimmedSubject: "fixup Bla", - expectedIsFixup: false, - }, - { - subject: "fixup! Bla", - expectedTrimmedSubject: "Bla", - expectedIsFixup: true, - }, - { - subject: "fixup! fixup! Bla", - expectedTrimmedSubject: "Bla", - expectedIsFixup: true, - }, - { - subject: "amend! squash! Bla", - expectedTrimmedSubject: "Bla", - expectedIsFixup: true, - }, - { - subject: "fixup!", - expectedTrimmedSubject: "fixup!", - expectedIsFixup: false, - }, - } - for _, s := range scenarios { - t.Run(s.subject, func(t *testing.T) { - trimmedSubject, isFixupCommit := isFixupCommit(s.subject) - assert.Equal(t, s.expectedTrimmedSubject, trimmedSubject) - assert.Equal(t, s.expectedIsFixup, isFixupCommit) - }) - } -} From df1e5512da90cc003767248d2df20e88a9691959 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 15 Jan 2026 20:20:55 +0100 Subject: [PATCH 3/5] Make getHashesAndSubjects a free-standing function There's no reason for this to be a method. --- pkg/gui/controllers/helpers/fixup_helper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index 201749761..054a5bbf1 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -114,7 +114,7 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error { // If there are multiple commits that could be the base commit, list // them in the error message. But only the candidates from the current // branch, not including any that are already merged. - subjects := self.getHashesAndSubjects(commits, hashGroups[NOT_MERGED]) + subjects := getHashesAndSubjects(commits, hashGroups[NOT_MERGED]) message := lo.Ternary(hasStagedChanges, self.c.Tr.MultipleBaseCommitsFoundStaged, self.c.Tr.MultipleBaseCommitsFoundUnstaged) @@ -144,7 +144,7 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error { }) } -func (self *FixupHelper) getHashesAndSubjects(commits []*models.Commit, hashes []string) string { +func getHashesAndSubjects(commits []*models.Commit, hashes []string) string { // This is called only for the NOT_MERGED commits, and we know that all of them are contained in // the commits slice. commitsSet := set.NewFromSlice(hashes) From 4bcf3e181cad97b90e6ce8e8f9341a6d2b1ee2f5 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 15 Jan 2026 20:18:35 +0100 Subject: [PATCH 4/5] Extract a helper method getCommitsForHashes --- pkg/gui/controllers/helpers/fixup_helper.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index 054a5bbf1..c670b9dfb 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -144,20 +144,27 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error { }) } -func getHashesAndSubjects(commits []*models.Commit, hashes []string) string { +func getCommitsForHashes(commits []*models.Commit, hashes []string) []*models.Commit { // This is called only for the NOT_MERGED commits, and we know that all of them are contained in // the commits slice. commitsSet := set.NewFromSlice(hashes) - subjects := make([]string, 0, len(hashes)) + result := make([]*models.Commit, 0, len(hashes)) for _, c := range commits { if commitsSet.Includes(c.Hash()) { - subjects = append(subjects, fmt.Sprintf("%s %s", c.ShortRefName(), c.Name)) + result = append(result, c) commitsSet.Remove(c.Hash()) if commitsSet.Len() == 0 { break } } } + return result +} + +func getHashesAndSubjects(commits []*models.Commit, hashes []string) string { + subjects := lo.Map(getCommitsForHashes(commits, hashes), func(c *models.Commit, _ int) string { + return fmt.Sprintf("%s %s", c.ShortRefName(), c.Name) + }) return strings.Join(subjects, "\n") } From 268d1934f83c1af1688063af20a245eead1120d0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 15 Jan 2026 20:36:58 +0100 Subject: [PATCH 5/5] Ignore fixup commits for a found base commit when doing ctrl-f --- pkg/gui/controllers/helpers/fixup_helper.go | 42 +++++- .../controllers/helpers/fixup_helper_test.go | 138 ++++++++++++++++++ ...p_disregard_fixups_for_same_base_commit.go | 16 +- 3 files changed, 180 insertions(+), 16 deletions(-) diff --git a/pkg/gui/controllers/helpers/fixup_helper.go b/pkg/gui/controllers/helpers/fixup_helper.go index c670b9dfb..dfde8365b 100644 --- a/pkg/gui/controllers/helpers/fixup_helper.go +++ b/pkg/gui/controllers/helpers/fixup_helper.go @@ -110,20 +110,24 @@ func (self *FixupHelper) HandleFindBaseCommitForFixupPress() error { return errors.New(self.c.Tr.BaseCommitIsAlreadyOnMainBranch) } - if len(hashGroups[NOT_MERGED]) > 1 { - // If there are multiple commits that could be the base commit, list + foundCommits := getCommitsForHashes(commits, hashGroups[NOT_MERGED]) + // If there are multiple commits that could be the base commit, remove all + // those that are fixups for the last one. + foundCommits = removeFixupCommits(foundCommits) + + if len(foundCommits) > 1 { + // If there are still multiple commits that could be the base commit, list // them in the error message. But only the candidates from the current // branch, not including any that are already merged. - subjects := getHashesAndSubjects(commits, hashGroups[NOT_MERGED]) + subjects := getHashesAndSubjects(foundCommits) message := lo.Ternary(hasStagedChanges, self.c.Tr.MultipleBaseCommitsFoundStaged, self.c.Tr.MultipleBaseCommitsFoundUnstaged) return fmt.Errorf("%s\n\n%s", message, subjects) } - // At this point we know that the NOT_MERGED bucket has exactly one commit, - // and that's the one we want to select. - _, index, _ := self.findCommit(commits, hashGroups[NOT_MERGED][0]) + // Now we know that foundCommits has exactly one commit, so find its index + _, index, _ := self.findCommit(commits, foundCommits[0].Hash()) return self.c.ConfirmIf(warnAboutAddedLines, types.ConfirmOpts{ Title: self.c.Tr.FindBaseCommitForFixup, @@ -161,13 +165,35 @@ func getCommitsForHashes(commits []*models.Commit, hashes []string) []*models.Co return result } -func getHashesAndSubjects(commits []*models.Commit, hashes []string) string { - subjects := lo.Map(getCommitsForHashes(commits, hashes), func(c *models.Commit, _ int) string { +func getHashesAndSubjects(commits []*models.Commit) string { + subjects := lo.Map(commits, func(c *models.Commit, _ int) string { return fmt.Sprintf("%s %s", c.ShortRefName(), c.Name) }) return strings.Join(subjects, "\n") } +func removeFixupCommits(commits []*models.Commit) []*models.Commit { + if len(commits) <= 1 { + return commits + } + + // If the last found commit is itself a fixup, don't eliminate anything. + baseSubject, lastIsFixup := IsFixupCommit(commits[len(commits)-1].Name) + if lastIsFixup { + return commits + } + + // need to go backwards because we are mutating the slice as we go + for i := len(commits) - 2; i >= 0; i-- { + subject, isFixup := IsFixupCommit(commits[i].Name) + if isFixup && subject == baseSubject { + commits = utils.Remove(commits, i) + } + } + + return commits +} + func (self *FixupHelper) getDiff() (string, bool, error) { args := []string{"-U0", "--ignore-submodules=all", "HEAD", "--"} diff --git a/pkg/gui/controllers/helpers/fixup_helper_test.go b/pkg/gui/controllers/helpers/fixup_helper_test.go index e281a4c28..466e90087 100644 --- a/pkg/gui/controllers/helpers/fixup_helper_test.go +++ b/pkg/gui/controllers/helpers/fixup_helper_test.go @@ -3,6 +3,9 @@ package helpers import ( "testing" + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" "github.com/stretchr/testify/assert" ) @@ -201,3 +204,138 @@ func TestFixupHelper_IsFixupCommit(t *testing.T) { }) } } + +func TestFixupHelper_removeFixupCommits(t *testing.T) { + hashPool := &utils.StringPool{} + + type commitDesc struct { + Hash string + Name string + } + + scenarios := []struct { + name string + commits []commitDesc + expectedResult []commitDesc + }{ + { + name: "empty list", + commits: []commitDesc{}, + expectedResult: []commitDesc{}, + }, + { + name: "single commit", + commits: []commitDesc{ + {"abc123", "Some feature"}, + }, + expectedResult: []commitDesc{ + {"abc123", "Some feature"}, + }, + }, + { + name: "two unrelated commits", + commits: []commitDesc{ + {"abc123", "First feature"}, + {"def456", "Second feature"}, + }, + expectedResult: []commitDesc{ + {"abc123", "First feature"}, + {"def456", "Second feature"}, + }, + }, + { + name: "fixup commit for last commit", + commits: []commitDesc{ + {"abc123", "fixup! Some feature"}, + {"def456", "Some feature"}, + }, + expectedResult: []commitDesc{ + {"def456", "Some feature"}, + }, + }, + { + name: "amend and squash commits for last commit", + commits: []commitDesc{ + {"abc123", "squash! Some feature"}, + {"def456", "amend! Some feature"}, + {"ghi789", "Some feature"}, + }, + expectedResult: []commitDesc{ + {"ghi789", "Some feature"}, + }, + }, + { + name: "fixup commit for different commit", + commits: []commitDesc{ + {"abc123", "fixup! Other feature"}, + {"def456", "Some feature"}, + }, + expectedResult: []commitDesc{ + {"abc123", "fixup! Other feature"}, + {"def456", "Some feature"}, + }, + }, + { + name: "last commit is a fixup itself", + commits: []commitDesc{ + {"abc123", "fixup! Some feature"}, + {"def456", "fixup! Some feature"}, + }, + expectedResult: []commitDesc{ + {"abc123", "fixup! Some feature"}, + {"def456", "fixup! Some feature"}, + }, + }, + { + name: "nested fixup commit", + commits: []commitDesc{ + {"abc123", "fixup! fixup! Some feature"}, + {"def456", "amend! squash! fixup! Some feature"}, + {"ghi789", "Some feature"}, + }, + expectedResult: []commitDesc{ + {"ghi789", "Some feature"}, + }, + }, + { + name: "fixup commits mixed with unrelated commits", + commits: []commitDesc{ + {Hash: "abc123", Name: "fixup! Base commit"}, + {Hash: "def456", Name: "Unrelated commit"}, + {Hash: "ghi789", Name: "fixup! Base commit"}, + {Hash: "jkl012", Name: "Base commit"}, + }, + expectedResult: []commitDesc{ + {Hash: "def456", Name: "Unrelated commit"}, + {Hash: "jkl012", Name: "Base commit"}, + }, + }, + { + name: "only fixup commits for last commit removed, others preserved", + commits: []commitDesc{ + {Hash: "abc123", Name: "fixup! First feature"}, + {Hash: "def456", Name: "fixup! Second feature"}, + {Hash: "ghi789", Name: "Second feature"}, + {Hash: "jkl012", Name: "First feature"}, + }, + expectedResult: []commitDesc{ + {Hash: "def456", Name: "fixup! Second feature"}, + {Hash: "ghi789", Name: "Second feature"}, + {Hash: "jkl012", Name: "First feature"}, + }, + }, + } + + 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.name, func(t *testing.T) { + commits := lo.Map(s.commits, makeCommitFromDesc) + result := removeFixupCommits(commits) + expectedCommits := lo.Map(s.expectedResult, makeCommitFromDesc) + assert.Equal(t, expectedCommits, result) + }) + } +} diff --git a/pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go b/pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go index 07f32126a..94b579190 100644 --- a/pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go +++ b/pkg/integration/tests/commit/find_base_commit_for_fixup_disregard_fixups_for_same_base_commit.go @@ -34,13 +34,13 @@ var FindBaseCommitForFixupDisregardFixupsForSameBaseCommit = NewIntegrationTest( Focus(). Press(keys.Files.FindBaseCommitForFixup) - t.ExpectPopup().Alert(). - Title(Equals("Error")). - Content( - MatchesRegexp("Multiple base commits found.*\n\n" + - ".*fixup! 2nd commit\n" + - ".*2nd commit"), - ). - Confirm() + t.Views().Commits(). + IsFocused(). + Lines( + Contains("3rd commit"), + Contains("fixup! 2nd commit"), + Contains("2nd commit").IsSelected(), + Contains("1st commit"), + ) }, })