From d210107caaf221c17f19d56fc0ec76bf157eb044 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 16 Apr 2023 08:56:08 +0200 Subject: [PATCH 01/23] Bump github.com/fsmiamoto/git-todo-parser to latest version --- go.mod | 2 +- go.sum | 4 ++-- .../github.com/fsmiamoto/git-todo-parser/todo/parse.go | 10 ++++++++-- vendor/modules.txt | 2 +- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index f86062a69..098d19c67 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.4-0.20230403011024-617a5a7ce980 + github.com/fsmiamoto/git-todo-parser v0.0.4 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 e6a83d627..dd4a8b81e 100644 --- a/go.sum +++ b/go.sum @@ -30,8 +30,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.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/fsmiamoto/git-todo-parser v0.0.4 h1:fzcGaoAFDHWzJRKw//CSZFrXucsLKplIvOSab3FtWWM= +github.com/fsmiamoto/git-todo-parser v0.0.4/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/todo/parse.go b/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go index 97c60db9d..ab3dd9ea9 100644 --- a/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go +++ b/vendor/github.com/fsmiamoto/git-todo-parser/todo/parse.go @@ -56,9 +56,11 @@ func parseLine(line string) (Todo, error) { fields := strings.Fields(line) + var commandLen int for i := Pick; i < Comment; i++ { if isCommand(i, fields[0]) { todo.Command = i + commandLen = len(fields[0]) fields = fields[1:] break } @@ -74,10 +76,14 @@ func parseLine(line string) (Todo, error) { } if todo.Command == Label || todo.Command == Reset { - if len(fields) == 0 { + restOfLine := strings.TrimSpace(line[commandLen:]) + if todo.Command == Reset && restOfLine == "[new root]" { + todo.Label = restOfLine + } else if len(fields) == 0 { return todo, ErrMissingLabel + } else { + todo.Label = fields[0] } - todo.Label = fields[0] return todo, nil } diff --git a/vendor/modules.txt b/vendor/modules.txt index 12028e79c..fd06a3ec6 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.4-0.20230403011024-617a5a7ce980 +# github.com/fsmiamoto/git-todo-parser v0.0.4 ## explicit; go 1.13 github.com/fsmiamoto/git-todo-parser/todo # github.com/fsnotify/fsnotify v1.4.7 From a41218551da19b4fb987c5090665692d24852d03 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 24 Apr 2023 12:43:45 +0200 Subject: [PATCH 02/23] Put gitCommon.version back in deps_test.go This was reverted in 3546ab8f21, but shouldn't have. --- pkg/commands/git_commands/deps_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pkg/commands/git_commands/deps_test.go b/pkg/commands/git_commands/deps_test.go index df2c96253..bcf36b168 100644 --- a/pkg/commands/git_commands/deps_test.go +++ b/pkg/commands/git_commands/deps_test.go @@ -15,6 +15,7 @@ import ( type commonDeps struct { runner *oscommands.FakeCmdObjRunner userConfig *config.UserConfig + gitVersion *GitVersion gitConfig *git_config.FakeGitConfig getenv func(string) string removeFile func(string) error @@ -48,6 +49,11 @@ func buildGitCommon(deps commonDeps) *GitCommon { gitCommon.Common.UserConfig = config.GetDefaultConfig() } + gitCommon.version = deps.gitVersion + if gitCommon.version == nil { + gitCommon.version = &GitVersion{2, 0, 0, ""} + } + gitConfig := deps.gitConfig if gitConfig == nil { gitConfig = git_config.NewFakeGitConfig(nil) From 5645a662de36bd742c36f26b34b9e74510ba54eb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 6 Apr 2023 06:46:48 +0200 Subject: [PATCH 03/23] Use --rebase-merges for interactive rebase At the moment it doesn't make a big difference, because the vast majority of callers create a list of todos themselves to completely replace what git came up with. We're changing this in the following commits though, and then it's helpful to preserve merges. --- pkg/commands/git_commands/rebase.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 5828ffbaa..b34f8886a 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -173,7 +173,12 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract debug = "TRUE" } - cmdStr := fmt.Sprintf("git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash %s", opts.baseShaOrRoot) + rebaseMergesArg := " --rebase-merges" + if self.version.IsOlderThan(2, 22, 0) { + rebaseMergesArg = "" + } + cmdStr := fmt.Sprintf("git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash%s %s", + rebaseMergesArg, opts.baseShaOrRoot) self.Log.WithField("command", cmdStr).Debug("RunCommand") cmdObj := self.cmd.New(cmdStr) From d50c58b4c669b1de94bf4d04f508b3eb5b84647c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 5 Apr 2023 13:03:50 +0200 Subject: [PATCH 04/23] Implement "edit commit" in terms of the new EditRebase function --- pkg/commands/git_commands/rebase.go | 14 -------------- pkg/gui/controllers/local_commits_controller.go | 2 +- .../drop_todo_commit_with_update_ref.go | 13 ++----------- 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index b34f8886a..26c0536e9 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -127,20 +127,6 @@ func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, index in }).Run() } -func (self *RebaseCommands) InteractiveRebaseBreakAfter(commits []*models.Commit, index int) error { - todo, sha, err := self.BuildSingleActionTodo(commits, index-1, "pick") - if err != nil { - return err - } - - todo = append(todo, TodoLine{Action: "break", Commit: nil}) - return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ - baseShaOrRoot: sha, - todoLines: todo, - overrideEditor: true, - }).Run() -} - func (self *RebaseCommands) EditRebase(branchRef string) error { commands := []TodoLine{{Action: "break"}} return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 35e314357..1d05385c9 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -301,7 +301,7 @@ func (self *LocalCommitsController) edit(commit *models.Commit) error { return self.c.WithWaitingStatus(self.c.Tr.RebasingStatus, func() error { self.c.LogAction(self.c.Tr.Actions.EditCommit) - err := self.git.Rebase.InteractiveRebaseBreakAfter(self.model.Commits, self.context().GetSelectedLineIdx()) + err := self.git.Rebase.EditRebase(commit.Sha) return self.helpers.MergeAndRebase.CheckMergeOrRebase(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 c247f1743..fcbf1abe5 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 @@ -30,17 +30,8 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{ 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() - }). + NavigateToLine(Contains("commit 01")). + Press(keys.Universal.Edit). Focus(). Lines( Contains("pick").Contains("(*) commit 06"), From ab25600ccbbc9b8880289c496f521710894e26cc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 6 Apr 2023 07:04:25 +0200 Subject: [PATCH 05/23] Extract EditRebaseTodo into a function in utils.rebaseTodo We want to reuse it from the daemon code in the next commit. --- pkg/commands/git_commands/rebase.go | 21 ++------------------- pkg/utils/rebase_todo.go | 23 +++++++++++++++++++++++ 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 26c0536e9..31eaf1366 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -231,25 +231,8 @@ func (self *RebaseCommands) AmendTo(commit *models.Commit) 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") - todos, err := utils.ReadRebaseTodoFile(fileName) - if err != nil { - return err - } - - 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) - } - } - - // Should never get here - return fmt.Errorf("Todo %s not found in git-rebase-todo", commit.Sha) + return utils.EditRebaseTodo( + filepath.Join(self.dotGitDir, "rebase-merge/git-rebase-todo"), commit.Sha, commit.Action, action) } // MoveTodoDown moves a rebase todo item down by one position diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index 0b6a6a40c..a61d4c36c 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -9,6 +9,29 @@ import ( "github.com/samber/lo" ) +// Read a git-rebase-todo file, change the action for the given sha to +// newAction, and write it back +func EditRebaseTodo(filePath string, sha string, oldAction todo.TodoCommand, newAction todo.TodoCommand) error { + todos, err := ReadRebaseTodoFile(filePath) + if err != nil { + return err + } + + 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 == oldAction && equalShas(t.Commit, sha) { + t.Command = newAction + return WriteRebaseTodoFile(filePath, todos) + } + } + + // Should never get here + return fmt.Errorf("Todo %s not found in git-rebase-todo", sha) +} + func equalShas(a, b string) bool { return strings.HasPrefix(a, b) || strings.HasPrefix(b, a) } From b8fbe9756e5b2ad22b39fe7d0b8e40d88b15df84 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 5 Apr 2023 19:01:49 +0200 Subject: [PATCH 06/23] Implement squash, fixup, drop, and reword in terms of daemon --- pkg/app/daemon/daemon.go | 61 +++++++++-- pkg/commands/git_commands/rebase.go | 101 ++++++++---------- .../controllers/local_commits_controller.go | 8 +- 3 files changed, 104 insertions(+), 66 deletions(-) diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index e040e79c2..c733c3bc1 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -1,13 +1,16 @@ package daemon import ( + "fmt" "log" "os" "path/filepath" "strings" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/jesseduffield/lazygit/pkg/common" "github.com/jesseduffield/lazygit/pkg/env" + "github.com/jesseduffield/lazygit/pkg/utils" ) // Sometimes lazygit will be invoked in daemon mode from a parent lazygit process. @@ -34,6 +37,15 @@ const ( // to prepend the content of `RebaseTODOEnvKey` to the default `git-rebase-todo` // file instead of using it as a replacement. PrependLinesEnvKey string = "LAZYGIT_PREPEND_LINES" + + // If this is set, it tells lazygit to read the original todo file, and + // change the action for one or more entries in it. The value of the variable + // will have one or more lines of the form "Sha1:newAction", e.g. + // a02b54e1b7e7e8dd8bc1958c11ef4ee4df459ea4:edit + // The existing action of the todo to be changed is expected to be "pick". + // + // If this is used, the value of RebaseTODOEnvKey must be empty. + ChangeTodoActionEnvKey string = "LAZYGIT_CHANGE_TODO_ACTION" ) type Daemon interface { @@ -94,19 +106,52 @@ func (self *rebaseDaemon) Run() error { } func (self *rebaseDaemon) writeTodoFile(path string) error { - todoContent := []byte(os.Getenv(RebaseTODOEnvKey)) + if changeTodoActionEnvValue := os.Getenv(ChangeTodoActionEnvKey); changeTodoActionEnvValue != "" { + return self.changeTodoAction(path, changeTodoActionEnvValue) + } else { + todoContent := []byte(os.Getenv(RebaseTODOEnvKey)) - prependLines := os.Getenv(PrependLinesEnvKey) != "" - if prependLines { - existingContent, err := os.ReadFile(path) - if err != nil { - return err + prependLines := os.Getenv(PrependLinesEnvKey) != "" + if prependLines { + existingContent, err := os.ReadFile(path) + if err != nil { + return err + } + + todoContent = append(todoContent, existingContent...) } - todoContent = append(todoContent, existingContent...) + return os.WriteFile(path, todoContent, 0o644) + } +} + +func (self *rebaseDaemon) changeTodoAction(path string, changeTodoActionEnvValue string) error { + lines := strings.Split(changeTodoActionEnvValue, "\n") + for _, line := range lines { + fields := strings.Split(line, ":") + if len(fields) != 2 { + return fmt.Errorf("Unexpected value for %s: %s", ChangeTodoActionEnvKey, changeTodoActionEnvValue) + } + sha, newAction := fields[0], self.actionFromString(fields[1]) + if int(newAction) == 0 { + return fmt.Errorf("Unknown action in %s", changeTodoActionEnvValue) + } + if err := utils.EditRebaseTodo(path, sha, todo.Pick, newAction); err != nil { + return err + } } - return os.WriteFile(path, todoContent, 0o644) + return nil +} + +func (self *rebaseDaemon) actionFromString(actionString string) todo.TodoCommand { + for t := todo.Pick; t < todo.Comment; t++ { + if t.String() == actionString { + return t + } + } + + return 0 } func gitDir() string { diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 31eaf1366..338917626 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -55,14 +55,14 @@ func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, me } func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int) (oscommands.ICmdObj, error) { - todo, sha, err := self.BuildSingleActionTodo(commits, index, "reword") - if err != nil { - return nil, err - } - return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ - baseShaOrRoot: sha, - todoLines: todo, + baseShaOrRoot: getBaseShaOrRoot(commits, index+1), + changeTodoActions: []ChangeTodoAction{ + { + sha: commits[index].Sha, + newAction: todo.Reword, + }, + }, }), nil } @@ -114,16 +114,21 @@ func (self *RebaseCommands) MoveCommitDown(commits []*models.Commit, index int) }).Run() } -func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, index int, action string) error { - todo, sha, err := self.BuildSingleActionTodo(commits, index, action) - if err != nil { - return err +func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, index int, action todo.TodoCommand) error { + baseIndex := index + 1 + if action == todo.Squash || action == todo.Fixup { + baseIndex++ } + baseShaOrRoot := getBaseShaOrRoot(commits, baseIndex) + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ - baseShaOrRoot: sha, - todoLines: todo, + baseShaOrRoot: baseShaOrRoot, overrideEditor: true, + changeTodoActions: []ChangeTodoAction{{ + sha: commits[index].Sha, + newAction: action, + }}, }).Run() } @@ -136,11 +141,17 @@ func (self *RebaseCommands) EditRebase(branchRef string) error { }).Run() } +type ChangeTodoAction struct { + sha string + newAction todo.TodoCommand +} + type PrepareInteractiveRebaseCommandOpts struct { - baseShaOrRoot string - todoLines []TodoLine - overrideEditor bool - prepend bool + baseShaOrRoot string + todoLines []TodoLine + overrideEditor bool + prepend bool + changeTodoActions []ChangeTodoAction } // PrepareInteractiveRebaseCommand returns the cmd for an interactive rebase @@ -159,6 +170,14 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract debug = "TRUE" } + changeTodoValue := strings.Join(slices.Map(opts.changeTodoActions, func(c ChangeTodoAction) string { + return fmt.Sprintf("%s:%s", c.sha, c.newAction) + }), "\n") + + if todo != "" && changeTodoValue != "" { + panic("It's not allowed to pass both todoLines and changeActionOpts") + } + rebaseMergesArg := " --rebase-merges" if self.version.IsOlderThan(2, 22, 0) { rebaseMergesArg = "" @@ -170,16 +189,19 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract cmdObj := self.cmd.New(cmdStr) gitSequenceEditor := ex - if todo == "" { - gitSequenceEditor = "true" - } else { + if todo != "" { self.os.LogCommand(fmt.Sprintf("Creating TODO file for interactive rebase: \n\n%s", todo), false) + } else if changeTodoValue != "" { + self.os.LogCommand(fmt.Sprintf("Changing TODO action: %s", changeTodoValue), false) + } else { + gitSequenceEditor = "true" } cmdObj.AddEnvVars( daemon.DaemonKindEnvKey+"="+string(daemon.InteractiveRebase), daemon.RebaseTODOEnvKey+"="+todo, daemon.PrependLinesEnvKey+"="+prependLines, + daemon.ChangeTodoActionEnvKey+"="+changeTodoValue, "DEBUG="+debug, "LANG=en_US.UTF-8", // Force using EN as language "LC_ALL=en_US.UTF-8", // Force using EN as language @@ -193,33 +215,6 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract return cmdObj } -// produces TodoLines where every commit is picked (or dropped for merge commits) except for the commit at the given index, which -// will have the given action applied to it. -func (self *RebaseCommands) BuildSingleActionTodo(commits []*models.Commit, actionIndex int, action string) ([]TodoLine, string, error) { - baseIndex := actionIndex + 1 - - if action == "squash" || action == "fixup" { - baseIndex++ - } - - todoLines := self.BuildTodoLines(commits[0:baseIndex], func(commit *models.Commit, i int) string { - if i == actionIndex { - return action - } else if commit.IsMerge() { - // your typical interactive rebase will actually drop merge commits by default. Damn git CLI, you scary! - // doing this means we don't need to worry about rebasing over merges which always causes problems. - // you typically shouldn't be doing rebases that pass over merge commits anyway. - return "drop" - } else { - return "pick" - } - }) - - baseShaOrRoot := getBaseShaOrRoot(commits, baseIndex) - - return todoLines, baseShaOrRoot, nil -} - // AmendTo amends the given commit with whatever files are staged func (self *RebaseCommands) AmendTo(commit *models.Commit) error { if err := self.commit.CreateFixupCommit(commit.Sha); err != nil { @@ -278,15 +273,13 @@ func (self *RebaseCommands) BeginInteractiveRebaseForCommit(commits []*models.Co return errors.New(self.Tr.DisabledForGPG) } - todo, sha, err := self.BuildSingleActionTodo(commits, commitIndex, "edit") - if err != nil { - return err - } - return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ - baseShaOrRoot: sha, - todoLines: todo, + baseShaOrRoot: getBaseShaOrRoot(commits, commitIndex+1), overrideEditor: true, + changeTodoActions: []ChangeTodoAction{{ + sha: commits[commitIndex].Sha, + newAction: todo.Edit, + }}, }).Run() } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 1d05385c9..a0adc8e5f 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -169,7 +169,7 @@ func (self *LocalCommitsController) squashDown(commit *models.Commit) error { HandleConfirm: func() error { return self.c.WithWaitingStatus(self.c.Tr.SquashingStatus, func() error { self.c.LogAction(self.c.Tr.Actions.SquashCommitDown) - return self.interactiveRebase("squash") + return self.interactiveRebase(todo.Squash) }) }, }) @@ -194,7 +194,7 @@ func (self *LocalCommitsController) fixup(commit *models.Commit) error { HandleConfirm: func() error { return self.c.WithWaitingStatus(self.c.Tr.FixingStatus, func() error { self.c.LogAction(self.c.Tr.Actions.FixupCommit) - return self.interactiveRebase("fixup") + return self.interactiveRebase(todo.Fixup) }) }, }) @@ -284,7 +284,7 @@ func (self *LocalCommitsController) drop(commit *models.Commit) error { HandleConfirm: func() error { return self.c.WithWaitingStatus(self.c.Tr.DeletingStatus, func() error { self.c.LogAction(self.c.Tr.Actions.DropCommit) - return self.interactiveRebase("drop") + return self.interactiveRebase(todo.Drop) }) }, }) @@ -320,7 +320,7 @@ func (self *LocalCommitsController) pick(commit *models.Commit) error { return self.pullFiles() } -func (self *LocalCommitsController) interactiveRebase(action string) error { +func (self *LocalCommitsController) interactiveRebase(action todo.TodoCommand) error { err := self.git.Rebase.InteractiveRebase(self.model.Commits, self.context().GetSelectedLineIdx(), action) return self.helpers.MergeAndRebase.CheckMergeOrRebase(err) } From 3791f0b2fa1a633b67752adb800895065abb17f2 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 6 Apr 2023 08:15:15 +0200 Subject: [PATCH 07/23] Implement "move patch to selected commit" in terms of daemon --- pkg/commands/git_commands/patch.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index 8c956529c..a09f9bce6 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -3,6 +3,7 @@ package git_commands import ( "fmt" + "github.com/fsmiamoto/git-todo-parser/todo" "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/patch" @@ -103,18 +104,13 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s baseIndex := sourceCommitIdx + 1 - todoLines := self.rebase.BuildTodoLines(commits[0:baseIndex], func(commit *models.Commit, i int) string { - if i == sourceCommitIdx || i == destinationCommitIdx { - return "edit" - } else { - return "pick" - } - }) - err := self.rebase.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: commits[baseIndex].Sha, - todoLines: todoLines, overrideEditor: true, + changeTodoActions: []ChangeTodoAction{ + {sha: commits[sourceCommitIdx].Sha, newAction: todo.Edit}, + {sha: commits[destinationCommitIdx].Sha, newAction: todo.Edit}, + }, }).Run() if err != nil { return err From dad7a70bf8c58d63a57a7846c91d2fca82160e4d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 6 Apr 2023 09:53:10 +0200 Subject: [PATCH 08/23] Implement moving commits up/down in terms of daemon --- pkg/app/daemon/daemon.go | 18 ++++++++++ pkg/commands/git_commands/rebase.go | 35 +++++++++++++++---- .../controllers/local_commits_controller.go | 2 +- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index c733c3bc1..0bd457099 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -2,6 +2,7 @@ package daemon import ( "fmt" + "io" "log" "os" "path/filepath" @@ -46,13 +47,24 @@ const ( // // If this is used, the value of RebaseTODOEnvKey must be empty. ChangeTodoActionEnvKey string = "LAZYGIT_CHANGE_TODO_ACTION" + + // Can be set to the sha of a "pick" todo that will be moved down by one. + MoveTodoDownEnvKey string = "LAZYGIT_MOVE_COMMIT_DOWN" + + // Can be set to the sha of a "pick" todo that will be moved up by one. + MoveTodoUpEnvKey string = "LAZYGIT_MOVE_COMMIT_UP" ) type Daemon interface { Run() error } +var logFile io.StringWriter + func Handle(common *common.Common) { + logFile, _ = os.Create("/tmp/daemon-log.txt") + _, _ = logFile.WriteString("Hello Daemon\n") + d := getDaemon(common) if d == nil { return @@ -108,6 +120,12 @@ func (self *rebaseDaemon) Run() error { func (self *rebaseDaemon) writeTodoFile(path string) error { if changeTodoActionEnvValue := os.Getenv(ChangeTodoActionEnvKey); changeTodoActionEnvValue != "" { return self.changeTodoAction(path, changeTodoActionEnvValue) + } else if shaToMoveDown := os.Getenv(MoveTodoDownEnvKey); shaToMoveDown != "" { + _, _ = logFile.WriteString(fmt.Sprintf("Moving commit down: %s\n", shaToMoveDown)) + return utils.MoveTodoDown(path, shaToMoveDown, todo.Pick) + } else if shaToMoveUp := os.Getenv(MoveTodoUpEnvKey); shaToMoveUp != "" { + _, _ = logFile.WriteString(fmt.Sprintf("Moving commit up: %s\n", shaToMoveUp)) + return utils.MoveTodoUp(path, shaToMoveUp, todo.Pick) } else { todoContent := []byte(os.Getenv(RebaseTODOEnvKey)) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 338917626..13c978a92 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -99,18 +99,22 @@ func (self *RebaseCommands) GenericAmend(commits []*models.Commit, index int, f } func (self *RebaseCommands) MoveCommitDown(commits []*models.Commit, index int) error { - // not appending to original slice so that we don't mutate it - orderedCommits := append([]*models.Commit{}, commits[0:index]...) - orderedCommits = append(orderedCommits, commits[index+1], commits[index]) - - todoLines := self.BuildTodoLinesSingleAction(orderedCommits, "pick") - baseShaOrRoot := getBaseShaOrRoot(commits, index+2) return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: baseShaOrRoot, - todoLines: todoLines, overrideEditor: true, + moveDown: commits[index].Sha, + }).Run() +} + +func (self *RebaseCommands) MoveCommitUp(commits []*models.Commit, index int) error { + baseShaOrRoot := getBaseShaOrRoot(commits, index+1) + + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ + baseShaOrRoot: baseShaOrRoot, + overrideEditor: true, + moveUp: commits[index].Sha, }).Run() } @@ -152,6 +156,8 @@ type PrepareInteractiveRebaseCommandOpts struct { overrideEditor bool prepend bool changeTodoActions []ChangeTodoAction + moveDown string + moveUp string } // PrepareInteractiveRebaseCommand returns the cmd for an interactive rebase @@ -174,6 +180,15 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract return fmt.Sprintf("%s:%s", c.sha, c.newAction) }), "\n") + moveDownValue := "" + if opts.moveDown != "" { + moveDownValue = opts.moveDown + } + moveUpValue := "" + if opts.moveUp != "" { + moveUpValue = opts.moveUp + } + if todo != "" && changeTodoValue != "" { panic("It's not allowed to pass both todoLines and changeActionOpts") } @@ -193,6 +208,10 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract self.os.LogCommand(fmt.Sprintf("Creating TODO file for interactive rebase: \n\n%s", todo), false) } else if changeTodoValue != "" { self.os.LogCommand(fmt.Sprintf("Changing TODO action: %s", changeTodoValue), false) + } else if moveDownValue != "" { + self.os.LogCommand(fmt.Sprintf("Moving TODO down: %s", moveDownValue), false) + } else if moveUpValue != "" { + self.os.LogCommand(fmt.Sprintf("Moving TODO up: %s", moveUpValue), false) } else { gitSequenceEditor = "true" } @@ -202,6 +221,8 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract daemon.RebaseTODOEnvKey+"="+todo, daemon.PrependLinesEnvKey+"="+prependLines, daemon.ChangeTodoActionEnvKey+"="+changeTodoValue, + daemon.MoveTodoDownEnvKey+"="+moveDownValue, + daemon.MoveTodoUpEnvKey+"="+moveUpValue, "DEBUG="+debug, "LANG=en_US.UTF-8", // Force using EN as language "LC_ALL=en_US.UTF-8", // Force using EN as language diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index a0adc8e5f..082d24fce 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -440,7 +440,7 @@ func (self *LocalCommitsController) moveUp(commit *models.Commit) error { return self.c.WithWaitingStatus(self.c.Tr.MovingStatus, func() error { self.c.LogAction(self.c.Tr.Actions.MoveCommitUp) - err := self.git.Rebase.MoveCommitDown(self.model.Commits, index-1) + err := self.git.Rebase.MoveCommitUp(self.model.Commits, index) if err == nil { self.context().MoveSelectedLine(-1) } From a8586ba57e15c1d069448044a1e7d5859547ea7d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 6 Apr 2023 11:13:42 +0200 Subject: [PATCH 09/23] Refactor: simplify PrepareInteractiveRebaseCommand API Instead of passing a bunch of different options in PrepareInteractiveRebaseCommandOpts, where it was unclear how they interact if several are set, have only a single field "instruction" which can be set to one of various different instructions. The functionality of replacing the entire todo file with our own is no longer available; it is only possible to prepend todos to the existing file. Also, instead of using different env vars for the various rebase operations that we want to tell the daemon to do, use a single one that contains a json-encoded struct with all available instructions. This makes the protocol much clearer, and makes it easier to extend in the future. --- pkg/app/daemon/daemon.go | 108 ++++++++------------ pkg/commands/git_commands/patch.go | 9 +- pkg/commands/git_commands/rebase.go | 149 ++++++++++++++++------------ pkg/utils/rebase_todo.go | 10 ++ 4 files changed, 142 insertions(+), 134 deletions(-) diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index 0bd457099..7053a8032 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -1,8 +1,7 @@ package daemon import ( - "fmt" - "io" + "encoding/json" "log" "os" "path/filepath" @@ -32,39 +31,39 @@ const ( const ( DaemonKindEnvKey string = "LAZYGIT_DAEMON_KIND" - RebaseTODOEnvKey string = "LAZYGIT_REBASE_TODO" - // The `PrependLinesEnvKey` env variable is set to `true` to tell our daemon - // to prepend the content of `RebaseTODOEnvKey` to the default `git-rebase-todo` - // file instead of using it as a replacement. - PrependLinesEnvKey string = "LAZYGIT_PREPEND_LINES" + // Contains a json-encoded instance of the InteractiveRebaseInstructions struct + InteractiveRebaseInstructionsEnvKey string = "LAZYGIT_DAEMON_INSTRUCTIONS" +) - // If this is set, it tells lazygit to read the original todo file, and - // change the action for one or more entries in it. The value of the variable - // will have one or more lines of the form "Sha1:newAction", e.g. - // a02b54e1b7e7e8dd8bc1958c11ef4ee4df459ea4:edit +// Exactly one of the fields in this struct is expected to be non-empty +type InteractiveRebaseInstructions struct { + // If this is non-empty, this string is prepended to the git-rebase-todo + // file. The string is expected to have newlines at the end of each line. + LinesToPrependToRebaseTODO string + + // If this is non-empty, it tells lazygit to read the original todo file, and + // change the action for one or more entries in it. // The existing action of the todo to be changed is expected to be "pick". - // - // If this is used, the value of RebaseTODOEnvKey must be empty. - ChangeTodoActionEnvKey string = "LAZYGIT_CHANGE_TODO_ACTION" + ChangeTodoActions []ChangeTodoAction // Can be set to the sha of a "pick" todo that will be moved down by one. - MoveTodoDownEnvKey string = "LAZYGIT_MOVE_COMMIT_DOWN" + ShaToMoveDown string // Can be set to the sha of a "pick" todo that will be moved up by one. - MoveTodoUpEnvKey string = "LAZYGIT_MOVE_COMMIT_UP" -) + ShaToMoveUp string +} + +type ChangeTodoAction struct { + Sha string + NewAction todo.TodoCommand +} type Daemon interface { Run() error } -var logFile io.StringWriter - func Handle(common *common.Common) { - logFile, _ = os.Create("/tmp/daemon-log.txt") - _, _ = logFile.WriteString("Hello Daemon\n") - d := getDaemon(common) if d == nil { return @@ -118,43 +117,30 @@ func (self *rebaseDaemon) Run() error { } func (self *rebaseDaemon) writeTodoFile(path string) error { - if changeTodoActionEnvValue := os.Getenv(ChangeTodoActionEnvKey); changeTodoActionEnvValue != "" { - return self.changeTodoAction(path, changeTodoActionEnvValue) - } else if shaToMoveDown := os.Getenv(MoveTodoDownEnvKey); shaToMoveDown != "" { - _, _ = logFile.WriteString(fmt.Sprintf("Moving commit down: %s\n", shaToMoveDown)) - return utils.MoveTodoDown(path, shaToMoveDown, todo.Pick) - } else if shaToMoveUp := os.Getenv(MoveTodoUpEnvKey); shaToMoveUp != "" { - _, _ = logFile.WriteString(fmt.Sprintf("Moving commit up: %s\n", shaToMoveUp)) - return utils.MoveTodoUp(path, shaToMoveUp, todo.Pick) - } else { - todoContent := []byte(os.Getenv(RebaseTODOEnvKey)) - - prependLines := os.Getenv(PrependLinesEnvKey) != "" - if prependLines { - existingContent, err := os.ReadFile(path) - if err != nil { - return err - } - - todoContent = append(todoContent, existingContent...) - } - - return os.WriteFile(path, todoContent, 0o644) + jsonData := os.Getenv(InteractiveRebaseInstructionsEnvKey) + instructions := InteractiveRebaseInstructions{} + err := json.Unmarshal([]byte(jsonData), &instructions) + if err != nil { + return err } + + if instructions.LinesToPrependToRebaseTODO != "" { + return utils.PrependStrToTodoFile(path, []byte(instructions.LinesToPrependToRebaseTODO)) + } else if len(instructions.ChangeTodoActions) != 0 { + return self.changeTodoAction(path, instructions.ChangeTodoActions) + } else if instructions.ShaToMoveDown != "" { + return utils.MoveTodoDown(path, instructions.ShaToMoveDown, todo.Pick) + } else if instructions.ShaToMoveUp != "" { + return utils.MoveTodoUp(path, instructions.ShaToMoveUp, todo.Pick) + } + + self.c.Log.Error("No instructions were given to daemon") + return nil } -func (self *rebaseDaemon) changeTodoAction(path string, changeTodoActionEnvValue string) error { - lines := strings.Split(changeTodoActionEnvValue, "\n") - for _, line := range lines { - fields := strings.Split(line, ":") - if len(fields) != 2 { - return fmt.Errorf("Unexpected value for %s: %s", ChangeTodoActionEnvKey, changeTodoActionEnvValue) - } - sha, newAction := fields[0], self.actionFromString(fields[1]) - if int(newAction) == 0 { - return fmt.Errorf("Unknown action in %s", changeTodoActionEnvValue) - } - if err := utils.EditRebaseTodo(path, sha, todo.Pick, newAction); err != nil { +func (self *rebaseDaemon) changeTodoAction(path string, changeTodoActions []ChangeTodoAction) error { + for _, c := range changeTodoActions { + if err := utils.EditRebaseTodo(path, c.Sha, todo.Pick, c.NewAction); err != nil { return err } } @@ -162,16 +148,6 @@ func (self *rebaseDaemon) changeTodoAction(path string, changeTodoActionEnvValue return nil } -func (self *rebaseDaemon) actionFromString(actionString string) todo.TodoCommand { - for t := todo.Pick; t < todo.Comment; t++ { - if t.String() == actionString { - return t - } - } - - return 0 -} - func gitDir() string { dir := env.GetGitDirEnv() if dir == "" { diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index a09f9bce6..0f8bfed9d 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -5,6 +5,7 @@ import ( "github.com/fsmiamoto/git-todo-parser/todo" "github.com/go-errors/errors" + "github.com/jesseduffield/lazygit/pkg/app/daemon" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/patch" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" @@ -107,9 +108,11 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s err := self.rebase.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: commits[baseIndex].Sha, overrideEditor: true, - changeTodoActions: []ChangeTodoAction{ - {sha: commits[sourceCommitIdx].Sha, newAction: todo.Edit}, - {sha: commits[destinationCommitIdx].Sha, newAction: todo.Edit}, + instruction: ChangeTodoActionsInstruction{ + actions: []daemon.ChangeTodoAction{ + {Sha: commits[sourceCommitIdx].Sha, NewAction: todo.Edit}, + {Sha: commits[destinationCommitIdx].Sha, NewAction: todo.Edit}, + }, }, }).Run() if err != nil { diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 13c978a92..ead8da27a 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -1,6 +1,7 @@ package git_commands import ( + "encoding/json" "fmt" "path/filepath" "strings" @@ -57,11 +58,11 @@ func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, me func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int) (oscommands.ICmdObj, error) { return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: getBaseShaOrRoot(commits, index+1), - changeTodoActions: []ChangeTodoAction{ - { - sha: commits[index].Sha, - newAction: todo.Reword, - }, + instruction: ChangeTodoActionsInstruction{ + actions: []daemon.ChangeTodoAction{{ + Sha: commits[index].Sha, + NewAction: todo.Reword, + }}, }, }), nil } @@ -103,8 +104,8 @@ func (self *RebaseCommands) MoveCommitDown(commits []*models.Commit, index int) return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: baseShaOrRoot, + instruction: MoveDownInstruction{sha: commits[index].Sha}, overrideEditor: true, - moveDown: commits[index].Sha, }).Run() } @@ -113,8 +114,8 @@ func (self *RebaseCommands) MoveCommitUp(commits []*models.Commit, index int) er return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: baseShaOrRoot, + instruction: MoveUpInstruction{sha: commits[index].Sha}, overrideEditor: true, - moveUp: commits[index].Sha, }).Run() } @@ -127,12 +128,14 @@ func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, index in baseShaOrRoot := getBaseShaOrRoot(commits, baseIndex) return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ - baseShaOrRoot: baseShaOrRoot, + baseShaOrRoot: baseShaOrRoot, + instruction: ChangeTodoActionsInstruction{ + actions: []daemon.ChangeTodoAction{{ + Sha: commits[index].Sha, + NewAction: action, + }}, + }, overrideEditor: true, - changeTodoActions: []ChangeTodoAction{{ - sha: commits[index].Sha, - newAction: action, - }}, }).Run() } @@ -140,59 +143,72 @@ func (self *RebaseCommands) EditRebase(branchRef string) error { commands := []TodoLine{{Action: "break"}} return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: branchRef, - todoLines: commands, - prepend: true, + instruction: PrependLinesInstruction{todoLines: commands}, }).Run() } -type ChangeTodoAction struct { - sha string - newAction todo.TodoCommand +type InteractiveRebaseInstruction interface { + // Add our data to the instructions struct, and return a log string + serialize(instructions *daemon.InteractiveRebaseInstructions) string +} + +type PrependLinesInstruction struct { + todoLines []TodoLine +} + +func (self PrependLinesInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { + todoStr := TodoLinesToString(self.todoLines) + instructions.LinesToPrependToRebaseTODO = todoStr + return fmt.Sprintf("Creating TODO file for interactive rebase: \n\n%s", todoStr) +} + +type ChangeTodoActionsInstruction struct { + actions []daemon.ChangeTodoAction +} + +func (self ChangeTodoActionsInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { + instructions.ChangeTodoActions = self.actions + changeTodoStr := strings.Join(slices.Map(self.actions, func(c daemon.ChangeTodoAction) string { + return fmt.Sprintf("%s:%s", c.Sha, c.NewAction) + }), "\n") + return fmt.Sprintf("Changing TODO actions: %s", changeTodoStr) +} + +type MoveDownInstruction struct { + sha string +} + +func (self MoveDownInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { + instructions.ShaToMoveDown = self.sha + return fmt.Sprintf("Moving TODO down: %s", self.sha) +} + +type MoveUpInstruction struct { + sha string +} + +func (self MoveUpInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { + instructions.ShaToMoveUp = self.sha + return fmt.Sprintf("Moving TODO up: %s", self.sha) } type PrepareInteractiveRebaseCommandOpts struct { - baseShaOrRoot string - todoLines []TodoLine - overrideEditor bool - prepend bool - changeTodoActions []ChangeTodoAction - moveDown string - moveUp string + baseShaOrRoot string + instruction InteractiveRebaseInstruction + overrideEditor bool } // PrepareInteractiveRebaseCommand returns the cmd for an interactive rebase // we tell git to run lazygit to edit the todo list, and we pass the client // lazygit a todo string to write to the todo file func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteractiveRebaseCommandOpts) oscommands.ICmdObj { - todo := self.buildTodo(opts.todoLines) ex := oscommands.GetLazygitPath() - prependLines := "" - if opts.prepend { - prependLines = "TRUE" - } debug := "FALSE" if self.Debug { debug = "TRUE" } - changeTodoValue := strings.Join(slices.Map(opts.changeTodoActions, func(c ChangeTodoAction) string { - return fmt.Sprintf("%s:%s", c.sha, c.newAction) - }), "\n") - - moveDownValue := "" - if opts.moveDown != "" { - moveDownValue = opts.moveDown - } - moveUpValue := "" - if opts.moveUp != "" { - moveUpValue = opts.moveUp - } - - if todo != "" && changeTodoValue != "" { - panic("It's not allowed to pass both todoLines and changeActionOpts") - } - rebaseMergesArg := " --rebase-merges" if self.version.IsOlderThan(2, 22, 0) { rebaseMergesArg = "" @@ -204,25 +220,24 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract cmdObj := self.cmd.New(cmdStr) gitSequenceEditor := ex - if todo != "" { - self.os.LogCommand(fmt.Sprintf("Creating TODO file for interactive rebase: \n\n%s", todo), false) - } else if changeTodoValue != "" { - self.os.LogCommand(fmt.Sprintf("Changing TODO action: %s", changeTodoValue), false) - } else if moveDownValue != "" { - self.os.LogCommand(fmt.Sprintf("Moving TODO down: %s", moveDownValue), false) - } else if moveUpValue != "" { - self.os.LogCommand(fmt.Sprintf("Moving TODO up: %s", moveUpValue), false) + + if opts.instruction != nil { + instructions := daemon.InteractiveRebaseInstructions{} + logStr := opts.instruction.serialize(&instructions) + jsonData, err := json.Marshal(instructions) + if err == nil { + envVar := fmt.Sprintf("%s=%s", daemon.InteractiveRebaseInstructionsEnvKey, jsonData) + cmdObj.AddEnvVars(envVar) + self.os.LogCommand(logStr, false) + } else { + self.Log.Error(err) + } } else { gitSequenceEditor = "true" } cmdObj.AddEnvVars( daemon.DaemonKindEnvKey+"="+string(daemon.InteractiveRebase), - daemon.RebaseTODOEnvKey+"="+todo, - daemon.PrependLinesEnvKey+"="+prependLines, - daemon.ChangeTodoActionEnvKey+"="+changeTodoValue, - daemon.MoveTodoDownEnvKey+"="+moveDownValue, - daemon.MoveTodoUpEnvKey+"="+moveUpValue, "DEBUG="+debug, "LANG=en_US.UTF-8", // Force using EN as language "LC_ALL=en_US.UTF-8", // Force using EN as language @@ -297,10 +312,12 @@ func (self *RebaseCommands) BeginInteractiveRebaseForCommit(commits []*models.Co return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: getBaseShaOrRoot(commits, commitIndex+1), overrideEditor: true, - changeTodoActions: []ChangeTodoAction{{ - sha: commits[commitIndex].Sha, - newAction: todo.Edit, - }}, + instruction: ChangeTodoActionsInstruction{ + actions: []daemon.ChangeTodoAction{{ + Sha: commits[commitIndex].Sha, + NewAction: todo.Edit, + }}, + }, }).Run() } @@ -393,11 +410,13 @@ func (self *RebaseCommands) CherryPickCommits(commits []*models.Commit) error { return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: "HEAD", - todoLines: todoLines, + instruction: PrependLinesInstruction{ + todoLines: todoLines, + }, }).Run() } -func (self *RebaseCommands) buildTodo(todoLines []TodoLine) string { +func TodoLinesToString(todoLines []TodoLine) string { lines := slices.Map(todoLines, func(todoLine TodoLine) string { return todoLine.ToString() }) diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index a61d4c36c..f34e11774 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -63,6 +63,16 @@ func WriteRebaseTodoFile(fileName string, todos []todo.Todo) error { return err } +func PrependStrToTodoFile(filePath string, linesToPrepend []byte) error { + existingContent, err := os.ReadFile(filePath) + if err != nil { + return err + } + + linesToPrepend = append(linesToPrepend, existingContent...) + return os.WriteFile(filePath, linesToPrepend, 0o644) +} + func MoveTodoDown(fileName string, sha string, action todo.TodoCommand) error { todos, err := ReadRebaseTodoFile(fileName) if err != nil { From 185bbf0c75f9ea0d645fb0df6893169f7ae07340 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sun, 16 Apr 2023 15:43:54 +1000 Subject: [PATCH 10/23] Refactor to tighten interface to lazygit daemon --- pkg/app/daemon/daemon.go | 314 +++++++++++++++-------- pkg/app/daemon/rebase.go | 64 +++++ pkg/commands/git_commands/patch.go | 13 +- pkg/commands/git_commands/rebase.go | 160 ++++-------- pkg/commands/git_commands/rebase_test.go | 3 +- 5 files changed, 331 insertions(+), 223 deletions(-) create mode 100644 pkg/app/daemon/rebase.go diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index 7053a8032..d8f32e608 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -2,15 +2,16 @@ package daemon import ( "encoding/json" + "fmt" "log" "os" - "path/filepath" - "strings" + "strconv" "github.com/fsmiamoto/git-todo-parser/todo" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/common" - "github.com/jesseduffield/lazygit/pkg/env" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) // Sometimes lazygit will be invoked in daemon mode from a parent lazygit process. @@ -18,58 +19,56 @@ import ( // For example, if we want to ensure that a git command doesn't hang due to // waiting for an editor to save a commit message, we can tell git to invoke lazygit // as the editor via 'GIT_EDITOR=lazygit', and use the env var -// 'LAZYGIT_DAEMON_KIND=EXIT_IMMEDIATELY' to specify that we want to run lazygit -// as a daemon which simply exits immediately. Any additional arguments we want -// to pass to a daemon can be done via other env vars. +// 'LAZYGIT_DAEMON_KIND=1' (exit immediately) to specify that we want to run lazygit +// as a daemon which simply exits immediately. +// +// 'Daemon' is not the best name for this, because it's not a persistent background +// process, but it's close enough. -type DaemonKind string +type DaemonKind int const ( - InteractiveRebase DaemonKind = "INTERACTIVE_REBASE" - ExitImmediately DaemonKind = "EXIT_IMMEDIATELY" + // for when we fail to parse the daemon kind + DaemonKindUnknown DaemonKind = iota + + DaemonKindExitImmediately + DaemonKindCherryPick + DaemonKindMoveTodoUp + DaemonKindMoveTodoDown + DaemonKindInsertBreak + DaemonKindChangeTodoActions ) const ( DaemonKindEnvKey string = "LAZYGIT_DAEMON_KIND" - // Contains a json-encoded instance of the InteractiveRebaseInstructions struct - InteractiveRebaseInstructionsEnvKey string = "LAZYGIT_DAEMON_INSTRUCTIONS" + // Contains json-encoded arguments to the daemon + DaemonInstructionEnvKey string = "LAZYGIT_DAEMON_INSTRUCTION" ) -// Exactly one of the fields in this struct is expected to be non-empty -type InteractiveRebaseInstructions struct { - // If this is non-empty, this string is prepended to the git-rebase-todo - // file. The string is expected to have newlines at the end of each line. - LinesToPrependToRebaseTODO string +func getInstruction() Instruction { + jsonData := os.Getenv(DaemonInstructionEnvKey) - // If this is non-empty, it tells lazygit to read the original todo file, and - // change the action for one or more entries in it. - // The existing action of the todo to be changed is expected to be "pick". - ChangeTodoActions []ChangeTodoAction + mapping := map[DaemonKind]func(string) Instruction{ + DaemonKindExitImmediately: deserializeInstruction[*ExitImmediatelyInstruction], + DaemonKindCherryPick: deserializeInstruction[*CherryPickCommitsInstruction], + DaemonKindChangeTodoActions: deserializeInstruction[*ChangeTodoActionsInstruction], + DaemonKindMoveTodoUp: deserializeInstruction[*MoveTodoUpInstruction], + DaemonKindMoveTodoDown: deserializeInstruction[*MoveTodoDownInstruction], + DaemonKindInsertBreak: deserializeInstruction[*InsertBreakInstruction], + } - // Can be set to the sha of a "pick" todo that will be moved down by one. - ShaToMoveDown string - - // Can be set to the sha of a "pick" todo that will be moved up by one. - ShaToMoveUp string -} - -type ChangeTodoAction struct { - Sha string - NewAction todo.TodoCommand -} - -type Daemon interface { - Run() error + return mapping[getDaemonKind()](jsonData) } func Handle(common *common.Common) { - d := getDaemon(common) - if d == nil { + if !InDaemonMode() { return } - if err := d.Run(); err != nil { + instruction := getInstruction() + + if err := instruction.run(common); err != nil { log.Fatal(err) } @@ -77,89 +76,200 @@ func Handle(common *common.Common) { } func InDaemonMode() bool { - return getDaemonKind() != "" -} - -func getDaemon(common *common.Common) Daemon { - switch getDaemonKind() { - case InteractiveRebase: - return &rebaseDaemon{c: common} - case ExitImmediately: - return &exitImmediatelyDaemon{c: common} - } - - return nil + return getDaemonKind() != DaemonKindUnknown } func getDaemonKind() DaemonKind { - return DaemonKind(os.Getenv(DaemonKindEnvKey)) -} - -type rebaseDaemon struct { - c *common.Common -} - -func (self *rebaseDaemon) Run() error { - self.c.Log.Info("Lazygit invoked as interactive rebase demon") - self.c.Log.Info("args: ", os.Args) - path := os.Args[1] - - if strings.HasSuffix(path, "git-rebase-todo") { - return self.writeTodoFile(path) - } else if strings.HasSuffix(path, filepath.Join(gitDir(), "COMMIT_EDITMSG")) { // TODO: test - // if we are rebasing and squashing, we'll see a COMMIT_EDITMSG - // but in this case we don't need to edit it, so we'll just return - } else { - self.c.Log.Info("Lazygit demon did not match on any use cases") - } - - return nil -} - -func (self *rebaseDaemon) writeTodoFile(path string) error { - jsonData := os.Getenv(InteractiveRebaseInstructionsEnvKey) - instructions := InteractiveRebaseInstructions{} - err := json.Unmarshal([]byte(jsonData), &instructions) + intValue, err := strconv.Atoi(os.Getenv(DaemonKindEnvKey)) if err != nil { - return err + return DaemonKindUnknown } - if instructions.LinesToPrependToRebaseTODO != "" { - return utils.PrependStrToTodoFile(path, []byte(instructions.LinesToPrependToRebaseTODO)) - } else if len(instructions.ChangeTodoActions) != 0 { - return self.changeTodoAction(path, instructions.ChangeTodoActions) - } else if instructions.ShaToMoveDown != "" { - return utils.MoveTodoDown(path, instructions.ShaToMoveDown, todo.Pick) - } else if instructions.ShaToMoveUp != "" { - return utils.MoveTodoUp(path, instructions.ShaToMoveUp, todo.Pick) + return DaemonKind(intValue) +} + +// An Instruction is a command to be run by lazygit in daemon mode. +// It is serialized to json and passed to lazygit via environment variables +type Instruction interface { + Kind() DaemonKind + SerializedInstructions() string + + // runs the instruction + run(common *common.Common) error +} + +func serializeInstruction[T any](instruction T) string { + jsonData, err := json.Marshal(instruction) + if err != nil { + // this should never happen + panic(err) } - self.c.Log.Error("No instructions were given to daemon") + return string(jsonData) +} + +func deserializeInstruction[T Instruction](jsonData string) Instruction { + var instruction T + err := json.Unmarshal([]byte(jsonData), &instruction) + if err != nil { + panic(err) + } + + return instruction +} + +func ToEnvVars(instruction Instruction) []string { + return []string{ + fmt.Sprintf("%s=%d", DaemonKindEnvKey, instruction.Kind()), + fmt.Sprintf("%s=%s", DaemonInstructionEnvKey, instruction.SerializedInstructions()), + } +} + +type ExitImmediatelyInstruction struct{} + +func (self *ExitImmediatelyInstruction) Kind() DaemonKind { + return DaemonKindExitImmediately +} + +func (self *ExitImmediatelyInstruction) SerializedInstructions() string { + return serializeInstruction(self) +} + +func (self *ExitImmediatelyInstruction) run(common *common.Common) error { return nil } -func (self *rebaseDaemon) changeTodoAction(path string, changeTodoActions []ChangeTodoAction) error { - for _, c := range changeTodoActions { - if err := utils.EditRebaseTodo(path, c.Sha, todo.Pick, c.NewAction); err != nil { - return err +func NewExitImmediatelyInstruction() Instruction { + return &ExitImmediatelyInstruction{} +} + +type CherryPickCommitsInstruction struct { + Todo string +} + +func NewCherryPickCommitsInstruction(commits []*models.Commit) Instruction { + todoLines := lo.Map(commits, func(commit *models.Commit, _ int) TodoLine { + return TodoLine{ + Action: "pick", + Commit: commit, } + }) + + todo := TodoLinesToString(todoLines) + + return &CherryPickCommitsInstruction{ + Todo: todo, } - - return nil } -func gitDir() string { - dir := env.GetGitDirEnv() - if dir == "" { - return ".git" +func (self *CherryPickCommitsInstruction) Kind() DaemonKind { + return DaemonKindCherryPick +} + +func (self *CherryPickCommitsInstruction) SerializedInstructions() string { + return serializeInstruction(self) +} + +func (self *CherryPickCommitsInstruction) run(common *common.Common) error { + return handleInteractiveRebase(common, func(path string) error { + return utils.PrependStrToTodoFile(path, []byte(self.Todo)) + }) +} + +type ChangeTodoActionsInstruction struct { + Changes []ChangeTodoAction +} + +func NewChangeTodoActionsInstruction(changes []ChangeTodoAction) Instruction { + return &ChangeTodoActionsInstruction{ + Changes: changes, } - return dir } -type exitImmediatelyDaemon struct { - c *common.Common +func (self *ChangeTodoActionsInstruction) Kind() DaemonKind { + return DaemonKindChangeTodoActions } -func (self *exitImmediatelyDaemon) Run() error { - return nil +func (self *ChangeTodoActionsInstruction) SerializedInstructions() string { + return serializeInstruction(self) +} + +func (self *ChangeTodoActionsInstruction) run(common *common.Common) error { + return handleInteractiveRebase(common, func(path string) error { + for _, c := range self.Changes { + if err := utils.EditRebaseTodo(path, c.Sha, todo.Pick, c.NewAction); err != nil { + return err + } + } + + return nil + }) +} + +type MoveTodoUpInstruction struct { + Sha string +} + +func NewMoveTodoUpInstruction(sha string) Instruction { + return &MoveTodoUpInstruction{ + Sha: sha, + } +} + +func (self *MoveTodoUpInstruction) Kind() DaemonKind { + return DaemonKindMoveTodoUp +} + +func (self *MoveTodoUpInstruction) SerializedInstructions() string { + return serializeInstruction(self) +} + +func (self *MoveTodoUpInstruction) run(common *common.Common) error { + return handleInteractiveRebase(common, func(path string) error { + return utils.MoveTodoUp(path, self.Sha, todo.Pick) + }) +} + +type MoveTodoDownInstruction struct { + Sha string +} + +func NewMoveTodoDownInstruction(sha string) Instruction { + return &MoveTodoDownInstruction{ + Sha: sha, + } +} + +func (self *MoveTodoDownInstruction) Kind() DaemonKind { + return DaemonKindMoveTodoDown +} + +func (self *MoveTodoDownInstruction) SerializedInstructions() string { + return serializeInstruction(self) +} + +func (self *MoveTodoDownInstruction) run(common *common.Common) error { + return handleInteractiveRebase(common, func(path string) error { + return utils.MoveTodoDown(path, self.Sha, todo.Pick) + }) +} + +type InsertBreakInstruction struct{} + +func NewInsertBreakInstruction() Instruction { + return &InsertBreakInstruction{} +} + +func (self *InsertBreakInstruction) Kind() DaemonKind { + return DaemonKindInsertBreak +} + +func (self *InsertBreakInstruction) SerializedInstructions() string { + return serializeInstruction(self) +} + +func (self *InsertBreakInstruction) run(common *common.Common) error { + return handleInteractiveRebase(common, func(path string) error { + return utils.PrependStrToTodoFile(path, []byte("break\n")) + }) } diff --git a/pkg/app/daemon/rebase.go b/pkg/app/daemon/rebase.go new file mode 100644 index 000000000..8702f0f69 --- /dev/null +++ b/pkg/app/daemon/rebase.go @@ -0,0 +1,64 @@ +package daemon + +import ( + "os" + "path/filepath" + "strings" + + "github.com/fsmiamoto/git-todo-parser/todo" + "github.com/jesseduffield/generics/slices" + "github.com/jesseduffield/lazygit/pkg/commands/models" + "github.com/jesseduffield/lazygit/pkg/common" + "github.com/jesseduffield/lazygit/pkg/env" +) + +type TodoLine struct { + Action string + Commit *models.Commit +} + +func (self *TodoLine) ToString() string { + if self.Action == "break" { + return self.Action + "\n" + } else { + return self.Action + " " + self.Commit.Sha + " " + self.Commit.Name + "\n" + } +} + +func TodoLinesToString(todoLines []TodoLine) string { + lines := slices.Map(todoLines, func(todoLine TodoLine) string { + return todoLine.ToString() + }) + + return strings.Join(slices.Reverse(lines), "") +} + +type ChangeTodoAction struct { + Sha string + NewAction todo.TodoCommand +} + +func handleInteractiveRebase(common *common.Common, f func(path string) error) error { + common.Log.Info("Lazygit invoked as interactive rebase demon") + common.Log.Info("args: ", os.Args) + path := os.Args[1] + + if strings.HasSuffix(path, "git-rebase-todo") { + return f(path) + } else if strings.HasSuffix(path, filepath.Join(gitDir(), "COMMIT_EDITMSG")) { // TODO: test + // if we are rebasing and squashing, we'll see a COMMIT_EDITMSG + // but in this case we don't need to edit it, so we'll just return + } else { + common.Log.Info("Lazygit demon did not match on any use cases") + } + + return nil +} + +func gitDir() string { + dir := env.GetGitDirEnv() + if dir == "" { + return ".git" + } + return dir +} diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index 0f8bfed9d..06e5e0f67 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -105,15 +105,16 @@ func (self *PatchCommands) MovePatchToSelectedCommit(commits []*models.Commit, s baseIndex := sourceCommitIdx + 1 + changes := []daemon.ChangeTodoAction{ + {Sha: commits[sourceCommitIdx].Sha, NewAction: todo.Edit}, + {Sha: commits[destinationCommitIdx].Sha, NewAction: todo.Edit}, + } + self.os.LogCommand(logTodoChanges(changes), false) + err := self.rebase.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: commits[baseIndex].Sha, overrideEditor: true, - instruction: ChangeTodoActionsInstruction{ - actions: []daemon.ChangeTodoAction{ - {Sha: commits[sourceCommitIdx].Sha, NewAction: todo.Edit}, - {Sha: commits[destinationCommitIdx].Sha, NewAction: todo.Edit}, - }, - }, + instruction: daemon.NewChangeTodoActionsInstruction(changes), }).Run() if err != nil { return err diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index ead8da27a..be5384773 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -1,7 +1,6 @@ package git_commands import ( - "encoding/json" "fmt" "path/filepath" "strings" @@ -13,6 +12,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) type RebaseCommands struct { @@ -56,14 +56,15 @@ func (self *RebaseCommands) RewordCommit(commits []*models.Commit, index int, me } func (self *RebaseCommands) RewordCommitInEditor(commits []*models.Commit, index int) (oscommands.ICmdObj, error) { + changes := []daemon.ChangeTodoAction{{ + Sha: commits[index].Sha, + NewAction: todo.Reword, + }} + self.os.LogCommand(logTodoChanges(changes), false) + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: getBaseShaOrRoot(commits, index+1), - instruction: ChangeTodoActionsInstruction{ - actions: []daemon.ChangeTodoAction{{ - Sha: commits[index].Sha, - NewAction: todo.Reword, - }}, - }, + instruction: daemon.NewChangeTodoActionsInstruction(changes), }), nil } @@ -102,9 +103,13 @@ func (self *RebaseCommands) GenericAmend(commits []*models.Commit, index int, f func (self *RebaseCommands) MoveCommitDown(commits []*models.Commit, index int) error { baseShaOrRoot := getBaseShaOrRoot(commits, index+2) + sha := commits[index].Sha + + self.os.LogCommand(fmt.Sprintf("Moving TODO down: %s", utils.ShortSha(sha)), false) + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: baseShaOrRoot, - instruction: MoveDownInstruction{sha: commits[index].Sha}, + instruction: daemon.NewMoveTodoDownInstruction(sha), overrideEditor: true, }).Run() } @@ -112,9 +117,13 @@ func (self *RebaseCommands) MoveCommitDown(commits []*models.Commit, index int) func (self *RebaseCommands) MoveCommitUp(commits []*models.Commit, index int) error { baseShaOrRoot := getBaseShaOrRoot(commits, index+1) + sha := commits[index].Sha + + self.os.LogCommand(fmt.Sprintf("Moving TODO up: %s", utils.ShortSha(sha)), false) + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: baseShaOrRoot, - instruction: MoveUpInstruction{sha: commits[index].Sha}, + instruction: daemon.NewMoveTodoUpInstruction(sha), overrideEditor: true, }).Run() } @@ -127,74 +136,37 @@ func (self *RebaseCommands) InteractiveRebase(commits []*models.Commit, index in baseShaOrRoot := getBaseShaOrRoot(commits, baseIndex) + changes := []daemon.ChangeTodoAction{{ + Sha: commits[index].Sha, + NewAction: action, + }} + self.os.LogCommand(logTodoChanges(changes), false) + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ - baseShaOrRoot: baseShaOrRoot, - instruction: ChangeTodoActionsInstruction{ - actions: []daemon.ChangeTodoAction{{ - Sha: commits[index].Sha, - NewAction: action, - }}, - }, + baseShaOrRoot: baseShaOrRoot, overrideEditor: true, + instruction: daemon.NewChangeTodoActionsInstruction(changes), }).Run() } func (self *RebaseCommands) EditRebase(branchRef string) error { - commands := []TodoLine{{Action: "break"}} + self.os.LogCommand(fmt.Sprintf("Beginning interactive rebase at '%s'", branchRef), false) return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: branchRef, - instruction: PrependLinesInstruction{todoLines: commands}, + instruction: daemon.NewInsertBreakInstruction(), }).Run() } -type InteractiveRebaseInstruction interface { - // Add our data to the instructions struct, and return a log string - serialize(instructions *daemon.InteractiveRebaseInstructions) string -} - -type PrependLinesInstruction struct { - todoLines []TodoLine -} - -func (self PrependLinesInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { - todoStr := TodoLinesToString(self.todoLines) - instructions.LinesToPrependToRebaseTODO = todoStr - return fmt.Sprintf("Creating TODO file for interactive rebase: \n\n%s", todoStr) -} - -type ChangeTodoActionsInstruction struct { - actions []daemon.ChangeTodoAction -} - -func (self ChangeTodoActionsInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { - instructions.ChangeTodoActions = self.actions - changeTodoStr := strings.Join(slices.Map(self.actions, func(c daemon.ChangeTodoAction) string { +func logTodoChanges(changes []daemon.ChangeTodoAction) string { + changeTodoStr := strings.Join(slices.Map(changes, func(c daemon.ChangeTodoAction) string { return fmt.Sprintf("%s:%s", c.Sha, c.NewAction) }), "\n") return fmt.Sprintf("Changing TODO actions: %s", changeTodoStr) } -type MoveDownInstruction struct { - sha string -} - -func (self MoveDownInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { - instructions.ShaToMoveDown = self.sha - return fmt.Sprintf("Moving TODO down: %s", self.sha) -} - -type MoveUpInstruction struct { - sha string -} - -func (self MoveUpInstruction) serialize(instructions *daemon.InteractiveRebaseInstructions) string { - instructions.ShaToMoveUp = self.sha - return fmt.Sprintf("Moving TODO up: %s", self.sha) -} - type PrepareInteractiveRebaseCommandOpts struct { baseShaOrRoot string - instruction InteractiveRebaseInstruction + instruction daemon.Instruction overrideEditor bool } @@ -222,22 +194,12 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract gitSequenceEditor := ex if opts.instruction != nil { - instructions := daemon.InteractiveRebaseInstructions{} - logStr := opts.instruction.serialize(&instructions) - jsonData, err := json.Marshal(instructions) - if err == nil { - envVar := fmt.Sprintf("%s=%s", daemon.InteractiveRebaseInstructionsEnvKey, jsonData) - cmdObj.AddEnvVars(envVar) - self.os.LogCommand(logStr, false) - } else { - self.Log.Error(err) - } + cmdObj.AddEnvVars(daemon.ToEnvVars(opts.instruction)...) } else { gitSequenceEditor = "true" } cmdObj.AddEnvVars( - daemon.DaemonKindEnvKey+"="+string(daemon.InteractiveRebase), "DEBUG="+debug, "LANG=en_US.UTF-8", // Force using EN as language "LC_ALL=en_US.UTF-8", // Force using EN as language @@ -309,15 +271,16 @@ func (self *RebaseCommands) BeginInteractiveRebaseForCommit(commits []*models.Co return errors.New(self.Tr.DisabledForGPG) } + changes := []daemon.ChangeTodoAction{{ + Sha: commits[commitIndex].Sha, + NewAction: todo.Edit, + }} + self.os.LogCommand(logTodoChanges(changes), false) + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: getBaseShaOrRoot(commits, commitIndex+1), overrideEditor: true, - instruction: ChangeTodoActionsInstruction{ - actions: []daemon.ChangeTodoAction{{ - Sha: commits[commitIndex].Sha, - NewAction: todo.Edit, - }}, - }, + instruction: daemon.NewChangeTodoActionsInstruction(changes), }).Run() } @@ -364,15 +327,16 @@ func (self *RebaseCommands) GenericMergeOrRebaseAction(commandType string, comma } func (self *RebaseCommands) runSkipEditorCommand(cmdObj oscommands.ICmdObj) error { + instruction := daemon.NewExitImmediatelyInstruction() lazyGitPath := oscommands.GetLazygitPath() return cmdObj. AddEnvVars( - daemon.DaemonKindEnvKey+"="+string(daemon.ExitImmediately), "GIT_EDITOR="+lazyGitPath, "GIT_SEQUENCE_EDITOR="+lazyGitPath, "EDITOR="+lazyGitPath, "VISUAL="+lazyGitPath, ). + AddEnvVars(daemon.ToEnvVars(instruction)...). Run() } @@ -406,49 +370,17 @@ func (self *RebaseCommands) DiscardOldFileChanges(commits []*models.Commit, comm // CherryPickCommits begins an interactive rebase with the given shas being cherry picked onto HEAD func (self *RebaseCommands) CherryPickCommits(commits []*models.Commit) error { - todoLines := self.BuildTodoLinesSingleAction(commits, "pick") + commitLines := lo.Map(commits, func(commit *models.Commit, _ int) string { + return fmt.Sprintf("%s %s", utils.ShortSha(commit.Sha), commit.Name) + }) + self.os.LogCommand(fmt.Sprintf("Cherry-picking commits:\n%s", strings.Join(commitLines, "\n")), false) return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ baseShaOrRoot: "HEAD", - instruction: PrependLinesInstruction{ - todoLines: todoLines, - }, + instruction: daemon.NewCherryPickCommitsInstruction(commits), }).Run() } -func TodoLinesToString(todoLines []TodoLine) string { - lines := slices.Map(todoLines, func(todoLine TodoLine) string { - return todoLine.ToString() - }) - - return strings.Join(slices.Reverse(lines), "") -} - -func (self *RebaseCommands) BuildTodoLines(commits []*models.Commit, f func(*models.Commit, int) string) []TodoLine { - return slices.MapWithIndex(commits, func(commit *models.Commit, i int) TodoLine { - return TodoLine{Action: f(commit, i), Commit: commit} - }) -} - -func (self *RebaseCommands) BuildTodoLinesSingleAction(commits []*models.Commit, action string) []TodoLine { - return self.BuildTodoLines(commits, func(commit *models.Commit, i int) string { - return action - }) -} - -type TodoLine struct { - Action string - Commit *models.Commit -} - -func (self *TodoLine) ToString() string { - if self.Action == "break" { - return self.Action + "\n" - } else { - return self.Action + " " + self.Commit.Sha + " " + self.Commit.Name + "\n" - } -} - // we can't start an interactive rebase from the first commit without passing the // '--root' arg func getBaseShaOrRoot(commits []*models.Commit, index int) string { diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index b2cd88323..1ef22ff5d 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -2,6 +2,7 @@ package git_commands import ( "regexp" + "strconv" "testing" "github.com/go-errors/errors" @@ -63,7 +64,7 @@ func TestRebaseSkipEditorCommand(t *testing.T) { `^EDITOR=.*$`, `^GIT_EDITOR=.*$`, `^GIT_SEQUENCE_EDITOR=.*$`, - "^" + daemon.DaemonKindEnvKey + "=" + string(daemon.ExitImmediately) + "$", + "^" + daemon.DaemonKindEnvKey + "=" + strconv.Itoa(int(daemon.DaemonKindExitImmediately)) + "$", } { regexStr := regexStr foundMatch := lo.ContainsBy(envVars, func(envVar string) bool { From 3fe4db9316f2bcfb168c6585acee425831f9b04b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 19 Apr 2023 08:19:17 +0200 Subject: [PATCH 11/23] Make RebaseCommands.AmendTo more robust This fixes two problems with the "amend commit with staged changes" command: 1. Amending to a fixup commit didn't work (this would create a commmit with the title "fixup! fixup! original title" and keep that at the top of the branch) 2. Unrelated fixup commits would be squashed too. The added integration test verifies that both of these problems are fixed. --- pkg/app/daemon/daemon.go | 43 +++++++++++++--- pkg/commands/git_commands/rebase.go | 16 +++++- .../controllers/local_commits_controller.go | 2 +- .../interactive_rebase/amend_fixup_commit.go | 50 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + pkg/utils/rebase_todo.go | 36 +++++++++++++ 6 files changed, 139 insertions(+), 9 deletions(-) create mode 100644 pkg/integration/tests/interactive_rebase/amend_fixup_commit.go diff --git a/pkg/app/daemon/daemon.go b/pkg/app/daemon/daemon.go index d8f32e608..adc287309 100644 --- a/pkg/app/daemon/daemon.go +++ b/pkg/app/daemon/daemon.go @@ -37,6 +37,7 @@ const ( DaemonKindMoveTodoDown DaemonKindInsertBreak DaemonKindChangeTodoActions + DaemonKindMoveFixupCommitDown ) const ( @@ -50,12 +51,13 @@ func getInstruction() Instruction { jsonData := os.Getenv(DaemonInstructionEnvKey) mapping := map[DaemonKind]func(string) Instruction{ - DaemonKindExitImmediately: deserializeInstruction[*ExitImmediatelyInstruction], - DaemonKindCherryPick: deserializeInstruction[*CherryPickCommitsInstruction], - DaemonKindChangeTodoActions: deserializeInstruction[*ChangeTodoActionsInstruction], - DaemonKindMoveTodoUp: deserializeInstruction[*MoveTodoUpInstruction], - DaemonKindMoveTodoDown: deserializeInstruction[*MoveTodoDownInstruction], - DaemonKindInsertBreak: deserializeInstruction[*InsertBreakInstruction], + DaemonKindExitImmediately: deserializeInstruction[*ExitImmediatelyInstruction], + DaemonKindCherryPick: deserializeInstruction[*CherryPickCommitsInstruction], + DaemonKindChangeTodoActions: deserializeInstruction[*ChangeTodoActionsInstruction], + DaemonKindMoveFixupCommitDown: deserializeInstruction[*MoveFixupCommitDownInstruction], + DaemonKindMoveTodoUp: deserializeInstruction[*MoveTodoUpInstruction], + DaemonKindMoveTodoDown: deserializeInstruction[*MoveTodoDownInstruction], + DaemonKindInsertBreak: deserializeInstruction[*InsertBreakInstruction], } return mapping[getDaemonKind()](jsonData) @@ -206,6 +208,35 @@ func (self *ChangeTodoActionsInstruction) run(common *common.Common) error { }) } +// Takes the sha of some commit, and the sha of a fixup commit that was created +// at the end of the branch, then moves the fixup commit down to right after the +// original commit, changing its type to "fixup" +type MoveFixupCommitDownInstruction struct { + OriginalSha string + FixupSha string +} + +func NewMoveFixupCommitDownInstruction(originalSha string, fixupSha string) Instruction { + return &MoveFixupCommitDownInstruction{ + OriginalSha: originalSha, + FixupSha: fixupSha, + } +} + +func (self *MoveFixupCommitDownInstruction) Kind() DaemonKind { + return DaemonKindMoveFixupCommitDown +} + +func (self *MoveFixupCommitDownInstruction) SerializedInstructions() string { + return serializeInstruction(self) +} + +func (self *MoveFixupCommitDownInstruction) run(common *common.Common) error { + return handleInteractiveRebase(common, func(path string) error { + return utils.MoveFixupCommitDown(path, self.OriginalSha, self.FixupSha) + }) +} + type MoveTodoUpInstruction struct { Sha string } diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index be5384773..50c599a66 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -214,12 +214,24 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract } // AmendTo amends the given commit with whatever files are staged -func (self *RebaseCommands) AmendTo(commit *models.Commit) error { +func (self *RebaseCommands) AmendTo(commits []*models.Commit, commitIndex int) error { + commit := commits[commitIndex] + if err := self.commit.CreateFixupCommit(commit.Sha); err != nil { return err } - return self.SquashAllAboveFixupCommits(commit) + // Get the sha of the commit we just created + fixupSha, err := self.cmd.New("git rev-parse --verify HEAD").RunWithOutput() + if err != nil { + return err + } + + return self.PrepareInteractiveRebaseCommand(PrepareInteractiveRebaseCommandOpts{ + baseShaOrRoot: getBaseShaOrRoot(commits, commitIndex+1), + overrideEditor: true, + instruction: daemon.NewMoveFixupCommitDownInstruction(commit.Sha, fixupSha), + }).Run() } // EditRebaseTodo sets the action for a given rebase commit in the git-rebase-todo file diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 082d24fce..285289121 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -466,7 +466,7 @@ func (self *LocalCommitsController) amendTo(commit *models.Commit) error { HandleConfirm: func() error { return self.c.WithWaitingStatus(self.c.Tr.AmendingStatus, func() error { self.c.LogAction(self.c.Tr.Actions.AmendCommit) - err := self.git.Rebase.AmendTo(commit) + err := self.git.Rebase.AmendTo(self.model.Commits, self.context().GetView().SelectedLineIdx()) return self.helpers.MergeAndRebase.CheckMergeOrRebase(err) }) }, diff --git a/pkg/integration/tests/interactive_rebase/amend_fixup_commit.go b/pkg/integration/tests/interactive_rebase/amend_fixup_commit.go new file mode 100644 index 000000000..0c39c756b --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/amend_fixup_commit.go @@ -0,0 +1,50 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var AmendFixupCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Amends a staged file to a fixup commit, and checks that other unrelated fixup commits are not auto-squashed.", + ExtraCmdArgs: "", + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + CreateNCommits(1). + CreateFileAndAdd("first-fixup-file", "").Commit("fixup! commit 01"). + CreateNCommitsStartingAt(2, 2). + CreateFileAndAdd("unrelated-fixup-file", "fixup 03").Commit("fixup! commit 03"). + CreateFileAndAdd("fixup-file", "fixup 01") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("fixup! commit 03"), + Contains("commit 03"), + Contains("commit 02"), + Contains("fixup! commit 01"), + Contains("commit 01"), + ). + NavigateToLine(Contains("fixup! commit 01")). + Press(keys.Commits.AmendToCommit). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Amend Commit")). + Content(Contains("Are you sure you want to amend this commit with your staged files?")). + Confirm() + }). + Lines( + Contains("fixup! commit 03"), + Contains("commit 03"), + Contains("commit 02"), + Contains("fixup! commit 01").IsSelected(), + Contains("commit 01"), + ) + + t.Views().Main(). + Content(Contains("fixup 01")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index d4da79732..8b9ad8ff5 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -85,6 +85,7 @@ var tests = []*components.IntegrationTest{ filter_by_path.TypeFile, interactive_rebase.AdvancedInteractiveRebase, interactive_rebase.AmendFirstCommit, + interactive_rebase.AmendFixupCommit, interactive_rebase.AmendHeadCommitDuringRebase, interactive_rebase.AmendMerge, interactive_rebase.AmendNonHeadCommitDuringRebase, diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index f34e11774..2b8248224 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -134,6 +134,42 @@ func moveTodoUp(todos []todo.Todo, sha string, action todo.TodoCommand) ([]todo. return rearrangedTodos, nil } +func MoveFixupCommitDown(fileName string, originalSha string, fixupSha string) error { + todos, err := ReadRebaseTodoFile(fileName) + if err != nil { + return err + } + + newTodos := []todo.Todo{} + numOriginalShaLinesFound := 0 + numFixupShaLinesFound := 0 + + for _, t := range todos { + if t.Command == todo.Pick { + if equalShas(t.Commit, originalSha) { + numOriginalShaLinesFound += 1 + // append the original commit, and then the fixup + newTodos = append(newTodos, t) + newTodos = append(newTodos, todo.Todo{Command: todo.Fixup, Commit: fixupSha}) + continue + } else if equalShas(t.Commit, fixupSha) { + numFixupShaLinesFound += 1 + // skip the fixup here + continue + } + } + + newTodos = append(newTodos, t) + } + + if numOriginalShaLinesFound != 1 || numFixupShaLinesFound != 1 { + return fmt.Errorf("Expected exactly one each of originalSha and fixupSha, got %d, %d", + numOriginalShaLinesFound, numFixupShaLinesFound) + } + + return WriteRebaseTodoFile(fileName, newTodos) +} + // 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 { From e63858215e8778f58d66bf064359d70a48eb6756 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Mon, 24 Apr 2023 11:42:23 +1000 Subject: [PATCH 12/23] refactor moveFixupCommitDown --- pkg/utils/rebase_todo.go | 57 ++++++++++-------- pkg/utils/rebase_todo_test.go | 108 ++++++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 25 deletions(-) diff --git a/pkg/utils/rebase_todo.go b/pkg/utils/rebase_todo.go index 2b8248224..15c06b1c4 100644 --- a/pkg/utils/rebase_todo.go +++ b/pkg/utils/rebase_todo.go @@ -140,36 +140,43 @@ func MoveFixupCommitDown(fileName string, originalSha string, fixupSha string) e return err } - newTodos := []todo.Todo{} - numOriginalShaLinesFound := 0 - numFixupShaLinesFound := 0 - - for _, t := range todos { - if t.Command == todo.Pick { - if equalShas(t.Commit, originalSha) { - numOriginalShaLinesFound += 1 - // append the original commit, and then the fixup - newTodos = append(newTodos, t) - newTodos = append(newTodos, todo.Todo{Command: todo.Fixup, Commit: fixupSha}) - continue - } else if equalShas(t.Commit, fixupSha) { - numFixupShaLinesFound += 1 - // skip the fixup here - continue - } - } - - newTodos = append(newTodos, t) - } - - if numOriginalShaLinesFound != 1 || numFixupShaLinesFound != 1 { - return fmt.Errorf("Expected exactly one each of originalSha and fixupSha, got %d, %d", - numOriginalShaLinesFound, numFixupShaLinesFound) + newTodos, err := moveFixupCommitDown(todos, originalSha, fixupSha) + if err != nil { + return err } return WriteRebaseTodoFile(fileName, newTodos) } +func moveFixupCommitDown(todos []todo.Todo, originalSha string, fixupSha string) ([]todo.Todo, error) { + isOriginal := func(t todo.Todo) bool { + return t.Command == todo.Pick && equalShas(t.Commit, originalSha) + } + + isFixup := func(t todo.Todo) bool { + return t.Command == todo.Pick && equalShas(t.Commit, fixupSha) + } + + originalShaCount := lo.CountBy(todos, isOriginal) + if originalShaCount != 1 { + return nil, fmt.Errorf("Expected exactly one original SHA, found %d", originalShaCount) + } + + fixupShaCount := lo.CountBy(todos, isFixup) + if fixupShaCount != 1 { + return nil, fmt.Errorf("Expected exactly one fixup SHA, found %d", fixupShaCount) + } + + _, fixupIndex, _ := lo.FindIndexOf(todos, isFixup) + _, originalIndex, _ := lo.FindIndexOf(todos, isOriginal) + + newTodos := MoveElement(todos, fixupIndex, originalIndex+1) + + newTodos[originalIndex+1].Command = todo.Fixup + + return newTodos, 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 { diff --git a/pkg/utils/rebase_todo_test.go b/pkg/utils/rebase_todo_test.go index a4b1a46d0..4f554e926 100644 --- a/pkg/utils/rebase_todo_test.go +++ b/pkg/utils/rebase_todo_test.go @@ -1,6 +1,7 @@ package utils import ( + "errors" "testing" "github.com/fsmiamoto/git-todo-parser/todo" @@ -228,3 +229,110 @@ func TestRebaseCommands_moveTodoUp(t *testing.T) { ) } } + +func TestRebaseCommands_moveFixupCommitDown(t *testing.T) { + scenarios := []struct { + name string + todos []todo.Todo + originalSha string + fixupSha string + expectedTodos []todo.Todo + expectedErr error + }{ + { + name: "fixup commit is the last commit", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Fixup, Commit: "fixup"}, + }, + expectedErr: nil, + }, + { + // TODO: is this something we actually want to support? + name: "fixup commit is separated from original commit", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "other"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Fixup, Commit: "fixup"}, + {Command: todo.Pick, Commit: "other"}, + }, + expectedErr: nil, + }, + { + name: "More original SHAs than expected", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one original SHA, found 2"), + }, + { + name: "More fixup SHAs than expected", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + {Command: todo.Pick, Commit: "fixup"}, + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one fixup SHA, found 2"), + }, + { + name: "No fixup SHAs found", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "original"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one fixup SHA, found 0"), + }, + { + name: "No original SHAs found", + todos: []todo.Todo{ + {Command: todo.Pick, Commit: "fixup"}, + }, + originalSha: "original", + fixupSha: "fixup", + expectedTodos: nil, + expectedErr: errors.New("Expected exactly one original SHA, found 0"), + }, + } + + for _, scenario := range scenarios { + t.Run(scenario.name, func(t *testing.T) { + actualTodos, actualErr := moveFixupCommitDown(scenario.todos, scenario.originalSha, scenario.fixupSha) + + if scenario.expectedErr == nil { + if !assert.NoError(t, actualErr) { + t.Errorf("Expected no error, got: %v", actualErr) + } + } else { + if !assert.EqualError(t, actualErr, scenario.expectedErr.Error()) { + t.Errorf("Expected err: %v, got: %v", scenario.expectedErr, actualErr) + } + } + + if !assert.EqualValues(t, actualTodos, scenario.expectedTodos) { + t.Errorf("Expected todos: %v, got: %v", scenario.expectedTodos, actualTodos) + } + }) + } +} From d675117289cc195b7cae105a808a29383f48d9d4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 13 Apr 2023 11:13:57 +0200 Subject: [PATCH 13/23] Fix activation of initial context This broke with 40f6767cfc77; the symptom is that starting lazygit with a git arg (e.g. "lazygit log") wouldn't activate the requested panel correctly. While it would show in the expanded view, it didn't have a green frame, and keyboard events would go to the files panel. --- pkg/gui/gui_common.go | 4 ++++ pkg/gui/layout.go | 2 +- pkg/gui/types/common.go | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/gui/gui_common.go b/pkg/gui/gui_common.go index 835aa4f54..01c73ad9a 100644 --- a/pkg/gui/gui_common.go +++ b/pkg/gui/gui_common.go @@ -70,6 +70,10 @@ func (self *guiCommon) IsCurrentContext(c types.Context) bool { return self.CurrentContext().GetKey() == c.GetKey() } +func (self *guiCommon) ActivateContext(context types.Context) error { + return self.gui.activateContext(context, types.OnFocusOpts{}) +} + func (self *guiCommon) GetAppState() *config.AppState { return self.gui.Config.GetAppState() } diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 0deb37d2e..14e1973bc 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -182,7 +182,7 @@ func (gui *Gui) onInitialViewsCreationForRepo() error { } initialContext := gui.currentSideContext() - if err := gui.c.PushContext(initialContext); err != nil { + if err := gui.c.ActivateContext(initialContext); err != nil { return err } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index aeb1da4c0..5711800f5 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -45,6 +45,7 @@ type IGuiCommon interface { CurrentContext() Context CurrentStaticContext() Context IsCurrentContext(Context) bool + ActivateContext(context Context) error // enters search mode for the current view OpenSearch() From fba1a2b5aceba3098ad9ffb4cacc19073de0e910 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 2 May 2023 09:36:03 +0200 Subject: [PATCH 14/23] Add config gui.experimentalShowBranchHeads People find the new (*) display for branch heads in the commits list confusing, so make it opt-in for now. --- docs/Config.md | 1 + pkg/config/user_config.go | 78 ++++++++++--------- pkg/gui/presentation/commits.go | 2 +- .../drop_todo_commit_with_update_ref.go | 12 +-- ...ommit_with_update_ref_show_branch_heads.go | 62 +++++++++++++++ pkg/integration/tests/test_list.go | 1 + 6 files changed, 111 insertions(+), 45 deletions(-) create mode 100644 pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go diff --git a/docs/Config.md b/docs/Config.md index 837230367..34e89c130 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -57,6 +57,7 @@ gui: showFileTree: true # for rendering changes files in a tree format showListFooter: true # for seeing the '5 of 20' message in list panels showRandomTip: true + experimentalShowBranchHeads: false # visualize branch heads with (*) in commits list showBottomLine: true # for hiding the bottom information line (unless it has important information to tell you) showCommandLog: true showIcons: false diff --git a/pkg/config/user_config.go b/pkg/config/user_config.go index fb36bb7ea..82361ddbd 100644 --- a/pkg/config/user_config.go +++ b/pkg/config/user_config.go @@ -27,32 +27,33 @@ type RefresherConfig struct { } type GuiConfig struct { - AuthorColors map[string]string `yaml:"authorColors"` - BranchColors map[string]string `yaml:"branchColors"` - ScrollHeight int `yaml:"scrollHeight"` - ScrollPastBottom bool `yaml:"scrollPastBottom"` - MouseEvents bool `yaml:"mouseEvents"` - SkipUnstageLineWarning bool `yaml:"skipUnstageLineWarning"` - SkipStashWarning bool `yaml:"skipStashWarning"` - SidePanelWidth float64 `yaml:"sidePanelWidth"` - ExpandFocusedSidePanel bool `yaml:"expandFocusedSidePanel"` - MainPanelSplitMode string `yaml:"mainPanelSplitMode"` - Language string `yaml:"language"` - TimeFormat string `yaml:"timeFormat"` - Theme ThemeConfig `yaml:"theme"` - CommitLength CommitLengthConfig `yaml:"commitLength"` - SkipNoStagedFilesWarning bool `yaml:"skipNoStagedFilesWarning"` - ShowListFooter bool `yaml:"showListFooter"` - ShowFileTree bool `yaml:"showFileTree"` - ShowRandomTip bool `yaml:"showRandomTip"` - ShowCommandLog bool `yaml:"showCommandLog"` - ShowBottomLine bool `yaml:"showBottomLine"` - ShowIcons bool `yaml:"showIcons"` - CommandLogSize int `yaml:"commandLogSize"` - SplitDiff string `yaml:"splitDiff"` - SkipRewordInEditorWarning bool `yaml:"skipRewordInEditorWarning"` - WindowSize string `yaml:"windowSize"` - Border string `yaml:"border"` + AuthorColors map[string]string `yaml:"authorColors"` + BranchColors map[string]string `yaml:"branchColors"` + ScrollHeight int `yaml:"scrollHeight"` + ScrollPastBottom bool `yaml:"scrollPastBottom"` + MouseEvents bool `yaml:"mouseEvents"` + SkipUnstageLineWarning bool `yaml:"skipUnstageLineWarning"` + SkipStashWarning bool `yaml:"skipStashWarning"` + SidePanelWidth float64 `yaml:"sidePanelWidth"` + ExpandFocusedSidePanel bool `yaml:"expandFocusedSidePanel"` + MainPanelSplitMode string `yaml:"mainPanelSplitMode"` + Language string `yaml:"language"` + TimeFormat string `yaml:"timeFormat"` + Theme ThemeConfig `yaml:"theme"` + CommitLength CommitLengthConfig `yaml:"commitLength"` + SkipNoStagedFilesWarning bool `yaml:"skipNoStagedFilesWarning"` + ShowListFooter bool `yaml:"showListFooter"` + ShowFileTree bool `yaml:"showFileTree"` + ShowRandomTip bool `yaml:"showRandomTip"` + ShowCommandLog bool `yaml:"showCommandLog"` + ShowBottomLine bool `yaml:"showBottomLine"` + ShowIcons bool `yaml:"showIcons"` + ExperimentalShowBranchHeads bool `yaml:"experimentalShowBranchHeads"` + CommandLogSize int `yaml:"commandLogSize"` + SplitDiff string `yaml:"splitDiff"` + SkipRewordInEditorWarning bool `yaml:"skipRewordInEditorWarning"` + WindowSize string `yaml:"windowSize"` + Border string `yaml:"border"` } type ThemeConfig struct { @@ -408,18 +409,19 @@ func GetDefaultConfig() *UserConfig { UnstagedChangesColor: []string{"red"}, DefaultFgColor: []string{"default"}, }, - CommitLength: CommitLengthConfig{Show: true}, - SkipNoStagedFilesWarning: false, - ShowListFooter: true, - ShowCommandLog: true, - ShowBottomLine: true, - ShowFileTree: true, - ShowRandomTip: true, - ShowIcons: false, - CommandLogSize: 8, - SplitDiff: "auto", - SkipRewordInEditorWarning: false, - Border: "single", + CommitLength: CommitLengthConfig{Show: true}, + SkipNoStagedFilesWarning: false, + ShowListFooter: true, + ShowCommandLog: true, + ShowBottomLine: true, + ShowFileTree: true, + ShowRandomTip: true, + ShowIcons: false, + ExperimentalShowBranchHeads: false, + CommandLogSize: 8, + SplitDiff: "auto", + SkipRewordInEditorWarning: false, + Border: "single", }, Git: GitConfig{ Paging: PagingConfig{ diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index 7e16197b5..6b83456b9 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -277,7 +277,7 @@ func displayCommit( } else { if len(commit.Tags) > 0 { tagString = theme.DiffTerminalColor.SetBold().Sprint(strings.Join(commit.Tags, " ")) + " " - } else if commit.ExtraInfo != "" { + } else if common.UserConfig.Gui.ExperimentalShowBranchHeads && commit.ExtraInfo != "" { tagString = style.FgMagenta.SetBold().Sprint("(*)") + " " } } 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 fcbf1abe5..ace5bed40 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 @@ -23,10 +23,10 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Focus(). Lines( - Contains("(*) commit 06").IsSelected(), + Contains("commit 06").IsSelected(), Contains("commit 05"), Contains("commit 04"), - Contains("(*) commit 03"), + Contains("commit 03"), Contains("commit 02"), Contains("commit 01"), ). @@ -34,11 +34,11 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{ Press(keys.Universal.Edit). Focus(). Lines( - Contains("pick").Contains("(*) commit 06"), + 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 03"), Contains("pick").Contains("commit 02"), Contains("<-- YOU ARE HERE --- commit 01"), ). @@ -50,9 +50,9 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). IsFocused(). Lines( - Contains("(*) commit 06"), + Contains("commit 06"), Contains("commit 04"), - Contains("(*) commit 03"), + Contains("commit 03"), Contains("commit 02"), Contains("commit 01"), ) diff --git a/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go new file mode 100644 index 000000000..cb42ce989 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go @@ -0,0 +1,62 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var DropTodoCommitWithUpdateRefShowBranchHeads = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Drops a commit during interactive rebase when there is an update-ref in the git-rebase-todo file (with experimentalShowBranchHeads on)", + ExtraCmdArgs: "", + Skip: false, + GitVersion: From("2.38.0"), + SetupConfig: func(config *config.AppConfig) { + config.UserConfig.Gui.ExperimentalShowBranchHeads = true + }, + 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"), + ). + NavigateToLine(Contains("commit 01")). + Press(keys.Universal.Edit). + Focus(). + Lines( + Contains("pick").Contains("(*) commit 06"), + Contains("pick").Contains("commit 05"), + Contains("pick").Contains("commit 04"), + Contains("update-ref").Contains("master"), + Contains("pick").Contains("(*) commit 03"), + Contains("pick").Contains("commit 02"), + Contains("<-- YOU ARE HERE --- commit 01"), + ). + NavigateToLine(Contains("commit 05")). + Press(keys.Universal.Remove) + + t.Common().ContinueRebase() + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("(*) commit 06"), + Contains("commit 04"), + Contains("(*) commit 03"), + Contains("commit 02"), + Contains("commit 01"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 1bb581614..b2cb12e14 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -93,6 +93,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.AmendMerge, interactive_rebase.AmendNonHeadCommitDuringRebase, interactive_rebase.DropTodoCommitWithUpdateRef, + interactive_rebase.DropTodoCommitWithUpdateRefShowBranchHeads, interactive_rebase.EditFirstCommit, interactive_rebase.EditNonTodoCommitDuringRebase, interactive_rebase.FixupFirstCommit, From 88d4313970783d37f4e27354983b5313c108cd2d Mon Sep 17 00:00:00 2001 From: README-bot Date: Tue, 2 May 2023 08:59:25 +0000 Subject: [PATCH 15/23] Updated README.md --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bb30e8aef..2e9483a54 100644 --- a/README.md +++ b/README.md @@ -22,7 +22,7 @@ A simple terminal UI for git commands, written in Go with the [gocui](https://gi

