go/lazygit
1
0
Fork 0
mirror of https://github.com/jesseduffield/lazygit.git synced 2025-04-27 12:32:37 +02:00
Code Issues Releases Activity

With stacked branches, create fixup commit at the end of the branch it belongs to (#3892)

Browse Source 
- **PR Description**

When working with stacked branches, and creating a fixup commit for a
commit in one of the lower branches of the stack, the fixup was created
at the top of the stack and the user needed to move it down to the right
branch manually. This is unnecessary extra work; create it at the end of
the right branch automatically.
This commit is contained in:
Stefan Haller 2024-09-15 11:21:34 +02:00 committed by GitHub
commit 647f533e71
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 172 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
pkg/app/daemon/daemon.go
View File

@ -246,16 +246,18 @@ func (self *ChangeTodoActionsInstruction) run(common *common.Common) error {
// Takes the hash of some commit, and the hash of a fixup commit that was created
// at the end of the branch, then moves the fixup commit down to right after the
// original commit, changing its type to "fixup"
// original commit, changing its type to "fixup" (only if ChangeToFixup is true)
type MoveFixupCommitDownInstruction struct {
OriginalHash string
FixupHash string
OriginalHash string
FixupHash string
ChangeToFixup bool
}
func NewMoveFixupCommitDownInstruction(originalHash string, fixupHash string) Instruction {
func NewMoveFixupCommitDownInstruction(originalHash string, fixupHash string, changeToFixup bool) Instruction {
return &MoveFixupCommitDownInstruction{
OriginalHash: originalHash,
FixupHash: fixupHash,
OriginalHash: originalHash,
FixupHash: fixupHash,
ChangeToFixup: changeToFixup,
}
}
@ -269,7 +271,7 @@ func (self *MoveFixupCommitDownInstruction) SerializedInstructions() string {
func (self *MoveFixupCommitDownInstruction) run(common *common.Common) error {
return handleInteractiveRebase(common, func(path string) error {
return utils.MoveFixupCommitDown(path, self.OriginalHash, self.FixupHash, getCommentChar())
return utils.MoveFixupCommitDown(path, self.OriginalHash, self.FixupHash, self.ChangeToFixup, getCommentChar())
})
}

24
pkg/commands/git_commands/rebase.go
View File

@ -284,6 +284,11 @@ func (self *RebaseCommands) GitRebaseEditTodo(todosFileContent []byte) error {
return cmdObj.Run()
}
func (self *RebaseCommands) getHashOfLastCommitMade() (string, error) {
cmdArgs := NewGitCmd("rev-parse").Arg("--verify", "HEAD").ToArgv()
return self.cmd.New(cmdArgs).RunWithOutput()
}
// AmendTo amends the given commit with whatever files are staged
func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) error {
commit := commits[commitIndex]
@ -292,9 +297,7 @@ func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) e
return err
}
// Get the hash of the commit we just created
cmdArgs := NewGitCmd("rev-parse").Arg("--verify", "HEAD").ToArgv()
fixupHash, err := self.cmd.New(cmdArgs).RunWithOutput()
fixupHash, err := self.getHashOfLastCommitMade()
if err != nil {
return err
}
@ -302,7 +305,20 @@ func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) e
return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
baseHashOrRoot: getBaseHashOrRoot(commits, commitIndex+1),
overrideEditor: true,
instruction: daemon.NewMoveFixupCommitDownInstruction(commit.Hash, fixupHash),
instruction: daemon.NewMoveFixupCommitDownInstruction(commit.Hash, fixupHash, true),
}).Run()
}
func (self *RebaseCommands) MoveFixupCommitDown(commits []*models.Commit, targetCommitIndex int) error {
fixupHash, err := self.getHashOfLastCommitMade()
if err != nil {
return err
}
return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{
baseHashOrRoot: getBaseHashOrRoot(commits, targetCommitIndex+1),
overrideEditor: true,
instruction: daemon.NewMoveFixupCommitDownInstruction(commits[targetCommitIndex].Hash, fixupHash, false),
}).Run()
}

52
pkg/gui/controllers/local_commits_controller.go
View File

