diff options
author | Jesse Duffield <jessedduffield@gmail.com> | 2023-04-24 11:42:23 +1000 |
---|---|---|
committer | Stefan Haller <stefan@haller-berlin.de> | 2023-04-29 07:28:33 +0200 |
commit | e63858215e8778f58d66bf064359d70a48eb6756 (patch) | |
tree | b2bca455955af976f6d02b3ae1e9d34477b054c8 | |
parent | 3fe4db9316f2bcfb168c6585acee425831f9b04b (diff) |
refactor moveFixupCommitDown
-rw-r--r-- | pkg/utils/rebase_todo.go | 53 | ||||
-rw-r--r-- | pkg/utils/rebase_todo_test.go | 108 |
2 files changed, 138 insertions, 23 deletions
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,34 +140,41 @@ 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, err := moveFixupCommitDown(todos, originalSha, fixupSha) + if err != nil { + return err + } - newTodos = append(newTodos, t) + 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) } - if numOriginalShaLinesFound != 1 || numFixupShaLinesFound != 1 { - return fmt.Errorf("Expected exactly one each of originalSha and fixupSha, got %d, %d", - numOriginalShaLinesFound, numFixupShaLinesFound) + isFixup := func(t todo.Todo) bool { + return t.Command == todo.Pick && equalShas(t.Commit, fixupSha) } - return WriteRebaseTodoFile(fileName, newTodos) + 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 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) + } + }) + } +} |