- +

## Elevator Pitch From 30656b5ac63181bb96d12b2fa2661b608c28c3d5 Mon Sep 17 00:00:00 2001 From: Ryooooooga Date: Thu, 23 Mar 2023 00:11:17 +0900 Subject: [PATCH 16/23] chore(git_commands): support old git version (`git rebase --empty=keep`) --- pkg/commands/git_commands/rebase.go | 10 ++++- pkg/commands/git_commands/rebase_test.go | 52 +++++++++++++++++------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/pkg/commands/git_commands/rebase.go b/pkg/commands/git_commands/rebase.go index 50c599a66..e7a2d766a 100644 --- a/pkg/commands/git_commands/rebase.go +++ b/pkg/commands/git_commands/rebase.go @@ -181,12 +181,18 @@ func (self *RebaseCommands) PrepareInteractiveRebaseCommand(opts PrepareInteract debug = "TRUE" } + emptyArg := " --empty=keep" + if self.version.IsOlderThan(2, 26, 0) { + emptyArg = "" + } + rebaseMergesArg := " --rebase-merges" if self.version.IsOlderThan(2, 22, 0) { rebaseMergesArg = "" } - cmdStr := fmt.Sprintf("git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash%s %s", - rebaseMergesArg, opts.baseShaOrRoot) + + cmdStr := fmt.Sprintf("git rebase --interactive --autostash --keep-empty%s --no-autosquash%s %s", + emptyArg, rebaseMergesArg, opts.baseShaOrRoot) self.Log.WithField("command", cmdStr).Debug("RunCommand") cmdObj := self.cmd.New(cmdStr) diff --git a/pkg/commands/git_commands/rebase_test.go b/pkg/commands/git_commands/rebase_test.go index 1ef22ff5d..f865468fb 100644 --- a/pkg/commands/git_commands/rebase_test.go +++ b/pkg/commands/git_commands/rebase_test.go @@ -16,37 +16,60 @@ import ( func TestRebaseRebaseBranch(t *testing.T) { type scenario struct { - testName string - arg string - runner *oscommands.FakeCmdObjRunner - test func(error) + testName string + arg string + gitVersion *GitVersion + runner *oscommands.FakeCmdObjRunner + test func(error) } scenarios := []scenario{ { - testName: "successful rebase", - arg: "master", + testName: "successful rebase", + arg: "master", + gitVersion: &GitVersion{2, 26, 0, ""}, runner: oscommands.NewFakeRunner(t). - Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash master`, "", nil), + Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges master`, "", nil), test: func(err error) { assert.NoError(t, err) }, }, { - testName: "unsuccessful rebase", - arg: "master", + testName: "unsuccessful rebase", + arg: "master", + gitVersion: &GitVersion{2, 26, 0, ""}, runner: oscommands.NewFakeRunner(t). - Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash master`, "", errors.New("error")), + Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges master`, "", errors.New("error")), test: func(err error) { assert.Error(t, err) }, }, + { + testName: "successful rebase (< 2.26.0)", + arg: "master", + gitVersion: &GitVersion{2, 25, 5, ""}, + runner: oscommands.NewFakeRunner(t). + Expect(`git rebase --interactive --autostash --keep-empty --no-autosquash --rebase-merges master`, "", nil), + test: func(err error) { + assert.NoError(t, err) + }, + }, + { + testName: "successful rebase (< 2.22.0)", + arg: "master", + gitVersion: &GitVersion{2, 21, 9, ""}, + runner: oscommands.NewFakeRunner(t). + Expect(`git rebase --interactive --autostash --keep-empty --no-autosquash master`, "", nil), + test: func(err error) { + assert.NoError(t, err) + }, + }, } for _, s := range scenarios { s := s t.Run(s.testName, func(t *testing.T) { - instance := buildRebaseCommands(commonDeps{runner: s.runner}) + instance := buildRebaseCommands(commonDeps{runner: s.runner, gitVersion: s.gitVersion}) s.test(instance.RebaseBranch(s.arg)) }) } @@ -126,7 +149,7 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { commitIndex: 0, fileName: "test999.txt", runner: oscommands.NewFakeRunner(t). - Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash abcdef`, "", nil). + Expect(`git rebase --interactive --autostash --keep-empty --empty=keep --no-autosquash --rebase-merges abcdef`, "", nil). Expect(`git cat-file -e HEAD^:"test999.txt"`, "", nil). Expect(`git checkout HEAD^ -- "test999.txt"`, "", nil). Expect(`git commit --amend --no-edit --allow-empty`, "", nil). @@ -143,8 +166,9 @@ func TestRebaseDiscardOldFileChanges(t *testing.T) { s := s t.Run(s.testName, func(t *testing.T) { instance := buildRebaseCommands(commonDeps{ - runner: s.runner, - gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses), + runner: s.runner, + gitVersion: &GitVersion{2, 26, 0, ""}, + gitConfig: git_config.NewFakeGitConfig(s.gitConfigMockResponses), }) s.test(instance.DiscardOldFileChanges(s.commits, s.commitIndex, s.fileName)) From c8f26aca6859aa052f240f0414395b939d7eb48d Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 15 Apr 2023 12:39:30 +0200 Subject: [PATCH 17/23] Rename From to AtLeast --- pkg/integration/components/test.go | 2 +- pkg/integration/components/test_test.go | 12 ++++++------ .../drop_todo_commit_with_update_ref.go | 2 +- ..._todo_commit_with_update_ref_show_branch_heads.go | 2 +- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/integration/components/test.go b/pkg/integration/components/test.go index 847781c8e..f4838ad86 100644 --- a/pkg/integration/components/test.go +++ b/pkg/integration/components/test.go @@ -60,7 +60,7 @@ type GitVersionRestriction struct { } // Verifies the version is at least the given version (inclusive) -func From(version string) GitVersionRestriction { +func AtLeast(version string) GitVersionRestriction { return GitVersionRestriction{from: version} } diff --git a/pkg/integration/components/test_test.go b/pkg/integration/components/test_test.go index 062382c2d..d15f86b0c 100644 --- a/pkg/integration/components/test_test.go +++ b/pkg/integration/components/test_test.go @@ -96,18 +96,18 @@ func TestGitVersionRestriction(t *testing.T) { expectedShouldRun bool }{ { - testName: "From, current is newer", - gitVersion: From("2.24.9"), + testName: "AtLeast, current is newer", + gitVersion: AtLeast("2.24.9"), expectedShouldRun: true, }, { - testName: "From, current is same", - gitVersion: From("2.25.0"), + testName: "AtLeast, current is same", + gitVersion: AtLeast("2.25.0"), expectedShouldRun: true, }, { - testName: "From, current is older", - gitVersion: From("2.26.0"), + testName: "AtLeast, current is older", + gitVersion: AtLeast("2.26.0"), expectedShouldRun: false, }, { 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 ace5bed40..e8d1e170a 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 @@ -9,7 +9,7 @@ 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"), + GitVersion: AtLeast("2.38.0"), SetupConfig: func(config *config.AppConfig) {}, SetupRepo: func(shell *Shell) { shell. diff --git a/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go index cb42ce989..6321891a7 100644 --- a/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go +++ b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref_show_branch_heads.go @@ -9,7 +9,7 @@ var DropTodoCommitWithUpdateRefShowBranchHeads = NewIntegrationTest(NewIntegrati Description: "Drops a commit during interactive rebase when there is an update-ref in the git-rebase-todo file (with experimentalShowBranchHeads on)", ExtraCmdArgs: "", Skip: false, - GitVersion: From("2.38.0"), + GitVersion: AtLeast("2.38.0"), SetupConfig: func(config *config.AppConfig) { config.UserConfig.Gui.ExperimentalShowBranchHeads = true }, From d607b366cb809e821c4d5531a2f2f8abe35e5ba0 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 15 Apr 2023 12:53:23 +0200 Subject: [PATCH 18/23] Add own version for test move_to_earlier_commit for older git versions --- .../patch_building/move_to_earlier_commit.go | 1 + .../move_to_earlier_commit_no_keep_empty.go | 77 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 3 files changed, 79 insertions(+) create mode 100644 pkg/integration/tests/patch_building/move_to_earlier_commit_no_keep_empty.go diff --git a/pkg/integration/tests/patch_building/move_to_earlier_commit.go b/pkg/integration/tests/patch_building/move_to_earlier_commit.go index 5803737f0..98bf6fa05 100644 --- a/pkg/integration/tests/patch_building/move_to_earlier_commit.go +++ b/pkg/integration/tests/patch_building/move_to_earlier_commit.go @@ -9,6 +9,7 @@ var MoveToEarlierCommit = NewIntegrationTest(NewIntegrationTestArgs{ Description: "Move a patch from a commit to an earlier commit", ExtraCmdArgs: "", Skip: false, + GitVersion: AtLeast("2.26.0"), SetupConfig: func(config *config.AppConfig) {}, SetupRepo: func(shell *Shell) { shell.CreateDir("dir") diff --git a/pkg/integration/tests/patch_building/move_to_earlier_commit_no_keep_empty.go b/pkg/integration/tests/patch_building/move_to_earlier_commit_no_keep_empty.go new file mode 100644 index 000000000..a44ba3438 --- /dev/null +++ b/pkg/integration/tests/patch_building/move_to_earlier_commit_no_keep_empty.go @@ -0,0 +1,77 @@ +package patch_building + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var MoveToEarlierCommitNoKeepEmpty = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Move a patch from a commit to an earlier commit, for older git versions that don't keep the empty commit", + ExtraCmdArgs: "", + Skip: false, + GitVersion: Before("2.26.0"), + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateDir("dir") + shell.CreateFileAndAdd("dir/file1", "file1 content") + shell.CreateFileAndAdd("dir/file2", "file2 content") + shell.Commit("first commit") + + shell.CreateFileAndAdd("unrelated-file", "") + shell.Commit("destination commit") + + shell.UpdateFileAndAdd("dir/file1", "file1 content with old changes") + shell.DeleteFileAndAdd("dir/file2") + shell.CreateFileAndAdd("dir/file3", "file3 content") + shell.Commit("commit to move from") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("commit to move from").IsSelected(), + Contains("destination commit"), + Contains("first commit"), + ). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("dir").IsSelected(), + Contains(" M file1"), + Contains(" D file2"), + Contains(" A file3"), + ). + PressPrimaryAction(). + PressEscape() + + t.Views().Information().Content(Contains("building patch")) + + t.Views().Commits(). + IsFocused(). + SelectNextItem() + + t.Common().SelectPatchOption(Contains("move patch to selected commit")) + + t.Views().Commits(). + IsFocused(). + Lines( + Contains("destination commit"), + Contains("first commit").IsSelected(), + ). + SelectPreviousItem(). + PressEnter() + + t.Views().CommitFiles(). + IsFocused(). + Lines( + Contains("dir").IsSelected(), + Contains(" M file1"), + Contains(" D file2"), + Contains(" A file3"), + Contains("A unrelated-file"), + ). + PressEscape() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index b2cb12e14..fe6604ac1 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -117,6 +117,7 @@ var tests = []*components.IntegrationTest{ patch_building.ApplyInReverseWithConflict, patch_building.CopyPatchToClipboard, patch_building.MoveToEarlierCommit, + patch_building.MoveToEarlierCommitNoKeepEmpty, patch_building.MoveToIndex, patch_building.MoveToIndexPartOfAdjacentAddedLines, patch_building.MoveToIndexPartial, From c88ecdf87cb8463103dab621b4242c853dde0816 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Wed, 3 May 2023 13:48:04 +1000 Subject: [PATCH 19/23] update open docs --- docs/Config.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/Config.md b/docs/Config.md index 34e89c130..9c5aa20b8 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -252,21 +252,21 @@ keybinding: ```yaml os: - openCommand: 'start "" {{filename}}' + open: 'start "" {{filename}}' ``` ### Linux ```yaml os: - openCommand: 'xdg-open {{filename}} >/dev/null' + open: 'xdg-open {{filename}} >/dev/null' ``` ### OSX ```yaml os: - openCommand: 'open {{filename}}' + open: 'open {{filename}}' ``` ### Configuring File Editing From 6e027f42dace96fd7074cf572eef0faebd89a2b5 Mon Sep 17 00:00:00 2001 From: Christian Rackerseder Date: Wed, 3 May 2023 13:30:51 +0200 Subject: [PATCH 20/23] Include "nvim" in supported edit presets --- docs/Config.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Config.md b/docs/Config.md index 9c5aa20b8..ee6fcc385 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -285,7 +285,7 @@ os: editPreset: 'vscode' ``` -Supported presets are `vim`, `emacs`, `nano`, `vscode`, `sublime`, `bbedit`, and +Supported presets are `vim`, `nvim`, `emacs`, `nano`, `vscode`, `sublime`, `bbedit`, and `xcode`. In many cases lazygit will be able to guess the right preset from your $(git config core.editor), or an environment variable such as $VISUAL or $EDITOR. From 1636931c2b11695846241933a4d7e8d61609e766 Mon Sep 17 00:00:00 2001 From: Christian Rackerseder Date: Thu, 4 May 2023 08:28:58 +0200 Subject: [PATCH 21/23] Include "kakoune" in supported edit presets --- docs/Config.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/Config.md b/docs/Config.md index ee6fcc385..e3890f9f6 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -285,9 +285,9 @@ os: editPreset: 'vscode' ``` -Supported presets are `vim`, `nvim`, `emacs`, `nano`, `vscode`, `sublime`, `bbedit`, and -`xcode`. In many cases lazygit will be able to guess the right preset from your -$(git config core.editor), or an environment variable such as $VISUAL or $EDITOR. +Supported presets are `vim`, `nvim`, `emacs`, `nano`, `vscode`, `sublime`, `bbedit`, +`kakoune` and `xcode`. In many cases lazygit will be able to guess the right preset +from your $(git config core.editor), or an environment variable such as $VISUAL or $EDITOR. If for some reason you are not happy with the default commands from a preset, or there simply is no preset for your editor, you can customize the commands by From 3cac14c76e5c968c5c134306d905a98d300bf308 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Thu, 4 May 2023 16:53:13 +1000 Subject: [PATCH 22/23] add comment to encourage keeping code and docs in sync --- pkg/config/editor_presets.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/config/editor_presets.go b/pkg/config/editor_presets.go index d461112d0..76b9d5996 100644 --- a/pkg/config/editor_presets.go +++ b/pkg/config/editor_presets.go @@ -35,6 +35,7 @@ type editPreset struct { editInTerminal bool } +// IF YOU ADD A PRESET TO THIS FUNCTION YOU MUST UPDATE THE `Supported presets` SECTION OF docs/Config.md func getPreset(osConfig *OSConfig, guessDefaultEditor func() string) *editPreset { presets := map[string]*editPreset{ "vi": standardTerminalEditorPreset("vi"), From e156e090ccb3f9d4f824a4292f9a79e6c74594b2 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Wed, 10 May 2023 22:31:27 +1000 Subject: [PATCH 23/23] add ability to update yaml path while preserving comments --- pkg/utils/yaml_utils/yaml_utils.go | 54 +++++++++++++++++++++ pkg/utils/yaml_utils/yaml_utils_test.go | 64 +++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 pkg/utils/yaml_utils/yaml_utils.go create mode 100644 pkg/utils/yaml_utils/yaml_utils_test.go diff --git a/pkg/utils/yaml_utils/yaml_utils.go b/pkg/utils/yaml_utils/yaml_utils.go new file mode 100644 index 000000000..9ed7ae875 --- /dev/null +++ b/pkg/utils/yaml_utils/yaml_utils.go @@ -0,0 +1,54 @@ +package yaml_utils + +import ( + "fmt" + + "gopkg.in/yaml.v3" +) + +// takes a yaml document in bytes, a path to a key, and a value to set. The value must be a scalar. +func UpdateYaml(yamlBytes []byte, path []string, value string) ([]byte, error) { + // Parse the YAML file. + var node yaml.Node + err := yaml.Unmarshal(yamlBytes, &node) + if err != nil { + return nil, fmt.Errorf("failed to parse YAML: %w", err) + } + + body := node.Content[0] + + updateYamlNode(body, path, value) + + // Convert the updated YAML node back to YAML bytes. + updatedYAMLBytes, err := yaml.Marshal(body) + if err != nil { + return nil, fmt.Errorf("failed to convert YAML node to bytes: %w", err) + } + + return updatedYAMLBytes, nil +} + +// Recursive function to update the YAML node. +func updateYamlNode(node *yaml.Node, path []string, value string) { + if len(path) == 0 { + node.Value = value + return + } + + key := path[0] + for i := 0; i < len(node.Content)-1; i += 2 { + if node.Content[i].Value == key { + updateYamlNode(node.Content[i+1], path[1:], value) + return + } + } + + // if the key doesn't exist, we'll add it + node.Content = append(node.Content, &yaml.Node{ + Kind: yaml.ScalarNode, + Value: key, + }, &yaml.Node{ + Kind: yaml.ScalarNode, + Value: value, + }) +} diff --git a/pkg/utils/yaml_utils/yaml_utils_test.go b/pkg/utils/yaml_utils/yaml_utils_test.go new file mode 100644 index 000000000..4bdfeb432 --- /dev/null +++ b/pkg/utils/yaml_utils/yaml_utils_test.go @@ -0,0 +1,64 @@ +package yaml_utils + +import "testing" + +func TestUpdateYaml(t *testing.T) { + tests := []struct { + name string + in string + path []string + value string + expectedOut string + expectedErr string + }{ + { + name: "update value", + in: "foo: bar\n", + path: []string{"foo"}, + value: "baz", + expectedOut: "foo: baz\n", + expectedErr: "", + }, + { + name: "add new key and value", + in: "foo: bar\n", + path: []string{"foo2"}, + value: "baz", + expectedOut: "foo: bar\nfoo2: baz\n", + expectedErr: "", + }, + { + name: "preserve inline comment", + in: "foo: bar # my comment\n", + path: []string{"foo2"}, + value: "baz", + expectedOut: "foo: bar # my comment\nfoo2: baz\n", + expectedErr: "", + }, + { + name: "nested update", + in: "foo:\n bar: baz\n", + path: []string{"foo", "bar"}, + value: "qux", + // indentation is not preserved. See https://github.com/go-yaml/yaml/issues/899 + expectedOut: "foo:\n bar: qux\n", + expectedErr: "", + }, + } + + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + out, err := UpdateYaml([]byte(test.in), test.path, test.value) + if test.expectedErr != "" { + if err == nil { + t.Errorf("expected error %q but got none", test.expectedErr) + } + } else if err != nil { + t.Errorf("unexpected error: %v", err) + } else if string(out) != test.expectedOut { + t.Errorf("expected %q but got %q", test.expectedOut, string(out)) + } + }) + } +}