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.

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 {