Don't keep commits that become empty during a rebase

The only exception is when moving a custom patch for an entire commit to an
earlier commit; in this case the source commit becomes empty, but we want to
keep it, mainly for consistency with moving the patch to a later commit, which
behaves the same.

In all other cases where we rebase, it's confusing when empty commits are kept;
the most common example is rebasing a branch onto master, where master already
contains some of the commits of our branch. In this case we simply want to drop
these.
This commit is contained in:
Stefan Haller 2023-05-19 18:46:19 +02:00
parent 2e0d0a92ee
commit 3cddd7cfa5
3 changed files with 25 additions and 18 deletions

View file

@ -91,7 +91,7 @@ func (self *PatchCommands) SaveTemporaryPatch(patch string) (string, error) {
// DeletePatchesFromCommit applies a patch in reverse for a commit
func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, commitIndex int) error {
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIndex); err != nil {
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIndex, false); err != nil {
return err
}
@ -117,7 +117,10 @@ func (self *PatchCommands) DeletePatchesFromCommit(commits []*models.Commit, com
func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, sourceCommitIdx int, destinationCommitIdx int) error {
if sourceCommitIdx < destinationCommitIdx {
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, destinationCommitIdx); err != nil {
// Passing true for keepCommitsThatBecomeEmpty: if the moved-from
// commit becomes empty, we want to keep it, mainly for consistency with
// moving the patch to a *later* commit, which behaves the same.
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, destinationCommitIdx, true); err != nil {
return err
}
@ -223,7 +226,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
}
}
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx); err != nil {
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx, false); err != nil {
return err
}
@ -272,7 +275,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId
}
func (self *PatchCommands) PullPatchIntoNewCommit(commits []*models.Commit, commitIdx int) error {
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx); err != nil {
if err := self.rebase.BeginInteractiveRebaseForCommit(commits, commitIdx, false); err != nil {
return err
}

View file

@ -41,7 +41,7 @@ func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, me
return self.commit.RewordLastCommit(message)
}
err := self.BeginInteractiveRebaseForCommit(commits, index)
err := self.BeginInteractiveRebaseForCommit(commits, index, false)
if err != nil {
return err
}
@ -86,7 +86,7 @@ func (self *RebaseCommands) GenericAmend(commits []*models.Commit, index int, f
return f()
}
err := self.BeginInteractiveRebaseForCommit(commits, index)
err := self.BeginInteractiveRebaseForCommit(commits, index, false)
if err != nil {
return err
}
@ -165,9 +165,10 @@ func logTodoChanges(changes []daemon.ChangeTodoAction) string {
}
type PrepareInteractiveRebaseCommandOpts struct {
baseShaOrRoot string
instruction daemon.Instruction
overrideEditor bool
baseShaOrRoot string
instruction daemon.Instruction
overrideEditor bool
keepCommitsThatBecomeEmpty bool
}
// PrepareInteractiveRebaseCommand returns the cmd for an interactive rebase
@ -180,7 +181,7 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract
Arg("--interactive").
Arg("--autostash").
Arg("--keep-empty").
ArgIf(!self.version.IsOlderThan(2, 26, 0), "--empty=keep").
ArgIf(opts.keepCommitsThatBecomeEmpty && !self.version.IsOlderThan(2, 26, 0), "--empty=keep").
Arg("--no-autosquash").
ArgIf(!self.version.IsOlderThan(2, 22, 0), "--rebase-merges").
Arg(opts.baseShaOrRoot).
@ -273,7 +274,9 @@ func (self *RebaseCommands) SquashAllAboveFixupCommits(commit *models.Commit) er
// BeginInteractiveRebaseForCommit starts an interactive rebase to edit the current
// commit and pick all others. After this you'll want to call `self.ContinueRebase()
func (self *RebaseCommands) BeginInteractiveRebaseForCommit(commits []*models.Commit, commitIndex int) error {
func (self *RebaseCommands) BeginInteractiveRebaseForCommit(
commits []*models.Commit, commitIndex int, keepCommitsThatBecomeEmpty bool,
) error {
if len(commits)-1 < commitIndex {
return errors.New("index outside of range of commits")
}
@ -292,9 +295,10 @@ func (self *RebaseCommands) BeginInteractiveRebaseForCommit(commits []*models.Co
self.os.LogCommand(logTodoChanges(changes), false)
return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
baseShaOrRoot: getBaseShaOrRoot(commits, commitIndex+1),
overrideEditor: true,
instruction: daemon.NewChangeTodoActionsInstruction(changes),
baseShaOrRoot: getBaseShaOrRoot(commits, commitIndex+1),
overrideEditor: true,
keepCommitsThatBecomeEmpty: keepCommitsThatBecomeEmpty,
instruction: daemon.NewChangeTodoActionsInstruction(changes),
}).Run()
}
@ -358,7 +362,7 @@ func (self *RebaseCommands) runSkipEditorCommand(cmdObj oscommands.ICmdObj) erro
// DiscardOldFileChanges discards changes to a file from an old commit
func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, commitIndex int, fileName string) error {
if err := self.BeginInteractiveRebaseForCommit(commits, commitIndex); err != nil {
if err := self.BeginInteractiveRebaseForCommit(commits, commitIndex, false); err != nil {
return err
}

View file

@ -29,7 +29,7 @@ func TestRebaseRebaseBranch(t *testing.T) {
arg: "master",
gitVersion: &GitVersion{2, 26, 0, ""},
runner: oscommands.NewFakeRunner(t).
Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges master`, "", nil),
Expect(`git rebase --interactive --autostash --keep-empty --no-autosquash --rebase-merges master`, "", nil),
test: func(err error) {
assert.NoError(t, err)
},
@ -39,7 +39,7 @@ func TestRebaseRebaseBranch(t *testing.T) {
arg: "master",
gitVersion: &GitVersion{2, 26, 0, ""},
runner: oscommands.NewFakeRunner(t).
Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges master`, "", errors.New("error")),
Expect(`git rebase --interactive --autostash --keep-empty --no-autosquash --rebase-merges master`, "", errors.New("error")),
test: func(err error) {
assert.Error(t, err)
},
@ -149,7 +149,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) {
commitIndex: 0,
fileName: "test999.txt",
runner: oscommands.NewFakeRunner(t).
Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges abcdef`, "", nil).
Expect(`git rebase --interactive --autostash --keep-empty --no-autosquash --rebase-merges abcdef`, "", nil).
Expect(`git cat-file -e HEAD^:"test999.txt"`, "", nil).
Expect(`git checkout HEAD^ -- "test999.txt"`, "", nil).
Expect(`git commit --amend --no-edit --allow-empty`, "", nil).