diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index 2b8248224..15c06b1c4 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -140,36 +140,43 @@ func MoveFixupCommitDown(fileName string, originalSha string, fixupSha string) e return err } - newTodos := []todo.Todo{} - numOriginalShaLinesFound := 0 - numFixupShaLinesFound := 0 - - for _, t := range todos { - if t.Command == todo.Pick { - if equalShas(t.Commit, originalSha) { - numOriginalShaLinesFound += 1 - // append the original commit, and then the fixup - newTodos = append(newTodos, t) - newTodos = append(newTodos, todo.Todo{Command: todo.Fixup, Commit: fixupSha}) - continue - } else if equalShas(t.Commit, fixupSha) { - numFixupShaLinesFound += 1 - // skip the fixup here - continue - } - } - - newTodos = append(newTodos, t) - } - - if numOriginalShaLinesFound != 1 || numFixupShaLinesFound != 1 { - return fmt.Errorf("Expected exactly one each of originalSha and fixupSha, got %d, %d", - numOriginalShaLinesFound, numFixupShaLinesFound) + newTodos, err := moveFixupCommitDown(todos, originalSha, fixupSha) + if err != nil { + return err } return WriteRebaseTodoFile(fileName, newTodos) } +func moveFixupCommitDown(todos []todo.Todo, originalSha string, fixupSha string) ([]todo.Todo, error) { + isOriginal := func(t todo.Todo) bool { + return t.Command == todo.Pick && equalShas(t.Commit, originalSha) + } + + isFixup := func(t todo.Todo) bool { + return t.Command == todo.Pick && equalShas(t.Commit, fixupSha) + } + + originalShaCount := lo.CountBy(todos, isOriginal) + if originalShaCount != 1 { + return nil, fmt.Errorf("Expected exactly one original SHA, found %d", originalShaCount) + } + + fixupShaCount := lo.CountBy(todos, isFixup) + if fixupShaCount != 1 { + return nil, fmt.Errorf("Expected exactly one fixup SHA, found %d", fixupShaCount) + } + + _, fixupIndex, _ := lo.FindIndexOf(todos, isFixup) + _, originalIndex, _ := lo.FindIndexOf(todos, isOriginal) + + newTodos := MoveElement(todos, fixupIndex, originalIndex+1) + + newTodos[originalIndex+1].Command = todo.Fixup + + return newTodos, nil +} + // 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 { diff --git a/pkg/utils/rebase_todo_test.go b/pkg/utils/rebase_todo_test.go index a4b1a46d0..4f554e926 100644 --- a/pkg/utils/rebase_todo_test.go +++ b/pkg/utils/rebase_todo_test.go @@ -1,6 +1,7 @@ package utils import ( + "errors" "testing" "github.com/fsmiamoto/git-todo-parser/todo" @@ -228,3 +229,110 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { ) } } + +func TestRebaseCommands_moveFixupCommitDown(t *testing.T) { + scenarios := []struct { + name string + todos []todo.Todo + originalSha string + fixupSha string + expectedTodos []todo.Todo + expectedErr error + }{ + { + name: "fixup commit is the last commit", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Fixup, Commit: "fixup"}, + }, + expectedErr: nil, + }, + { + // TODO: is this something we actually want to support? + name: "fixup commit is separated from original commit", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "other"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Fixup, Commit: "fixup"}, + {Command: todo.Pick, Commit: "other"}, + }, + expectedErr: nil, + }, + { + name: "More original SHAs than expected", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one original SHA, found 2"), + }, + { + name: "More fixup SHAs than expected", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "fixup"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one fixup SHA, found 2"), + }, + { + name: "No fixup SHAs found", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one fixup SHA, found 0"), + }, + { + name: "No original SHAs found", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one original SHA, found 0"), + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + actualTodos, actualErr := moveFixupCommitDown(scenario.todos, scenario.originalSha, scenario.fixupSha) + + if scenario.expectedErr == nil { + if !assert.NoError(t, actualErr) { + t.Errorf("Expected no error, got: %v", actualErr) + } + } else { + if !assert.EqualError(t, actualErr, scenario.expectedErr.Error()) { + t.Errorf("Expected err: %v, got: %v", scenario.expectedErr, actualErr) + } + } + + if !assert.EqualValues(t, actualTodos, scenario.expectedTodos) { + t.Errorf("Expected todos: %v, got: %v", scenario.expectedTodos, actualTodos) + } + }) + } +}