From e8d84a1f2ce48845cd31c0fde04334e1f1bc3f76 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 16 Mar 2024 15:21:17 +0100 Subject: Refactor: pass Todo to moveTodoUp/Down instead of Sha and Action We need this because we want to enable moving update-ref todos, which don't have a sha. --- pkg/utils/rebase_todo.go | 12 ++++---- pkg/utils/rebase_todo_test.go | 64 +++++++++++++++++++++---------------------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index e4bfe25d0..c0eda8f0b 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -116,8 +116,8 @@ func MoveTodosUp(fileName string, todosToMove []Todo, commentChar byte) error { return WriteRebaseTodoFile(fileName, rearrangedTodos, commentChar) } -func moveTodoDown(todos []todo.Todo, sha string, action todo.TodoCommand) ([]todo.Todo, error) { - rearrangedTodos, err := moveTodoUp(lo.Reverse(todos), sha, action) +func moveTodoDown(todos []todo.Todo, todoToMove Todo) ([]todo.Todo, error) { + rearrangedTodos, err := moveTodoUp(lo.Reverse(todos), todoToMove) return lo.Reverse(rearrangedTodos), err } @@ -126,17 +126,17 @@ func moveTodosDown(todos []todo.Todo, todosToMove []Todo) ([]todo.Todo, error) { return lo.Reverse(rearrangedTodos), err } -func moveTodoUp(todos []todo.Todo, sha string, action todo.TodoCommand) ([]todo.Todo, error) { +func moveTodoUp(todos []todo.Todo, todoToMove Todo) ([]todo.Todo, error) { _, sourceIdx, ok := lo.FindIndexOf(todos, func(t todo.Todo) bool { // Comparing just the sha is not enough; we need to compare both the // action and the sha, as the sha could appear multiple times (e.g. in a // pick and later in a merge) - return t.Command == action && equalShas(t.Commit, sha) + return t.Command == todoToMove.Action && equalShas(t.Commit, todoToMove.Sha) }) if !ok { // Should never happen - return []todo.Todo{}, fmt.Errorf("Todo %s not found in git-rebase-todo", sha) + return []todo.Todo{}, fmt.Errorf("Todo %s not found in git-rebase-todo", todoToMove.Sha) } // The todos are ordered backwards compared to our model commits, so @@ -161,7 +161,7 @@ func moveTodoUp(todos []todo.Todo, sha string, action todo.TodoCommand) ([]todo. func moveTodosUp(todos []todo.Todo, todosToMove []Todo) ([]todo.Todo, error) { for _, todoToMove := range todosToMove { var newTodos []todo.Todo - newTodos, err := moveTodoUp(todos, todoToMove.Sha, todoToMove.Action) + newTodos, err := moveTodoUp(todos, todoToMove) if err != nil { return nil, err } diff --git a/pkg/utils/rebase_todo_test.go b/pkg/utils/rebase_todo_test.go index 40f44a3cb..fd736d7a0 100644 --- a/pkg/utils/rebase_todo_test.go +++ b/pkg/utils/rebase_todo_test.go @@ -10,11 +10,11 @@ import ( func TestRebaseCommands_moveTodoDown(t *testing.T) { type scenario struct { - testName string - todos []todo.Todo - shaToMoveDown string - expectedErr string - expectedTodos []todo.Todo + testName string + todos []todo.Todo + todoToMoveDown Todo + expectedErr string + expectedTodos []todo.Todo } scenarios := []scenario{ @@ -25,8 +25,8 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "5678", - expectedErr: "", + todoToMoveDown: Todo{Sha: "5678", Action: todo.Pick}, + expectedErr: "", expectedTodos: []todo.Todo{ {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "1234"}, @@ -40,8 +40,8 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "abcd", - expectedErr: "", + todoToMoveDown: Todo{Sha: "abcd", Action: todo.Pick}, + expectedErr: "", expectedTodos: []todo.Todo{ {Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "abcd"}, @@ -57,8 +57,8 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "def0"}, }, - shaToMoveDown: "5678", - expectedErr: "", + todoToMoveDown: Todo{Sha: "5678", Action: todo.Pick}, + expectedErr: "", expectedTodos: []todo.Todo{ {Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "5678"}, @@ -76,9 +76,9 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "def0", - expectedErr: "Todo def0 not found in git-rebase-todo", - expectedTodos: []todo.Todo{}, + todoToMoveDown: Todo{Sha: "def0", Action: todo.Pick}, + expectedErr: "Todo def0 not found in git-rebase-todo", + expectedTodos: []todo.Todo{}, }, { testName: "trying to move first commit down", @@ -87,9 +87,9 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "1234", - expectedErr: "Destination position for moving todo is out of range", - expectedTodos: []todo.Todo{}, + todoToMoveDown: Todo{Sha: "1234", Action: todo.Pick}, + expectedErr: "Destination position for moving todo is out of range", + expectedTodos: []todo.Todo{}, }, { testName: "trying to move commit down when all commits before are invisible", @@ -99,15 +99,15 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) { {Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "5678"}, }, - shaToMoveDown: "1234", - expectedErr: "Destination position for moving todo is out of range", - expectedTodos: []todo.Todo{}, + todoToMoveDown: Todo{Sha: "1234", Action: todo.Pick}, + expectedErr: "Destination position for moving todo is out of range", + expectedTodos: []todo.Todo{}, }, } for _, s := range scenarios { t.Run(s.testName, func(t *testing.T) { - rearrangedTodos, err := moveTodoDown(s.todos, s.shaToMoveDown, todo.Pick) + rearrangedTodos, err := moveTodoDown(s.todos, s.todoToMoveDown) if s.expectedErr == "" { assert.NoError(t, err) } else { @@ -123,7 +123,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { type scenario struct { testName string todos []todo.Todo - shaToMoveDown string + todoToMoveUp Todo expectedErr string expectedTodos []todo.Todo } @@ -136,8 +136,8 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "5678", - expectedErr: "", + todoToMoveUp: Todo{Sha: "5678", Action: todo.Pick}, + expectedErr: "", expectedTodos: []todo.Todo{ {Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "abcd"}, @@ -151,8 +151,8 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "1234", - expectedErr: "", + todoToMoveUp: Todo{Sha: "1234", Action: todo.Pick}, + expectedErr: "", expectedTodos: []todo.Todo{ {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "1234"}, @@ -168,8 +168,8 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "def0"}, }, - shaToMoveDown: "abcd", - expectedErr: "", + todoToMoveUp: Todo{Sha: "abcd", Action: todo.Pick}, + expectedErr: "", expectedTodos: []todo.Todo{ {Command: todo.Pick, Commit: "1234"}, {Command: todo.Label, Label: "myLabel"}, @@ -187,7 +187,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "def0", + todoToMoveUp: Todo{Sha: "def0", Action: todo.Pick}, expectedErr: "Todo def0 not found in git-rebase-todo", expectedTodos: []todo.Todo{}, }, @@ -198,7 +198,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { {Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "abcd"}, }, - shaToMoveDown: "abcd", + todoToMoveUp: Todo{Sha: "abcd", Action: todo.Pick}, expectedErr: "Destination position for moving todo is out of range", expectedTodos: []todo.Todo{}, }, @@ -210,7 +210,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { {Command: todo.Label, Label: "myLabel"}, {Command: todo.Reset, Label: "otherlabel"}, }, - shaToMoveDown: "5678", + todoToMoveUp: Todo{Sha: "5678", Action: todo.Pick}, expectedErr: "Destination position for moving todo is out of range", expectedTodos: []todo.Todo{}, }, @@ -218,7 +218,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.shaToMoveDown, todo.Pick) + rearrangedTodos, err := moveTodoUp(s.todos, s.todoToMoveUp) if s.expectedErr == "" { assert.NoError(t, err) } else { -- cgit v1.2.3