@ -895,6 +895,10 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err
return err
}
if err := self.moveFixupCommitToOwnerStackedBranch(commit); err != nil {
return err
}
self.context().MoveSelectedLine(1)
return self.c.Refresh(types.RefreshOptions{Mode: types.SYNC})
})
@ -924,6 +928,50 @@ func (self *LocalCommitsController) createFixupCommit(commit *models.Commit) err
})
}
func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCommit *models.Commit) error {
if self.c.Git().Version.IsOlderThan(2, 38, 0) {
// Git 2.38.0 introduced the `rebase.updateRefs` config option. Don't
// move the commit down with older versions, as it would break the stack.
return nil
}
if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE {
// Can't move commits while rebasing
return nil
}
if targetCommit.Status == models.StatusMerged {
// Target commit is already on main. It's a bit questionable that we
// allow creating a fixup commit for it in the first place, but we
// always did, so why restrict that now; however, it doesn't make sense
// to move the created fixup commit down in that case.
return nil
}
if !self.c.Git().Config.GetRebaseUpdateRefs() {
// If the user has disabled rebase.updateRefs, we don't move the fixup
// because this would break the stack of branches (presumably they like
// to manage it themselves manually, or something).
return nil
}
headOfOwnerBranchIdx := -1
for i := self.context().GetSelectedLineIdx(); i > 0; i-- {
if lo.SomeBy(self.c.Model().Branches, func(b *models.Branch) bool {
return b.CommitHash == self.c.Model().Commits[i].Hash
}) {
headOfOwnerBranchIdx = i
break
}
}
if headOfOwnerBranchIdx == -1 {
return nil
}
return self.c.Git().Rebase.MoveFixupCommitDown(self.c.Model().Commits, headOfOwnerBranchIdx)
}
func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, includeFileChanges bool) error {
commitMessage, err := self.c.Git().Commit.GetCommitMessage(commit.Hash)
if err != nil {
@ -947,6 +995,10 @@ func (self *LocalCommitsController) createAmendCommit(commit *models.Commit, inc
return err
}
if err := self.moveFixupCommitToOwnerStackedBranch(commit); err != nil {
return err
}
self.context().MoveSelectedLine(1)
return self.c.Refresh(types.RefreshOptions{Mode: types.SYNC})
})

53
pkg/integration/tests/commit/create_fixup_commit_in_branch_stack.go Normal file
View File

@ -0,0 +1,53 @@
package commit
import (
"github.com/jesseduffield/lazygit/pkg/config"
. "github.com/jesseduffield/lazygit/pkg/integration/components"
)
var CreateFixupCommitInBranchStack = NewIntegrationTest(NewIntegrationTestArgs{
Description: "Create a fixup commit in a stack of branches, verify that it is created at the end of the branch it belongs to",
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("branch2 commit 2")
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 ◯ branch2 commit 2"),
Contains("CI ◯ branch2 commit 1"),
Contains("CI ◯ * branch1 commit 3"),
Contains("CI ◯ branch1 commit 2"),
Contains("CI ◯ branch1 commit 1"),
).
NavigateToLine(Contains("branch1 commit 2")).
Press(keys.Commits.CreateFixupCommit).
Tap(func() {
t.ExpectPopup().Menu().
Title(Equals("Create fixup commit")).
Select(Contains("fixup! commit")).
Confirm()
}).
Lines(
Contains("CI ◯ branch2 commit 2"),
Contains("CI ◯ branch2 commit 1"),
Contains("CI ◯ * fixup! branch1 commit 2"),
Contains("CI ◯ branch1 commit 3"),
Contains("CI ◯ branch1 commit 2"),
Contains("CI ◯ branch1 commit 1"),
)
},
})

1
pkg/integration/tests/test_list.go
View File

