diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index ce1cc2ae0..782df5f1f 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -325,7 +325,7 @@ func (self *MoveTodosUpInstruction) run(common *common.Common) error { }) return handleInteractiveRebase(common, func(path string) error { - return utils.MoveTodosUp(path, todosToMove, getCommentChar()) + return utils.MoveTodosUp(path, todosToMove, false, getCommentChar()) }) } @@ -355,7 +355,7 @@ func (self *MoveTodosDownInstruction) run(common *common.Common) error { }) return handleInteractiveRebase(common, func(path string) error { - return utils.MoveTodosDown(path, todosToMove, getCommentChar()) + return utils.MoveTodosDown(path, todosToMove, false, getCommentChar()) }) } diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 5646b5898..757257750 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -369,7 +369,7 @@ func (self *RebaseCommands) MoveTodosDown(commits []*models.Commit) error { return todoFromCommit(commit) }) - return utils.MoveTodosDown(fileName, todosToMove, self.config.GetCoreCommentChar()) + return utils.MoveTodosDown(fileName, todosToMove, true, self.config.GetCoreCommentChar()) } func (self *RebaseCommands) MoveTodosUp(commits []*models.Commit) error { @@ -378,7 +378,7 @@ func (self *RebaseCommands) MoveTodosUp(commits []*models.Commit) error { return todoFromCommit(commit) }) - return utils.MoveTodosUp(fileName, todosToMove, self.config.GetCoreCommentChar()) + return utils.MoveTodosUp(fileName, todosToMove, true, self.config.GetCoreCommentChar()) } // SquashAllAboveFixupCommits squashes all fixup! commits above the given one diff --git a/pkg/integration/tests/interactive_rebase/move_across_branch_boundary_outside_rebase.go b/pkg/integration/tests/interactive_rebase/move_across_branch_boundary_outside_rebase.go new file mode 100644 index 000000000..d925acffe --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/move_across_branch_boundary_outside_rebase.go @@ -0,0 +1,47 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveAcrossBranchBoundaryOutsideRebase = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move a commit across a branch boundary in a stack of branches", + ExtraCmdArgs: []string{}, + Skip: false, + GitVersion: AtLeast("2.38.0"), + SetupConfig: func(config *config.AppConfig) { + config.GetUserConfig().Git.MainBranches = []string{"master"} + config.GetAppState().GitLogShowGraph = "never" + }, + SetupRepo: func(shell *Shell) { + shell. + CreateNCommits(1). + NewBranch("branch1"). + CreateNCommitsStartingAt(2, 2). + NewBranch("branch2"). + CreateNCommitsStartingAt(2, 4) + + shell.SetConfig("rebase.updateRefs", "true") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("CI commit 05").IsSelected(), + Contains("CI commit 04"), + Contains("CI * commit 03"), + Contains("CI commit 02"), + Contains("CI commit 01"), + ). + NavigateToLine(Contains("commit 04")). + Press(keys.Commits.MoveDownCommit). + Lines( + Contains("CI commit 05"), + Contains("CI * commit 03"), + Contains("CI commit 04").IsSelected(), + Contains("CI commit 02"), + Contains("CI commit 01"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 041f0416f..782bdcb1b 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -223,6 +223,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.InteractiveRebaseOfCopiedBranch, interactive_rebase.MidRebaseRangeSelect, interactive_rebase.Move, + interactive_rebase.MoveAcrossBranchBoundaryOutsideRebase, interactive_rebase.MoveInRebase, interactive_rebase.MoveUpdateRefTodo, interactive_rebase.MoveWithCustomCommentChar, diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index eedb3bab1..e2c9dc442 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -141,41 +141,41 @@ func deleteTodos(todos []todo.Todo, todosToDelete []Todo) ([]todo.Todo, error) { return todos, nil } -func MoveTodosDown(fileName string, todosToMove []Todo, commentChar byte) error { +func MoveTodosDown(fileName string, todosToMove []Todo, isInRebase bool, commentChar byte) error { todos, err := ReadRebaseTodoFile(fileName, commentChar) if err != nil { return err } - rearrangedTodos, err := moveTodosDown(todos, todosToMove) + rearrangedTodos, err := moveTodosDown(todos, todosToMove, isInRebase) if err != nil { return err } return WriteRebaseTodoFile(fileName, rearrangedTodos, commentChar) } -func MoveTodosUp(fileName string, todosToMove []Todo, commentChar byte) error { +func MoveTodosUp(fileName string, todosToMove []Todo, isInRebase bool, commentChar byte) error { todos, err := ReadRebaseTodoFile(fileName, commentChar) if err != nil { return err } - rearrangedTodos, err := moveTodosUp(todos, todosToMove) + rearrangedTodos, err := moveTodosUp(todos, todosToMove, isInRebase) if err != nil { return err } return WriteRebaseTodoFile(fileName, rearrangedTodos, commentChar) } -func moveTodoDown(todos []todo.Todo, todoToMove Todo) ([]todo.Todo, error) { - rearrangedTodos, err := moveTodoUp(lo.Reverse(todos), todoToMove) +func moveTodoDown(todos []todo.Todo, todoToMove Todo, isInRebase bool) ([]todo.Todo, error) { + rearrangedTodos, err := moveTodoUp(lo.Reverse(todos), todoToMove, isInRebase) return lo.Reverse(rearrangedTodos), err } -func moveTodosDown(todos []todo.Todo, todosToMove []Todo) ([]todo.Todo, error) { - rearrangedTodos, err := moveTodosUp(lo.Reverse(todos), lo.Reverse(todosToMove)) +func moveTodosDown(todos []todo.Todo, todosToMove []Todo, isInRebase bool) ([]todo.Todo, error) { + rearrangedTodos, err := moveTodosUp(lo.Reverse(todos), lo.Reverse(todosToMove), isInRebase) return lo.Reverse(rearrangedTodos), err } -func moveTodoUp(todos []todo.Todo, todoToMove Todo) ([]todo.Todo, error) { +func moveTodoUp(todos []todo.Todo, todoToMove Todo, isInRebase bool) ([]todo.Todo, error) { sourceIdx, ok := findTodo(todos, todoToMove) if !ok { @@ -188,7 +188,7 @@ func moveTodoUp(todos []todo.Todo, todoToMove Todo) ([]todo.Todo, error) { // the end of the slice) // Find the next todo that we show in lazygit's commits view (skipping the rest) - _, skip, ok := lo.FindIndexOf(todos[sourceIdx+1:], isRenderedTodo) + _, skip, ok := lo.FindIndexOf(todos[sourceIdx+1:], func(t todo.Todo) bool { return isRenderedTodo(t, isInRebase) }) if !ok { // We expect callers to guard against this @@ -202,10 +202,10 @@ func moveTodoUp(todos []todo.Todo, todoToMove Todo) ([]todo.Todo, error) { return rearrangedTodos, nil } -func moveTodosUp(todos []todo.Todo, todosToMove []Todo) ([]todo.Todo, error) { +func moveTodosUp(todos []todo.Todo, todosToMove []Todo, isInRebase bool) ([]todo.Todo, error) { for _, todoToMove := range todosToMove { var newTodos []todo.Todo - newTodos, err := moveTodoUp(todos, todoToMove) + newTodos, err := moveTodoUp(todos, todoToMove, isInRebase) if err != nil { return nil, err } @@ -286,9 +286,9 @@ func RemoveUpdateRefsForCopiedBranch(fileName string, commentChar byte) error { } // We render a todo in the commits view if it's a commit or if it's an -// update-ref. We don't render label, reset, or comment lines. -func isRenderedTodo(t todo.Todo) bool { - return t.Commit != "" || t.Command == todo.UpdateRef +// update-ref or exec. We don't render label, reset, or comment lines. +func isRenderedTodo(t todo.Todo, isInRebase bool) bool { + return t.Commit != "" || (isInRebase && (t.Command == todo.UpdateRef || t.Command == todo.Exec)) } func DropMergeCommit(fileName string, hash string, commentChar byte) error { diff --git a/pkg/utils/rebase_todo_test.go b/pkg/utils/rebase_todo_test.go index 4896c8a1d..9daf7db01 100644 --- a/pkg/utils/rebase_todo_test.go +++ b/pkg/utils/rebase_todo_test.go @@ -14,6 +14,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { testName string todos []todo.Todo todoToMoveDown Todo + isInRebase bool expectedErr string expectedTodos []todo.Todo } @@ -64,6 +65,54 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, }, }, + { + testName: "move across update-ref todo in rebase", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "5678"}, + }, + todoToMoveDown: Todo{Hash: "5678"}, + isInRebase: true, + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + }, + }, + { + testName: "move across update-ref todo outside of rebase", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "5678"}, + }, + todoToMoveDown: Todo{Hash: "5678"}, + isInRebase: false, + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + }, + }, + { + testName: "move across exec todo", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Exec, ExecCommand: "make test"}, + {Command: todo.Pick, Commit: "5678"}, + }, + todoToMoveDown: Todo{Hash: "5678"}, + isInRebase: true, + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Exec, ExecCommand: "make test"}, + }, + }, { testName: "skip an invisible todo", todos: []todo.Todo{ @@ -123,7 +172,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { for _, s := range scenarios { t.Run(s.testName, func(t *testing.T) { - rearrangedTodos, err := moveTodoDown(s.todos, s.todoToMoveDown) + rearrangedTodos, err := moveTodoDown(s.todos, s.todoToMoveDown, s.isInRebase) if s.expectedErr == "" { assert.NoError(t, err) } else { @@ -140,6 +189,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { testName string todos []todo.Todo todoToMoveUp Todo + isInRebase bool expectedErr string expectedTodos []todo.Todo } @@ -190,6 +240,54 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, }, }, + { + testName: "move across update-ref todo in rebase", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "5678"}, + }, + todoToMoveUp: Todo{Hash: "1234"}, + isInRebase: true, + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + }, + }, + { + testName: "move across update-ref todo outside of rebase", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "5678"}, + }, + todoToMoveUp: Todo{Hash: "1234"}, + isInRebase: false, + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "1234"}, + }, + }, + { + testName: "move across exec todo", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Exec, ExecCommand: "make test"}, + {Command: todo.Pick, Commit: "5678"}, + }, + todoToMoveUp: Todo{Hash: "1234"}, + isInRebase: true, + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Exec, ExecCommand: "make test"}, + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + }, + }, { testName: "skip an invisible todo", todos: []todo.Todo{ @@ -249,7 +347,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { for _, s := range scenarios { t.Run(s.testName, func(t *testing.T) { - rearrangedTodos, err := moveTodoUp(s.todos, s.todoToMoveUp) + rearrangedTodos, err := moveTodoUp(s.todos, s.todoToMoveUp, s.isInRebase) if s.expectedErr == "" { assert.NoError(t, err) } else {