diff --git a/go.mod b/go.mod index c710eee99..0a6a3871c 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/cli/safeexec v1.0.0 github.com/cloudfoundry/jibber_jabber v0.0.0-20151120183258-bcc4c8345a21 github.com/creack/pty v1.1.11 - github.com/fsmiamoto/git-todo-parser v0.0.2 + github.com/fsmiamoto/git-todo-parser v0.0.4-0.20230403011024-617a5a7ce980 github.com/fsnotify/fsnotify v1.4.7 github.com/gdamore/tcell/v2 v2.6.0 github.com/go-errors/errors v1.4.2 diff --git a/go.sum b/go.sum index 194b5f494..46f3c08e0 100644 --- a/go.sum +++ b/go.sum @@ -28,8 +28,8 @@ github.com/fatih/color v1.7.1-0.20180516100307-2d684516a886/go.mod h1:Zm6kSWBoL9 github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s= github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU= github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI435gkrCt3MPfRiAkVrwSbHsst4LCFVfpJc= -github.com/fsmiamoto/git-todo-parser v0.0.2 h1:l6Y+9q7jbM+yK/w6kASpHO7ejL9ARCErm3tCEqOT278= -github.com/fsmiamoto/git-todo-parser v0.0.2/go.mod h1:B+AgTbNE2BARvJqzXygThzqxLIaEWvwr2sxKYYb0Fas= +github.com/fsmiamoto/git-todo-parser v0.0.4-0.20230403011024-617a5a7ce980 h1:ay9aM+Ay9I4LJttUVF4EFVmeNUkS9/snYVFK6lwieVQ= +github.com/fsmiamoto/git-todo-parser v0.0.4-0.20230403011024-617a5a7ce980/go.mod h1:B+AgTbNE2BARvJqzXygThzqxLIaEWvwr2sxKYYb0Fas= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko= diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 119566f08..bce16de15 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -89,7 +89,7 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, if commit.Sha == firstPushedCommit { passedFirstPushedCommit = true } - commit.Status = map[bool]string{true: "unpushed", false: "pushed"}[!passedFirstPushedCommit] + commit.Status = map[bool]models.CommitStatus{true: models.StatusUnpushed, false: models.StatusPushed}[!passedFirstPushedCommit] commits = append(commits, commit) return false, nil }) @@ -194,8 +194,8 @@ func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode return nil, nil } - commitShas := slices.Map(commits, func(commit *models.Commit) string { - return commit.Sha + commitShas := slices.FilterMap(commits, func(commit *models.Commit) (string, bool) { + return commit.Sha, commit.Sha != "" }) // note that we're not filtering these as we do non-rebasing commits just because @@ -209,20 +209,26 @@ func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode ), ).DontLog() - hydratedCommits := make([]*models.Commit, 0, len(commits)) - i := 0 + fullCommits := map[string]*models.Commit{} err = cmdObj.RunAndProcessLines(func(line string) (bool, error) { commit := self.extractCommitFromLine(line) - matchingCommit := commits[i] - commit.Action = matchingCommit.Action - commit.Status = matchingCommit.Status - hydratedCommits = append(hydratedCommits, commit) - i++ + fullCommits[commit.Sha] = commit return false, nil }) if err != nil { return nil, err } + + hydratedCommits := make([]*models.Commit, 0, len(commits)) + for _, rebasingCommit := range commits { + if rebasingCommit.Sha == "" { + hydratedCommits = append(hydratedCommits, rebasingCommit) + } else if commit := fullCommits[rebasingCommit.Sha]; commit != nil { + commit.Action = rebasingCommit.Action + commit.Status = rebasingCommit.Status + hydratedCommits = append(hydratedCommits, commit) + } + } return hydratedCommits, nil } @@ -305,15 +311,17 @@ func (self *CommitLoader) getInteractiveRebasingCommits() ([]*models.Commit, err } for _, t := range todos { - if t.Commit == "" { + if t.Command == todo.UpdateRef { + t.Msg = strings.TrimPrefix(t.Ref, "refs/heads/") + } else if t.Commit == "" { // Command does not have a commit associated, skip continue } commits = slices.Prepend(commits, &models.Commit{ Sha: t.Commit, Name: t.Msg, - Status: "rebasing", - Action: t.Command.String(), + Status: models.StatusRebasing, + Action: t.Command, }) } @@ -332,7 +340,7 @@ func (self *CommitLoader) commitFromPatch(content string) *models.Commit { return &models.Commit{ Sha: sha, Name: name, - Status: "rebasing", + Status: models.StatusRebasing, } } @@ -349,11 +357,11 @@ func (self *CommitLoader) setCommitMergedStatuses(refName string, commits []*mod if strings.HasPrefix(ancestor, commit.Sha) { passedAncestor = true } - if commit.Status != "pushed" { + if commit.Status != models.StatusPushed { continue } if passedAncestor { - commits[i].Status = "merged" + commits[i].Status = models.StatusMerged } } return commits, nil diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 8ba3e6649..140541545 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -78,8 +78,8 @@ func TestGetCommits(t *testing.T) { { Sha: "0eea75e8c631fba6b58135697835d58ba4c18dbc", Name: "better typing for rebase mode", - Status: "unpushed", - Action: "", + Status: models.StatusUnpushed, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "(HEAD -> better-tests)", AuthorName: "Jesse Duffield", @@ -92,8 +92,8 @@ func TestGetCommits(t *testing.T) { { Sha: "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", Name: "fix logging", - Status: "pushed", - Action: "", + Status: models.StatusPushed, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "(origin/better-tests)", AuthorName: "Jesse Duffield", @@ -106,8 +106,8 @@ func TestGetCommits(t *testing.T) { { Sha: "e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c", Name: "refactor", - Status: "pushed", - Action: "", + Status: models.StatusPushed, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -120,8 +120,8 @@ func TestGetCommits(t *testing.T) { { Sha: "d8084cd558925eb7c9c38afeed5725c21653ab90", Name: "WIP", - Status: "pushed", - Action: "", + Status: models.StatusPushed, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -134,8 +134,8 @@ func TestGetCommits(t *testing.T) { { Sha: "65f910ebd85283b5cce9bf67d03d3f1a9ea3813a", Name: "WIP", - Status: "pushed", - Action: "", + Status: models.StatusPushed, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -148,8 +148,8 @@ func TestGetCommits(t *testing.T) { { Sha: "26c07b1ab33860a1a7591a0638f9925ccf497ffa", Name: "WIP", - Status: "merged", - Action: "", + Status: models.StatusMerged, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -162,8 +162,8 @@ func TestGetCommits(t *testing.T) { { Sha: "3d4470a6c072208722e5ae9a54bcb9634959a1c5", Name: "WIP", - Status: "merged", - Action: "", + Status: models.StatusMerged, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -176,8 +176,8 @@ func TestGetCommits(t *testing.T) { { Sha: "053a66a7be3da43aacdc7aa78e1fe757b82c4dd2", Name: "refactoring the config struct", - Status: "merged", - Action: "", + Status: models.StatusMerged, + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 6af83a27f..e4c20426f 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -2,15 +2,16 @@ package git_commands import ( "fmt" - "os" "path/filepath" "strings" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/go-errors/errors" "github.com/jesseduffield/generics/slices" "github.com/jesseduffield/lazygit/pkg/app/daemon" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" + "github.com/jesseduffield/lazygit/pkg/utils" ) type RebaseCommands struct { @@ -201,55 +202,39 @@ func (self *RebaseCommands) AmendTo(commit *models.Commit) error { return self.SquashAllAboveFixupCommits(commit) } -// EditRebaseTodo sets the action at a given index in the git-rebase-todo file -func (self *RebaseCommands) EditRebaseTodo(index int, action string) error { +// EditRebaseTodo sets the action for a given rebase commit in the git-rebase-todo file +func (self *RebaseCommands) EditRebaseTodo(commit *models.Commit, action todo.TodoCommand) error { fileName := filepath.Join(self.dotGitDir, "rebase-merge/git-rebase-todo") - bytes, err := os.ReadFile(fileName) + todos, err := utils.ReadRebaseTodoFile(fileName) if err != nil { return err } - content := strings.Split(string(bytes), "\n") - commitCount := self.getTodoCommitCount(content) - - // we have the most recent commit at the bottom whereas the todo file has - // it at the bottom, so we need to subtract our index from the commit count - contentIndex := commitCount - 1 - index - splitLine := strings.Split(content[contentIndex], " ") - content[contentIndex] = action + " " + strings.Join(splitLine[1:], " ") - result := strings.Join(content, "\n") - - return os.WriteFile(fileName, []byte(result), 0o644) -} - -func (self *RebaseCommands) getTodoCommitCount(content []string) int { - // count lines that are not blank and are not comments - commitCount := 0 - for _, line := range content { - if line != "" && !strings.HasPrefix(line, "#") { - commitCount++ + for i := range todos { + t := &todos[i] + // 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) + if t.Command == commit.Action && t.Commit == commit.Sha { + t.Command = action + return utils.WriteRebaseTodoFile(fileName, todos) } } - return commitCount + + // Should never get here + return fmt.Errorf("Todo %s not found in git-rebase-todo", commit.Sha) } // MoveTodoDown moves a rebase todo item down by one position -func (self *RebaseCommands) MoveTodoDown(index int) error { +func (self *RebaseCommands) MoveTodoDown(commit *models.Commit) error { fileName := filepath.Join(self.dotGitDir, "rebase-merge/git-rebase-todo") - bytes, err := os.ReadFile(fileName) - if err != nil { - return err - } + return utils.MoveTodoDown(fileName, commit.Sha, commit.Action) +} - content := strings.Split(string(bytes), "\n") - commitCount := self.getTodoCommitCount(content) - contentIndex := commitCount - 1 - index - - rearrangedContent := append(content[0:contentIndex-1], content[contentIndex], content[contentIndex-1]) - rearrangedContent = append(rearrangedContent, content[contentIndex+1:]...) - result := strings.Join(rearrangedContent, "\n") - - return os.WriteFile(fileName, []byte(result), 0o644) +// MoveTodoDown moves a rebase todo item down by one position +func (self *RebaseCommands) MoveTodoUp(commit *models.Commit) error { + fileName := filepath.Join(self.dotGitDir, "rebase-merge/git-rebase-todo") + return utils.MoveTodoUp(fileName, commit.Sha, commit.Action) } // SquashAllAboveFixupCommits squashes all fixup! commits above the given one diff --git a/pkg/commands/git_commands/reflog_commit_loader.go b/pkg/commands/git_commands/reflog_commit_loader.go index e2d3a7dad..c5c14fb72 100644 --- a/pkg/commands/git_commands/reflog_commit_loader.go +++ b/pkg/commands/git_commands/reflog_commit_loader.go @@ -52,7 +52,7 @@ func (self *ReflogCommitLoader) GetReflogCommits(lastReflogCommit *models.Commit Sha: fields[0], Name: fields[2], UnixTimestamp: int64(unixTimestamp), - Status: "reflog", + Status: models.StatusReflog, Parents: parents, } diff --git a/pkg/commands/git_commands/reflog_commit_loader_test.go b/pkg/commands/git_commands/reflog_commit_loader_test.go index 7170196e9..a51a40e69 100644 --- a/pkg/commands/git_commands/reflog_commit_loader_test.go +++ b/pkg/commands/git_commands/reflog_commit_loader_test.go @@ -51,35 +51,35 @@ func TestGetReflogCommits(t *testing.T) { { Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, { Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, { Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, { Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from master to A", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, { Sha: "f4ddf2f0d4be4ccc7efa", Name: "checkout: moving from A to master", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643149435, Parents: []string{"51baa8c1"}, }, @@ -95,7 +95,7 @@ func TestGetReflogCommits(t *testing.T) { lastReflogCommit: &models.Commit{ Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, @@ -103,7 +103,7 @@ func TestGetReflogCommits(t *testing.T) { { Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, @@ -119,7 +119,7 @@ func TestGetReflogCommits(t *testing.T) { lastReflogCommit: &models.Commit{ Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from B to A", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, @@ -128,7 +128,7 @@ func TestGetReflogCommits(t *testing.T) { { Sha: "c3c4b66b64c97ffeecde", Name: "checkout: moving from A to B", - Status: "reflog", + Status: models.StatusReflog, UnixTimestamp: 1643150483, Parents: []string{"51baa8c1"}, }, diff --git a/pkg/commands/git_commands/version.go b/pkg/commands/git_commands/version.go index 0cf4b485c..1b1351df4 100644 --- a/pkg/commands/git_commands/version.go +++ b/pkg/commands/git_commands/version.go @@ -32,7 +32,7 @@ func ParseGitVersion(versionStr string) (*GitVersion, error) { // versionStr should be something like: // git version 2.39.0 // git version 2.37.1 (Apple Git-137.1) - re := regexp.MustCompile(`[^\d]+(\d+)(\.\d+)?(\.\d+)?(.*)`) + re := regexp.MustCompile(`[^\d]*(\d+)(\.\d+)?(\.\d+)?(.*)`) matches := re.FindStringSubmatch(versionStr) if len(matches) < 5 { @@ -65,3 +65,7 @@ func (v *GitVersion) IsOlderThan(major, minor, patch int) bool { required := major*1000*1000 + minor*1000 + patch return actual < required } + +func (v *GitVersion) IsOlderThanVersion(version *GitVersion) bool { + return v.IsOlderThan(version.Major, version.Minor, version.Patch) +} diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index b3a171934..e0ff875a0 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -3,18 +3,37 @@ package models import ( "fmt" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/lazygit/pkg/utils" ) // Special commit hash for empty tree object const EmptyTreeCommitHash = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" +type CommitStatus int + +const ( + StatusNone CommitStatus = iota + StatusUnpushed + StatusPushed + StatusMerged + StatusRebasing + StatusSelected + StatusReflog +) + +const ( + // Conveniently for us, the todo package starts the enum at 1, and given + // that it doesn't have a "none" value, we're setting ours to 0 + ActionNone todo.TodoCommand = 0 +) + // Commit : A git commit type Commit struct { Sha string Name string - Status string // one of "unpushed", "pushed", "merged", "rebasing" or "selected" - Action string // one of "", "pick", "edit", "squash", "reword", "drop", "fixup" + Status CommitStatus + Action todo.TodoCommand Tags []string ExtraInfo string // something like 'HEAD -> master, tag: v0.15.2' AuthorName string // something like 'Jesse Duffield' @@ -63,7 +82,7 @@ func (c *Commit) IsMerge() bool { // returns true if this commit is not actually in the git log but instead // is from a TODO file for an interactive rebase. func (c *Commit) IsTODO() bool { - return c.Action != "" + return c.Action != ActionNone } func IsHeadCommit(commits []*Commit, index int) bool { diff --git a/pkg/gui/commits_panel.go b/pkg/gui/commits_panel.go index 8b8c74eec..2b75be4d0 100644 --- a/pkg/gui/commits_panel.go +++ b/pkg/gui/commits_panel.go @@ -1,6 +1,7 @@ package gui import ( + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" @@ -34,6 +35,13 @@ func (gui *Gui) branchCommitsRenderToMain() error { commit := gui.State.Contexts.LocalCommits.GetSelected() if commit == nil { task = types.NewRenderStringTask(gui.c.Tr.NoCommitsThisBranch) + } else if commit.Action == todo.UpdateRef { + task = types.NewRenderStringTask( + utils.ResolvePlaceholderString( + gui.c.Tr.UpdateRefHere, + map[string]string{ + "ref": commit.Name, + })) } else { cmdObj := gui.git.Commit.ShowCmdObj(commit.Sha, gui.State.Modes.Filtering.GetPath(), gui.IgnoreWhitespaceInDiffView) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 284534ffa..4610aed12 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -3,11 +3,13 @@ package controllers import ( "fmt" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) type ( @@ -153,7 +155,7 @@ func (self *LocalCommitsController) squashDown(commit *models.Commit) error { return self.c.ErrorMsg(self.c.Tr.CannotSquashOrFixupFirstCommit) } - applied, err := self.handleMidRebaseCommand("squash", commit) + applied, err := self.handleMidRebaseCommand(todo.Squash, commit) if err != nil { return err } @@ -178,7 +180,7 @@ func (self *LocalCommitsController) fixup(commit *models.Commit) error { return self.c.ErrorMsg(self.c.Tr.CannotSquashOrFixupFirstCommit) } - applied, err := self.handleMidRebaseCommand("fixup", commit) + applied, err := self.handleMidRebaseCommand(todo.Fixup, commit) if err != nil { return err } @@ -199,7 +201,7 @@ func (self *LocalCommitsController) fixup(commit *models.Commit) error { } func (self *LocalCommitsController) reword(commit *models.Commit) error { - applied, err := self.handleMidRebaseCommand("reword", commit) + applied, err := self.handleMidRebaseCommand(todo.Reword, commit) if err != nil { return err } @@ -248,7 +250,7 @@ func (self *LocalCommitsController) doRewordEditor() error { } func (self *LocalCommitsController) rewordEditor(commit *models.Commit) error { - midRebase, err := self.handleMidRebaseCommand("reword", commit) + midRebase, err := self.handleMidRebaseCommand(todo.Reword, commit) if err != nil { return err } @@ -268,7 +270,7 @@ func (self *LocalCommitsController) rewordEditor(commit *models.Commit) error { } func (self *LocalCommitsController) drop(commit *models.Commit) error { - applied, err := self.handleMidRebaseCommand("drop", commit) + applied, err := self.handleMidRebaseCommand(todo.Drop, commit) if err != nil { return err } @@ -289,7 +291,7 @@ func (self *LocalCommitsController) drop(commit *models.Commit) error { } func (self *LocalCommitsController) edit(commit *models.Commit) error { - applied, err := self.handleMidRebaseCommand("edit", commit) + applied, err := self.handleMidRebaseCommand(todo.Edit, commit) if err != nil { return err } @@ -305,7 +307,7 @@ func (self *LocalCommitsController) edit(commit *models.Commit) error { } func (self *LocalCommitsController) pick(commit *models.Commit) error { - applied, err := self.handleMidRebaseCommand("pick", commit) + applied, err := self.handleMidRebaseCommand(todo.Pick, commit) if err != nil { return err } @@ -326,12 +328,12 @@ func (self *LocalCommitsController) interactiveRebase(action string) error { // handleMidRebaseCommand sees if the selected commit is in fact a rebasing // commit meaning you are trying to edit the todo file rather than actually // begin a rebase. It then updates the todo file with that action -func (self *LocalCommitsController) handleMidRebaseCommand(action string, commit *models.Commit) (bool, error) { +func (self *LocalCommitsController) handleMidRebaseCommand(action todo.TodoCommand, commit *models.Commit) (bool, error) { if !commit.IsTODO() { if self.git.Status.WorkingTreeState() != enums.REBASE_MODE_NONE { // If we are in a rebase, the only action that is allowed for // non-todo commits is rewording the current head commit - if !(action == "reword" && self.isHeadCommit()) { + if !(action == todo.Reword && self.isHeadCommit()) { return true, self.c.ErrorMsg(self.c.Tr.AlreadyRebasing) } } @@ -343,19 +345,21 @@ func (self *LocalCommitsController) handleMidRebaseCommand(action string, commit // and that means we either unconditionally wait around for the subprocess to ask for // our input or we set a lazygit client as the EDITOR env variable and have it // request us to edit the commit message when prompted. - if action == "reword" { + if action == todo.Reword { return true, self.c.ErrorMsg(self.c.Tr.LcRewordNotSupported) } + if allowed := isChangeOfRebaseTodoAllowed(action); !allowed { + return true, self.c.ErrorMsg(self.c.Tr.LcChangingThisActionIsNotAllowed) + } + self.c.LogAction("Update rebase TODO") self.c.LogCommand( - fmt.Sprintf("Updating rebase action of commit %s to '%s'", commit.ShortSha(), action), + fmt.Sprintf("Updating rebase action of commit %s to '%s'", commit.ShortSha(), action.String()), false, ) - if err := self.git.Rebase.EditRebaseTodo( - self.context().GetSelectedLineIdx(), action, - ); err != nil { + if err := self.git.Rebase.EditRebaseTodo(commit, action); err != nil { return false, self.c.Error(err) } @@ -383,7 +387,7 @@ func (self *LocalCommitsController) moveDown(commit *models.Commit) error { self.c.LogAction(self.c.Tr.Actions.MoveCommitDown) self.c.LogCommand(fmt.Sprintf("Moving commit %s down", commit.ShortSha()), false) - if err := self.git.Rebase.MoveTodoDown(index); err != nil { + if err := self.git.Rebase.MoveTodoDown(commit); err != nil { return self.c.Error(err) } self.context().MoveSelectedLine(1) @@ -421,7 +425,7 @@ func (self *LocalCommitsController) moveUp(commit *models.Commit) error { false, ) - if err := self.git.Rebase.MoveTodoDown(index - 1); err != nil { + if err := self.git.Rebase.MoveTodoUp(self.model.Commits[index]); err != nil { return self.c.Error(err) } self.context().MoveSelectedLine(-1) @@ -760,3 +764,16 @@ func (self *LocalCommitsController) paste() error { func (self *LocalCommitsController) isHeadCommit() bool { return models.IsHeadCommit(self.model.Commits, self.context().GetSelectedLineIdx()) } + +func isChangeOfRebaseTodoAllowed(action todo.TodoCommand) bool { + allowedActions := []todo.TodoCommand{ + todo.Pick, + todo.Drop, + todo.Edit, + todo.Fixup, + todo.Squash, + todo.Reword, + } + + return lo.Contains(allowedActions, action) +} diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 77b946a64..7e16197b5 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" @@ -263,8 +264,9 @@ func displayCommit( bisectString := getBisectStatusText(bisectStatus, bisectInfo) actionString := "" - if commit.Action != "" { - actionString = actionColorMap(commit.Action).Sprint(commit.Action) + " " + if commit.Action != models.ActionNone { + todoString := commit.Action.String() + actionString = actionColorMap(commit.Action).Sprint(todoString) + " " } tagString := "" @@ -275,6 +277,8 @@ func displayCommit( } else { if len(commit.Tags) > 0 { tagString = theme.DiffTerminalColor.SetBold().Sprint(strings.Join(commit.Tags, " ")) + " " + } else if commit.ExtraInfo != "" { + tagString = style.FgMagenta.SetBold().Sprint("(*)") + " " } } @@ -343,19 +347,20 @@ func getShaColor( return getBisectStatusColor(bisectStatus) } - diffed := commit.Sha == diffName + diffed := commit.Sha != "" && commit.Sha == diffName shaColor := theme.DefaultTextColor switch commit.Status { - case "unpushed": + case models.StatusUnpushed: shaColor = style.FgRed - case "pushed": + case models.StatusPushed: shaColor = style.FgYellow - case "merged": + case models.StatusMerged: shaColor = style.FgGreen - case "rebasing": + case models.StatusRebasing: shaColor = style.FgBlue - case "reflog": + case models.StatusReflog: shaColor = style.FgBlue + default: } if diffed { @@ -367,15 +372,15 @@ func getShaColor( return shaColor } -func actionColorMap(str string) style.TextStyle { - switch str { - case "pick": +func actionColorMap(action todo.TodoCommand) style.TextStyle { + switch action { + case todo.Pick: return style.FgCyan - case "drop": + case todo.Drop: return style.FgRed - case "edit": + case todo.Edit: return style.FgGreen - case "fixup": + case todo.Fixup: return style.FgMagenta default: return style.FgYellow diff --git a/pkg/gui/presentation/commits_test.go b/pkg/gui/presentation/commits_test.go index 4c9efbe15..55155a704 100644 --- a/pkg/gui/presentation/commits_test.go +++ b/pkg/gui/presentation/commits_test.go @@ -5,6 +5,7 @@ import ( "strings" "testing" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/gookit/color" "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" @@ -91,8 +92,8 @@ func TestGetCommitListDisplayStrings(t *testing.T) { { testName: "showing graph, including rebase commits", commits: []*models.Commit{ - {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"}, - {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"}, + {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: todo.Pick}, + {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: todo.Pick}, {Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}}, {Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}}, {Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}}, @@ -114,8 +115,8 @@ func TestGetCommitListDisplayStrings(t *testing.T) { { testName: "showing graph, including rebase commits, with offset", commits: []*models.Commit{ - {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"}, - {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"}, + {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: todo.Pick}, + {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: todo.Pick}, {Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}}, {Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}}, {Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}}, @@ -136,8 +137,8 @@ func TestGetCommitListDisplayStrings(t *testing.T) { { testName: "startIdx is past TODO commits", commits: []*models.Commit{ - {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"}, - {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"}, + {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: todo.Pick}, + {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: todo.Pick}, {Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}}, {Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}}, {Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}}, @@ -156,8 +157,8 @@ func TestGetCommitListDisplayStrings(t *testing.T) { { testName: "only showing TODO commits", commits: []*models.Commit{ - {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"}, - {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"}, + {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: todo.Pick}, + {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: todo.Pick}, {Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}}, {Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}}, {Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}}, @@ -195,10 +196,10 @@ func TestGetCommitListDisplayStrings(t *testing.T) { { testName: "only TODO commits except last", commits: []*models.Commit{ - {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: "pick"}, - {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: "pick"}, - {Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}, Action: "pick"}, - {Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}, Action: "pick"}, + {Name: "commit1", Sha: "sha1", Parents: []string{"sha2", "sha3"}, Action: todo.Pick}, + {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}, Action: todo.Pick}, + {Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}, Action: todo.Pick}, + {Name: "commit4", Sha: "sha4", Parents: []string{"sha5"}, Action: todo.Pick}, {Name: "commit5", Sha: "sha5", Parents: []string{"sha7"}}, }, startIdx: 0, @@ -215,7 +216,7 @@ func TestGetCommitListDisplayStrings(t *testing.T) { { testName: "don't show YOU ARE HERE label when not asked for (e.g. in branches panel)", commits: []*models.Commit{ - {Name: "commit1", Sha: "sha1", Parents: []string{"sha2"}, Action: "pick"}, + {Name: "commit1", Sha: "sha1", Parents: []string{"sha2"}, Action: todo.Pick}, {Name: "commit2", Sha: "sha2", Parents: []string{"sha3"}}, {Name: "commit3", Sha: "sha3", Parents: []string{"sha4"}}, }, diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index e7756d03e..80179f365 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -105,6 +105,7 @@ type TranslationSet struct { SureResetCommitAuthor string LcRenameCommitEditor string NoCommitsThisBranch string + UpdateRefHere string Error string LcSelectHunk string LcNavigateConflicts string @@ -218,6 +219,7 @@ type TranslationSet struct { YouAreHere string YouDied string LcRewordNotSupported string + LcChangingThisActionIsNotAllowed string LcCherryPickCopy string LcCherryPickCopyRange string LcPasteCommits string @@ -754,6 +756,7 @@ func EnglishTranslationSet() TranslationSet { LcSquashDown: "squash down", LcFixupCommit: "fixup commit", NoCommitsThisBranch: "No commits for this branch", + UpdateRefHere: "Update branch '{{.ref}}' here", CannotSquashOrFixupFirstCommit: "There's no commit below to squash into", Fixup: "Fixup", SureFixupThisCommit: "Are you sure you want to 'fixup' this commit? It will be merged into the commit below", @@ -885,6 +888,7 @@ func EnglishTranslationSet() TranslationSet { YouAreHere: "YOU ARE HERE", YouDied: "YOU DIED!", LcRewordNotSupported: "rewording commits while interactively rebasing is not currently supported", + LcChangingThisActionIsNotAllowed: "changing this kind of rebase todo entry is not allowed", LcCherryPickCopy: "copy commit (cherry-pick)", LcCherryPickCopyRange: "copy commit range (cherry-pick)", LcPasteCommits: "paste commits (cherry-pick)", diff --git a/pkg/integration/components/runner.go b/pkg/integration/components/runner.go index 43e4f9304..03ca81295 100644 --- a/pkg/integration/components/runner.go +++ b/pkg/integration/components/runner.go @@ -8,6 +8,7 @@ import ( "path/filepath" "github.com/jesseduffield/lazycore/pkg/utils" + "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" ) @@ -42,6 +43,11 @@ func RunTests( return err } + gitVersion, err := getGitVersion() + if err != nil { + return err + } + for _, test := range tests { test := test @@ -51,7 +57,7 @@ func RunTests( ) for i := 0; i < maxAttempts; i++ { - err := runTest(test, paths, projectRootDir, logf, runCmd, sandbox, keyPressDelay) + err := runTest(test, paths, projectRootDir, logf, runCmd, sandbox, keyPressDelay, gitVersion) if err != nil { if i == maxAttempts-1 { return err @@ -77,12 +83,18 @@ func runTest( runCmd func(cmd *exec.Cmd) error, sandbox bool, keyPressDelay int, + gitVersion *git_commands.GitVersion, ) error { if test.Skip() { logf("Skipping test %s", test.Name()) return nil } + if !test.ShouldRunForGitVersion(gitVersion) { + logf("Skipping test %s for git version %d.%d.%d", test.Name(), gitVersion.Major, gitVersion.Minor, gitVersion.Patch) + return nil + } + logf("path: %s", paths.Root()) if err := prepareTestDir(test, paths, projectRootDir); err != nil { @@ -144,6 +156,16 @@ func globalGitConfigPath(rootDir string) string { return filepath.Join(rootDir, "test", "global_git_config") } +func getGitVersion() (*git_commands.GitVersion, error) { + osCommand := oscommands.NewDummyOSCommand() + cmdObj := osCommand.Cmd.New("git --version") + versionStr, err := cmdObj.RunWithOutput() + if err != nil { + return nil, err + } + return git_commands.ParseGitVersion(versionStr) +} + func getLazygitCommand(test *IntegrationTest, paths Paths, rootDir string, sandbox bool, keyPressDelay int) (*exec.Cmd, error) { osCommand := oscommands.NewDummyOSCommand() diff --git a/pkg/integration/components/shell.go b/pkg/integration/components/shell.go index d55fa8303..890f9efc4 100644 --- a/pkg/integration/components/shell.go +++ b/pkg/integration/components/shell.go @@ -192,7 +192,11 @@ func (self *Shell) DeleteFileAndAdd(fileName string) *Shell { // The reason for padding with zeroes is so that it's easier to do string // matches on the commit messages when there are many of them func (self *Shell) CreateNCommits(n int) *Shell { - for i := 1; i <= n; i++ { + return self.CreateNCommitsStartingAt(n, 1) +} + +func (self *Shell) CreateNCommitsStartingAt(n, startIndex int) *Shell { + for i := startIndex; i < startIndex+n; i++ { self.CreateFileAndAdd( fmt.Sprintf("file%02d.txt", i), fmt.Sprintf("file%02d content", i), diff --git a/pkg/integration/components/test.go b/pkg/integration/components/test.go index e1767f475..847781c8e 100644 --- a/pkg/integration/components/test.go +++ b/pkg/integration/components/test.go @@ -5,6 +5,8 @@ import ( "strconv" "strings" + "github.com/jesseduffield/generics/slices" + "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/env" integrationTypes "github.com/jesseduffield/lazygit/pkg/integration/types" @@ -28,6 +30,7 @@ type IntegrationTest struct { testDriver *TestDriver, keys config.KeybindingConfig, ) + gitVersion GitVersionRestriction } var _ integrationTypes.IntegrationTest = &IntegrationTest{} @@ -45,6 +48,56 @@ type NewIntegrationTestArgs struct { ExtraCmdArgs string // for when a test is flakey Skip bool + // to run a test only on certain git versions + GitVersion GitVersionRestriction +} + +type GitVersionRestriction struct { + // Only one of these fields can be non-empty; use functions below to construct + from string + before string + includes []string +} + +// Verifies the version is at least the given version (inclusive) +func From(version string) GitVersionRestriction { + return GitVersionRestriction{from: version} +} + +// Verifies the version is before the given version (exclusive) +func Before(version string) GitVersionRestriction { + return GitVersionRestriction{before: version} +} + +func Includes(versions ...string) GitVersionRestriction { + return GitVersionRestriction{includes: versions} +} + +func (self GitVersionRestriction) shouldRunOnVersion(version *git_commands.GitVersion) bool { + if self.from != "" { + from, err := git_commands.ParseGitVersion(self.from) + if err != nil { + panic("Invalid git version string: " + self.from) + } + return !version.IsOlderThanVersion(from) + } + if self.before != "" { + before, err := git_commands.ParseGitVersion(self.before) + if err != nil { + panic("Invalid git version string: " + self.before) + } + return version.IsOlderThanVersion(before) + } + if len(self.includes) != 0 { + return slices.Some(self.includes, func(str string) bool { + v, err := git_commands.ParseGitVersion(str) + if err != nil { + panic("Invalid git version string: " + str) + } + return version.Major == v.Major && version.Minor == v.Minor && version.Patch == v.Patch + }) + } + return true } func NewIntegrationTest(args NewIntegrationTestArgs) *IntegrationTest { @@ -63,6 +116,7 @@ func NewIntegrationTest(args NewIntegrationTestArgs) *IntegrationTest { setupRepo: args.SetupRepo, setupConfig: args.SetupConfig, run: args.Run, + gitVersion: args.GitVersion, } } @@ -82,6 +136,10 @@ func (self *IntegrationTest) Skip() bool { return self.skip } +func (self *IntegrationTest) ShouldRunForGitVersion(version *git_commands.GitVersion) bool { + return self.gitVersion.shouldRunOnVersion(version) +} + func (self *IntegrationTest) SetupConfig(config *config.AppConfig) { self.setupConfig(config) } diff --git a/pkg/integration/components/test_test.go b/pkg/integration/components/test_test.go index 4216b8130..062382c2d 100644 --- a/pkg/integration/components/test_test.go +++ b/pkg/integration/components/test_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/jesseduffield/gocui" + "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gui/types" @@ -87,3 +88,65 @@ func TestSuccess(t *testing.T) { assert.EqualValues(t, []string{"a", "b"}, driver.pressedKeys) assert.Equal(t, "", driver.failureMessage) } + +func TestGitVersionRestriction(t *testing.T) { + scenarios := []struct { + testName string + gitVersion GitVersionRestriction + expectedShouldRun bool + }{ + { + testName: "From, current is newer", + gitVersion: From("2.24.9"), + expectedShouldRun: true, + }, + { + testName: "From, current is same", + gitVersion: From("2.25.0"), + expectedShouldRun: true, + }, + { + testName: "From, current is older", + gitVersion: From("2.26.0"), + expectedShouldRun: false, + }, + { + testName: "Before, current is older", + gitVersion: Before("2.24.9"), + expectedShouldRun: false, + }, + { + testName: "Before, current is same", + gitVersion: Before("2.25.0"), + expectedShouldRun: false, + }, + { + testName: "Before, current is newer", + gitVersion: Before("2.26.0"), + expectedShouldRun: true, + }, + { + testName: "Includes, current is included", + gitVersion: Includes("2.23.0", "2.25.0"), + expectedShouldRun: true, + }, + { + testName: "Includes, current is not included", + gitVersion: Includes("2.23.0", "2.27.0"), + expectedShouldRun: false, + }, + } + + currentGitVersion := git_commands.GitVersion{Major: 2, Minor: 25, Patch: 0} + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + test := NewIntegrationTest(NewIntegrationTestArgs{ + Description: unitTestDescription, + GitVersion: s.gitVersion, + }) + shouldRun := test.ShouldRunForGitVersion(¤tGitVersion) + assert.Equal(t, shouldRun, s.expectedShouldRun) + }) + } +} diff --git a/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go new file mode 100644 index 000000000..c247f1743 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go @@ -0,0 +1,69 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Drops a commit during interactive rebase when there is an update-ref in the git-rebase-todo file", + ExtraCmdArgs: "", + Skip: false, + GitVersion: From("2.38.0"), + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + CreateNCommits(3). + NewBranch("mybranch"). + CreateNCommitsStartingAt(3, 4) + + shell.SetConfig("rebase.updateRefs", "true") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("(*) commit 06").IsSelected(), + Contains("commit 05"), + Contains("commit 04"), + Contains("(*) commit 03"), + Contains("commit 02"), + Contains("commit 01"), + ). + // Once "e" is fixed we can just hit "e", but for now we need to + // manually do a command-line rebase + // NavigateToLine(Contains("commit 01")). + // Press(keys.Universal.Edit). + Tap(func() { + t.GlobalPress(keys.Universal.ExecuteCustomCommand) + t.ExpectPopup().Prompt(). + Title(Equals("Custom Command:")). + Type(`git -c core.editor="perl -i -lpe 'print \"break\" if $.==1'" rebase -i HEAD~5`). + Confirm() + }). + Focus(). + Lines( + Contains("pick").Contains("(*) commit 06"), + Contains("pick").Contains("commit 05"), + Contains("pick").Contains("commit 04"), + Contains("update-ref").Contains("master"), + Contains("pick").Contains("(*) commit 03"), + Contains("pick").Contains("commit 02"), + Contains("<-- YOU ARE HERE --- commit 01"), + ). + NavigateToLine(Contains("commit 05")). + Press(keys.Universal.Remove) + + t.Common().ContinueRebase() + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("(*) commit 06"), + Contains("commit 04"), + Contains("(*) commit 03"), + Contains("commit 02"), + Contains("commit 01"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 19f002bda..a8959ee24 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -87,6 +87,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.AmendHeadCommitDuringRebase, interactive_rebase.AmendMerge, interactive_rebase.AmendNonHeadCommitDuringRebase, + interactive_rebase.DropTodoCommitWithUpdateRef, interactive_rebase.EditFirstCommit, interactive_rebase.EditNonTodoCommitDuringRebase, interactive_rebase.FixupFirstCommit, diff --git a/pkg/utils/rebaseTodo.go b/pkg/utils/rebaseTodo.go new file mode 100644 index 000000000..0b6a6a40c --- /dev/null +++ b/pkg/utils/rebaseTodo.go @@ -0,0 +1,108 @@ +package utils + +import ( + "fmt" + "os" + "strings" + + "github.com/fsmiamoto/git-todo-parser/todo" + "github.com/samber/lo" +) + +func equalShas(a, b string) bool { + return strings.HasPrefix(a, b) || strings.HasPrefix(b, a) +} + +func ReadRebaseTodoFile(fileName string) ([]todo.Todo, error) { + f, err := os.Open(fileName) + if err != nil { + return nil, err + } + + todos, err := todo.Parse(f) + err2 := f.Close() + if err == nil { + err = err2 + } + return todos, err +} + +func WriteRebaseTodoFile(fileName string, todos []todo.Todo) error { + f, err := os.Create(fileName) + if err != nil { + return err + } + err = todo.Write(f, todos) + err2 := f.Close() + if err == nil { + err = err2 + } + return err +} + +func MoveTodoDown(fileName string, sha string, action todo.TodoCommand) error { + todos, err := ReadRebaseTodoFile(fileName) + if err != nil { + return err + } + rearrangedTodos, err := moveTodoDown(todos, sha, action) + if err != nil { + return err + } + return WriteRebaseTodoFile(fileName, rearrangedTodos) +} + +func MoveTodoUp(fileName string, sha string, action todo.TodoCommand) error { + todos, err := ReadRebaseTodoFile(fileName) + if err != nil { + return err + } + rearrangedTodos, err := moveTodoUp(todos, sha, action) + if err != nil { + return err + } + return WriteRebaseTodoFile(fileName, rearrangedTodos) +} + +func moveTodoDown(todos []todo.Todo, sha string, action todo.TodoCommand) ([]todo.Todo, error) { + rearrangedTodos, err := moveTodoUp(lo.Reverse(todos), sha, action) + 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) + }) + + if !ok { + // Should never happen + return []todo.Todo{}, fmt.Errorf("Todo %s not found in git-rebase-todo", sha) + } + + // The todos are ordered backwards compared to our model commits, so + // actually move the commit _down_ in the todos slice (i.e. towards + // the end of the slice) + + // Find the next todo that we show in lazygit's commits view (skipping the rest) + _, skip, ok := lo.FindIndexOf(todos[sourceIdx+1:], isRenderedTodo) + + if !ok { + // We expect callers to guard against this + return []todo.Todo{}, fmt.Errorf("Destination position for moving todo is out of range") + } + + destinationIdx := sourceIdx + 1 + skip + + rearrangedTodos := MoveElement(todos, sourceIdx, destinationIdx) + + return rearrangedTodos, 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 { + return t.Commit != "" || t.Command == todo.UpdateRef +} diff --git a/pkg/utils/rebaseTodo_test.go b/pkg/utils/rebaseTodo_test.go new file mode 100644 index 000000000..a4b1a46d0 --- /dev/null +++ b/pkg/utils/rebaseTodo_test.go @@ -0,0 +1,230 @@ +package utils + +import ( + "testing" + + "github.com/fsmiamoto/git-todo-parser/todo" + "github.com/stretchr/testify/assert" +) + +func TestRebaseCommands_moveTodoDown(t *testing.T) { + type scenario struct { + testName string + todos []todo.Todo + shaToMoveDown string + expectedErr string + expectedTodos []todo.Todo + } + + scenarios := []scenario{ + { + testName: "simple case 1 - move to beginning", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + shaToMoveDown: "5678", + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + }, + { + testName: "simple case 2 - move from end", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + shaToMoveDown: "abcd", + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "abcd"}, + {Command: todo.Pick, Commit: "5678"}, + }, + }, + { + testName: "skip an invisible todo", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "abcd"}, + {Command: todo.Label, Label: "myLabel"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "def0"}, + }, + shaToMoveDown: "5678", + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + {Command: todo.Label, Label: "myLabel"}, + {Command: todo.Pick, Commit: "def0"}, + }, + }, + + // Error cases + { + testName: "commit not found", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + shaToMoveDown: "def0", + expectedErr: "Todo def0 not found in git-rebase-todo", + expectedTodos: []todo.Todo{}, + }, + { + testName: "trying to move first commit down", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {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{}, + }, + { + testName: "trying to move commit down when all commits before are invisible", + todos: []todo.Todo{ + {Command: todo.Label, Label: "myLabel"}, + {Command: todo.Reset, Label: "otherlabel"}, + {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{}, + }, + } + + for _, s := range scenarios { + t.Run(s.testName, func(t *testing.T) { + rearrangedTodos, err := moveTodoDown(s.todos, s.shaToMoveDown, todo.Pick) + if s.expectedErr == "" { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, s.expectedErr) + } + assert.Equal(t, s.expectedTodos, rearrangedTodos) + }, + ) + } +} + +func TestRebaseCommands_moveTodoUp(t *testing.T) { + type scenario struct { + testName string + todos []todo.Todo + shaToMoveDown string + expectedErr string + expectedTodos []todo.Todo + } + + scenarios := []scenario{ + { + testName: "simple case 1 - move to end", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + shaToMoveDown: "5678", + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "abcd"}, + {Command: todo.Pick, Commit: "5678"}, + }, + }, + { + testName: "simple case 2 - move from beginning", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + shaToMoveDown: "1234", + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + }, + { + testName: "skip an invisible todo", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "abcd"}, + {Command: todo.Label, Label: "myLabel"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "def0"}, + }, + shaToMoveDown: "abcd", + expectedErr: "", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Label, Label: "myLabel"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + {Command: todo.Pick, Commit: "def0"}, + }, + }, + + // Error cases + { + testName: "commit not found", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + shaToMoveDown: "def0", + expectedErr: "Todo def0 not found in git-rebase-todo", + expectedTodos: []todo.Todo{}, + }, + { + testName: "trying to move last commit up", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Pick, Commit: "abcd"}, + }, + shaToMoveDown: "abcd", + expectedErr: "Destination position for moving todo is out of range", + expectedTodos: []todo.Todo{}, + }, + { + testName: "trying to move commit up when all commits after it are invisible", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "1234"}, + {Command: todo.Pick, Commit: "5678"}, + {Command: todo.Label, Label: "myLabel"}, + {Command: todo.Reset, Label: "otherlabel"}, + }, + shaToMoveDown: "5678", + 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 := moveTodoUp(s.todos, s.shaToMoveDown, todo.Pick) + if s.expectedErr == "" { + assert.NoError(t, err) + } else { + assert.ErrorContains(t, err, s.expectedErr) + } + assert.Equal(t, s.expectedTodos, rearrangedTodos) + }, + ) + } +} diff --git a/pkg/utils/slice.go b/pkg/utils/slice.go index 2281d8a73..aff6ae470 100644 --- a/pkg/utils/slice.go +++ b/pkg/utils/slice.go @@ -92,3 +92,24 @@ func MuiltiGroupBy[T any, K comparable](slice []T, f func(T) []K) map[K][]T { } return result } + +// Returns a new slice with the element at index 'from' moved to index 'to'. +// Does not mutate original slice. +func MoveElement[T any](slice []T, from int, to int) []T { + newSlice := make([]T, len(slice)) + copy(newSlice, slice) + + if from == to { + return newSlice + } + + if from < to { + copy(newSlice[from:to+1], newSlice[from+1:to+1]) + } else { + copy(newSlice[to+1:from+1], newSlice[to:from]) + } + + newSlice[to] = slice[from] + + return newSlice +} diff --git a/pkg/utils/slice_test.go b/pkg/utils/slice_test.go index e66edcd61..b3cad8885 100644 --- a/pkg/utils/slice_test.go +++ b/pkg/utils/slice_test.go @@ -233,3 +233,85 @@ func TestLimitStr(t *testing.T) { } } } + +func TestMoveElement(t *testing.T) { + type scenario struct { + testName string + list []int + from int + to int + expected []int + } + + scenarios := []scenario{ + { + "no elements", + []int{}, + 0, + 0, + []int{}, + }, + { + "one element", + []int{1}, + 0, + 0, + []int{1}, + }, + { + "two elements, moving first to second", + []int{1, 2}, + 0, + 1, + []int{2, 1}, + }, + { + "two elements, moving second to first", + []int{1, 2}, + 1, + 0, + []int{2, 1}, + }, + { + "three elements, moving first to second", + []int{1, 2, 3}, + 0, + 1, + []int{2, 1, 3}, + }, + { + "three elements, moving second to first", + []int{1, 2, 3}, + 1, + 0, + []int{2, 1, 3}, + }, + { + "three elements, moving second to third", + []int{1, 2, 3}, + 1, + 2, + []int{1, 3, 2}, + }, + { + "three elements, moving third to second", + []int{1, 2, 3}, + 2, + 1, + []int{1, 3, 2}, + }, + } + + for _, s := range scenarios { + s := s + t.Run(s.testName, func(t *testing.T) { + assert.EqualValues(t, s.expected, MoveElement(s.list, s.from, s.to)) + }) + } + + t.Run("from out of bounds", func(t *testing.T) { + assert.Panics(t, func() { + MoveElement([]int{1, 2, 3}, 3, 0) + }) + }) +} diff --git a/vendor/github.com/fsmiamoto/git-todo-parser/LICENSE b/vendor/github.com/fsmiamoto/git-todo-parser/LICENSE new file mode 100644 index 000000000..56bc157ae --- /dev/null +++ b/vendor/github.com/fsmiamoto/git-todo-parser/LICENSE @@ -0,0 +1,7 @@ +Copyright © 2023 Flavio Miamoto + +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the “Software”), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED “AS IS”, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go b/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go index 8203d3151..97c60db9d 100644 --- a/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go +++ b/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go @@ -13,6 +13,7 @@ var ( ErrMissingLabel = errors.New("missing label") ErrMissingCommit = errors.New("missing commit") ErrMissingExecCmd = errors.New("missing command for exec") + ErrMissingRef = errors.New("missing ref") ) func Parse(f io.Reader) ([]Todo, error) { @@ -55,9 +56,9 @@ func parseLine(line string) (Todo, error) { fields := strings.Fields(line) - for i := TodoCommand(Pick); i < Comment; i++ { + for i := Pick; i < Comment; i++ { if isCommand(i, fields[0]) { - todo.Command = TodoCommand(i) + todo.Command = i fields = fields[1:] break } @@ -90,6 +91,7 @@ func parseLine(line string) (Todo, error) { if todo.Command == Merge { if fields[0] == "-C" || fields[0] == "-c" { + todo.Flag = fields[0] fields = fields[1:] if len(fields) == 0 { return todo, ErrMissingCommit @@ -115,10 +117,19 @@ func parseLine(line string) (Todo, error) { } // Skip flags if fields[0] == "-C" || fields[0] == "-c" { + todo.Flag = fields[0] fields = fields[1:] } } + if todo.Command == UpdateRef { + if len(fields) == 0 { + return todo, ErrMissingRef + } + todo.Ref = fields[0] + return todo, nil + } + if len(fields) == 0 { return todo, ErrMissingCommit } diff --git a/vendor/github.com/fsmiamoto/git-todo-parser/todo/todo.go b/vendor/github.com/fsmiamoto/git-todo-parser/todo/todo.go index ce16652db..77bb5dc71 100644 --- a/vendor/github.com/fsmiamoto/git-todo-parser/todo/todo.go +++ b/vendor/github.com/fsmiamoto/git-todo-parser/todo/todo.go @@ -18,6 +18,7 @@ const ( NoOp Drop + UpdateRef Comment ) @@ -27,10 +28,12 @@ const CommentChar = "#" type Todo struct { Command TodoCommand Commit string + Flag string Comment string ExecCommand string Label string Msg string + Ref string } func (t TodoCommand) String() string { @@ -38,23 +41,24 @@ func (t TodoCommand) String() string { } var commandToString = map[TodoCommand]string{ - Pick: "pick", - Revert: "revert", - Edit: "edit", - Reword: "reword", - Fixup: "fixup", - Squash: "squash", - Exec: "exec", - Break: "break", - Label: "label", - Reset: "reset", - Merge: "merge", - NoOp: "noop", - Drop: "drop", - Comment: "comment", + Pick: "pick", + Revert: "revert", + Edit: "edit", + Reword: "reword", + Fixup: "fixup", + Squash: "squash", + Exec: "exec", + Break: "break", + Label: "label", + Reset: "reset", + Merge: "merge", + NoOp: "noop", + Drop: "drop", + UpdateRef: "update-ref", + Comment: "comment", } -var todoCommandInfo = [14]struct { +var todoCommandInfo = [15]struct { nickname string cmd string }{ @@ -72,4 +76,5 @@ var todoCommandInfo = [14]struct { {"m", "merge"}, {"", "noop"}, {"d", "drop"}, + {"u", "update-ref"}, } diff --git a/vendor/github.com/fsmiamoto/git-todo-parser/todo/write.go b/vendor/github.com/fsmiamoto/git-todo-parser/todo/write.go new file mode 100644 index 000000000..c96b06cc0 --- /dev/null +++ b/vendor/github.com/fsmiamoto/git-todo-parser/todo/write.go @@ -0,0 +1,92 @@ +package todo + +import ( + "io" + "strings" +) + +func Write(f io.Writer, todos []Todo) error { + for _, todo := range todos { + if err := writeTodo(f, todo); err != nil { + return err + } + } + + return nil +} + +func writeTodo(f io.Writer, todo Todo) error { + var sb strings.Builder + if todo.Command != Comment { + sb.WriteString(todo.Command.String()) + } + + switch todo.Command { + case NoOp: + return nil + + case Comment: + sb.WriteString(CommentChar) + sb.WriteString(todo.Comment) + + case Break: + + case Label: + fallthrough + case Reset: + sb.WriteByte(' ') + sb.WriteString(todo.Label) + + case Exec: + sb.WriteByte(' ') + sb.WriteString(todo.ExecCommand) + + case Merge: + sb.WriteByte(' ') + if todo.Commit != "" { + sb.WriteString(todo.Flag) + sb.WriteByte(' ') + sb.WriteString(todo.Commit) + sb.WriteByte(' ') + } + sb.WriteString(todo.Label) + if todo.Msg != "" { + sb.WriteString(" # ") + sb.WriteString(todo.Msg) + } + + case Fixup: + sb.WriteByte(' ') + if todo.Flag != "" { + sb.WriteString(todo.Flag) + sb.WriteByte(' ') + } + sb.WriteString(todo.Commit) + + case UpdateRef: + sb.WriteByte(' ') + sb.WriteString(todo.Ref) + + case Pick: + fallthrough + case Revert: + fallthrough + case Edit: + fallthrough + case Reword: + fallthrough + case Squash: + fallthrough + case Drop: + sb.WriteByte(' ') + sb.WriteString(todo.Commit) + if todo.Msg != "" { + sb.WriteByte(' ') + sb.WriteString(todo.Msg) + } + } + + sb.WriteByte('\n') + _, err := f.Write([]byte(sb.String())) + return err +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 788171757..06224918c 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -30,7 +30,7 @@ github.com/emirpasic/gods/utils # github.com/fatih/color v1.9.0 ## explicit; go 1.13 github.com/fatih/color -# github.com/fsmiamoto/git-todo-parser v0.0.2 +# github.com/fsmiamoto/git-todo-parser v0.0.4-0.20230403011024-617a5a7ce980 ## explicit; go 1.13 github.com/fsmiamoto/git-todo-parser/todo # github.com/fsnotify/fsnotify v1.4.7