@ -87,6 +87,7 @@ var tests = []*components.IntegrationTest{
commit.CommitWithNonMatchingBranchName,
commit.CommitWithPrefix,
commit.CreateAmendCommit,
commit.CreateFixupCommitInBranchStack,
commit.CreateTag,
commit.DiscardOldFileChanges,
commit.FindBaseCommitForFixup,

10
pkg/utils/rebase_todo.go
View File

@ -221,13 +221,13 @@ func moveTodosUp(todos []todo.Todo, todosToMove []Todo) ([]todo.Todo, error) {
return todos, nil
}
func MoveFixupCommitDown(fileName string, originalHash string, fixupHash string, commentChar byte) error {
func MoveFixupCommitDown(fileName string, originalHash string, fixupHash string, changeToFixup bool, commentChar byte) error {
todos, err := ReadRebaseTodoFile(fileName, commentChar)
if err != nil {
return err
}
newTodos, err := moveFixupCommitDown(todos, originalHash, fixupHash)
newTodos, err := moveFixupCommitDown(todos, originalHash, fixupHash, changeToFixup)
if err != nil {
return err
}
@ -235,7 +235,7 @@ func MoveFixupCommitDown(fileName string, originalHash string, fixupHash string,
return WriteRebaseTodoFile(fileName, newTodos, commentChar)
}
func moveFixupCommitDown(todos []todo.Todo, originalHash string, fixupHash string) ([]todo.Todo, error) {
func moveFixupCommitDown(todos []todo.Todo, originalHash string, fixupHash string, changeToFixup bool) ([]todo.Todo, error) {
isOriginal := func(t todo.Todo) bool {
return (t.Command == todo.Pick || t.Command == todo.Merge) && equalHash(t.Commit, originalHash)
}
@ -259,7 +259,9 @@ func moveFixupCommitDown(todos []todo.Todo, originalHash string, fixupHash strin
newTodos := MoveElement(todos, fixupIndex, originalIndex+1)
newTodos[originalIndex+1].Command = todo.Fixup
if changeToFixup {
newTodos[originalIndex+1].Command = todo.Fixup
}
return newTodos, nil
}

39
pkg/utils/rebase_todo_test.go
View File

@ -266,23 +266,40 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
todos []todo.Todo
originalHash string
fixupHash string
changeToFixup bool
expectedTodos []todo.Todo
expectedErr error
}{
{
name: "fixup commit is the last commit",
name: "fixup commit is the last commit (change to fixup)",
todos: []todo.Todo{
{Command: todo.Pick, Commit: "original"},
{Command: todo.Pick, Commit: "fixup"},
},
originalHash: "original",
fixupHash: "fixup",
originalHash: "original",
fixupHash: "fixup",
changeToFixup: true,
expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "original"},
{Command: todo.Fixup, Commit: "fixup"},
},
expectedErr: nil,
},
{
name: "fixup commit is the last commit (don't change to fixup)",
todos: []todo.Todo{
{Command: todo.Pick, Commit: "original"},
{Command: todo.Pick, Commit: "fixup"},
},
originalHash: "original",
fixupHash: "fixup",
changeToFixup: false,
expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "original"},
{Command: todo.Pick, Commit: "fixup"},
},
expectedErr: nil,
},
{
name: "fixup commit is separated from original commit",
todos: []todo.Todo{
@ -290,8 +307,9 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
{Command: todo.Pick, Commit: "other"},
{Command: todo.Pick, Commit: "fixup"},
},
originalHash: "original",
fixupHash: "fixup",
originalHash: "original",
fixupHash: "fixup",
changeToFixup: true,
expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "original"},
{Command: todo.Fixup, Commit: "fixup"},
@ -306,8 +324,9 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
{Command: todo.Pick, Commit: "other"},
{Command: todo.Pick, Commit: "fixup"},
},
originalHash: "original",
fixupHash: "fixup",
originalHash: "original",
fixupHash: "fixup",
changeToFixup: true,
expectedTodos: []todo.Todo{
{Command: todo.Merge, Commit: "original"},
{Command: todo.Fixup, Commit: "fixup"},
@ -324,6 +343,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
},
originalHash: "original",
fixupHash: "fixup",
changeToFixup: true,
expectedTodos: nil,
expectedErr: errors.New("Expected exactly one original hash, found 2"),
},
@ -336,6 +356,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
},
originalHash: "original",
fixupHash: "fixup",
changeToFixup: true,
expectedTodos: nil,
expectedErr: errors.New("Expected exactly one fixup hash, found 2"),
},
@ -346,6 +367,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
},
originalHash: "original",
fixupHash: "fixup",
changeToFixup: true,
expectedTodos: nil,
expectedErr: errors.New("Expected exactly one fixup hash, found 0"),
},
@ -356,6 +378,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
},
originalHash: "original",
fixupHash: "fixup",
changeToFixup: true,
expectedTodos: nil,
expectedErr: errors.New("Expected exactly one original hash, found 0"),
},
@ -363,7 +386,7 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
for _, scenario := range scenarios {
t.Run(scenario.name, func(t *testing.T) {
actualTodos, actualErr := moveFixupCommitDown(scenario.todos, scenario.originalHash, scenario.fixupHash)
actualTodos, actualErr := moveFixupCommitDown(scenario.todos, scenario.originalHash, scenario.fixupHash, scenario.changeToFixup)
if scenario.expectedErr == nil {
assert.NoError(t, actualErr)