From c53c5e47efc36c1b9bf9fa51f4840caf70ceba28 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 3 Apr 2023 12:42:29 +0200 Subject: [PATCH] Store commit.Action as an enum instead of a string The main reason for doing this (besides the reasons given for Status in the previous commit) is that it allows us to easily convert from TodoCommand to Action and back. This will be needed later in the branch. Fortunately, TodoCommand is one-based, so this allows us to add an ActionNone constant with the value 0. --- pkg/commands/git_commands/commit_loader.go | 2 +- .../git_commands/commit_loader_test.go | 16 +++++------ pkg/commands/git_commands/rebase.go | 5 ++-- pkg/commands/models/commit.go | 11 ++++++-- .../controllers/local_commits_controller.go | 23 ++++++++-------- pkg/gui/presentation/commits.go | 18 +++++++------ pkg/gui/presentation/commits_test.go | 27 ++++++++++--------- 7 files changed, 57 insertions(+), 45 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 8c85de402..3b2e52ec4 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -313,7 +313,7 @@ func (self *CommitLoader) getInteractiveRebasingCommits() ([]*models.Commit, err Sha: t.Commit, Name: t.Msg, Status: models.StatusRebasing, - Action: t.Command.String(), + Action: t.Command, }) } diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index dab321753..140541545 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -79,7 +79,7 @@ func TestGetCommits(t *testing.T) { Sha: "0eea75e8c631fba6b58135697835d58ba4c18dbc", Name: "better typing for rebase mode", Status: models.StatusUnpushed, - Action: "", + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "(HEAD -> better-tests)", AuthorName: "Jesse Duffield", @@ -93,7 +93,7 @@ func TestGetCommits(t *testing.T) { Sha: "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", Name: "fix logging", Status: models.StatusPushed, - Action: "", + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "(origin/better-tests)", AuthorName: "Jesse Duffield", @@ -107,7 +107,7 @@ func TestGetCommits(t *testing.T) { Sha: "e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c", Name: "refactor", Status: models.StatusPushed, - Action: "", + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -121,7 +121,7 @@ func TestGetCommits(t *testing.T) { Sha: "d8084cd558925eb7c9c38afeed5725c21653ab90", Name: "WIP", Status: models.StatusPushed, - Action: "", + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -135,7 +135,7 @@ func TestGetCommits(t *testing.T) { Sha: "65f910ebd85283b5cce9bf67d03d3f1a9ea3813a", Name: "WIP", Status: models.StatusPushed, - Action: "", + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -149,7 +149,7 @@ func TestGetCommits(t *testing.T) { Sha: "26c07b1ab33860a1a7591a0638f9925ccf497ffa", Name: "WIP", Status: models.StatusMerged, - Action: "", + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -163,7 +163,7 @@ func TestGetCommits(t *testing.T) { Sha: "3d4470a6c072208722e5ae9a54bcb9634959a1c5", Name: "WIP", Status: models.StatusMerged, - Action: "", + Action: models.ActionNone, Tags: []string{}, ExtraInfo: "", AuthorName: "Jesse Duffield", @@ -177,7 +177,7 @@ func TestGetCommits(t *testing.T) { Sha: "053a66a7be3da43aacdc7aa78e1fe757b82c4dd2", Name: "refactoring the config struct", Status: models.StatusMerged, - Action: "", + 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..1207bdd56 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -6,6 +6,7 @@ import ( "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" @@ -202,7 +203,7 @@ func (self *RebaseCommands) AmendTo(commit *models.Commit) error { } // EditRebaseTodo sets the action at a given index in the git-rebase-todo file -func (self *RebaseCommands) EditRebaseTodo(index int, action string) error { +func (self *RebaseCommands) EditRebaseTodo(index int, action todo.TodoCommand) error { fileName := filepath.Join(self.dotGitDir, "rebase-merge/git-rebase-todo") bytes, err := os.ReadFile(fileName) if err != nil { @@ -216,7 +217,7 @@ func (self *RebaseCommands) EditRebaseTodo(index int, action string) error { // 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:], " ") + content[contentIndex] = action.String() + " " + strings.Join(splitLine[1:], " ") result := strings.Join(content, "\n") return os.WriteFile(fileName, []byte(result), 0o644) diff --git a/pkg/commands/models/commit.go b/pkg/commands/models/commit.go index adc7a5a51..e0ff875a0 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -3,6 +3,7 @@ package models import ( "fmt" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -21,12 +22,18 @@ const ( 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 CommitStatus - Action string // one of "", "pick", "edit", "squash", "reword", "drop", "fixup" + Action todo.TodoCommand Tags []string ExtraInfo string // something like 'HEAD -> master, tag: v0.15.2' AuthorName string // something like 'Jesse Duffield' @@ -75,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/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 284534ffa..7afb7e0ed 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -3,6 +3,7 @@ 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" @@ -153,7 +154,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 +179,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 +200,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 +249,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 +269,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 +290,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 +306,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 +327,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,13 +344,13 @@ 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) } 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, ) diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index f7d3e1308..7875d51da 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 := "" @@ -368,15 +370,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"}}, },