go/lazygit
1
0
Fork 0
mirror of https://github.com/jesseduffield/lazygit.git synced 2025-06-23 00:39:13 +02:00
Code Issues Releases Activity

Simplify finding rebase todos

Browse Source 
One of the comments we are deleting here said:

  // Comparing just the hash is not enough; we need to compare both the
  // action and the hash, as the hash could appear multiple times (e.g. in a
  // pick and later in a merge).

I don't remember what I was thinking when I wrote this code, but it's nonsense
of course. Maybe I was thinking that the hash that appears in a "merge" todo
would be the hash of the commit that is being merged in (which would then
actually appear in an earlier pick), but it isn't, it's the hash of the merge
commit itself (so that the rebase can reuse its commit message). Which means
that hashes are unique, no need to compare the action.
This commit is contained in:
Stefan Haller
2024-11-30 19:53:42 +01:00
parent 2823a7cff0
commit 64eb3d560b
4 changed files with 26 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
pkg/app/daemon/daemon.go
View File

@ -12,7 +12,6 @@ import (
"github.com/jesseduffield/lazygit/pkg/common" "github.com/jesseduffield/lazygit/pkg/common"
"github.com/jesseduffield/lazygit/pkg/utils" "github.com/jesseduffield/lazygit/pkg/utils"
"github.com/samber/lo" "github.com/samber/lo"
"github.com/stefanhaller/git-todo-parser/todo"
) )
// Sometimes lazygit will be invoked in daemon mode from a parent lazygit process. // Sometimes lazygit will be invoked in daemon mode from a parent lazygit process.
@ -235,7 +234,6 @@ func (self *ChangeTodoActionsInstruction) run(common *common.Common) error {
changes := lo.Map(self.Changes, func(c ChangeTodoAction, _ int) utils.TodoChange { changes := lo.Map(self.Changes, func(c ChangeTodoAction, _ int) utils.TodoChange {
return utils.TodoChange{ return utils.TodoChange{
Hash: c.Hash, Hash: c.Hash,
OldAction: todo.Pick,
NewAction: c.NewAction, NewAction: c.NewAction,
} }
}) })
@ -296,8 +294,7 @@ func (self *MoveTodosUpInstruction) SerializedInstructions() string {
func (self *MoveTodosUpInstruction) run(common *common.Common) error { func (self *MoveTodosUpInstruction) run(common *common.Common) error {
todosToMove := lo.Map(self.Hashes, func(hash string, _ int) utils.Todo { todosToMove := lo.Map(self.Hashes, func(hash string, _ int) utils.Todo {
return utils.Todo{ return utils.Todo{
Hash: hash, Hash: hash,
Action: todo.Pick,
} }
}) })
@ -327,8 +324,7 @@ func (self *MoveTodosDownInstruction) SerializedInstructions() string {
func (self *MoveTodosDownInstruction) run(common *common.Common) error { func (self *MoveTodosDownInstruction) run(common *common.Common) error {
todosToMove := lo.Map(self.Hashes, func(hash string, _ int) utils.Todo { todosToMove := lo.Map(self.Hashes, func(hash string, _ int) utils.Todo {
return utils.Todo{ return utils.Todo{
Hash: hash, Hash: hash,
Action: todo.Pick,
} }
}) })

5
pkg/commands/git_commands/rebase.go
View File

@ -324,9 +324,9 @@ func (self *RebaseCommands) MoveFixupCommitDown(commits []*models.Commit, target
func todoFromCommit(commit *models.Commit) utils.Todo { func todoFromCommit(commit *models.Commit) utils.Todo {
if commit.Action == todo.UpdateRef { if commit.Action == todo.UpdateRef {
return utils.Todo{Ref: commit.Name, Action: commit.Action} return utils.Todo{Ref: commit.Name}
} else { } else {
return utils.Todo{Hash: commit.Hash, Action: commit.Action} return utils.Todo{Hash: commit.Hash}
} }
} }
@ -335,7 +335,6 @@ func (self *RebaseCommands) EditRebaseTodo(commits []*models.Commit, action todo
commitsWithAction := lo.Map(commits, func(commit *models.Commit, _ int) utils.TodoChange { commitsWithAction := lo.Map(commits, func(commit *models.Commit, _ int) utils.TodoChange {
return utils.TodoChange{ return utils.TodoChange{
Hash: commit.Hash, Hash: commit.Hash,
OldAction: commit.Action,
NewAction: action, NewAction: action,
} }
}) })

21
pkg/utils/rebase_todo.go
View File

@ -6,24 +6,18 @@ import (
"fmt" "fmt"
"os" "os"
"slices" "slices"
"strings"
"github.com/samber/lo" "github.com/samber/lo"
"github.com/stefanhaller/git-todo-parser/todo" "github.com/stefanhaller/git-todo-parser/todo"
) )
type Todo struct { type Todo struct {
Hash string // for todos that have one, e.g. pick, drop, fixup, etc. Hash string // for todos that have one, e.g. pick, drop, fixup, etc.
Ref string // for update-ref todos Ref string // for update-ref todos
Action todo.TodoCommand
} }
// In order to change a TODO in git-rebase-todo, we need to specify the old action,
// because sometimes the same hash appears multiple times in the file (e.g. in a pick
// and later in a merge)
type TodoChange struct { type TodoChange struct {
Hash string Hash string
OldAction todo.TodoCommand
NewAction todo.TodoCommand NewAction todo.TodoCommand
} }
@ -40,7 +34,7 @@ func EditRebaseTodo(filePath string, changes []TodoChange, commentChar byte) err
t := &todos[i] t := &todos[i]
// This is a nested loop, but it's ok because the number of todos should be small // This is a nested loop, but it's ok because the number of todos should be small
for _, change := range changes { for _, change := range changes {
if t.Command == change.OldAction && equalHash(t.Commit, change.Hash) { if equalHash(t.Commit, change.Hash) {
matchCount++ matchCount++
t.Command = change.NewAction t.Command = change.NewAction
} }
@ -66,13 +60,8 @@ func equalHash(a, b string) bool {
func findTodo(todos []todo.Todo, todoToFind Todo) (int, bool) { func findTodo(todos []todo.Todo, todoToFind Todo) (int, bool) {
_, idx, ok := lo.FindIndexOf(todos, func(t todo.Todo) bool { _, idx, ok := lo.FindIndexOf(todos, func(t todo.Todo) bool {
// Comparing just the hash is not enough; we need to compare both the // For update-ref todos we also must compare the Ref (they have an empty hash)
// action and the hash, as the hash could appear multiple times (e.g. in a return equalHash(t.Commit, todoToFind.Hash) && t.Ref == todoToFind.Ref
// pick and later in a merge). For update-ref todos we also must compare
// the Ref.
return t.Command == todoToFind.Action &&
equalHash(t.Commit, todoToFind.Hash) &&
t.Ref == todoToFind.Ref
}) })
return idx, ok return idx, ok
} }

34
pkg/utils/rebase_todo_test.go
View File

@ -26,7 +26,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveDown: Todo{Hash: "5678", Action: todo.Pick}, todoToMoveDown: Todo{Hash: "5678"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
@ -41,7 +41,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveDown: Todo{Hash: "abcd", Action: todo.Pick}, todoToMoveDown: Todo{Hash: "abcd"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
@ -56,7 +56,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"},
}, },
todoToMoveDown: Todo{Ref: "refs/heads/some_branch", Action: todo.UpdateRef}, todoToMoveDown: Todo{Ref: "refs/heads/some_branch"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
@ -73,7 +73,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "def0"}, {Command: todo.Pick, Commit: "def0"},
}, },
todoToMoveDown: Todo{Hash: "5678", Action: todo.Pick}, todoToMoveDown: Todo{Hash: "5678"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
@ -92,7 +92,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveDown: Todo{Hash: "def0", Action: todo.Pick}, todoToMoveDown: Todo{Hash: "def0"},
expectedErr: "Todo def0 not found in git-rebase-todo", expectedErr: "Todo def0 not found in git-rebase-todo",
expectedTodos: []todo.Todo{}, expectedTodos: []todo.Todo{},
}, },
@ -103,7 +103,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveDown: Todo{Hash: "1234", Action: todo.Pick}, todoToMoveDown: Todo{Hash: "1234"},
expectedErr: "Destination position for moving todo is out of range", expectedErr: "Destination position for moving todo is out of range",
expectedTodos: []todo.Todo{}, expectedTodos: []todo.Todo{},
}, },
@ -115,7 +115,7 @@ func TestRebaseCommands_moveTodoDown(t *testing.T) {
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
}, },
todoToMoveDown: Todo{Hash: "1234", Action: todo.Pick}, todoToMoveDown: Todo{Hash: "1234"},
expectedErr: "Destination position for moving todo is out of range", expectedErr: "Destination position for moving todo is out of range",
expectedTodos: []todo.Todo{}, expectedTodos: []todo.Todo{},
}, },
@ -152,7 +152,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveUp: Todo{Hash: "5678", Action: todo.Pick}, todoToMoveUp: Todo{Hash: "5678"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
@ -167,7 +167,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveUp: Todo{Hash: "1234", Action: todo.Pick}, todoToMoveUp: Todo{Hash: "1234"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
@ -182,7 +182,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
{Command: todo.UpdateRef, Ref: "refs/heads/some_branch"}, {Command: todo.UpdateRef, Ref: "refs/heads/some_branch"},
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
}, },
todoToMoveUp: Todo{Ref: "refs/heads/some_branch", Action: todo.UpdateRef}, todoToMoveUp: Todo{Ref: "refs/heads/some_branch"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
@ -199,7 +199,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "def0"}, {Command: todo.Pick, Commit: "def0"},
}, },
todoToMoveUp: Todo{Hash: "abcd", Action: todo.Pick}, todoToMoveUp: Todo{Hash: "abcd"},
expectedErr: "", expectedErr: "",
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
@ -218,7 +218,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveUp: Todo{Hash: "def0", Action: todo.Pick}, todoToMoveUp: Todo{Hash: "def0"},
expectedErr: "Todo def0 not found in git-rebase-todo", expectedErr: "Todo def0 not found in git-rebase-todo",
expectedTodos: []todo.Todo{}, expectedTodos: []todo.Todo{},
}, },
@ -229,7 +229,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todoToMoveUp: Todo{Hash: "abcd", Action: todo.Pick}, todoToMoveUp: Todo{Hash: "abcd"},
expectedErr: "Destination position for moving todo is out of range", expectedErr: "Destination position for moving todo is out of range",
expectedTodos: []todo.Todo{}, expectedTodos: []todo.Todo{},
}, },
@ -241,7 +241,7 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) {
{Command: todo.Label, Label: "myLabel"}, {Command: todo.Label, Label: "myLabel"},
{Command: todo.Reset, Label: "otherlabel"}, {Command: todo.Reset, Label: "otherlabel"},
}, },
todoToMoveUp: Todo{Hash: "5678", Action: todo.Pick}, todoToMoveUp: Todo{Hash: "5678"},
expectedErr: "Destination position for moving todo is out of range", expectedErr: "Destination position for moving todo is out of range",
expectedTodos: []todo.Todo{}, expectedTodos: []todo.Todo{},
}, },
@ -417,8 +417,8 @@ func TestRebaseCommands_deleteTodos(t *testing.T) {
{Command: todo.Pick, Commit: "abcd"}, {Command: todo.Pick, Commit: "abcd"},
}, },
todosToDelete: []Todo{ todosToDelete: []Todo{
{Ref: "refs/heads/some_branch", Action: todo.UpdateRef}, {Ref: "refs/heads/some_branch"},
{Hash: "abcd", Action: todo.Pick}, {Hash: "abcd"},
}, },
expectedTodos: []todo.Todo{ expectedTodos: []todo.Todo{
{Command: todo.Pick, Commit: "1234"}, {Command: todo.Pick, Commit: "1234"},
@ -433,7 +433,7 @@ func TestRebaseCommands_deleteTodos(t *testing.T) {
{Command: todo.Pick, Commit: "5678"}, {Command: todo.Pick, Commit: "5678"},
}, },
todosToDelete: []Todo{ todosToDelete: []Todo{
{Hash: "abcd", Action: todo.Pick}, {Hash: "abcd"},
}, },
expectedTodos: []todo.Todo{}, expectedTodos: []todo.Todo{},
expectedErr: errors.New("Todo abcd not found in git-rebase-todo"), expectedErr: errors.New("Todo abcd not found in git-rebase-todo"),