From 62c5c32fbb0a44713933724e5c941ee1d42c2b27 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 4 Apr 2023 21:29:19 +0200 Subject: [PATCH 01/11] Bump github.com/fsmiamoto/git-todo-parser to latest main version --- go.mod | 2 +- go.sum | 4 +- .../fsmiamoto/git-todo-parser/LICENSE | 7 ++ .../fsmiamoto/git-todo-parser/todo/parse.go | 15 ++- .../fsmiamoto/git-todo-parser/todo/todo.go | 35 ++++--- .../fsmiamoto/git-todo-parser/todo/write.go | 92 +++++++++++++++++++ vendor/modules.txt | 2 +- 7 files changed, 136 insertions(+), 21 deletions(-) create mode 100644 vendor/github.com/fsmiamoto/git-todo-parser/LICENSE create mode 100644 vendor/github.com/fsmiamoto/git-todo-parser/todo/write.go 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/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 From 188773511e0c94a4ec338328865e824402ec8b00 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 3 Apr 2023 12:40:29 +0200 Subject: [PATCH 02/11] Store commit.Status as an enum instead of a string This is unrelated to the changes in this PR, but since we are doing the same thing for the commit.Action field in the next commit, it makes sense to do it for Status too for consistency. Modelling this as an enum feels more natural than modelling it as a string, since there's a finite set of possible values. And it saves a little bit of memory (not very much, since none of the strings were heap-allocated, but still). --- pkg/commands/git_commands/commit_loader.go | 10 +++++----- .../git_commands/commit_loader_test.go | 16 ++++++++-------- .../git_commands/reflog_commit_loader.go | 2 +- .../git_commands/reflog_commit_loader_test.go | 18 +++++++++--------- pkg/commands/models/commit.go | 14 +++++++++++++- pkg/gui/presentation/commits.go | 11 ++++++----- 6 files changed, 42 insertions(+), 29 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 119566f08..8c85de402 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 }) @@ -312,7 +312,7 @@ func (self *CommitLoader) getInteractiveRebasingCommits() ([]*models.Commit, err commits = slices.Prepend(commits, &models.Commit{ Sha: t.Commit, Name: t.Msg, - Status: "rebasing", + Status: models.StatusRebasing, Action: t.Command.String(), }) } @@ -332,7 +332,7 @@ func (self *CommitLoader) commitFromPatch(content string) *models.Commit { return &models.Commit{ Sha: sha, Name: name, - Status: "rebasing", + Status: models.StatusRebasing, } } @@ -349,11 +349,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..dab321753 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -78,7 +78,7 @@ func TestGetCommits(t *testing.T) { { Sha: "0eea75e8c631fba6b58135697835d58ba4c18dbc", Name: "better typing for rebase mode", - Status: "unpushed", + Status: models.StatusUnpushed, Action: "", Tags: []string{}, ExtraInfo: "(HEAD -> better-tests)", @@ -92,7 +92,7 @@ func TestGetCommits(t *testing.T) { { Sha: "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", Name: "fix logging", - Status: "pushed", + Status: models.StatusPushed, Action: "", Tags: []string{}, ExtraInfo: "(origin/better-tests)", @@ -106,7 +106,7 @@ func TestGetCommits(t *testing.T) { { Sha: "e94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c", Name: "refactor", - Status: "pushed", + Status: models.StatusPushed, Action: "", Tags: []string{}, ExtraInfo: "", @@ -120,7 +120,7 @@ func TestGetCommits(t *testing.T) { { Sha: "d8084cd558925eb7c9c38afeed5725c21653ab90", Name: "WIP", - Status: "pushed", + Status: models.StatusPushed, Action: "", Tags: []string{}, ExtraInfo: "", @@ -134,7 +134,7 @@ func TestGetCommits(t *testing.T) { { Sha: "65f910ebd85283b5cce9bf67d03d3f1a9ea3813a", Name: "WIP", - Status: "pushed", + Status: models.StatusPushed, Action: "", Tags: []string{}, ExtraInfo: "", @@ -148,7 +148,7 @@ func TestGetCommits(t *testing.T) { { Sha: "26c07b1ab33860a1a7591a0638f9925ccf497ffa", Name: "WIP", - Status: "merged", + Status: models.StatusMerged, Action: "", Tags: []string{}, ExtraInfo: "", @@ -162,7 +162,7 @@ func TestGetCommits(t *testing.T) { { Sha: "3d4470a6c072208722e5ae9a54bcb9634959a1c5", Name: "WIP", - Status: "merged", + Status: models.StatusMerged, Action: "", Tags: []string{}, ExtraInfo: "", @@ -176,7 +176,7 @@ func TestGetCommits(t *testing.T) { { Sha: "053a66a7be3da43aacdc7aa78e1fe757b82c4dd2", Name: "refactoring the config struct", - Status: "merged", + Status: models.StatusMerged, Action: "", Tags: []string{}, ExtraInfo: "", 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/models/commit.go b/pkg/commands/models/commit.go index b3a171934..adc7a5a51 100644 --- a/pkg/commands/models/commit.go +++ b/pkg/commands/models/commit.go @@ -9,11 +9,23 @@ import ( // Special commit hash for empty tree object const EmptyTreeCommitHash = "4b825dc642cb6eb9a060e54bf8d69288fbee4904" +type CommitStatus int + +const ( + StatusNone CommitStatus = iota + StatusUnpushed + StatusPushed + StatusMerged + StatusRebasing + StatusSelected + StatusReflog +) + // Commit : A git commit type Commit struct { Sha string Name string - Status string // one of "unpushed", "pushed", "merged", "rebasing" or "selected" + Status CommitStatus Action string // one of "", "pick", "edit", "squash", "reword", "drop", "fixup" Tags []string ExtraInfo string // something like 'HEAD -> master, tag: v0.15.2' diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 77b946a64..f7d3e1308 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -346,16 +346,17 @@ func getShaColor( diffed := 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 { From c53c5e47efc36c1b9bf9fa51f4840caf70ceba28 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 3 Apr 2023 12:42:29 +0200 Subject: [PATCH 03/11] 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"}}, }, From a0d179b6dcb1dc27690c447dd55f264bcc9cbcb0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 7 Apr 2023 19:28:31 +0200 Subject: [PATCH 04/11] Make getHydratedRebasingCommits more robust So far the algorithm worked on the assumption that the output of the "git show" command corresponds one-to-one to the lines of the rebase-todo file. This assumption doesn't hold once we start to include todo lines that don't have a sha (like update-ref), or when the todo file contains multiple entries for the same sha. This should never happen normally, but it can if users manually edit the todo file and duplicate a line. --- pkg/commands/git_commands/commit_loader.go | 24 ++++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 3b2e52ec4..6e409d717 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -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 } From 740474c10c7d2038a33f9861573a4a85213c2270 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 4 Apr 2023 09:19:06 +0200 Subject: [PATCH 05/11] Visualize branch heads in commits panel Useful when working with stacked branches. --- pkg/gui/presentation/commits.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 7875d51da..8a4d947c4 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -277,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("(*)") + " " } } From 227b0b781cb4aab2ae613a66f2e73467670ae208 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 5 Apr 2023 09:33:11 +0200 Subject: [PATCH 06/11] Show update-ref commands in rebase todo list This is useful when working with stacked branches, because you can now move "pick" entries across an update-ref command and you can tell exactly which branch the commit will end up in. It's also useful to spot situations where the --update-refs option didn't work as desired. For example, if you duplicate a branch and want to rebase only one of the branches but not the other (maybe for testing); if you have rebase.updateRefs=true in your git config, then rebasing one branch will move the other branch along. To solve this we'll have to introduce a way to delete the update-ref entry (maybe by hitting backspace?); this is out of scope for this PR, so for now users will have to type "git rebase --edit-todo" into the custom command prompt to sort this out. We will also have to prevent users from trying to turn update-ref commands into other commands like "pick" or "drop"; we'll do this later in this branch. --- pkg/commands/git_commands/commit_loader.go | 4 +++- pkg/gui/commits_panel.go | 8 ++++++++ pkg/gui/presentation/commits.go | 2 +- pkg/i18n/english.go | 2 ++ 4 files changed, 14 insertions(+), 2 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 6e409d717..bce16de15 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -311,7 +311,9 @@ 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 } 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/presentation/commits.go b/pkg/gui/presentation/commits.go index 8a4d947c4..7e16197b5 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -347,7 +347,7 @@ func getShaColor( return getBisectStatusColor(bisectStatus) } - diffed := commit.Sha == diffName + diffed := commit.Sha != "" && commit.Sha == diffName shaColor := theme.DefaultTextColor switch commit.Status { case models.StatusUnpushed: diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index e7756d03e..7b6642882 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 @@ -754,6 +755,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", From a304fed68cd2208c3e78afd9ea99446d5b5f4444 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 13 Apr 2023 23:14:56 +0200 Subject: [PATCH 07/11] Add GitVersion field to NewIntegrationTestArgs It can be used to specify which git versions a given test should or should not run on. --- pkg/commands/git_commands/version.go | 6 ++- pkg/integration/components/runner.go | 24 +++++++++- pkg/integration/components/test.go | 58 +++++++++++++++++++++++ pkg/integration/components/test_test.go | 63 +++++++++++++++++++++++++ 4 files changed, 149 insertions(+), 2 deletions(-) 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/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/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) + }) + } +} From 860a8d102b7eac47aaefc3ae17b443e93cd10c9f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 13 Apr 2023 20:24:28 +0200 Subject: [PATCH 08/11] Add integration test for dropping a todo commit when there's an update-ref The test shows how we are accidentally dropping the wrong commit in this case. We'll fix that in the next commit. --- pkg/integration/components/shell.go | 6 +- .../drop_todo_commit_with_update_ref.go | 72 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 78 insertions(+), 1 deletion(-) create mode 100644 pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go 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/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..11d693d29 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go @@ -0,0 +1,72 @@ +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"), + /* EXPECTED: + Contains("commit 04"), + ACTUAL: */ + Contains("commit 05"), + 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, From 120dd1530ae329199928c3494ea6063d741fc54d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 2 Apr 2023 19:16:28 +0200 Subject: [PATCH 09/11] Make EditRebaseTodo more robust It used to work on the assumption that rebasing commits in lazygit's model correspond one-to-one to lines in the git-rebase-todo file, which isn't necessarily true (e.g. when users use "git rebase --edit-todo" at the custom command prompt and add a "break" between lines). --- pkg/commands/git_commands/rebase.go | 29 +++++++++------- .../controllers/local_commits_controller.go | 4 +-- .../drop_todo_commit_with_update_ref.go | 3 -- pkg/utils/rebaseTodo.go | 34 +++++++++++++++++++ 4 files changed, 51 insertions(+), 19 deletions(-) create mode 100644 pkg/utils/rebaseTodo.go diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 1207bdd56..27b5a6ba4 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -12,6 +12,7 @@ import ( "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 { @@ -202,25 +203,27 @@ 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 todo.TodoCommand) 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) + 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) + } + } - // 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.String() + " " + strings.Join(splitLine[1:], " ") - result := strings.Join(content, "\n") - - return os.WriteFile(fileName, []byte(result), 0o644) + // Should never get here + return fmt.Errorf("Todo %s not found in git-rebase-todo", commit.Sha) } func (self *RebaseCommands) getTodoCommitCount(content []string) int { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 7afb7e0ed..72169a5f0 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -354,9 +354,7 @@ func (self *LocalCommitsController) handleMidRebaseCommand(action todo.TodoComma 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) } 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 index 11d693d29..c247f1743 100644 --- 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 @@ -60,10 +60,7 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{ IsFocused(). Lines( Contains("(*) commit 06"), - /* EXPECTED: Contains("commit 04"), - ACTUAL: */ - Contains("commit 05"), Contains("(*) commit 03"), Contains("commit 02"), Contains("commit 01"), diff --git a/pkg/utils/rebaseTodo.go b/pkg/utils/rebaseTodo.go new file mode 100644 index 000000000..3e071321b --- /dev/null +++ b/pkg/utils/rebaseTodo.go @@ -0,0 +1,34 @@ +package utils + +import ( + "os" + + "github.com/fsmiamoto/git-todo-parser/todo" +) + +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 +} From dc4e88f8a48bd52160a76b79da56e13af7b9ffc0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 4 Apr 2023 10:23:50 +0200 Subject: [PATCH 10/11] Make moving todo commits more robust --- pkg/commands/git_commands/rebase.go | 31 +-- .../controllers/local_commits_controller.go | 4 +- pkg/utils/rebaseTodo.go | 74 ++++++ pkg/utils/rebaseTodo_test.go | 230 ++++++++++++++++++ pkg/utils/slice.go | 21 ++ pkg/utils/slice_test.go | 82 +++++++ 6 files changed, 415 insertions(+), 27 deletions(-) create mode 100644 pkg/utils/rebaseTodo_test.go diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 27b5a6ba4..e4c20426f 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -2,7 +2,6 @@ package git_commands import ( "fmt" - "os" "path/filepath" "strings" @@ -226,34 +225,16 @@ func (self *RebaseCommands) EditRebaseTodo(commit *models.Commit, action todo.To return fmt.Errorf("Todo %s not found in git-rebase-todo", commit.Sha) } -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++ - } - } - return commitCount +// MoveTodoDown moves a rebase todo item down by one position +func (self *RebaseCommands) MoveTodoDown(commit *models.Commit) error { + fileName := filepath.Join(self.dotGitDir, "rebase-merge/git-rebase-todo") + return utils.MoveTodoDown(fileName, commit.Sha, commit.Action) } // MoveTodoDown moves a rebase todo item down by one position -func (self *RebaseCommands) MoveTodoDown(index int) error { +func (self *RebaseCommands) MoveTodoUp(commit *models.Commit) error { fileName := filepath.Join(self.dotGitDir, "rebase-merge/git-rebase-todo") - bytes, err := os.ReadFile(fileName) - if err != nil { - return err - } - - 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) + return utils.MoveTodoUp(fileName, commit.Sha, commit.Action) } // SquashAllAboveFixupCommits squashes all fixup! commits above the given one diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 72169a5f0..48fafd4e6 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -382,7 +382,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) @@ -420,7 +420,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) diff --git a/pkg/utils/rebaseTodo.go b/pkg/utils/rebaseTodo.go index 3e071321b..0b6a6a40c 100644 --- a/pkg/utils/rebaseTodo.go +++ b/pkg/utils/rebaseTodo.go @@ -1,11 +1,18 @@ 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 { @@ -32,3 +39,70 @@ func WriteRebaseTodoFile(fileName string, todos []todo.Todo) error { } 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) + }) + }) +} From d675eb65077fee840d2ed7b13a0ee038611664b6 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 5 Apr 2023 16:09:36 +0200 Subject: [PATCH 11/11] Don't allow changing the type of certain rebase todos We already show "merge" todo entries when starting an interactive rebase with --rebase-merges outside of lazygit. Changing the type of a merge entry to "pick" or "edit" doesn't make sense and shouldn't be allowed. Earlier in this branch we have started to show "update-ref" entries, these can't be changed either (they can be moved, though). You might argue that it should be possible to change them to "drop", but in the case of "update-ref" this doesn't make sense either, because "drop" needs a Sha and we don't have one here. Also, you would then be able to later change it back to "pick", so we would have to remember that this isn't allowed for this particular drop entry; that's messy, so just disallow all editing. --- .../controllers/local_commits_controller.go | 18 ++++++++++++++++++ pkg/i18n/english.go | 2 ++ 2 files changed, 20 insertions(+) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 48fafd4e6..4610aed12 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -9,6 +9,7 @@ import ( "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 ( @@ -348,6 +349,10 @@ func (self *LocalCommitsController) handleMidRebaseCommand(action todo.TodoComma 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.String()), @@ -759,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/i18n/english.go b/pkg/i18n/english.go index 7b6642882..80179f365 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -219,6 +219,7 @@ type TranslationSet struct { YouAreHere string YouDied string LcRewordNotSupported string + LcChangingThisActionIsNotAllowed string LcCherryPickCopy string LcCherryPickCopyRange string LcPasteCommits string @@ -887,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)",