diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index d290192e4..ef6b5be80 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -2,6 +2,7 @@ package controllers import ( "fmt" + "strings" "github.com/fsmiamoto/git-todo-parser/todo" "github.com/go-errors/errors" @@ -811,11 +812,14 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err HandleConfirm: func() error { return self.c.Helpers().WorkingTree.WithEnsureCommitableFiles(func() error { self.c.LogAction(self.c.Tr.Actions.CreateFixupCommit) - if err := self.c.Git().Commit.CreateFixupCommit(commit.Sha); err != nil { - return self.c.Error(err) - } + return self.c.WithWaitingStatusSync(self.c.Tr.CreatingFixupCommitStatus, func() error { + if err := self.c.Git().Commit.CreateFixupCommit(commit.Sha); err != nil { + return self.c.Error(err) + } - return self.c.Refresh(types.RefreshOptions{Mode: types.ASYNC}) + self.context().MoveSelectedLine(1) + return self.c.Refresh(types.RefreshOptions{Mode: types.SYNC}) + }) }) }, }) @@ -844,37 +848,89 @@ func (self *LocalCommitsController) squashFixupCommits() error { } func (self *LocalCommitsController) squashAllFixupsAboveSelectedCommit(commit *models.Commit) error { - return self.c.WithWaitingStatus(self.c.Tr.SquashingStatus, func(gocui.Task) error { - self.c.LogAction(self.c.Tr.Actions.SquashAllAboveFixupCommits) - err := self.c.Git().Rebase.SquashAllAboveFixupCommits(commit) - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) - }) + return self.squashFixupsImpl(commit, self.context().GetSelectedLineIdx()) } func (self *LocalCommitsController) squashAllFixupsInCurrentBranch() error { - commit, err := self.findCommitForSquashFixupsInCurrentBranch() + commit, rebaseStartIdx, err := self.findCommitForSquashFixupsInCurrentBranch() if err != nil { return self.c.Error(err) } - return self.c.WithWaitingStatus(self.c.Tr.SquashingStatus, func(gocui.Task) error { + return self.squashFixupsImpl(commit, rebaseStartIdx) +} + +func (self *LocalCommitsController) squashFixupsImpl(commit *models.Commit, rebaseStartIdx int) error { + selectionOffset := countSquashableCommitsAbove(self.c.Model().Commits, self.context().GetSelectedLineIdx(), rebaseStartIdx) + return self.c.WithWaitingStatusSync(self.c.Tr.SquashingStatus, func() error { self.c.LogAction(self.c.Tr.Actions.SquashAllAboveFixupCommits) err := self.c.Git().Rebase.SquashAllAboveFixupCommits(commit) - return self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(err) + self.context().MoveSelectedLine(-selectionOffset) + return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions( + err, types.RefreshOptions{Mode: types.SYNC}) }) } -func (self *LocalCommitsController) findCommitForSquashFixupsInCurrentBranch() (*models.Commit, error) { +func (self *LocalCommitsController) findCommitForSquashFixupsInCurrentBranch() (*models.Commit, int, error) { commits := self.c.Model().Commits _, index, ok := lo.FindIndexOf(commits, func(c *models.Commit) bool { return c.IsMerge() || c.Status == models.StatusMerged }) if !ok || index == 0 { - return nil, errors.New(self.c.Tr.CannotSquashCommitsInCurrentBranch) + return nil, -1, errors.New(self.c.Tr.CannotSquashCommitsInCurrentBranch) } - return commits[index-1], nil + return commits[index-1], index - 1, nil +} + +// Anticipate how many commits above the selectedIdx are going to get squashed +// by the SquashAllAboveFixupCommits call, so that we can adjust the selection +// afterwards. Let's hope we're matching git's behavior correctly here. +func countSquashableCommitsAbove(commits []*models.Commit, selectedIdx int, rebaseStartIdx int) int { + result := 0 + + // 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 { + // 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++ + } + } + } + } + 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 strings.HasPrefix(s, prefix) { + return strings.TrimPrefix(s, prefix), 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 { @@ -1067,7 +1123,7 @@ func (self *LocalCommitsController) canFindCommitForQuickStart() *types.Disabled } func (self *LocalCommitsController) canFindCommitForSquashFixupsInCurrentBranch() *types.DisabledReason { - if _, err := self.findCommitForSquashFixupsInCurrentBranch(); err != nil { + if _, _, err := self.findCommitForSquashFixupsInCurrentBranch(); err != nil { return &types.DisabledReason{Text: err.Error()} } diff --git a/pkg/gui/controllers/local_commits_controller_test.go b/pkg/gui/controllers/local_commits_controller_test.go new file mode 100644 index 000000000..d425821d7 --- /dev/null +++ b/pkg/gui/controllers/local_commits_controller_test.go @@ -0,0 +1,141 @@ +package controllers + +import ( + "testing" + + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/stretchr/testify/assert" +) + +func Test_countSquashableCommitsAbove(t *testing.T) { + scenarios := []struct { + name string + commits []*models.Commit + selectedIdx int + rebaseStartIdx int + expectedResult int + }{ + { + name: "no squashable commits", + commits: []*models.Commit{ + {Name: "abc"}, + {Name: "def"}, + {Name: "ghi"}, + }, + selectedIdx: 2, + rebaseStartIdx: 2, + expectedResult: 0, + }, + { + name: "some squashable commits, including for the selected commit", + commits: []*models.Commit{ + {Name: "fixup! def"}, + {Name: "fixup! ghi"}, + {Name: "abc"}, + {Name: "def"}, + {Name: "ghi"}, + }, + selectedIdx: 4, + rebaseStartIdx: 4, + expectedResult: 2, + }, + { + name: "base commit is below rebase start", + commits: []*models.Commit{ + {Name: "fixup! def"}, + {Name: "abc"}, + {Name: "def"}, + }, + selectedIdx: 1, + rebaseStartIdx: 1, + expectedResult: 0, + }, + { + name: "base commit does not exist at all", + commits: []*models.Commit{ + {Name: "fixup! xyz"}, + {Name: "abc"}, + {Name: "def"}, + }, + selectedIdx: 2, + rebaseStartIdx: 2, + expectedResult: 0, + }, + { + name: "selected commit is in the middle of fixups", + commits: []*models.Commit{ + {Name: "fixup! def"}, + {Name: "abc"}, + {Name: "fixup! ghi"}, + {Name: "def"}, + {Name: "ghi"}, + }, + selectedIdx: 1, + rebaseStartIdx: 4, + expectedResult: 1, + }, + { + name: "selected commit is after rebase start", + commits: []*models.Commit{ + {Name: "fixup! def"}, + {Name: "abc"}, + {Name: "def"}, + {Name: "ghi"}, + }, + selectedIdx: 3, + rebaseStartIdx: 2, + expectedResult: 1, + }, + } + for _, s := range scenarios { + t.Run(s.name, func(t *testing.T) { + assert.Equal(t, s.expectedResult, countSquashableCommitsAbove(s.commits, s.selectedIdx, s.rebaseStartIdx)) + }) + } +} + +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) + }) + } +} diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index c05033834..b1d0e01eb 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -341,6 +341,7 @@ type TranslationSet struct { CheckingOutStatus string CommittingStatus string RevertingStatus string + CreatingFixupCommitStatus string CommitFiles string SubCommitsDynamicTitle string CommitFilesDynamicTitle string @@ -1289,6 +1290,7 @@ func EnglishTranslationSet() TranslationSet { CheckingOutStatus: "Checking out", CommittingStatus: "Committing", RevertingStatus: "Reverting", + CreatingFixupCommitStatus: "Creating fixup commit", CommitFiles: "Commit files", SubCommitsDynamicTitle: "Commits (%s)", CommitFilesDynamicTitle: "Diff files (%s)", diff --git a/pkg/integration/tests/interactive_rebase/squash_fixups_above.go b/pkg/integration/tests/interactive_rebase/squash_fixups_above.go new file mode 100644 index 000000000..e87addce0 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/squash_fixups_above.go @@ -0,0 +1,56 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SquashFixupsAbove = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Squashes all fixups above a commit and checks that the selected line stays correct.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + CreateNCommits(3). + CreateFileAndAdd("fixup-file", "fixup content") + }, + 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.Commits.CreateFixupCommit). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Create fixup commit")). + Content(Contains("Are you sure you want to create a fixup! commit for commit")). + Confirm() + }). + Lines( + Contains("fixup! commit 02"), + Contains("commit 03"), + Contains("commit 02").IsSelected(), + Contains("commit 01"), + ). + Press(keys.Commits.SquashAboveCommits). + Tap(func() { + t.ExpectPopup().Menu(). + Title(Equals("Apply fixup commits")). + Select(Contains("Above the selected commit")). + Confirm() + }). + Lines( + Contains("commit 03"), + Contains("commit 02").IsSelected(), + Contains("commit 01"), + ) + + t.Views().Main(). + Content(Contains("fixup content")) + }, +}) diff --git a/pkg/integration/tests/interactive_rebase/squash_fixups_in_current_branch.go b/pkg/integration/tests/interactive_rebase/squash_fixups_in_current_branch.go index 636810533..c6721d829 100644 --- a/pkg/integration/tests/interactive_rebase/squash_fixups_in_current_branch.go +++ b/pkg/integration/tests/interactive_rebase/squash_fixups_in_current_branch.go @@ -27,10 +27,12 @@ var SquashFixupsInCurrentBranch = NewIntegrationTest(NewIntegrationTestArgs{ Run: func(t *TestDriver, keys config.KeybindingConfig) { t.Views().Commits(). Focus(). + SelectNextItem(). + SelectNextItem(). Lines( Contains("fixup! commit 01"), Contains("commit 02"), - Contains("commit 01"), + Contains("commit 01").IsSelected(), Contains("fixup! master commit"), Contains("master commit"), ). @@ -43,11 +45,10 @@ var SquashFixupsInCurrentBranch = NewIntegrationTest(NewIntegrationTestArgs{ }). Lines( Contains("commit 02"), - Contains("commit 01"), + Contains("commit 01").IsSelected(), Contains("fixup! master commit"), Contains("master commit"), - ). - NavigateToLine(Contains("commit 01")) + ) t.Views().Main(). Content(Contains("fixup content")) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index d0525fa59..402a40acf 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -188,6 +188,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.RewordYouAreHereCommitWithEditor, interactive_rebase.SquashDownFirstCommit, interactive_rebase.SquashDownSecondCommit, + interactive_rebase.SquashFixupsAbove, interactive_rebase.SquashFixupsAboveFirstCommit, interactive_rebase.SquashFixupsInCurrentBranch, interactive_rebase.SwapInRebaseWithConflict,