summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorStefan Haller <stefan@haller-berlin.de>2024-03-17 07:38:25 +0100
committerGitHub <noreply@github.com>2024-03-17 07:38:25 +0100
commite9d050c5d5f377f263686584f432a0935c358809 (patch)
treec7157bc0767cea290069b2ef7ac4aceabf8b64d1
parentfd18db6ba285eb280aee1cb8c381295df6556cad (diff)
parent5c56bc7015ec635245d6270287508851276a040f (diff)
Allow moving and deleting update-ref todos (#3391)
- **PR Description** Previously it wasn't possible to move an update-ref entry up or down using ctrl-j and ctrl-k, or to delete an update-ref entry. For moving, a work-around was to move the surrounding commits instead, but it's still nice to be able to do it directly. Deleting is very much necessary though, since there are situations where git adds the update-ref entries but they are not wanted; one example is if you want to make a copy of a branch and rebase to a different place, without the original branch following it. (There's a long discussion about this [here](https://public-inbox.org/git/adb7f680-5bfa-6fa5-6d8a-61323fee7f53@haller-berlin.de/).) Update-ref todos can't be set to "drop" like other todos, because they have no sha associated with them, so we need to delete them immediately. We show a confirmation before doing that, because you'd have to abort the rebase if you do it accidentally. We allow range selecting normal todos and update-ref todos at the same time; in that case, we delete all the update-ref todos and set all the other ones to "drop". Not that this is an absolutely necessary feature, but it wasn't hard to do.
-rw-r--r--pkg/commands/git_commands/commit_loader.go2
-rw-r--r--pkg/commands/git_commands/rebase.go30
-rw-r--r--pkg/gui/context/traits/list_cursor.go6
-rw-r--r--pkg/gui/controllers/local_commits_controller.go81
-rw-r--r--pkg/gui/presentation/commits.go3
-rw-r--r--pkg/i18n/english.go2
-rw-r--r--pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go65
-rw-r--r--pkg/integration/tests/interactive_rebase/move_update_ref_todo.go61
-rw-r--r--pkg/integration/tests/test_list.go2
-rw-r--r--pkg/utils/rebase_todo.go60
-rw-r--r--pkg/utils/rebase_todo_test.go149
11 files changed, 403 insertions, 58 deletions
diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go
index 7cd7f38b0..df22b91bf 100644
--- a/pkg/commands/git_commands/commit_loader.go
+++ b/pkg/commands/git_commands/commit_loader.go
@@ -348,7 +348,7 @@ func (self *CommitLoader) getRebasingCommits(rebaseMode enums.RebaseMode) []*mod
for _, t := range todos {
if t.Command == todo.UpdateRef {
- t.Msg = strings.TrimPrefix(t.Ref, "refs/heads/")
+ t.Msg = t.Ref
} else if t.Commit == "" {
// Command does not have a commit associated, skip
continue
diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go
index 0bcfa5f67..eeae66167 100644
--- a/pkg/commands/git_commands/rebase.go
+++ b/pkg/commands/git_commands/rebase.go
@@ -272,6 +272,14 @@ func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) e
}).Run()
}
+func todoFromCommit(commit *models.Commit) utils.Todo {
+ if commit.Action == todo.UpdateRef {
+ return utils.Todo{Ref: commit.Name, Action: commit.Action}
+ } else {
+ return utils.Todo{Sha: commit.Sha, Action: commit.Action}
+ }
+}
+
// Sets the action for the given commits in the git-rebase-todo file
func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo.TodoCommand) error {
commitsWithAction := lo.Map(commits, func(commit *models.Commit, _ int) utils.TodoChange {
@@ -289,13 +297,22 @@ func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo
)
}
+func (self *RebaseCommands) DeleteUpdateRefTodos(commits []*models.Commit) error {
+ todosToDelete := lo.Map(commits, func(commit *models.Commit, _ int) utils.Todo {
+ return todoFromCommit(commit)
+ })
+
+ return utils.DeleteTodos(
+ filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"),
+ todosToDelete,
+ self.config.GetCoreCommentChar(),
+ )
+}
+
func (self *RebaseCommands) MoveTodosDown(commits []*models.Commit) error {
fileName := filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo")
todosToMove := lo.Map(commits, func(commit *models.Commit, _ int) utils.Todo {
- return utils.Todo{
- Sha: commit.Sha,
- Action: commit.Action,
- }
+ return todoFromCommit(commit)
})
return utils.MoveTodosDown(fileName, todosToMove, self.config.GetCoreCommentChar())
@@ -304,10 +321,7 @@ func (self *RebaseCommands) MoveTodosDown(commits []*models.Commit) error {
func (self *RebaseCommands) MoveTodosUp(commits []*models.Commit) error {
fileName := filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo")
todosToMove := lo.Map(commits, func(commit *models.Commit, _ int) utils.Todo {
- return utils.Todo{
- Sha: commit.Sha,
- Action: commit.Action,
- }
+ return todoFromCommit(commit)
})
return utils.MoveTodosUp(fileName, todosToMove, self.config.GetCoreCommentChar())
diff --git a/pkg/gui/context/traits/list_cursor.go b/pkg/gui/context/traits/list_cursor.go
index 3b4ab1c3d..31f3de7a4 100644
--- a/pkg/gui/context/traits/list_cursor.go
+++ b/pkg/gui/context/traits/list_cursor.go
@@ -65,7 +65,11 @@ func (self *ListCursor) SetSelection(value int) {
func (self *ListCursor) SetSelectionRangeAndMode(selectedIdx, rangeStartIdx int, mode RangeSelectMode) {
self.selectedIdx = self.clampValue(selectedIdx)
self.rangeStartIdx = self.clampValue(rangeStartIdx)
- self.rangeSelectMode = mode
+ if mode == RangeSelectModeNonSticky && selectedIdx == rangeStartIdx {
+ self.rangeSelectMode = RangeSelectModeNone
+ } else {
+ self.rangeSelectMode = mode
+ }
}
// Returns the selectedIdx, the rangeStartIdx, and the mode of the current selection.
diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go
index 085504662..fe15a37d3 100644
--- a/pkg/gui/controllers/local_commits_controller.go
+++ b/pkg/gui/controllers/local_commits_controller.go
@@ -106,7 +106,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
Handler: self.withItemsRange(self.drop),
GetDisabledReason: self.require(
self.itemRangeSelected(
- self.midRebaseCommandEnabled,
+ self.canDropCommits,
),
),
Description: self.c.Tr.DropCommit,
@@ -179,7 +179,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
Key: opts.GetKey(opts.Config.Commits.MoveDownCommit),
Handler: self.withItemsRange(self.moveDown),
GetDisabledReason: self.require(self.itemRangeSelected(
- self.midRebaseCommandEnabled,
+ self.midRebaseMoveCommandEnabled,
self.canMoveDown,
)),
Description: self.c.Tr.MoveDownCommit,
@@ -188,7 +188,7 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [
Key: opts.GetKey(opts.Config.Commits.MoveUpCommit),
Handler: self.withItemsRange(self.moveUp),
GetDisabledReason: self.require(self.itemRangeSelected(
- self.midRebaseCommandEnabled,
+ self.midRebaseMoveCommandEnabled,
self.canMoveUp,
)),
Description: self.c.Tr.MoveUpCommit,
@@ -282,7 +282,7 @@ func (self *LocalCommitsController) GetOnRenderToMain() func() error {
utils.ResolvePlaceholderString(
self.c.Tr.UpdateRefHere,
map[string]string{
- "ref": commit.Name,
+ "ref": strings.TrimPrefix(commit.Name, "refs/heads/"),
}))
} else {
cmdObj := self.c.Git().Commit.ShowCmdObj(commit.Sha, self.c.Modes().Filtering.GetPath())
@@ -439,6 +439,36 @@ func (self *LocalCommitsController) rewordEditor(commit *models.Commit) error {
func (self *LocalCommitsController) drop(selectedCommits []*models.Commit, startIdx int, endIdx int) error {
if self.isRebasing() {
+ groupedTodos := lo.GroupBy(selectedCommits, func(c *models.Commit) bool {
+ return c.Action == todo.UpdateRef
+ })
+ updateRefTodos := groupedTodos[true]
+ nonUpdateRefTodos := groupedTodos[false]
+
+ if len(updateRefTodos) > 0 {
+ return self.c.Confirm(types.ConfirmOpts{
+ Title: self.c.Tr.DropCommitTitle,
+ Prompt: self.c.Tr.DropUpdateRefPrompt,
+ HandleConfirm: func() error {
+ selectedIdx, rangeStartIdx, rangeSelectMode := self.context().GetSelectionRangeAndMode()
+
+ if err := self.c.Git().Rebase.DeleteUpdateRefTodos(updateRefTodos); err != nil {
+ return err
+ }
+
+ if selectedIdx > rangeStartIdx {
+ selectedIdx = utils.Max(selectedIdx-len(updateRefTodos), rangeStartIdx)
+ } else {
+ rangeStartIdx = utils.Max(rangeStartIdx-len(updateRefTodos), selectedIdx)
+ }
+
+ self.context().SetSelectionRangeAndMode(selectedIdx, rangeStartIdx, rangeSelectMode)
+
+ return self.updateTodos(todo.Drop, nonUpdateRefTodos)
+ },
+ })
+ }
+
return self.updateTodos(todo.Drop, selectedCommits)
}
@@ -1192,6 +1222,49 @@ func (self *LocalCommitsController) midRebaseCommandEnabled(selectedCommits []*m
return nil
}
+// Ensures that if we are mid-rebase, we're only selecting commits that can be moved
+func (self *LocalCommitsController) midRebaseMoveCommandEnabled(selectedCommits []*models.Commit, startIdx int, endIdx int) *types.DisabledReason {
+ if !self.isRebasing() {
+ return nil
+ }
+
+ for _, commit := range selectedCommits {
+ if !commit.IsTODO() {
+ return &types.DisabledReason{Text: self.c.Tr.MustSelectTodoCommits}
+ }
+
+ // All todo types that can be edited are allowed to be moved, plus
+ // update-ref todos
+ if !isChangeOfRebaseTodoAllowed(commit.Action) && commit.Action != todo.UpdateRef {
+ return &types.DisabledReason{Text: self.c.Tr.ChangingThisActionIsNotAllowed}
+ }
+ }
+
+ return nil
+}
+
+func (self *LocalCommitsController) canDropCommits(selectedCommits []*models.Commit, startIdx int, endIdx int) *types.DisabledReason {
+ if !self.isRebasing() {
+ return nil
+ }
+
+ nonUpdateRefTodos := lo.Filter(selectedCommits, func(c *models.Commit, _ int) bool {
+ return c.Action != todo.UpdateRef
+ })
+
+ for _, commit := range nonUpdateRefTodos {
+ if !commit.IsTODO() {
+ return &types.DisabledReason{Text: self.c.Tr.MustSelectTodoCommits}
+ }
+
+ if !isChangeOfRebaseTodoAllowed(commit.Action) {
+ return &types.DisabledReason{Text: self.c.Tr.ChangingThisActionIsNotAllowed}
+ }
+ }
+
+ return nil
+}
+
// These actions represent standard things you might want to do with a commit,
// as opposed to TODO actions like 'merge', 'update-ref', etc.
var standardActions = []todo.TodoCommand{
diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go
index bd4057e41..611fdde8c 100644
--- a/pkg/gui/presentation/commits.go
+++ b/pkg/gui/presentation/commits.go
@@ -342,6 +342,9 @@ func displayCommit(
}
name := commit.Name
+ if commit.Action == todo.UpdateRef {
+ name = strings.TrimPrefix(name, "refs/heads/")
+ }
if parseEmoji {
name = emoji.Sprint(name)
}
diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go
index 1ec3af2c1..d38bb1180 100644
--- a/pkg/i18n/english.go
+++ b/pkg/i18n/english.go
@@ -325,6 +325,7 @@ type TranslationSet struct {
AmendCommitPrompt string
DropCommitTitle string
DropCommitPrompt string
+ DropUpdateRefPrompt string
PullingStatus string
PushingStatus string
FetchingStatus string
@@ -1280,6 +1281,7 @@ func EnglishTranslationSet() TranslationSet {
AmendCommitPrompt: "Are you sure you want to amend this commit with your staged files?",
DropCommitTitle: "Drop commit",
DropCommitPrompt: "Are you sure you want to drop the selected commit(s)?",
+ DropUpdateRefPrompt: "Are you sure you want to delete the selected update-ref todo(s)? This is irreversible except by aborting the rebase.",
PullingStatus: "Pulling",
PushingStatus: "Pushing",
FetchingStatus: "Fetching",
diff --git a/pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go b/pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go
new file mode 100644
index 000000000..d08f518ec
--- /dev/null
+++ b/pkg/integration/tests/interactive_rebase/delete_update_ref_todo.go
@@ -0,0 +1,65 @@
+package interactive_rebase
+
+import (
+ "github.com/jesseduffield/lazygit/pkg/config"
+ . "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var DeleteUpdateRefTodo = NewIntegrationTest(NewIntegrationTestArgs{
+ Description: "Delete an update-ref item from the rebase todo list",
+ ExtraCmdArgs: []string{},
+ Skip: false,
+ GitVersion: AtLeast("2.38.0"),
+ SetupConfig: func(config *config.AppConfig) {},
+ SetupRepo: func(shell *Shell) {
+ shell.
+ NewBranch("branch1").
+ CreateNCommits(3).
+ NewBranch("branch2").
+ CreateNCommitsStartingAt(3, 4)
+
+ shell.SetConfig("rebase.updateRefs", "true")
+ },
+ Run: func(t *TestDriver, keys config.KeybindingConfig) {
+ t.Views().Commits().
+ Focus().
+ NavigateToLine(Contains("commit 01")).
+ Press(keys.Universal.Edit).
+ Lines(
+ Contains("pick").Contains("CI commit 06"),
+ Contains("pick").Contains("CI commit 05"),
+ Contains("pick").Contains("CI commit 04"),
+ Contains("update-ref").Contains("branch1"),
+ Contains("pick").Contains("CI commit 03"),
+ Contains("pick").Contains("CI commit 02"),
+ Contains("CI ◯ <-- YOU ARE HERE --- commit 01"),
+ ).
+ NavigateToLine(Contains("update-ref")).
+ Press(keys.Universal.Remove).
+ Tap(func() {
+ t.ExpectPopup().Confirmation().
+ Title(Equals("Drop commit")).
+ Content(Contains("Are you sure you want to delete the selected update-ref todo(s)?")).
+ Confirm()
+ }).
+ Lines(
+ Contains("pick").Contains("CI commit 06"),
+ Contains("pick").Contains("CI commit 05"),
+ Contains("pick").Contains("CI commit 04"),
+ Contains("pick").Contains("CI commit 03").IsSelected(),
+ Contains("pick").Contains("CI commit 02"),
+ Contains("CI ◯ <-- YOU ARE HERE --- commit 01"),
+ ).
+ Tap(func() {
+ t.Common().ContinueRebase()
+ }).
+ Lines(
+ Contains("CI ◯ commit 06"),
+ Contains("CI ◯ commit 05"),
+ Contains("CI ◯ commit 04"),
+ Contains("CI ◯ commit 03"), // No start on this commit, so there's no branch head here
+ Contains("CI ◯ commit 02"),
+ Contains("CI ◯ commit 01"),
+ )
+ },
+})
diff --git a/pkg/integration/tests/interactive_rebase/move_update_ref_todo.go b/pkg/integration/tests/interactive_rebase/move_update_ref_todo.go
new file mode 100644
index 000000000..3dbfcd9cb
--- /dev/null
+++ b/pkg/integration/tests/interactive_rebase/move_update_ref_todo.go
@@ -0,0 +1,61 @@
+package interactive_rebase
+
+import (
+ "github.com/jesseduffield/lazygit/pkg/config"
+ . "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var MoveUpdateRefTodo = NewIntegrationTest(NewIntegrationTestArgs{
+ Description: "Move an update-ref item in the rebase todo list",
+ ExtraCmdArgs: []string{},
+ Skip: false,
+ GitVersion: AtLeast("2.38.0"),
+ SetupConfig: func(config *config.AppConfig) {},
+ SetupRepo: func(shell *Shell) {
+ shell.
+ NewBranch("branch1").
+ CreateNCommits(3).
+ NewBranch("branch2").
+ CreateNCommitsStartingAt(3, 4)
+
+ shell.SetConfig("rebase.updateRefs", "true")
+ },
+ Run: func(t *TestDriver, keys config.KeybindingConfig) {
+ t.Views().Commits().
+ Focus().
+ NavigateToLine(Contains("commit 01")).
+ Press(keys.Universal.Edit).
+ Lines(
+ Contains("pick").Contains("CI commit 06"),
+ Contains("pick").Contains("CI commit 05"),
+ Contains("pick").Contains("CI commit 04"),
+ Contains("update-ref").Contains("branch1"),
+ Contains("pick").Contains("CI commit 03"),
+ Contains("pick").Contains("CI commit 02"),
+ Contains("CI ◯ <-- YOU ARE HERE --- commit 01"),
+ ).
+ NavigateToLine(Contains("update-ref")).
+ Press(keys.Commits.MoveUpCommit).
+ Press(keys.Commits.MoveUpCommit).
+ Lines(
+ Contains("pick").Contains("CI commit 06"),
+ Contains("update-ref").Contains("branch1"),
+ Contains("pick").Contains("CI commit 05"),
+ Contains("pick").Contains("CI commit 04"),
+ Contains("pick").Contains("CI commit 03"),
+ Contains("pick").Contains("CI commit 02"),
+ Contains("CI ◯ <-- YOU ARE HERE --- commit 01"),
+ ).
+ Tap(func() {
+ t.Common().ContinueRebase()
+ }).
+ Lines(
+ Contains("CI ◯ commit 06"),
+ Contains("CI ◯ * commit 05"),
+ Contains("CI ◯ commit 04"),
+ Contains("CI ◯ commit 03"),
+ 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 e5a540e8b..0a78d0193 100644
--- a/pkg/integration/tests/test_list.go
+++ b/pkg/integration/tests/test_list.go
@@ -164,6 +164,7 @@ var tests = []*components.IntegrationTest{
interactive_rebase.AmendHeadCommitDuringRebase,
interactive_rebase.AmendMerge,
interactive_rebase.AmendNonHeadCommitDuringRebase,
+ interactive_rebase.DeleteUpdateRefTodo,
interactive_rebase.DontShowBranchHeadsForTodoItems,
interactive_rebase.DropTodoCommitWithUpdateRef,
interactive_rebase.DropWithCustomCommentChar,
@@ -176,6 +177,7 @@ var tests = []*components.IntegrationTest{
interactive_rebase.MidRebaseRangeSelect,
interactive_rebase.Move,
interactive_rebase.MoveInRebase,
+ interactive_rebase.MoveUpdateRefTodo,
interactive_rebase.MoveWithCustomCommentChar,
interactive_rebase.OutsideRebaseRangeSelect,
interactive_rebase.PickRescheduled,
diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go
index e4bfe25d0..3403eef97 100644
--- a/pkg/utils/rebase_todo.go
+++ b/pkg/utils/rebase_todo.go
@@ -10,7 +10,8 @@ import (
)
type Todo struct {
- Sha string
+ Sha string // for todos that have one, e.g. pick, drop, fixup, etc.
+ Ref string // for update-ref todos
Action todo.TodoCommand
}
@@ -55,6 +56,19 @@ func equalShas(a, b string) bool {
return strings.HasPrefix(a, b) || strings.HasPrefix(b, a)
}
+func findTodo(todos []todo.Todo, todoToFind Todo) (int, bool) {
+ _, idx, 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). For update-ref todos we also must compare
+ // the Ref.
+ return t.Command == todoToFind.Action &&
+ equalShas(t.Commit, todoToFind.Sha) &&
+ t.Ref == todoToFind.Ref
+ })
+ return idx, ok
+}
+
func ReadRebaseTodoFile(fileName string, commentChar byte) ([]todo.Todo, error) {
f, err := os.Open(fileName)
if err != nil {
@@ -92,6 +106,33 @@ func PrependStrToTodoFile(filePath string, linesToPrepend []byte) error {
return os.WriteFile(filePath, linesToPrepend, 0o644)
}
+func DeleteTodos(fileName string, todosToDelete []Todo, commentChar byte) error {
+ todos, err := ReadRebaseTodoFile(fileName, commentChar)
+ if err != nil {
+ return err
+ }
+ rearrangedTodos, err := deleteTodos(todos, todosToDelete)
+ if err != nil {
+ return err
+ }
+ return WriteRebaseTodoFile(fileName, rearrangedTodos, commentChar)
+}
+
+func deleteTodos(todos []todo.Todo, todosToDelete []Todo) ([]todo.Todo, error) {
+ for _, todoToDelete := range todosToDelete {
+ idx, ok := findTodo(todos, todoToDelete)
+
+ if !ok {
+ // Should never happen
+ return []todo.Todo{}, fmt.Errorf("Todo %s not found in git-rebase-todo", todoToDelete.Sha)
+ }
+
+ todos = Remove(todos, idx)
+ }
+
+ return todos, nil
+}
+
func MoveTodosDown(fileName string, todosToMove []Todo, commentChar byte) error {
todos, err := ReadRebaseTodoFile(fileName, commentChar)
if err != nil {
@@ -116,8 +157,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 +167,12 @@ 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) {
- _, 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)
- })
+func moveTodoUp(todos []todo.Todo, todoToMove Todo) ([]todo.Todo, error) {
+ sourceIdx, ok := findTodo(todos, todoToMove)
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 +197,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..fd87d7c7c 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"},
@@ -49,6 +49,21 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
},
},
{
+ testName: "move update-ref todo",
+ todos: []todo.Todo{
+ {Command: todo.Pick, Commit: "1234"},
+ {Command: todo.Pick, Commit: "5678"},
+ {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"},
+ },
+ todoToMoveDown: Todo{Ref: "refs/heads/some_branch", Action: todo.UpdateRef},
+ expectedErr: "",
+ expectedTodos: []todo.Todo{
+ {Command: todo.Pick, Commit: "1234"},
+ {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"},
+ {Command: todo.Pick, Commit: "5678"},
+ },
+ },
+ {
testName: "skip an invisible todo",
todos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"},
@@ -57,8 +72,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 +91,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 +102,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 +114,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 +138,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 +151,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 +166,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"},
@@ -160,6 +175,21 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
},
},
{
+ testName: "move update-ref todo",
+ todos: []todo.Todo{
+ {Command: todo.Pick, Commit: "1234"},
+ {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"},
+ {Command: todo.Pick, Commit: "5678"},
+ },
+ todoToMoveUp: Todo{Ref: "refs/heads/some_branch", Action: todo.UpdateRef},
+ expectedErr: "",
+ expectedTodos: []todo.Todo{
+ {Command: todo.Pick, Commit: "1234"},
+ {Command: todo.Pick, Commit: "5678"},
+ {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"},
+ },
+ },
+ {
testName: "skip an invisible todo",
todos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"},
@@ -168,8 +198,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 +217,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 +228,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 +240,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 +248,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 {
@@ -330,3 +360,58 @@ func TestRebaseCommands_moveFixupCommitDown(t *testing.T) {
})
}
}
+
+func TestRebaseCommands_deleteTodos(t *testing.T) {
+ scenarios := []struct {
+ name string
+ todos []todo.Todo
+ todosToDelete []Todo
+ expectedTodos []todo.Todo
+ expectedErr error
+ }{
+ {
+ name: "success",
+ todos: []todo.Todo{
+ {Command: todo.Pick, Commit: "1234"},
+ {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"},
+ {Command: todo.Pick, Commit: "5678"},
+ {Command: todo.Pick, Commit: "abcd"},
+ },
+ todosToDelete: []Todo{
+ {Ref: "refs/heads/some_branch", Action: todo.UpdateRef},
+ {Sha: "abcd", Action: todo.Pick},
+ },
+ expectedTodos: []todo.Todo{
+ {Command: todo.Pick, Commit: "1234"},
+ {Command: todo.Pick, Commit: "5678"},
+ },
+ expectedErr: nil,
+ },
+ {
+ name: "failure",
+ todos: []todo.Todo{
+ {Command: todo.Pick, Commit: "1234"},
+ {Command: todo.Pick, Commit: "5678"},
+ },
+ todosToDelete: []Todo{
+ {Sha: "abcd", Action: todo.Pick},
+ },
+ expectedTodos: []todo.Todo{},
+ expectedErr: errors.New("Todo abcd not found in git-rebase-todo"),
+ },
+ }
+
+ for _, scenario := range scenarios {
+ t.Run(scenario.name, func(t *testing.T) {
+ actualTodos, actualErr := deleteTodos(scenario.todos, scenario.todosToDelete)
+
+ if scenario.expectedErr == nil {
+ assert.NoError(t, actualErr)
+ } else {
+ assert.EqualError(t, actualErr, scenario.expectedErr.Error())
+ }
+
+ assert.EqualValues(t, scenario.expectedTodos, actualTodos)
+ })
+ }
+}