From 682e54222c5f552d3cc4d18d23169c707179e1e4 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 14 Jun 2024 17:21:34 +0200 Subject: [PATCH 01/13] Don't wait for debugger in daemon mode When debugging an integration test that involves some behind-the-scenes rebasing, the daemon lazygit would also wait for a debugger to attach. This is very confusing because the test seems to hang; once you figured out what's going on, it's inconvenient because you need to attach a debugger to the daemon every time you debug the test. Now, it would sometimes be useful to be able to debug the daemon itself (whether inside an integration test, or during normal usage), and I have often wished to be able to do that. We might introduce an additional env var (and command-line option?) to enable this; but that's out of scope here. --- pkg/integration/clients/injector/main.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/integration/clients/injector/main.go b/pkg/integration/clients/injector/main.go index 1f4c845e8..96a4fb1f0 100644 --- a/pkg/integration/clients/injector/main.go +++ b/pkg/integration/clients/injector/main.go @@ -31,7 +31,7 @@ func main() { integrationTest := getIntegrationTest() - if os.Getenv(components.WAIT_FOR_DEBUGGER_ENV_VAR) != "" { + if os.Getenv(components.WAIT_FOR_DEBUGGER_ENV_VAR) != "" && !daemon.InDaemonMode() { println("Waiting for debugger to attach...") for !isDebuggerAttached() { time.Sleep(time.Millisecond * 100) From 94fc4d7eb4d60c9a67096190f80620a058fe5f53 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 10 Jun 2024 16:22:50 +0200 Subject: [PATCH 02/13] Remove rebaseMode field from TestGetCommits scenario All test cases set it to enums.REBASE_MODE_NONE, so we can simplify things a little bit by hard-coding that. This makes the changes in the following commits a little easier. --- .../git_commands/commit_loader_test.go | 34 +++++++------------ 1 file changed, 13 insertions(+), 21 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index f9c4cdff6..a6b4e58ae 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -33,17 +33,15 @@ func TestGetCommits(t *testing.T) { expectedCommits []*models.Commit expectedError error logOrder string - rebaseMode enums.RebaseMode opts GetCommitsOptions mainBranches []string } scenarios := []scenario{ { - testName: "should return no commits if there are none", - logOrder: "topo-order", - rebaseMode: enums.REBASE_MODE_NONE, - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, + testName: "should return no commits if there are none", + logOrder: "topo-order", + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), @@ -52,10 +50,9 @@ func TestGetCommits(t *testing.T) { expectedError: nil, }, { - testName: "should use proper upstream name for branch", - logOrder: "topo-order", - rebaseMode: enums.REBASE_MODE_NONE, - opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false}, + testName: "should use proper upstream name for branch", + logOrder: "topo-order", + opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: "refs/heads/mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), @@ -66,7 +63,6 @@ func TestGetCommits(t *testing.T) { { testName: "should return commits if they are present", logOrder: "topo-order", - rebaseMode: enums.REBASE_MODE_NONE, opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, mainBranches: []string{"master", "main", "develop"}, runner: oscommands.NewFakeRunner(t). @@ -203,7 +199,6 @@ func TestGetCommits(t *testing.T) { { testName: "should not call merge-base for mainBranches if none exist", logOrder: "topo-order", - rebaseMode: enums.REBASE_MODE_NONE, opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, mainBranches: []string{"master", "main"}, runner: oscommands.NewFakeRunner(t). @@ -240,7 +235,6 @@ func TestGetCommits(t *testing.T) { { testName: "should call merge-base for all main branches that exist", logOrder: "topo-order", - rebaseMode: enums.REBASE_MODE_NONE, opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, mainBranches: []string{"master", "main", "develop", "1.0-hotfixes"}, runner: oscommands.NewFakeRunner(t). @@ -277,10 +271,9 @@ func TestGetCommits(t *testing.T) { expectedError: nil, }, { - testName: "should not specify order if `log.order` is `default`", - logOrder: "default", - rebaseMode: enums.REBASE_MODE_NONE, - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, + testName: "should not specify order if `log.order` is `default`", + logOrder: "default", + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), @@ -289,10 +282,9 @@ func TestGetCommits(t *testing.T) { expectedError: nil, }, { - testName: "should set filter path", - logOrder: "default", - rebaseMode: enums.REBASE_MODE_NONE, - opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"}, + testName: "should set filter path", + logOrder: "default", + opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: "mybranch", FilterPath: "src"}, runner: oscommands.NewFakeRunner(t). ExpectGitArgs([]string{"merge-base", "mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:%H%x00%at%x00%aN%x00%ae%x00%D%x00%p%x00%m%x00%s", "--abbrev=40", "--follow", "--no-show-signature", "--", "src"}, "", nil), @@ -312,7 +304,7 @@ func TestGetCommits(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: cmd, - getRebaseMode: func() (enums.RebaseMode, error) { return scenario.rebaseMode, nil }, + getRebaseMode: func() (enums.RebaseMode, error) { return enums.REBASE_MODE_NONE, nil }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil From 37f835244dd3ba48b7167d0ee82471df5d5cae47 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 11 Jun 2024 14:20:32 +0200 Subject: [PATCH 03/13] Use WorkingTreeState instead of RebaseMode in CommitLoader We want to get rid of RebaseMode, and using the more general WorkingTreeState will later allow us to also show cherry-pick or revert todos. --- pkg/commands/git.go | 2 +- pkg/commands/git_commands/commit_loader.go | 41 ++++++++----------- .../git_commands/commit_loader_test.go | 16 ++++---- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 7e7d9354f..4aba9a7a8 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -136,7 +136,7 @@ func NewGitCommandAux( branchLoader := git_commands.NewBranchLoader(cmn, gitCommon, cmd, branchCommands.CurrentBranchInfo, configCommands) commitFileLoader := git_commands.NewCommitFileLoader(cmn, cmd) - commitLoader := git_commands.NewCommitLoader(cmn, cmd, statusCommands.RebaseMode, gitCommon) + commitLoader := git_commands.NewCommitLoader(cmn, cmd, statusCommands.WorkingTreeState, gitCommon) reflogCommitLoader := git_commands.NewReflogCommitLoader(cmn, cmd) remoteLoader := git_commands.NewRemoteLoader(cmn, cmd, repo.Remotes) worktreeLoader := git_commands.NewWorktreeLoader(gitCommon) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index f9cfff141..6bce06360 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -31,10 +31,10 @@ type CommitLoader struct { *common.Common cmd oscommands.ICmdObjBuilder - getRebaseMode func() (enums.RebaseMode, error) - readFile func(filename string) ([]byte, error) - walkFiles func(root string, fn filepath.WalkFunc) error - dotGitDir string + getWorkingTreeState func() enums.RebaseMode + readFile func(filename string) ([]byte, error) + walkFiles func(root string, fn filepath.WalkFunc) error + dotGitDir string *GitCommon } @@ -42,16 +42,16 @@ type CommitLoader struct { func NewCommitLoader( cmn *common.Common, cmd oscommands.ICmdObjBuilder, - getRebaseMode func() (enums.RebaseMode, error), + getWorkingTreeState func() enums.RebaseMode, gitCommon *GitCommon, ) *CommitLoader { return &CommitLoader{ - Common: cmn, - cmd: cmd, - getRebaseMode: getRebaseMode, - readFile: os.ReadFile, - walkFiles: filepath.Walk, - GitCommon: gitCommon, + Common: cmn, + cmd: cmd, + getWorkingTreeState: getWorkingTreeState, + readFile: os.ReadFile, + walkFiles: filepath.Walk, + GitCommon: gitCommon, } } @@ -172,17 +172,12 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod } } - rebaseMode, err := self.getRebaseMode() - if err != nil { - return nil, err - } - - if rebaseMode == enums.REBASE_MODE_NONE { + if !self.getWorkingTreeState().IsRebasing() { // not in rebase mode so return original commits return result, nil } - rebasingCommits, err := self.getHydratedRebasingCommits(rebaseMode) + rebasingCommits, err := self.getHydratedRebasingCommits() if err != nil { return nil, err } @@ -248,8 +243,8 @@ func (self *CommitLoader) extractCommitFromLine(line string, showDivergence bool } } -func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode) ([]*models.Commit, error) { - commits := self.getRebasingCommits(rebaseMode) +func (self *CommitLoader) getHydratedRebasingCommits() ([]*models.Commit, error) { + commits := self.getRebasingCommits() if len(commits) == 0 { return nil, nil @@ -310,11 +305,7 @@ func (self *CommitLoader) getHydratedRebasingCommits(rebaseMode enums.RebaseMode // git-rebase-todo example: // pick ac446ae94ee560bdb8d1d057278657b251aaef17 ac446ae // pick afb893148791a2fbd8091aeb81deba4930c73031 afb8931 -func (self *CommitLoader) getRebasingCommits(rebaseMode enums.RebaseMode) []*models.Commit { - if rebaseMode != enums.REBASE_MODE_INTERACTIVE { - return nil - } - +func (self *CommitLoader) getRebasingCommits() []*models.Commit { bytesContent, err := self.readFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo")) if err != nil { self.Log.Error(fmt.Sprintf("error occurred reading git-rebase-todo: %s", err.Error())) diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index a6b4e58ae..73cae73a2 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -302,10 +302,10 @@ func TestGetCommits(t *testing.T) { cmd := oscommands.NewDummyCmdObjBuilder(scenario.runner) builder := &CommitLoader{ - Common: common, - cmd: cmd, - getRebaseMode: func() (enums.RebaseMode, error) { return enums.REBASE_MODE_NONE, nil }, - dotGitDir: ".git", + Common: common, + cmd: cmd, + getWorkingTreeState: func() enums.RebaseMode { return enums.REBASE_MODE_NONE }, + dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil }, @@ -485,10 +485,10 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { common := utils.NewDummyCommon() builder := &CommitLoader{ - Common: common, - cmd: oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)), - getRebaseMode: func() (enums.RebaseMode, error) { return enums.REBASE_MODE_INTERACTIVE, nil }, - dotGitDir: ".git", + Common: common, + cmd: oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)), + getWorkingTreeState: func() enums.RebaseMode { return enums.REBASE_MODE_INTERACTIVE }, + dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil }, From 1a736975462a6e532e6f8c6963154be43df71025 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 10 Jun 2024 16:52:02 +0200 Subject: [PATCH 04/13] Simplify the RebaseMode enum - Remove REBASE_MODE_NORMAL. It is not the "normal" mode anyway, rather a legacy mode; we have removed support for it in eb0f7e3d02, so there's no point in representing it in the enum. - Remove distinction between REBASE_MODE_REBASING and REBASE_MODE_INTERACTIVE; these are the same now. - Rename StatusCommands.IsInInteractiveRebase to IsInRebase. - Remove StatusCommands.RebaseMode; use StatusCommands.IsInRebase instead. --- .../git_commands/commit_loader_test.go | 2 +- pkg/commands/git_commands/status.go | 29 +++++-------------- pkg/commands/types/enums/enums.go | 6 +--- .../controllers/helpers/cherry_pick_helper.go | 4 +-- 4 files changed, 11 insertions(+), 30 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 73cae73a2..631270abc 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -487,7 +487,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)), - getWorkingTreeState: func() enums.RebaseMode { return enums.REBASE_MODE_INTERACTIVE }, + getWorkingTreeState: func() enums.RebaseMode { return enums.REBASE_MODE_REBASING }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil diff --git a/pkg/commands/git_commands/status.go b/pkg/commands/git_commands/status.go index 0e0ef37fc..ff1499ec9 100644 --- a/pkg/commands/git_commands/status.go +++ b/pkg/commands/git_commands/status.go @@ -20,24 +20,9 @@ func NewStatusCommands( } } -// RebaseMode returns "" for non-rebase mode, "normal" for normal rebase -// and "interactive" for interactive rebase -func (self *StatusCommands) RebaseMode() (enums.RebaseMode, error) { - ok, err := self.IsInNormalRebase() - if err == nil && ok { - return enums.REBASE_MODE_NORMAL, nil - } - ok, err = self.IsInInteractiveRebase() - if err == nil && ok { - return enums.REBASE_MODE_INTERACTIVE, err - } - - return enums.REBASE_MODE_NONE, err -} - func (self *StatusCommands) WorkingTreeState() enums.RebaseMode { - rebaseMode, _ := self.RebaseMode() - if rebaseMode != enums.REBASE_MODE_NONE { + isInRebase, _ := self.IsInRebase() + if isInRebase { return enums.REBASE_MODE_REBASING } merging, _ := self.IsInMergeState() @@ -51,14 +36,14 @@ func (self *StatusCommands) IsBareRepo() bool { return self.repoPaths.isBareRepo } -func (self *StatusCommands) IsInNormalRebase() (bool, error) { +func (self *StatusCommands) IsInRebase() (bool, error) { + exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge")) + if err == nil && exists { + return true, nil + } return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-apply")) } -func (self *StatusCommands) IsInInteractiveRebase() (bool, error) { - return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge")) -} - // IsInMergeState states whether we are still mid-merge func (self *StatusCommands) IsInMergeState() (bool, error) { return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "MERGE_HEAD")) diff --git a/pkg/commands/types/enums/enums.go b/pkg/commands/types/enums/enums.go index 68a29d8f0..790cb1f5c 100644 --- a/pkg/commands/types/enums/enums.go +++ b/pkg/commands/types/enums/enums.go @@ -5,10 +5,6 @@ type RebaseMode int const ( // this means we're neither rebasing nor merging REBASE_MODE_NONE RebaseMode = iota - // this means normal rebase as opposed to interactive rebase - REBASE_MODE_NORMAL - REBASE_MODE_INTERACTIVE - // REBASE_MODE_REBASING is a general state that captures both REBASE_MODE_NORMAL and REBASE_MODE_INTERACTIVE REBASE_MODE_REBASING REBASE_MODE_MERGING ) @@ -18,5 +14,5 @@ func (self RebaseMode) IsMerging() bool { } func (self RebaseMode) IsRebasing() bool { - return self == REBASE_MODE_INTERACTIVE || self == REBASE_MODE_NORMAL || self == REBASE_MODE_REBASING + return self == REBASE_MODE_REBASING } diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index 47108df16..1cb2285d0 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -77,7 +77,7 @@ func (self *CherryPickHelper) Paste() error { "numCommits": strconv.Itoa(len(self.getData().CherryPickedCommits)), }), HandleConfirm: func() error { - isInRebase, err := self.c.Git().Status.IsInInteractiveRebase() + isInRebase, err := self.c.Git().Status.IsInRebase() if err != nil { return err } @@ -107,7 +107,7 @@ func (self *CherryPickHelper) Paste() error { // be because there were conflicts. Don't clear the copied // commits in this case, since we might want to abort and // try pasting them again. - isInRebase, err = self.c.Git().Status.IsInInteractiveRebase() + isInRebase, err = self.c.Git().Status.IsInRebase() if err != nil { return err } From cd36e95a826a7868ac6956de9a0e59f97a76d44b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 10 Jun 2024 17:36:34 +0200 Subject: [PATCH 05/13] Rename RebaseMode to WorkingTreeState We're about to add more possible values (reverting and cherry-picking), so working tree state seems like a more suitable name. --- pkg/commands/git_commands/commit_loader.go | 4 ++-- .../git_commands/commit_loader_test.go | 4 ++-- pkg/commands/git_commands/patch.go | 4 ++-- pkg/commands/git_commands/status.go | 8 ++++---- pkg/commands/types/enums/enums.go | 16 +++++++-------- pkg/gui/context/local_commits_context.go | 2 +- .../custom_patch_options_menu_action.go | 4 ++-- .../helpers/merge_and_rebase_helper.go | 20 +++++++++---------- pkg/gui/controllers/helpers/mode_helper.go | 2 +- .../helpers/patch_building_helper.go | 2 +- pkg/gui/controllers/helpers/refresh_helper.go | 2 +- .../controllers/local_commits_controller.go | 4 ++-- pkg/gui/controllers/status_controller.go | 2 +- pkg/gui/controllers/undo_controller.go | 4 ++-- pkg/gui/presentation/status.go | 4 ++-- pkg/gui/presentation/working_tree.go | 16 +++++++-------- pkg/gui/types/common.go | 2 +- 17 files changed, 50 insertions(+), 50 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 6bce06360..08ee08313 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -31,7 +31,7 @@ type CommitLoader struct { *common.Common cmd oscommands.ICmdObjBuilder - getWorkingTreeState func() enums.RebaseMode + getWorkingTreeState func() enums.WorkingTreeState readFile func(filename string) ([]byte, error) walkFiles func(root string, fn filepath.WalkFunc) error dotGitDir string @@ -42,7 +42,7 @@ type CommitLoader struct { func NewCommitLoader( cmn *common.Common, cmd oscommands.ICmdObjBuilder, - getWorkingTreeState func() enums.RebaseMode, + getWorkingTreeState func() enums.WorkingTreeState, gitCommon *GitCommon, ) *CommitLoader { return &CommitLoader{ diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 631270abc..9aac43d11 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -304,7 +304,7 @@ func TestGetCommits(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: cmd, - getWorkingTreeState: func() enums.RebaseMode { return enums.REBASE_MODE_NONE }, + getWorkingTreeState: func() enums.WorkingTreeState { return enums.WORKING_TREE_STATE_NONE }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil @@ -487,7 +487,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)), - getWorkingTreeState: func() enums.RebaseMode { return enums.REBASE_MODE_REBASING }, + getWorkingTreeState: func() enums.WorkingTreeState { return enums.WORKING_TREE_STATE_REBASING }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index 36e094337..daa1030d4 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -229,7 +229,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId } if err := self.ApplyCustomPatch(true, true); err != nil { - if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING { + if self.status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { _ = self.rebase.AbortRebase() } return err @@ -253,7 +253,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId self.rebase.onSuccessfulContinue = func() error { // add patches to index if err := self.ApplyPatch(patch, ApplyPatchOpts{Index: true, ThreeWay: true}); err != nil { - if self.status.WorkingTreeState() == enums.REBASE_MODE_REBASING { + if self.status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { _ = self.rebase.AbortRebase() } return err diff --git a/pkg/commands/git_commands/status.go b/pkg/commands/git_commands/status.go index ff1499ec9..ba4fdb679 100644 --- a/pkg/commands/git_commands/status.go +++ b/pkg/commands/git_commands/status.go @@ -20,16 +20,16 @@ func NewStatusCommands( } } -func (self *StatusCommands) WorkingTreeState() enums.RebaseMode { +func (self *StatusCommands) WorkingTreeState() enums.WorkingTreeState { isInRebase, _ := self.IsInRebase() if isInRebase { - return enums.REBASE_MODE_REBASING + return enums.WORKING_TREE_STATE_REBASING } merging, _ := self.IsInMergeState() if merging { - return enums.REBASE_MODE_MERGING + return enums.WORKING_TREE_STATE_MERGING } - return enums.REBASE_MODE_NONE + return enums.WORKING_TREE_STATE_NONE } func (self *StatusCommands) IsBareRepo() bool { diff --git a/pkg/commands/types/enums/enums.go b/pkg/commands/types/enums/enums.go index 790cb1f5c..8513b074e 100644 --- a/pkg/commands/types/enums/enums.go +++ b/pkg/commands/types/enums/enums.go @@ -1,18 +1,18 @@ package enums -type RebaseMode int +type WorkingTreeState int const ( // this means we're neither rebasing nor merging - REBASE_MODE_NONE RebaseMode = iota - REBASE_MODE_REBASING - REBASE_MODE_MERGING + WORKING_TREE_STATE_NONE WorkingTreeState = iota + WORKING_TREE_STATE_REBASING + WORKING_TREE_STATE_MERGING ) -func (self RebaseMode) IsMerging() bool { - return self == REBASE_MODE_MERGING +func (self WorkingTreeState) IsMerging() bool { + return self == WORKING_TREE_STATE_MERGING } -func (self RebaseMode) IsRebasing() bool { - return self == REBASE_MODE_REBASING +func (self WorkingTreeState) IsRebasing() bool { + return self == WORKING_TREE_STATE_REBASING } diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 4f2f797e5..9f6c2f5f1 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -41,7 +41,7 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext { } } - showYouAreHereLabel := c.Model().WorkingTreeStateAtLastCommitRefresh == enums.REBASE_MODE_REBASING + showYouAreHereLabel := c.Model().WorkingTreeStateAtLastCommitRefresh == enums.WORKING_TREE_STATE_REBASING hasRebaseUpdateRefsConfig := c.Git().Config.GetRebaseUpdateRefs() return presentation.GetCommitListDisplayStrings( diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index ced98b072..5208cfb15 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -44,7 +44,7 @@ func (self *CustomPatchOptionsMenuAction) Call() error { }, } - if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState() == enums.REBASE_MODE_NONE { + if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_NONE { menuItems = append(menuItems, []*types.MenuItem{ { Label: fmt.Sprintf(self.c.Tr.RemovePatchFromOriginalCommit, self.c.Git().Patch.PatchBuilder.To), @@ -115,7 +115,7 @@ func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() int { } func (self *CustomPatchOptionsMenuAction) validateNormalWorkingTreeState() (bool, error) { - if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE { + if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE { return false, errors.New(self.c.Tr.CantPatchWhileRebasingError) } return true, nil diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index fba0b9014..565df9226 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -51,7 +51,7 @@ func (self *MergeAndRebaseHelper) CreateRebaseOptionsMenu() error { {option: REBASE_OPTION_ABORT, key: 'a'}, } - if self.c.Git().Status.WorkingTreeState() == enums.REBASE_MODE_REBASING { + if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { options = append(options, optionAndKey{ option: REBASE_OPTION_SKIP, key: 's', }) @@ -68,7 +68,7 @@ func (self *MergeAndRebaseHelper) CreateRebaseOptionsMenu() error { }) var title string - if self.c.Git().Status.WorkingTreeState() == enums.REBASE_MODE_MERGING { + if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_MERGING { title = self.c.Tr.MergeOptionsTitle } else { title = self.c.Tr.RebaseOptionsTitle @@ -84,12 +84,12 @@ func (self *MergeAndRebaseHelper) ContinueRebase() error { func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { status := self.c.Git().Status.WorkingTreeState() - if status != enums.REBASE_MODE_MERGING && status != enums.REBASE_MODE_REBASING { + if status != enums.WORKING_TREE_STATE_MERGING && status != enums.WORKING_TREE_STATE_REBASING { return errors.New(self.c.Tr.NotMergingOrRebasing) } self.c.LogAction(fmt.Sprintf("Merge/Rebase: %s", command)) - if status == enums.REBASE_MODE_REBASING { + if status == enums.WORKING_TREE_STATE_REBASING { todoFile, err := os.ReadFile( filepath.Join(self.c.Git().RepoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"), ) @@ -105,9 +105,9 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { commandType := "" switch status { - case enums.REBASE_MODE_MERGING: + case enums.WORKING_TREE_STATE_MERGING: commandType = "merge" - case enums.REBASE_MODE_REBASING: + case enums.WORKING_TREE_STATE_REBASING: commandType = "rebase" default: // shouldn't be possible to land here @@ -116,10 +116,10 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { // we should end up with a command like 'git merge --continue' // it's impossible for a rebase to require a commit so we'll use a subprocess only if it's a merge - needsSubprocess := (status == enums.REBASE_MODE_MERGING && command != REBASE_OPTION_ABORT && self.c.UserConfig().Git.Merging.ManualCommit) || + needsSubprocess := (status == enums.WORKING_TREE_STATE_MERGING && command != REBASE_OPTION_ABORT && self.c.UserConfig().Git.Merging.ManualCommit) || // but we'll also use a subprocess if we have exec todos; those are likely to be lengthy build // tasks whose output the user will want to see in the terminal - (status == enums.REBASE_MODE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos()) + (status == enums.WORKING_TREE_STATE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos()) if needsSubprocess { // TODO: see if we should be calling more of the code from self.Git.Rebase.GenericMergeOrRebaseAction @@ -239,9 +239,9 @@ func (self *MergeAndRebaseHelper) AbortMergeOrRebaseWithConfirm() error { func (self *MergeAndRebaseHelper) workingTreeStateNoun() string { workingTreeState := self.c.Git().Status.WorkingTreeState() switch workingTreeState { - case enums.REBASE_MODE_NONE: + case enums.WORKING_TREE_STATE_NONE: return "" - case enums.REBASE_MODE_MERGING: + case enums.WORKING_TREE_STATE_MERGING: return "merge" default: return "rebase" diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index 579862155..0259be97e 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -116,7 +116,7 @@ func (self *ModeHelper) Statuses() []ModeStatus { }, { IsActive: func() bool { - return !self.suppressRebasingMode && self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE + return !self.suppressRebasingMode && self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE }, Description: func() string { workingTreeState := self.c.Git().Status.WorkingTreeState() diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index 1cf64b3f8..2deb19d9c 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/pkg/gui/controllers/helpers/patch_building_helper.go @@ -22,7 +22,7 @@ func NewPatchBuildingHelper( } func (self *PatchBuildingHelper) ValidateNormalWorkingTreeState() (bool, error) { - if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE { + if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE { return false, errors.New(self.c.Tr.CantPatchWhileRebasingError) } return true, nil diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 33347a363..9e6dff1dd 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -583,7 +583,7 @@ func (self *RefreshHelper) refreshStateFiles() error { } } - if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE && conflictFileCount == 0 && prevConflictFileCount > 0 { + if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE && conflictFileCount == 0 && prevConflictFileCount > 0 { self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 79e3a6549..4e7f652e0 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -682,7 +682,7 @@ func (self *LocalCommitsController) rewordEnabled(commit *models.Commit) *types. } func (self *LocalCommitsController) isRebasing() bool { - return self.c.Model().WorkingTreeStateAtLastCommitRefresh != enums.REBASE_MODE_NONE + return self.c.Model().WorkingTreeStateAtLastCommitRefresh != enums.WORKING_TREE_STATE_NONE } func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, startIdx int, endIdx int) error { @@ -976,7 +976,7 @@ func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCo return nil } - if self.c.Git().Status.WorkingTreeState() != enums.REBASE_MODE_NONE { + if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE { // Can't move commits while rebasing return nil } diff --git a/pkg/gui/controllers/status_controller.go b/pkg/gui/controllers/status_controller.go index d517b870c..a0012227b 100644 --- a/pkg/gui/controllers/status_controller.go +++ b/pkg/gui/controllers/status_controller.go @@ -110,7 +110,7 @@ func (self *StatusController) onClick(opts gocui.ViewMouseBindingOpts) error { repoName := self.c.Git().RepoPaths.RepoName() workingTreeState := self.c.Git().Status.WorkingTreeState() switch workingTreeState { - case enums.REBASE_MODE_REBASING, enums.REBASE_MODE_MERGING: + case enums.WORKING_TREE_STATE_REBASING, enums.WORKING_TREE_STATE_MERGING: workingTreeStatus := fmt.Sprintf("(%s)", presentation.FormatWorkingTreeStateLower(self.c.Tr, workingTreeState)) if cursorInSubstring(opts.X, upstreamStatus+" ", workingTreeStatus) { return self.c.Helpers().MergeAndRebase.CreateRebaseOptionsMenu() diff --git a/pkg/gui/controllers/undo_controller.go b/pkg/gui/controllers/undo_controller.go index 198aa7d7b..b00eb802f 100644 --- a/pkg/gui/controllers/undo_controller.go +++ b/pkg/gui/controllers/undo_controller.go @@ -78,7 +78,7 @@ func (self *UndoController) reflogUndo() error { undoEnvVars := []string{"GIT_REFLOG_ACTION=[lazygit undo]"} undoingStatus := self.c.Tr.UndoingStatus - if self.c.Git().Status.WorkingTreeState() == enums.REBASE_MODE_REBASING { + if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { return errors.New(self.c.Tr.CantUndoWhileRebasing) } @@ -142,7 +142,7 @@ func (self *UndoController) reflogRedo() error { redoEnvVars := []string{"GIT_REFLOG_ACTION=[lazygit redo]"} redoingStatus := self.c.Tr.RedoingStatus - if self.c.Git().Status.WorkingTreeState() == enums.REBASE_MODE_REBASING { + if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { return errors.New(self.c.Tr.CantRedoWhileRebasing) } diff --git a/pkg/gui/presentation/status.go b/pkg/gui/presentation/status.go index b3b210067..3cc137b35 100644 --- a/pkg/gui/presentation/status.go +++ b/pkg/gui/presentation/status.go @@ -18,7 +18,7 @@ func FormatStatus( currentBranch *models.Branch, itemOperation types.ItemOperation, linkedWorktreeName string, - workingTreeState enums.RebaseMode, + workingTreeState enums.WorkingTreeState, tr *i18n.TranslationSet, userConfig *config.UserConfig, ) string { @@ -31,7 +31,7 @@ func FormatStatus( } } - if workingTreeState != enums.REBASE_MODE_NONE { + if workingTreeState != enums.WORKING_TREE_STATE_NONE { status += style.FgYellow.Sprintf("(%s) ", FormatWorkingTreeStateLower(tr, workingTreeState)) } diff --git a/pkg/gui/presentation/working_tree.go b/pkg/gui/presentation/working_tree.go index 1e1954bc9..1e2eadd1a 100644 --- a/pkg/gui/presentation/working_tree.go +++ b/pkg/gui/presentation/working_tree.go @@ -5,11 +5,11 @@ import ( "github.com/jesseduffield/lazygit/pkg/i18n" ) -func FormatWorkingTreeStateTitle(tr *i18n.TranslationSet, rebaseMode enums.RebaseMode) string { - switch rebaseMode { - case enums.REBASE_MODE_REBASING: +func FormatWorkingTreeStateTitle(tr *i18n.TranslationSet, workingTreeState enums.WorkingTreeState) string { + switch workingTreeState { + case enums.WORKING_TREE_STATE_REBASING: return tr.RebasingStatus - case enums.REBASE_MODE_MERGING: + case enums.WORKING_TREE_STATE_MERGING: return tr.MergingStatus default: // should never actually display this @@ -17,11 +17,11 @@ func FormatWorkingTreeStateTitle(tr *i18n.TranslationSet, rebaseMode enums.Rebas } } -func FormatWorkingTreeStateLower(tr *i18n.TranslationSet, rebaseMode enums.RebaseMode) string { - switch rebaseMode { - case enums.REBASE_MODE_REBASING: +func FormatWorkingTreeStateLower(tr *i18n.TranslationSet, workingTreeState enums.WorkingTreeState) string { + switch workingTreeState { + case enums.WORKING_TREE_STATE_REBASING: return tr.LowercaseRebasingStatus - case enums.REBASE_MODE_MERGING: + case enums.WORKING_TREE_STATE_MERGING: return tr.LowercaseMergingStatus default: // should never actually display this diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index 82a309fb1..afa0bf2a4 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -288,7 +288,7 @@ type Model struct { ReflogCommits []*models.Commit BisectInfo *git_commands.BisectInfo - WorkingTreeStateAtLastCommitRefresh enums.RebaseMode + WorkingTreeStateAtLastCommitRefresh enums.WorkingTreeState RemoteBranches []*models.RemoteBranch Tags []*models.Tag From b210b4363dd1ee2f752b8dabd81dc47045f544fb Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 11 Jun 2024 12:05:42 +0200 Subject: [PATCH 06/13] Centralize logic regarding WorkingTreeState close to its definition --- pkg/commands/types/enums/enums.go | 45 +++++++++++++++++++ .../helpers/merge_and_rebase_helper.go | 34 ++------------ pkg/gui/controllers/helpers/mode_helper.go | 3 +- pkg/gui/controllers/status_controller.go | 2 +- pkg/gui/presentation/status.go | 2 +- pkg/gui/presentation/working_tree.go | 30 ------------- 6 files changed, 52 insertions(+), 64 deletions(-) delete mode 100644 pkg/gui/presentation/working_tree.go diff --git a/pkg/commands/types/enums/enums.go b/pkg/commands/types/enums/enums.go index 8513b074e..09fb1211a 100644 --- a/pkg/commands/types/enums/enums.go +++ b/pkg/commands/types/enums/enums.go @@ -1,5 +1,7 @@ package enums +import "github.com/jesseduffield/lazygit/pkg/i18n" + type WorkingTreeState int const ( @@ -16,3 +18,46 @@ func (self WorkingTreeState) IsMerging() bool { func (self WorkingTreeState) IsRebasing() bool { return self == WORKING_TREE_STATE_REBASING } + +func (self WorkingTreeState) Title(tr *i18n.TranslationSet) string { + switch self { + case WORKING_TREE_STATE_REBASING: + return tr.RebasingStatus + case WORKING_TREE_STATE_MERGING: + return tr.MergingStatus + default: + // should never actually display this + return "none" + } +} + +func (self WorkingTreeState) LowerCaseTitle(tr *i18n.TranslationSet) string { + switch self { + case WORKING_TREE_STATE_REBASING: + return tr.LowercaseRebasingStatus + case WORKING_TREE_STATE_MERGING: + return tr.LowercaseMergingStatus + default: + // should never actually display this + return "none" + } +} + +func (self WorkingTreeState) OptionsMenuTitle(tr *i18n.TranslationSet) string { + if self == WORKING_TREE_STATE_MERGING { + return tr.MergeOptionsTitle + } + return tr.RebaseOptionsTitle +} + +func (self WorkingTreeState) CommandName() string { + switch self { + case WORKING_TREE_STATE_MERGING: + return "merge" + case WORKING_TREE_STATE_REBASING: + return "rebase" + default: + // shouldn't be possible to land here + return "" + } +} diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 565df9226..55a322292 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -67,13 +67,7 @@ func (self *MergeAndRebaseHelper) CreateRebaseOptionsMenu() error { } }) - var title string - if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_MERGING { - title = self.c.Tr.MergeOptionsTitle - } else { - title = self.c.Tr.RebaseOptionsTitle - } - + title := self.c.Git().Status.WorkingTreeState().OptionsMenuTitle(self.c.Tr) return self.c.Menu(types.CreateMenuOptions{Title: title, Items: menuItems}) } @@ -103,15 +97,7 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { } } - commandType := "" - switch status { - case enums.WORKING_TREE_STATE_MERGING: - commandType = "merge" - case enums.WORKING_TREE_STATE_REBASING: - commandType = "rebase" - default: - // shouldn't be possible to land here - } + commandType := status.CommandName() // we should end up with a command like 'git merge --continue' @@ -199,7 +185,7 @@ func (self *MergeAndRebaseHelper) CheckForConflicts(result error) error { } func (self *MergeAndRebaseHelper) PromptForConflictHandling() error { - mode := self.workingTreeStateNoun() + mode := self.c.Git().Status.WorkingTreeState().CommandName() return self.c.Menu(types.CreateMenuOptions{ Title: self.c.Tr.FoundConflictsTitle, Items: []*types.MenuItem{ @@ -224,7 +210,7 @@ func (self *MergeAndRebaseHelper) PromptForConflictHandling() error { func (self *MergeAndRebaseHelper) AbortMergeOrRebaseWithConfirm() error { // prompt user to confirm that they want to abort, then do it - mode := self.workingTreeStateNoun() + mode := self.c.Git().Status.WorkingTreeState().CommandName() self.c.Confirm(types.ConfirmOpts{ Title: fmt.Sprintf(self.c.Tr.AbortTitle, mode), Prompt: fmt.Sprintf(self.c.Tr.AbortPrompt, mode), @@ -236,18 +222,6 @@ func (self *MergeAndRebaseHelper) AbortMergeOrRebaseWithConfirm() error { return nil } -func (self *MergeAndRebaseHelper) workingTreeStateNoun() string { - workingTreeState := self.c.Git().Status.WorkingTreeState() - switch workingTreeState { - case enums.WORKING_TREE_STATE_NONE: - return "" - case enums.WORKING_TREE_STATE_MERGING: - return "merge" - default: - return "rebase" - } -} - // PromptToContinueRebase asks the user if they want to continue the rebase/merge that's in progress func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { self.c.Confirm(types.ConfirmOpts{ diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index 0259be97e..0c964d2de 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -5,7 +5,6 @@ import ( "strings" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" - "github.com/jesseduffield/lazygit/pkg/gui/presentation" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/samber/lo" @@ -121,7 +120,7 @@ func (self *ModeHelper) Statuses() []ModeStatus { Description: func() string { workingTreeState := self.c.Git().Status.WorkingTreeState() return self.withResetButton( - presentation.FormatWorkingTreeStateTitle(self.c.Tr, workingTreeState), style.FgYellow, + workingTreeState.Title(self.c.Tr), style.FgYellow, ) }, Reset: self.mergeAndRebaseHelper.AbortMergeOrRebaseWithConfirm, diff --git a/pkg/gui/controllers/status_controller.go b/pkg/gui/controllers/status_controller.go index a0012227b..31b264e17 100644 --- a/pkg/gui/controllers/status_controller.go +++ b/pkg/gui/controllers/status_controller.go @@ -111,7 +111,7 @@ func (self *StatusController) onClick(opts gocui.ViewMouseBindingOpts) error { workingTreeState := self.c.Git().Status.WorkingTreeState() switch workingTreeState { case enums.WORKING_TREE_STATE_REBASING, enums.WORKING_TREE_STATE_MERGING: - workingTreeStatus := fmt.Sprintf("(%s)", presentation.FormatWorkingTreeStateLower(self.c.Tr, workingTreeState)) + workingTreeStatus := fmt.Sprintf("(%s)", workingTreeState.LowerCaseTitle(self.c.Tr)) if cursorInSubstring(opts.X, upstreamStatus+" ", workingTreeStatus) { return self.c.Helpers().MergeAndRebase.CreateRebaseOptionsMenu() } diff --git a/pkg/gui/presentation/status.go b/pkg/gui/presentation/status.go index 3cc137b35..3c0fe068a 100644 --- a/pkg/gui/presentation/status.go +++ b/pkg/gui/presentation/status.go @@ -32,7 +32,7 @@ func FormatStatus( } if workingTreeState != enums.WORKING_TREE_STATE_NONE { - status += style.FgYellow.Sprintf("(%s) ", FormatWorkingTreeStateLower(tr, workingTreeState)) + status += style.FgYellow.Sprintf("(%s) ", workingTreeState.LowerCaseTitle(tr)) } name := GetBranchTextStyle(currentBranch.Name).Sprint(currentBranch.Name) diff --git a/pkg/gui/presentation/working_tree.go b/pkg/gui/presentation/working_tree.go deleted file mode 100644 index 1e2eadd1a..000000000 --- a/pkg/gui/presentation/working_tree.go +++ /dev/null @@ -1,30 +0,0 @@ -package presentation - -import ( - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" - "github.com/jesseduffield/lazygit/pkg/i18n" -) - -func FormatWorkingTreeStateTitle(tr *i18n.TranslationSet, workingTreeState enums.WorkingTreeState) string { - switch workingTreeState { - case enums.WORKING_TREE_STATE_REBASING: - return tr.RebasingStatus - case enums.WORKING_TREE_STATE_MERGING: - return tr.MergingStatus - default: - // should never actually display this - return "none" - } -} - -func FormatWorkingTreeStateLower(tr *i18n.TranslationSet, workingTreeState enums.WorkingTreeState) string { - switch workingTreeState { - case enums.WORKING_TREE_STATE_REBASING: - return tr.LowercaseRebasingStatus - case enums.WORKING_TREE_STATE_MERGING: - return tr.LowercaseMergingStatus - default: - // should never actually display this - return "none" - } -} From e1eb95b2b3492819d9732cae05bbf0e22f473914 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 31 Mar 2025 19:21:40 +0200 Subject: [PATCH 07/13] Add test for a special situation in rebases involving empty commits The situation is that you perform a rebase, and one of the commits becomes empty during the rebase, in a way that couldn't be predicted before the rebase started. To explain: git rebase has some logic where it immediately discards commits if it can tell that they already exist in the branch being rebased onto; it does this by looking at the patch ids of the commits to work out which commits already exist upstream. But for those commits that become empty even though there isn't a corresponding commit upstream, git stops with an error, and lazygit detects this (in CheckMergeOrRebaseWithRefreshOptions) and automatically continues the rebase. This all works fine; I'm adding this test because I almost broke this during development of this branch, so I'm adding it to guard against regressions. --- .../rebase_with_commit_that_becomes_empty.go | 45 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 46 insertions(+) create mode 100644 pkg/integration/tests/interactive_rebase/rebase_with_commit_that_becomes_empty.go diff --git a/pkg/integration/tests/interactive_rebase/rebase_with_commit_that_becomes_empty.go b/pkg/integration/tests/interactive_rebase/rebase_with_commit_that_becomes_empty.go new file mode 100644 index 000000000..e9b743856 --- /dev/null +++ b/pkg/integration/tests/interactive_rebase/rebase_with_commit_that_becomes_empty.go @@ -0,0 +1,45 @@ +package interactive_rebase + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RebaseWithCommitThatBecomesEmpty = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Performs a rebase involving a commit that becomes empty during the rebase, and gets dropped.", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.EmptyCommit("initial commit") + // It is important that we create two separate commits for the two + // changes to the file, but only one commit for the same changes on our + // branch; otherwise, the commit would be discarded at the start of the + // rebase already. + shell.CreateFileAndAdd("file", "change 1\n") + shell.Commit("master change 1") + shell.UpdateFileAndAdd("file", "change 1\nchange 2\n") + shell.Commit("master change 2") + shell.NewBranchFrom("branch", "HEAD^^") + shell.CreateFileAndAdd("file", "change 1\nchange 2\n") + shell.Commit("branch change") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + NavigateToLine(Contains("master")). + Press(keys.Branches.RebaseBranch) + + t.ExpectPopup().Menu(). + Title(Equals("Rebase 'branch'")). + Select(Contains("Simple rebase")). + Confirm() + + t.Views().Commits(). + Lines( + Contains("master change 2"), + Contains("master change 1"), + Contains("initial commit"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index cbcf4ab5a..f381f8bea 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -260,6 +260,7 @@ var tests = []*components.IntegrationTest{ interactive_rebase.QuickStartKeepSelection, interactive_rebase.QuickStartKeepSelectionRange, interactive_rebase.Rebase, + interactive_rebase.RebaseWithCommitThatBecomesEmpty, interactive_rebase.RewordCommitWithEditorAndFail, interactive_rebase.RewordFirstCommit, interactive_rebase.RewordLastCommit, From 8af8f7754b027834e39b098df8ecf28e13e082e7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 7 Apr 2025 10:44:11 +0200 Subject: [PATCH 08/13] Move types/enums/enums.go to working_tree_state.go It looks like enums.go was supposed to be file that collects a bunch of enums, but actually there's only one in there, and since it has methods, it deserves to be in a file of its own, named after the type. --- pkg/commands/git_commands/commit_loader.go | 5 ++--- pkg/commands/git_commands/commit_loader_test.go | 5 ++--- pkg/commands/git_commands/patch.go | 5 ++--- pkg/commands/git_commands/status.go | 10 +++++----- .../enums/enums.go => models/working_tree_state.go} | 2 +- pkg/gui/context/local_commits_context.go | 3 +-- .../controllers/custom_patch_options_menu_action.go | 6 +++--- .../controllers/helpers/merge_and_rebase_helper.go | 11 +++++------ pkg/gui/controllers/helpers/mode_helper.go | 4 ++-- pkg/gui/controllers/helpers/patch_building_helper.go | 4 ++-- pkg/gui/controllers/helpers/refresh_helper.go | 3 +-- pkg/gui/controllers/local_commits_controller.go | 5 ++--- pkg/gui/controllers/status_controller.go | 4 ++-- pkg/gui/controllers/undo_controller.go | 6 +++--- pkg/gui/presentation/status.go | 5 ++--- pkg/gui/types/common.go | 3 +-- 16 files changed, 36 insertions(+), 45 deletions(-) rename pkg/commands/{types/enums/enums.go => models/working_tree_state.go} (98%) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 08ee08313..fdaf8d9ff 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -13,7 +13,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/common" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" @@ -31,7 +30,7 @@ type CommitLoader struct { *common.Common cmd oscommands.ICmdObjBuilder - getWorkingTreeState func() enums.WorkingTreeState + getWorkingTreeState func() models.WorkingTreeState readFile func(filename string) ([]byte, error) walkFiles func(root string, fn filepath.WalkFunc) error dotGitDir string @@ -42,7 +41,7 @@ type CommitLoader struct { func NewCommitLoader( cmn *common.Common, cmd oscommands.ICmdObjBuilder, - getWorkingTreeState func() enums.WorkingTreeState, + getWorkingTreeState func() models.WorkingTreeState, gitCommon *GitCommon, ) *CommitLoader { return &CommitLoader{ diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 9aac43d11..0641f2476 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -8,7 +8,6 @@ import ( "github.com/go-errors/errors" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/stefanhaller/git-todo-parser/todo" @@ -304,7 +303,7 @@ func TestGetCommits(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: cmd, - getWorkingTreeState: func() enums.WorkingTreeState { return enums.WORKING_TREE_STATE_NONE }, + getWorkingTreeState: func() models.WorkingTreeState { return models.WORKING_TREE_STATE_NONE }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil @@ -487,7 +486,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)), - getWorkingTreeState: func() enums.WorkingTreeState { return enums.WORKING_TREE_STATE_REBASING }, + getWorkingTreeState: func() models.WorkingTreeState { return models.WORKING_TREE_STATE_REBASING }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index daa1030d4..a76121186 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -8,7 +8,6 @@ import ( "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" "github.com/stefanhaller/git-todo-parser/todo" ) @@ -229,7 +228,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId } if err := self.ApplyCustomPatch(true, true); err != nil { - if self.status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { + if self.status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { _ = self.rebase.AbortRebase() } return err @@ -253,7 +252,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId self.rebase.onSuccessfulContinue = func() error { // add patches to index if err := self.ApplyPatch(patch, ApplyPatchOpts{Index: true, ThreeWay: true}); err != nil { - if self.status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { + if self.status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { _ = self.rebase.AbortRebase() } return err diff --git a/pkg/commands/git_commands/status.go b/pkg/commands/git_commands/status.go index ba4fdb679..a0d68e241 100644 --- a/pkg/commands/git_commands/status.go +++ b/pkg/commands/git_commands/status.go @@ -5,7 +5,7 @@ import ( "path/filepath" "strings" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" + "github.com/jesseduffield/lazygit/pkg/commands/models" ) type StatusCommands struct { @@ -20,16 +20,16 @@ func NewStatusCommands( } } -func (self *StatusCommands) WorkingTreeState() enums.WorkingTreeState { +func (self *StatusCommands) WorkingTreeState() models.WorkingTreeState { isInRebase, _ := self.IsInRebase() if isInRebase { - return enums.WORKING_TREE_STATE_REBASING + return models.WORKING_TREE_STATE_REBASING } merging, _ := self.IsInMergeState() if merging { - return enums.WORKING_TREE_STATE_MERGING + return models.WORKING_TREE_STATE_MERGING } - return enums.WORKING_TREE_STATE_NONE + return models.WORKING_TREE_STATE_NONE } func (self *StatusCommands) IsBareRepo() bool { diff --git a/pkg/commands/types/enums/enums.go b/pkg/commands/models/working_tree_state.go similarity index 98% rename from pkg/commands/types/enums/enums.go rename to pkg/commands/models/working_tree_state.go index 09fb1211a..bb8f6536b 100644 --- a/pkg/commands/types/enums/enums.go +++ b/pkg/commands/models/working_tree_state.go @@ -1,4 +1,4 @@ -package enums +package models import "github.com/jesseduffield/lazygit/pkg/i18n" diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 9f6c2f5f1..a286241da 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -7,7 +7,6 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/commands/models" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/gui/presentation" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/samber/lo" @@ -41,7 +40,7 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext { } } - showYouAreHereLabel := c.Model().WorkingTreeStateAtLastCommitRefresh == enums.WORKING_TREE_STATE_REBASING + showYouAreHereLabel := c.Model().WorkingTreeStateAtLastCommitRefresh == models.WORKING_TREE_STATE_REBASING hasRebaseUpdateRefsConfig := c.Git().Config.GetRebaseUpdateRefs() return presentation.GetCommitListDisplayStrings( diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index 5208cfb15..ca06d5fd2 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -5,7 +5,7 @@ import ( "fmt" "github.com/jesseduffield/gocui" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -44,7 +44,7 @@ func (self *CustomPatchOptionsMenuAction) Call() error { }, } - if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_NONE { + if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_NONE { menuItems = append(menuItems, []*types.MenuItem{ { Label: fmt.Sprintf(self.c.Tr.RemovePatchFromOriginalCommit, self.c.Git().Patch.PatchBuilder.To), @@ -115,7 +115,7 @@ func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() int { } func (self *CustomPatchOptionsMenuAction) validateNormalWorkingTreeState() (bool, error) { - if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE { + if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE { return false, errors.New(self.c.Tr.CantPatchWhileRebasingError) } return true, nil diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 55a322292..dbb10fac6 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -10,7 +10,6 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" @@ -51,7 +50,7 @@ func (self *MergeAndRebaseHelper) CreateRebaseOptionsMenu() error { {option: REBASE_OPTION_ABORT, key: 'a'}, } - if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { + if self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { options = append(options, optionAndKey{ option: REBASE_OPTION_SKIP, key: 's', }) @@ -78,12 +77,12 @@ func (self *MergeAndRebaseHelper) ContinueRebase() error { func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { status := self.c.Git().Status.WorkingTreeState() - if status != enums.WORKING_TREE_STATE_MERGING && status != enums.WORKING_TREE_STATE_REBASING { + if status != models.WORKING_TREE_STATE_MERGING && status != models.WORKING_TREE_STATE_REBASING { return errors.New(self.c.Tr.NotMergingOrRebasing) } self.c.LogAction(fmt.Sprintf("Merge/Rebase: %s", command)) - if status == enums.WORKING_TREE_STATE_REBASING { + if status == models.WORKING_TREE_STATE_REBASING { todoFile, err := os.ReadFile( filepath.Join(self.c.Git().RepoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"), ) @@ -102,10 +101,10 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { // we should end up with a command like 'git merge --continue' // it's impossible for a rebase to require a commit so we'll use a subprocess only if it's a merge - needsSubprocess := (status == enums.WORKING_TREE_STATE_MERGING && command != REBASE_OPTION_ABORT && self.c.UserConfig().Git.Merging.ManualCommit) || + needsSubprocess := (status == models.WORKING_TREE_STATE_MERGING && command != REBASE_OPTION_ABORT && self.c.UserConfig().Git.Merging.ManualCommit) || // but we'll also use a subprocess if we have exec todos; those are likely to be lengthy build // tasks whose output the user will want to see in the terminal - (status == enums.WORKING_TREE_STATE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos()) + (status == models.WORKING_TREE_STATE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos()) if needsSubprocess { // TODO: see if we should be calling more of the code from self.Git.Rebase.GenericMergeOrRebaseAction diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index 0c964d2de..b5b3df7ac 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -4,7 +4,7 @@ import ( "fmt" "strings" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/samber/lo" @@ -115,7 +115,7 @@ func (self *ModeHelper) Statuses() []ModeStatus { }, { IsActive: func() bool { - return !self.suppressRebasingMode && self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE + return !self.suppressRebasingMode && self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE }, Description: func() string { workingTreeState := self.c.Git().Status.WorkingTreeState() diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index 2deb19d9c..700cfff6a 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/pkg/gui/controllers/helpers/patch_building_helper.go @@ -3,8 +3,8 @@ package helpers import ( "errors" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/patch" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/gui/patch_exploring" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -22,7 +22,7 @@ func NewPatchBuildingHelper( } func (self *PatchBuildingHelper) ValidateNormalWorkingTreeState() (bool, error) { - if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE { + if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE { return false, errors.New(self.c.Tr.CantPatchWhileRebasingError) } return true, nil diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 9e6dff1dd..a05860324 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -9,7 +9,6 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/filetree" "github.com/jesseduffield/lazygit/pkg/gui/mergeconflicts" @@ -583,7 +582,7 @@ func (self *RefreshHelper) refreshStateFiles() error { } } - if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE && conflictFileCount == 0 && prevConflictFileCount > 0 { + if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE && conflictFileCount == 0 && prevConflictFileCount > 0 { self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 4e7f652e0..7487d3def 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -8,7 +8,6 @@ import ( "github.com/jesseduffield/gocui" "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/context/traits" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" @@ -682,7 +681,7 @@ func (self *LocalCommitsController) rewordEnabled(commit *models.Commit) *types. } func (self *LocalCommitsController) isRebasing() bool { - return self.c.Model().WorkingTreeStateAtLastCommitRefresh != enums.WORKING_TREE_STATE_NONE + return self.c.Model().WorkingTreeStateAtLastCommitRefresh != models.WORKING_TREE_STATE_NONE } func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, startIdx int, endIdx int) error { @@ -976,7 +975,7 @@ func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCo return nil } - if self.c.Git().Status.WorkingTreeState() != enums.WORKING_TREE_STATE_NONE { + if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE { // Can't move commits while rebasing return nil } diff --git a/pkg/gui/controllers/status_controller.go b/pkg/gui/controllers/status_controller.go index 31b264e17..73d36c0f7 100644 --- a/pkg/gui/controllers/status_controller.go +++ b/pkg/gui/controllers/status_controller.go @@ -7,7 +7,7 @@ import ( "time" "github.com/jesseduffield/gocui" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/constants" "github.com/jesseduffield/lazygit/pkg/gui/presentation" "github.com/jesseduffield/lazygit/pkg/gui/style" @@ -110,7 +110,7 @@ func (self *StatusController) onClick(opts gocui.ViewMouseBindingOpts) error { repoName := self.c.Git().RepoPaths.RepoName() workingTreeState := self.c.Git().Status.WorkingTreeState() switch workingTreeState { - case enums.WORKING_TREE_STATE_REBASING, enums.WORKING_TREE_STATE_MERGING: + case models.WORKING_TREE_STATE_REBASING, models.WORKING_TREE_STATE_MERGING: workingTreeStatus := fmt.Sprintf("(%s)", workingTreeState.LowerCaseTitle(self.c.Tr)) if cursorInSubstring(opts.X, upstreamStatus+" ", workingTreeStatus) { return self.c.Helpers().MergeAndRebase.CreateRebaseOptionsMenu() diff --git a/pkg/gui/controllers/undo_controller.go b/pkg/gui/controllers/undo_controller.go index b00eb802f..1150975bc 100644 --- a/pkg/gui/controllers/undo_controller.go +++ b/pkg/gui/controllers/undo_controller.go @@ -5,7 +5,7 @@ import ( "fmt" "github.com/jesseduffield/gocui" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" + "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -78,7 +78,7 @@ func (self *UndoController) reflogUndo() error { undoEnvVars := []string{"GIT_REFLOG_ACTION=[lazygit undo]"} undoingStatus := self.c.Tr.UndoingStatus - if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { + if self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { return errors.New(self.c.Tr.CantUndoWhileRebasing) } @@ -142,7 +142,7 @@ func (self *UndoController) reflogRedo() error { redoEnvVars := []string{"GIT_REFLOG_ACTION=[lazygit redo]"} redoingStatus := self.c.Tr.RedoingStatus - if self.c.Git().Status.WorkingTreeState() == enums.WORKING_TREE_STATE_REBASING { + if self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { return errors.New(self.c.Tr.CantRedoWhileRebasing) } diff --git a/pkg/gui/presentation/status.go b/pkg/gui/presentation/status.go index 3c0fe068a..e09aab5f3 100644 --- a/pkg/gui/presentation/status.go +++ b/pkg/gui/presentation/status.go @@ -5,7 +5,6 @@ import ( "time" "github.com/jesseduffield/lazygit/pkg/commands/models" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/gui/presentation/icons" "github.com/jesseduffield/lazygit/pkg/gui/style" @@ -18,7 +17,7 @@ func FormatStatus( currentBranch *models.Branch, itemOperation types.ItemOperation, linkedWorktreeName string, - workingTreeState enums.WorkingTreeState, + workingTreeState models.WorkingTreeState, tr *i18n.TranslationSet, userConfig *config.UserConfig, ) string { @@ -31,7 +30,7 @@ func FormatStatus( } } - if workingTreeState != enums.WORKING_TREE_STATE_NONE { + if workingTreeState != models.WORKING_TREE_STATE_NONE { status += style.FgYellow.Sprintf("(%s) ", workingTreeState.LowerCaseTitle(tr)) } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index afa0bf2a4..e01c66423 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -6,7 +6,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/git_commands" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" - "github.com/jesseduffield/lazygit/pkg/commands/types/enums" "github.com/jesseduffield/lazygit/pkg/common" "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/utils" @@ -288,7 +287,7 @@ type Model struct { ReflogCommits []*models.Commit BisectInfo *git_commands.BisectInfo - WorkingTreeStateAtLastCommitRefresh enums.WorkingTreeState + WorkingTreeStateAtLastCommitRefresh models.WorkingTreeState RemoteBranches []*models.RemoteBranch Tags []*models.Tag From 542525743c57801c5efc834e2228c0eb53d255f9 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Mon, 10 Jun 2024 17:54:04 +0200 Subject: [PATCH 09/13] Make WorkingTreeState a struct, and add cherry-picking and reverting states --- pkg/commands/git_commands/commit_loader.go | 2 +- .../git_commands/commit_loader_test.go | 4 +- pkg/commands/git_commands/patch.go | 4 +- pkg/commands/git_commands/status.go | 51 +++++-- pkg/commands/models/working_tree_state.go | 137 ++++++++++++------ pkg/gui/context/local_commits_context.go | 2 +- .../custom_patch_options_menu_action.go | 5 +- .../helpers/merge_and_rebase_helper.go | 11 +- pkg/gui/controllers/helpers/mode_helper.go | 3 +- .../helpers/patch_building_helper.go | 3 +- pkg/gui/controllers/helpers/refresh_helper.go | 2 +- .../controllers/local_commits_controller.go | 4 +- pkg/gui/controllers/status_controller.go | 10 +- pkg/gui/controllers/undo_controller.go | 5 +- pkg/gui/options_map.go | 10 +- pkg/gui/presentation/status.go | 2 +- pkg/i18n/english.go | 16 +- 17 files changed, 176 insertions(+), 95 deletions(-) diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index fdaf8d9ff..22050b384 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -171,7 +171,7 @@ func (self *CommitLoader) MergeRebasingCommits(commits []*models.Commit) ([]*mod } } - if !self.getWorkingTreeState().IsRebasing() { + if !self.getWorkingTreeState().Rebasing { // not in rebase mode so return original commits return result, nil } diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index 0641f2476..f7b075134 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -303,7 +303,7 @@ func TestGetCommits(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: cmd, - getWorkingTreeState: func() models.WorkingTreeState { return models.WORKING_TREE_STATE_NONE }, + getWorkingTreeState: func() models.WorkingTreeState { return models.WorkingTreeState{} }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil @@ -486,7 +486,7 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { builder := &CommitLoader{ Common: common, cmd: oscommands.NewDummyCmdObjBuilder(oscommands.NewFakeRunner(t)), - getWorkingTreeState: func() models.WorkingTreeState { return models.WORKING_TREE_STATE_REBASING }, + getWorkingTreeState: func() models.WorkingTreeState { return models.WorkingTreeState{Rebasing: true} }, dotGitDir: ".git", readFile: func(filename string) ([]byte, error) { return []byte(""), nil diff --git a/pkg/commands/git_commands/patch.go b/pkg/commands/git_commands/patch.go index a76121186..77563ab88 100644 --- a/pkg/commands/git_commands/patch.go +++ b/pkg/commands/git_commands/patch.go @@ -228,7 +228,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId } if err := self.ApplyCustomPatch(true, true); err != nil { - if self.status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { + if self.status.WorkingTreeState().Rebasing { _ = self.rebase.AbortRebase() } return err @@ -252,7 +252,7 @@ func (self *PatchCommands) MovePatchIntoIndex(commits []*models.Commit, commitId self.rebase.onSuccessfulContinue = func() error { // add patches to index if err := self.ApplyPatch(patch, ApplyPatchOpts{Index: true, ThreeWay: true}); err != nil { - if self.status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { + if self.status.WorkingTreeState().Rebasing { _ = self.rebase.AbortRebase() } return err diff --git a/pkg/commands/git_commands/status.go b/pkg/commands/git_commands/status.go index a0d68e241..ff09e22bc 100644 --- a/pkg/commands/git_commands/status.go +++ b/pkg/commands/git_commands/status.go @@ -21,15 +21,12 @@ func NewStatusCommands( } func (self *StatusCommands) WorkingTreeState() models.WorkingTreeState { - isInRebase, _ := self.IsInRebase() - if isInRebase { - return models.WORKING_TREE_STATE_REBASING - } - merging, _ := self.IsInMergeState() - if merging { - return models.WORKING_TREE_STATE_MERGING - } - return models.WORKING_TREE_STATE_NONE + result := models.WorkingTreeState{} + result.Rebasing, _ = self.IsInRebase() + result.Merging, _ = self.IsInMergeState() + result.CherryPicking, _ = self.IsInCherryPick() + result.Reverting, _ = self.IsInRevert() + return result } func (self *StatusCommands) IsBareRepo() bool { @@ -49,6 +46,42 @@ func (self *StatusCommands) IsInMergeState() (bool, error) { return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "MERGE_HEAD")) } +func (self *StatusCommands) IsInCherryPick() (bool, error) { + exists, err := self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD")) + if err != nil || !exists { + return exists, err + } + // Sometimes, CHERRY_PICK_HEAD is present during rebases even if no + // cherry-pick is in progress. I suppose this is because rebase used to be + // implemented as a series of cherry-picks, so this could be remnants of + // code that is shared between cherry-pick and rebase, or something. The way + // to tell if this is the case is to check for the presence of the + // stopped-sha file, which records the sha of the last pick that was + // executed before the rebase stopped, and seeing if the sha in that file is + // the same as the one in CHERRY_PICK_HEAD. + cherryPickHead, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "CHERRY_PICK_HEAD")) + if err != nil { + return false, err + } + stoppedSha, err := os.ReadFile(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "rebase-merge", "stopped-sha")) + if err != nil { + // If we get an error we assume the file doesn't exist + return true, nil + } + cherryPickHeadStr := strings.TrimSpace(string(cherryPickHead)) + stoppedShaStr := strings.TrimSpace(string(stoppedSha)) + // Need to use HasPrefix here because the cherry-pick HEAD is a full sha1, + // but stopped-sha is an abbreviated sha1 + if strings.HasPrefix(cherryPickHeadStr, stoppedShaStr) { + return false, nil + } + return true, nil +} + +func (self *StatusCommands) IsInRevert() (bool, error) { + return self.os.FileExists(filepath.Join(self.repoPaths.WorktreeGitDirPath(), "REVERT_HEAD")) +} + // Full ref (e.g. "refs/heads/mybranch") of the branch that is currently // being rebased, or empty string when we're not in a rebase func (self *StatusCommands) BranchBeingRebased() string { diff --git a/pkg/commands/models/working_tree_state.go b/pkg/commands/models/working_tree_state.go index bb8f6536b..3e7bdbf1b 100644 --- a/pkg/commands/models/working_tree_state.go +++ b/pkg/commands/models/working_tree_state.go @@ -2,62 +2,111 @@ package models import "github.com/jesseduffield/lazygit/pkg/i18n" -type WorkingTreeState int - -const ( - // this means we're neither rebasing nor merging - WORKING_TREE_STATE_NONE WorkingTreeState = iota - WORKING_TREE_STATE_REBASING - WORKING_TREE_STATE_MERGING -) - -func (self WorkingTreeState) IsMerging() bool { - return self == WORKING_TREE_STATE_MERGING +// The state of the working tree. Several of these can be true at once. +// In particular, the concrete multi-state combinations that can occur in +// practice are Rebasing+CherryPicking, and Rebasing+Reverting. Theoretically, I +// guess Rebasing+Merging could also happen, but it probably won't in practice. +type WorkingTreeState struct { + Rebasing bool + Merging bool + CherryPicking bool + Reverting bool } -func (self WorkingTreeState) IsRebasing() bool { - return self == WORKING_TREE_STATE_REBASING +func (self WorkingTreeState) Any() bool { + return self.Rebasing || self.Merging || self.CherryPicking || self.Reverting +} + +func (self WorkingTreeState) None() bool { + return !self.Any() +} + +type EffectiveWorkingTreeState int + +const ( + // this means we're neither rebasing nor merging, cherry-picking, or reverting + WORKING_TREE_STATE_NONE EffectiveWorkingTreeState = iota + WORKING_TREE_STATE_REBASING + WORKING_TREE_STATE_MERGING + WORKING_TREE_STATE_CHERRY_PICKING + WORKING_TREE_STATE_REVERTING +) + +// Effective returns the "current" state; if several states are true at once, +// this is the one that should be displayed in status views, and it's the one +// that the user can continue or abort. +// +// As an example, if you are stopped in an interactive rebase, and then you +// perform a cherry-pick, and the cherry-pick conflicts, then both +// WorkingTreeState.Rebasing and WorkingTreeState.CherryPicking are true. +// The effective state is cherry-picking, because that's the one you can +// continue or abort. It is not possible to continue the rebase without first +// aborting the cherry-pick. +func (self WorkingTreeState) Effective() EffectiveWorkingTreeState { + if self.Reverting { + return WORKING_TREE_STATE_REVERTING + } + if self.CherryPicking { + return WORKING_TREE_STATE_CHERRY_PICKING + } + if self.Merging { + return WORKING_TREE_STATE_MERGING + } + if self.Rebasing { + return WORKING_TREE_STATE_REBASING + } + return WORKING_TREE_STATE_NONE } func (self WorkingTreeState) Title(tr *i18n.TranslationSet) string { - switch self { - case WORKING_TREE_STATE_REBASING: - return tr.RebasingStatus - case WORKING_TREE_STATE_MERGING: - return tr.MergingStatus - default: - // should never actually display this - return "none" - } + return map[EffectiveWorkingTreeState]string{ + WORKING_TREE_STATE_REBASING: tr.RebasingStatus, + WORKING_TREE_STATE_MERGING: tr.MergingStatus, + WORKING_TREE_STATE_CHERRY_PICKING: tr.CherryPickingStatus, + WORKING_TREE_STATE_REVERTING: tr.RevertingStatus, + }[self.Effective()] } func (self WorkingTreeState) LowerCaseTitle(tr *i18n.TranslationSet) string { - switch self { - case WORKING_TREE_STATE_REBASING: - return tr.LowercaseRebasingStatus - case WORKING_TREE_STATE_MERGING: - return tr.LowercaseMergingStatus - default: - // should never actually display this - return "none" - } + return map[EffectiveWorkingTreeState]string{ + WORKING_TREE_STATE_REBASING: tr.LowercaseRebasingStatus, + WORKING_TREE_STATE_MERGING: tr.LowercaseMergingStatus, + WORKING_TREE_STATE_CHERRY_PICKING: tr.LowercaseCherryPickingStatus, + WORKING_TREE_STATE_REVERTING: tr.LowercaseRevertingStatus, + }[self.Effective()] } func (self WorkingTreeState) OptionsMenuTitle(tr *i18n.TranslationSet) string { - if self == WORKING_TREE_STATE_MERGING { - return tr.MergeOptionsTitle - } - return tr.RebaseOptionsTitle + return map[EffectiveWorkingTreeState]string{ + WORKING_TREE_STATE_REBASING: tr.RebaseOptionsTitle, + WORKING_TREE_STATE_MERGING: tr.MergeOptionsTitle, + WORKING_TREE_STATE_CHERRY_PICKING: tr.CherryPickOptionsTitle, + WORKING_TREE_STATE_REVERTING: tr.RevertOptionsTitle, + }[self.Effective()] +} + +func (self WorkingTreeState) OptionsMapTitle(tr *i18n.TranslationSet) string { + return map[EffectiveWorkingTreeState]string{ + WORKING_TREE_STATE_REBASING: tr.ViewRebaseOptions, + WORKING_TREE_STATE_MERGING: tr.ViewMergeOptions, + WORKING_TREE_STATE_CHERRY_PICKING: tr.ViewCherryPickOptions, + WORKING_TREE_STATE_REVERTING: tr.ViewRevertOptions, + }[self.Effective()] } func (self WorkingTreeState) CommandName() string { - switch self { - case WORKING_TREE_STATE_MERGING: - return "merge" - case WORKING_TREE_STATE_REBASING: - return "rebase" - default: - // shouldn't be possible to land here - return "" - } + return map[EffectiveWorkingTreeState]string{ + WORKING_TREE_STATE_REBASING: "rebase", + WORKING_TREE_STATE_MERGING: "merge", + WORKING_TREE_STATE_CHERRY_PICKING: "cherry-pick", + WORKING_TREE_STATE_REVERTING: "revert", + }[self.Effective()] +} + +func (self WorkingTreeState) CanShowTodos() bool { + return self.Rebasing || self.CherryPicking || self.Reverting +} + +func (self WorkingTreeState) CanSkip() bool { + return self.Rebasing || self.CherryPicking || self.Reverting } diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index a286241da..f40f7f0ff 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -40,7 +40,7 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext { } } - showYouAreHereLabel := c.Model().WorkingTreeStateAtLastCommitRefresh == models.WORKING_TREE_STATE_REBASING + showYouAreHereLabel := c.Model().WorkingTreeStateAtLastCommitRefresh.CanShowTodos() hasRebaseUpdateRefsConfig := c.Git().Config.GetRebaseUpdateRefs() return presentation.GetCommitListDisplayStrings( diff --git a/pkg/gui/controllers/custom_patch_options_menu_action.go b/pkg/gui/controllers/custom_patch_options_menu_action.go index ca06d5fd2..f231152db 100644 --- a/pkg/gui/controllers/custom_patch_options_menu_action.go +++ b/pkg/gui/controllers/custom_patch_options_menu_action.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/jesseduffield/gocui" - "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/controllers/helpers" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -44,7 +43,7 @@ func (self *CustomPatchOptionsMenuAction) Call() error { }, } - if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_NONE { + if self.c.Git().Patch.PatchBuilder.CanRebase && self.c.Git().Status.WorkingTreeState().None() { menuItems = append(menuItems, []*types.MenuItem{ { Label: fmt.Sprintf(self.c.Tr.RemovePatchFromOriginalCommit, self.c.Git().Patch.PatchBuilder.To), @@ -115,7 +114,7 @@ func (self *CustomPatchOptionsMenuAction) getPatchCommitIndex() int { } func (self *CustomPatchOptionsMenuAction) validateNormalWorkingTreeState() (bool, error) { - if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE { + if self.c.Git().Status.WorkingTreeState().Any() { return false, errors.New(self.c.Tr.CantPatchWhileRebasingError) } return true, nil diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index dbb10fac6..f8c328958 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -50,7 +50,7 @@ func (self *MergeAndRebaseHelper) CreateRebaseOptionsMenu() error { {option: REBASE_OPTION_ABORT, key: 'a'}, } - if self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { + if self.c.Git().Status.WorkingTreeState().CanSkip() { options = append(options, optionAndKey{ option: REBASE_OPTION_SKIP, key: 's', }) @@ -77,12 +77,13 @@ func (self *MergeAndRebaseHelper) ContinueRebase() error { func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { status := self.c.Git().Status.WorkingTreeState() - if status != models.WORKING_TREE_STATE_MERGING && status != models.WORKING_TREE_STATE_REBASING { + if status.None() { return errors.New(self.c.Tr.NotMergingOrRebasing) } self.c.LogAction(fmt.Sprintf("Merge/Rebase: %s", command)) - if status == models.WORKING_TREE_STATE_REBASING { + effectiveStatus := status.Effective() + if effectiveStatus == models.WORKING_TREE_STATE_REBASING { todoFile, err := os.ReadFile( filepath.Join(self.c.Git().RepoPaths.WorktreeGitDirPath(), "rebase-merge/git-rebase-todo"), ) @@ -101,10 +102,10 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { // we should end up with a command like 'git merge --continue' // it's impossible for a rebase to require a commit so we'll use a subprocess only if it's a merge - needsSubprocess := (status == models.WORKING_TREE_STATE_MERGING && command != REBASE_OPTION_ABORT && self.c.UserConfig().Git.Merging.ManualCommit) || + needsSubprocess := (effectiveStatus == models.WORKING_TREE_STATE_MERGING && command != REBASE_OPTION_ABORT && self.c.UserConfig().Git.Merging.ManualCommit) || // but we'll also use a subprocess if we have exec todos; those are likely to be lengthy build // tasks whose output the user will want to see in the terminal - (status == models.WORKING_TREE_STATE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos()) + (effectiveStatus == models.WORKING_TREE_STATE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos()) if needsSubprocess { // TODO: see if we should be calling more of the code from self.Git.Rebase.GenericMergeOrRebaseAction diff --git a/pkg/gui/controllers/helpers/mode_helper.go b/pkg/gui/controllers/helpers/mode_helper.go index b5b3df7ac..8c6b46eb6 100644 --- a/pkg/gui/controllers/helpers/mode_helper.go +++ b/pkg/gui/controllers/helpers/mode_helper.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/samber/lo" @@ -115,7 +114,7 @@ func (self *ModeHelper) Statuses() []ModeStatus { }, { IsActive: func() bool { - return !self.suppressRebasingMode && self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE + return !self.suppressRebasingMode && self.c.Git().Status.WorkingTreeState().Any() }, Description: func() string { workingTreeState := self.c.Git().Status.WorkingTreeState() diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index 700cfff6a..73d0fb608 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/pkg/gui/controllers/helpers/patch_building_helper.go @@ -3,7 +3,6 @@ package helpers import ( "errors" - "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/patch" "github.com/jesseduffield/lazygit/pkg/gui/patch_exploring" "github.com/jesseduffield/lazygit/pkg/gui/types" @@ -22,7 +21,7 @@ func NewPatchBuildingHelper( } func (self *PatchBuildingHelper) ValidateNormalWorkingTreeState() (bool, error) { - if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE { + if self.c.Git().Status.WorkingTreeState().Any() { return false, errors.New(self.c.Tr.CantPatchWhileRebasingError) } return true, nil diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index a05860324..93ad110f3 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -582,7 +582,7 @@ func (self *RefreshHelper) refreshStateFiles() error { } } - if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE && conflictFileCount == 0 && prevConflictFileCount > 0 { + if self.c.Git().Status.WorkingTreeState().Any() && conflictFileCount == 0 && prevConflictFileCount > 0 { self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() }) } diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 7487d3def..260f42cf7 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -681,7 +681,7 @@ func (self *LocalCommitsController) rewordEnabled(commit *models.Commit) *types. } func (self *LocalCommitsController) isRebasing() bool { - return self.c.Model().WorkingTreeStateAtLastCommitRefresh != models.WORKING_TREE_STATE_NONE + return self.c.Model().WorkingTreeStateAtLastCommitRefresh.Any() } func (self *LocalCommitsController) moveDown(selectedCommits []*models.Commit, startIdx int, endIdx int) error { @@ -975,7 +975,7 @@ func (self *LocalCommitsController) moveFixupCommitToOwnerStackedBranch(targetCo return nil } - if self.c.Git().Status.WorkingTreeState() != models.WORKING_TREE_STATE_NONE { + if self.c.Git().Status.WorkingTreeState().Any() { // Can't move commits while rebasing return nil } diff --git a/pkg/gui/controllers/status_controller.go b/pkg/gui/controllers/status_controller.go index 73d36c0f7..8d173f46c 100644 --- a/pkg/gui/controllers/status_controller.go +++ b/pkg/gui/controllers/status_controller.go @@ -7,7 +7,6 @@ import ( "time" "github.com/jesseduffield/gocui" - "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/constants" "github.com/jesseduffield/lazygit/pkg/gui/presentation" "github.com/jesseduffield/lazygit/pkg/gui/style" @@ -109,8 +108,7 @@ func (self *StatusController) onClick(opts gocui.ViewMouseBindingOpts) error { upstreamStatus := utils.Decolorise(presentation.BranchStatus(currentBranch, types.ItemOperationNone, self.c.Tr, time.Now(), self.c.UserConfig())) repoName := self.c.Git().RepoPaths.RepoName() workingTreeState := self.c.Git().Status.WorkingTreeState() - switch workingTreeState { - case models.WORKING_TREE_STATE_REBASING, models.WORKING_TREE_STATE_MERGING: + if workingTreeState.Any() { workingTreeStatus := fmt.Sprintf("(%s)", workingTreeState.LowerCaseTitle(self.c.Tr)) if cursorInSubstring(opts.X, upstreamStatus+" ", workingTreeStatus) { return self.c.Helpers().MergeAndRebase.CreateRebaseOptionsMenu() @@ -118,10 +116,8 @@ func (self *StatusController) onClick(opts gocui.ViewMouseBindingOpts) error { if cursorInSubstring(opts.X, upstreamStatus+" "+workingTreeStatus+" ", repoName) { return self.c.Helpers().Repos.CreateRecentReposMenu() } - default: - if cursorInSubstring(opts.X, upstreamStatus+" ", repoName) { - return self.c.Helpers().Repos.CreateRecentReposMenu() - } + } else if cursorInSubstring(opts.X, upstreamStatus+" ", repoName) { + return self.c.Helpers().Repos.CreateRecentReposMenu() } return nil diff --git a/pkg/gui/controllers/undo_controller.go b/pkg/gui/controllers/undo_controller.go index 1150975bc..9922f73d6 100644 --- a/pkg/gui/controllers/undo_controller.go +++ b/pkg/gui/controllers/undo_controller.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/jesseduffield/gocui" - "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" ) @@ -78,7 +77,7 @@ func (self *UndoController) reflogUndo() error { undoEnvVars := []string{"GIT_REFLOG_ACTION=[lazygit undo]"} undoingStatus := self.c.Tr.UndoingStatus - if self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { + if self.c.Git().Status.WorkingTreeState().Any() { return errors.New(self.c.Tr.CantUndoWhileRebasing) } @@ -142,7 +141,7 @@ func (self *UndoController) reflogRedo() error { redoEnvVars := []string{"GIT_REFLOG_ACTION=[lazygit redo]"} redoingStatus := self.c.Tr.RedoingStatus - if self.c.Git().Status.WorkingTreeState() == models.WORKING_TREE_STATE_REBASING { + if self.c.Git().Status.WorkingTreeState().Any() { return errors.New(self.c.Tr.CantRedoWhileRebasing) } diff --git a/pkg/gui/options_map.go b/pkg/gui/options_map.go index 4f9836551..347db7589 100644 --- a/pkg/gui/options_map.go +++ b/pkg/gui/options_map.go @@ -82,16 +82,10 @@ func (self *OptionsMapMgr) renderContextOptionsMap() { } // Mode-specific global keybindings - if self.c.Model().WorkingTreeStateAtLastCommitRefresh.IsRebasing() { + if state := self.c.Model().WorkingTreeStateAtLastCommitRefresh; state.Any() { optionsMap = utils.Prepend(optionsMap, bindingInfo{ key: keybindings.Label(self.c.KeybindingsOpts().Config.Universal.CreateRebaseOptionsMenu), - description: self.c.Tr.ViewRebaseOptions, - style: style.FgYellow, - }) - } else if self.c.Model().WorkingTreeStateAtLastCommitRefresh.IsMerging() { - optionsMap = utils.Prepend(optionsMap, bindingInfo{ - key: keybindings.Label(self.c.KeybindingsOpts().Config.Universal.CreateRebaseOptionsMenu), - description: self.c.Tr.ViewMergeOptions, + description: state.OptionsMapTitle(self.c.Tr), style: style.FgYellow, }) } diff --git a/pkg/gui/presentation/status.go b/pkg/gui/presentation/status.go index e09aab5f3..b636df647 100644 --- a/pkg/gui/presentation/status.go +++ b/pkg/gui/presentation/status.go @@ -30,7 +30,7 @@ func FormatStatus( } } - if workingTreeState != models.WORKING_TREE_STATE_NONE { + if workingTreeState.Any() { status += style.FgYellow.Sprintf("(%s) ", workingTreeState.LowerCaseTitle(tr)) } diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index b2deb7ab1..f3713e048 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -306,11 +306,15 @@ type TranslationSet struct { ViewMergeRebaseOptionsTooltip string ViewMergeOptions string ViewRebaseOptions string + ViewCherryPickOptions string + ViewRevertOptions string NotMergingOrRebasing string AlreadyRebasing string RecentRepos string MergeOptionsTitle string RebaseOptionsTitle string + CherryPickOptionsTitle string + RevertOptionsTitle string CommitSummaryTitle string CommitDescriptionTitle string CommitDescriptionSubTitle string @@ -392,6 +396,8 @@ type TranslationSet struct { MergingStatus string LowercaseRebasingStatus string LowercaseMergingStatus string + LowercaseCherryPickingStatus string + LowercaseRevertingStatus string AmendingStatus string CherryPickingStatus string UndoingStatus string @@ -1359,11 +1365,15 @@ func EnglishTranslationSet() *TranslationSet { ViewMergeRebaseOptionsTooltip: "View options to abort/continue/skip the current merge/rebase.", ViewMergeOptions: "View merge options", ViewRebaseOptions: "View rebase options", + ViewCherryPickOptions: "View cherry-pick options", + ViewRevertOptions: "View revert options", NotMergingOrRebasing: "You are currently neither rebasing nor merging", AlreadyRebasing: "Can't perform this action during a rebase", RecentRepos: "Recent repositories", MergeOptionsTitle: "Merge options", RebaseOptionsTitle: "Rebase options", + CherryPickOptionsTitle: "Cherry-pick options", + RevertOptionsTitle: "Revert options", CommitSummaryTitle: "Commit summary", CommitDescriptionTitle: "Commit description", CommitDescriptionSubTitle: "Press {{.togglePanelKeyBinding}} to toggle focus, {{.commitMenuKeybinding}} to open menu", @@ -1451,8 +1461,10 @@ func EnglishTranslationSet() *TranslationSet { MovingStatus: "Moving", RebasingStatus: "Rebasing", MergingStatus: "Merging", - LowercaseRebasingStatus: "rebasing", // lowercase because it shows up in parentheses - LowercaseMergingStatus: "merging", // lowercase because it shows up in parentheses + LowercaseRebasingStatus: "rebasing", // lowercase because it shows up in parentheses + LowercaseMergingStatus: "merging", // lowercase because it shows up in parentheses + LowercaseCherryPickingStatus: "cherry-picking", // lowercase because it shows up in parentheses + LowercaseRevertingStatus: "reverting", // lowercase because it shows up in parentheses AmendingStatus: "Amending", CherryPickingStatus: "Cherry-picking", UndoingStatus: "Undoing", From 362678e2ef9cd886bd42fae362b1128b21913c43 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 12 Jun 2024 14:45:45 +0200 Subject: [PATCH 10/13] Mention which command is continued in PromptToContinueRebase When you are in the middle of a rebase, and you cherry-pick a commit which conflicts, it helps to be clear on whether you are prompted to continue the cherry-pick or the rebase. --- pkg/gui/controllers/helpers/merge_and_rebase_helper.go | 2 +- pkg/i18n/english.go | 2 +- pkg/integration/components/common.go | 6 ++++-- pkg/integration/tests/branch/rebase.go | 2 +- pkg/integration/tests/branch/rebase_and_drop.go | 2 +- .../tests/branch/rebase_conflicts_fix_build_errors.go | 2 +- pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go | 2 +- pkg/integration/tests/commit/shared.go | 2 +- pkg/integration/tests/conflicts/resolve_externally.go | 2 +- pkg/integration/tests/conflicts/resolve_multiple_files.go | 2 +- .../tests/conflicts/resolve_without_trailing_lf.go | 2 +- pkg/integration/tests/file/discard_all_dir_changes.go | 2 +- pkg/integration/tests/file/discard_various_changes.go | 2 +- .../tests/file/discard_various_changes_range_select.go | 2 +- .../tests/interactive_rebase/amend_commit_with_conflict.go | 2 +- pkg/integration/tests/interactive_rebase/shared.go | 4 ++-- .../move_to_earlier_commit_from_added_file.go | 2 +- .../move_to_index_from_added_file_with_conflict.go | 2 +- .../tests/patch_building/move_to_index_with_conflict.go | 2 +- pkg/integration/tests/sync/pull_merge_conflict.go | 2 +- pkg/integration/tests/sync/pull_rebase_conflict.go | 2 +- .../tests/sync/pull_rebase_interactive_conflict.go | 2 +- .../tests/sync/pull_rebase_interactive_conflict_drop.go | 2 +- 23 files changed, 27 insertions(+), 25 deletions(-) diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index f8c328958..0e63c516f 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -226,7 +226,7 @@ func (self *MergeAndRebaseHelper) AbortMergeOrRebaseWithConfirm() error { func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { self.c.Confirm(types.ConfirmOpts{ Title: self.c.Tr.Continue, - Prompt: self.c.Tr.ConflictsResolved, + Prompt: fmt.Sprintf(self.c.Tr.ConflictsResolved, self.c.Git().Status.WorkingTreeState().CommandName()), HandleConfirm: func() error { // By the time we get here, we might have unstaged changes again, // e.g. if the user had to fix build errors after resolving the diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index f3713e048..aa413c6b5 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -1391,7 +1391,7 @@ func EnglishTranslationSet() *TranslationSet { SecondaryTitle: "Secondary", ReflogCommitsTitle: "Reflog", GlobalTitle: "Global keybindings", - ConflictsResolved: "All merge conflicts resolved. Continue?", + ConflictsResolved: "All merge conflicts resolved. Continue the %s?", Continue: "Continue", UnstagedFilesAfterConflictsResolved: "Files have been modified since conflicts were resolved. Auto-stage them and continue?", Keybindings: "Keybindings", diff --git a/pkg/integration/components/common.go b/pkg/integration/components/common.go index 2033a9442..2d62e9ea3 100644 --- a/pkg/integration/components/common.go +++ b/pkg/integration/components/common.go @@ -1,5 +1,7 @@ package components +import "fmt" + // for running common actions type Common struct { t *TestDriver @@ -43,10 +45,10 @@ func (self *Common) AcknowledgeConflicts() { Confirm() } -func (self *Common) ContinueOnConflictsResolved() { +func (self *Common) ContinueOnConflictsResolved(command string) { self.t.ExpectPopup().Confirmation(). Title(Equals("Continue")). - Content(Contains("All merge conflicts resolved. Continue?")). + Content(Contains(fmt.Sprintf("All merge conflicts resolved. Continue the %s?", command))). Confirm() } diff --git a/pkg/integration/tests/branch/rebase.go b/pkg/integration/tests/branch/rebase.go index c7dc028af..6bc2083c4 100644 --- a/pkg/integration/tests/branch/rebase.go +++ b/pkg/integration/tests/branch/rebase.go @@ -48,7 +48,7 @@ var Rebase = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Information().Content(Contains("Rebasing")) - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Information().Content(DoesNotContain("Rebasing")) diff --git a/pkg/integration/tests/branch/rebase_and_drop.go b/pkg/integration/tests/branch/rebase_and_drop.go index 594ae4854..d6c655cf7 100644 --- a/pkg/integration/tests/branch/rebase_and_drop.go +++ b/pkg/integration/tests/branch/rebase_and_drop.go @@ -77,7 +77,7 @@ var RebaseAndDrop = NewIntegrationTest(NewIntegrationTestArgs{ IsFocused(). PressPrimaryAction() - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Information().Content(DoesNotContain("Rebasing")) diff --git a/pkg/integration/tests/branch/rebase_conflicts_fix_build_errors.go b/pkg/integration/tests/branch/rebase_conflicts_fix_build_errors.go index 9cd42cd2e..2ae75dafb 100644 --- a/pkg/integration/tests/branch/rebase_conflicts_fix_build_errors.go +++ b/pkg/integration/tests/branch/rebase_conflicts_fix_build_errors.go @@ -51,7 +51,7 @@ var RebaseConflictsFixBuildErrors = NewIntegrationTest(NewIntegrationTestArgs{ popup := t.ExpectPopup().Confirmation(). Title(Equals("Continue")). - Content(Contains("All merge conflicts resolved. Continue?")) + Content(Contains("All merge conflicts resolved. Continue the rebase?")) // While the popup is showing, fix some build errors t.Shell().UpdateFile("file", "make it compile again") diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go index cd968adf8..2f001ebe4 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go @@ -69,7 +69,7 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{ SelectNextItem(). PressPrimaryAction() - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Files().IsEmpty() diff --git a/pkg/integration/tests/commit/shared.go b/pkg/integration/tests/commit/shared.go index 072cc8154..b6861f135 100644 --- a/pkg/integration/tests/commit/shared.go +++ b/pkg/integration/tests/commit/shared.go @@ -61,7 +61,7 @@ func doTheRebaseForAmendTests(t *TestDriver, keys config.KeybindingConfig) { t.ExpectPopup().Confirmation(). Title(Equals("Continue")). - Content(Contains("All merge conflicts resolved. Continue?")). + Content(Contains("All merge conflicts resolved. Continue the rebase?")). Cancel() } diff --git a/pkg/integration/tests/conflicts/resolve_externally.go b/pkg/integration/tests/conflicts/resolve_externally.go index a8d717351..ab045f233 100644 --- a/pkg/integration/tests/conflicts/resolve_externally.go +++ b/pkg/integration/tests/conflicts/resolve_externally.go @@ -25,7 +25,7 @@ var ResolveExternally = NewIntegrationTest(NewIntegrationTestArgs{ }). Press(keys.Universal.Refresh) - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("merge") t.Views().Files(). IsEmpty() diff --git a/pkg/integration/tests/conflicts/resolve_multiple_files.go b/pkg/integration/tests/conflicts/resolve_multiple_files.go index 542a74d54..7dd88e02c 100644 --- a/pkg/integration/tests/conflicts/resolve_multiple_files.go +++ b/pkg/integration/tests/conflicts/resolve_multiple_files.go @@ -51,6 +51,6 @@ var ResolveMultipleFiles = NewIntegrationTest(NewIntegrationTestArgs{ ). PressPrimaryAction() - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("merge") }, }) diff --git a/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go index 2af1eac54..3deafb288 100644 --- a/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go +++ b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go @@ -43,7 +43,7 @@ var ResolveWithoutTrailingLf = NewIntegrationTest(NewIntegrationTestArgs{ t.ExpectPopup().Alert(). Title(Equals("Continue")). - Content(Contains("All merge conflicts resolved. Continue?")). + Content(Contains("All merge conflicts resolved. Continue the merge?")). Cancel() t.Views().Files(). diff --git a/pkg/integration/tests/file/discard_all_dir_changes.go b/pkg/integration/tests/file/discard_all_dir_changes.go index e03818534..6caa3d519 100644 --- a/pkg/integration/tests/file/discard_all_dir_changes.go +++ b/pkg/integration/tests/file/discard_all_dir_changes.go @@ -93,7 +93,7 @@ var DiscardAllDirChanges = NewIntegrationTest(NewIntegrationTestArgs{ Confirm() }). Tap(func() { - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("merge") t.ExpectPopup().Confirmation(). Title(Equals("Continue")). Content(Contains("Files have been modified since conflicts were resolved. Auto-stage them and continue?")). diff --git a/pkg/integration/tests/file/discard_various_changes.go b/pkg/integration/tests/file/discard_various_changes.go index 0cc188f65..bc68fd218 100644 --- a/pkg/integration/tests/file/discard_various_changes.go +++ b/pkg/integration/tests/file/discard_various_changes.go @@ -53,7 +53,7 @@ var DiscardVariousChanges = NewIntegrationTest(NewIntegrationTestArgs{ t.ExpectPopup().Confirmation(). Title(Equals("Continue")). - Content(Contains("All merge conflicts resolved. Continue?")). + Content(Contains("All merge conflicts resolved. Continue the merge?")). Cancel() discardOneByOne([]statusFile{ diff --git a/pkg/integration/tests/file/discard_various_changes_range_select.go b/pkg/integration/tests/file/discard_various_changes_range_select.go index 403ece7f7..937c50114 100644 --- a/pkg/integration/tests/file/discard_various_changes_range_select.go +++ b/pkg/integration/tests/file/discard_various_changes_range_select.go @@ -40,7 +40,7 @@ var DiscardVariousChangesRangeSelect = NewIntegrationTest(NewIntegrationTestArgs t.ExpectPopup().Confirmation(). Title(Equals("Continue")). - Content(Contains("All merge conflicts resolved. Continue?")). + Content(Contains("All merge conflicts resolved. Continue the merge?")). Cancel() }). Lines( diff --git a/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go b/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go index 2226b5196..c20ab51e0 100644 --- a/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go +++ b/pkg/integration/tests/interactive_rebase/amend_commit_with_conflict.go @@ -60,7 +60,7 @@ var AmendCommitWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ SelectNextItem(). PressPrimaryAction() // pick "4" - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Common().AcknowledgeConflicts() diff --git a/pkg/integration/tests/interactive_rebase/shared.go b/pkg/integration/tests/interactive_rebase/shared.go index 2e42c7f76..f34cbd7e2 100644 --- a/pkg/integration/tests/interactive_rebase/shared.go +++ b/pkg/integration/tests/interactive_rebase/shared.go @@ -33,7 +33,7 @@ func handleConflictsFromSwap(t *TestDriver) { SelectNextItem(). PressPrimaryAction() // pick "three" - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Common().AcknowledgeConflicts() @@ -56,7 +56,7 @@ func handleConflictsFromSwap(t *TestDriver) { SelectNextItem(). PressPrimaryAction() // pick "two" - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Commits(). Focus(). diff --git a/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go b/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go index 122be3884..a619128fa 100644 --- a/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go +++ b/pkg/integration/tests/patch_building/move_to_earlier_commit_from_added_file.go @@ -71,7 +71,7 @@ var MoveToEarlierCommitFromAddedFile = NewIntegrationTest(NewIntegrationTestArgs SelectNextItem(). PressPrimaryAction() // choose the version with all three lines - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Commits(). Focus(). diff --git a/pkg/integration/tests/patch_building/move_to_index_from_added_file_with_conflict.go b/pkg/integration/tests/patch_building/move_to_index_from_added_file_with_conflict.go index c7acfcd50..177e76e04 100644 --- a/pkg/integration/tests/patch_building/move_to_index_from_added_file_with_conflict.go +++ b/pkg/integration/tests/patch_building/move_to_index_from_added_file_with_conflict.go @@ -67,7 +67,7 @@ var MoveToIndexFromAddedFileWithConflict = NewIntegrationTest(NewIntegrationTest SelectNextItem(). PressPrimaryAction() - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.ExpectPopup().Alert(). Title(Equals("Error")). diff --git a/pkg/integration/tests/patch_building/move_to_index_with_conflict.go b/pkg/integration/tests/patch_building/move_to_index_with_conflict.go index 51b6f4766..bdf0765d9 100644 --- a/pkg/integration/tests/patch_building/move_to_index_with_conflict.go +++ b/pkg/integration/tests/patch_building/move_to_index_with_conflict.go @@ -62,7 +62,7 @@ var MoveToIndexWithConflict = NewIntegrationTest(NewIntegrationTestArgs{ ). PressPrimaryAction() - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.ExpectPopup().Alert(). Title(Equals("Error")). diff --git a/pkg/integration/tests/sync/pull_merge_conflict.go b/pkg/integration/tests/sync/pull_merge_conflict.go index 2161f6abd..45ca39b1f 100644 --- a/pkg/integration/tests/sync/pull_merge_conflict.go +++ b/pkg/integration/tests/sync/pull_merge_conflict.go @@ -60,7 +60,7 @@ var PullMergeConflict = NewIntegrationTest(NewIntegrationTestArgs{ ). PressPrimaryAction() // choose 'content4' - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("merge") t.Views().Status().Content(Equals("↑2 repo → master")) diff --git a/pkg/integration/tests/sync/pull_rebase_conflict.go b/pkg/integration/tests/sync/pull_rebase_conflict.go index d9541e0ed..1d41b2ac3 100644 --- a/pkg/integration/tests/sync/pull_rebase_conflict.go +++ b/pkg/integration/tests/sync/pull_rebase_conflict.go @@ -61,7 +61,7 @@ var PullRebaseConflict = NewIntegrationTest(NewIntegrationTestArgs{ SelectNextItem(). PressPrimaryAction() // choose 'content4' - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Status().Content(Equals("↑1 repo → master")) diff --git a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go index bf0fc050b..eb767a1a3 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go @@ -74,7 +74,7 @@ var PullRebaseInteractiveConflict = NewIntegrationTest(NewIntegrationTestArgs{ SelectNextItem(). PressPrimaryAction() // choose 'content4' - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Status().Content(Equals("↑2 repo → master")) diff --git a/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go b/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go index 3eee12efd..4aff8b4e0 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go @@ -83,7 +83,7 @@ var PullRebaseInteractiveConflictDrop = NewIntegrationTest(NewIntegrationTestArg SelectNextItem(). PressPrimaryAction() // choose 'content4' - t.Common().ContinueOnConflictsResolved() + t.Common().ContinueOnConflictsResolved("rebase") t.Views().Status().Content(Equals("↑1 repo → master")) From 4b3262ab3edda84f587db1a41219a71c4257f30c Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 30 Mar 2025 21:20:40 +0200 Subject: [PATCH 11/13] Add test for reverting a commit that conflicts This works already (except that the "you are here" marker is not right yet, but that's an issue for another PR), just add a test that verifies it. --- .../revert_with_conflict_single_commit.go | 82 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 2 files changed, 83 insertions(+) create mode 100644 pkg/integration/tests/commit/revert_with_conflict_single_commit.go diff --git a/pkg/integration/tests/commit/revert_with_conflict_single_commit.go b/pkg/integration/tests/commit/revert_with_conflict_single_commit.go new file mode 100644 index 000000000..9762e1f49 --- /dev/null +++ b/pkg/integration/tests/commit/revert_with_conflict_single_commit.go @@ -0,0 +1,82 @@ +package commit + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var RevertWithConflictSingleCommit = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Reverts a commit that conflicts", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("myfile", "") + shell.Commit("add empty file") + shell.CreateFileAndAdd("myfile", "first line\n") + shell.Commit("add first line") + shell.UpdateFileAndAdd("myfile", "first line\nsecond line\n") + shell.Commit("add second line") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + Lines( + Contains("CI ◯ add second line").IsSelected(), + Contains("CI ◯ add first line"), + Contains("CI ◯ add empty file"), + ). + SelectNextItem(). + Press(keys.Commits.RevertCommit). + Tap(func() { + t.ExpectPopup().Confirmation(). + Title(Equals("Revert commit")). + Content(MatchesRegexp(`Are you sure you want to revert \w+?`)). + Confirm() + t.ExpectPopup().Alert(). + Title(Equals("Error")). + // The exact error message is different on different git versions, + // but they all contain the word 'conflict' somewhere. + Content(Contains("conflict")). + Confirm() + }). + Lines( + /* EXPECTED: + Proper display of revert commits is not implemented yet; we'll do this in the next PR + Contains("revert").Contains("CI <-- CONFLICT --- add first line"), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ add empty file"), + ACTUAL: */ + Contains("CI ◯ <-- YOU ARE HERE --- add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ add empty file"), + ) + + t.Views().Options().Content(Contains("View revert options: m")) + t.Views().Information().Content(Contains("Reverting (Reset)")) + + t.Views().Files().Focus(). + Lines( + Contains("UU myfile").IsSelected(), + ). + PressEnter() + + t.Views().MergeConflicts().IsFocused(). + SelectNextItem(). + PressPrimaryAction() + + t.ExpectPopup().Alert(). + Title(Equals("Continue")). + Content(Contains("All merge conflicts resolved. Continue the revert?")). + Confirm() + + t.Views().Commits(). + Lines( + Contains(`CI ◯ Revert "add first line"`), + Contains("CI ◯ add second line"), + Contains("CI ◯ add first line"), + Contains("CI ◯ add empty file"), + ) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index f381f8bea..2c1e2d169 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -128,6 +128,7 @@ var tests = []*components.IntegrationTest{ commit.ResetAuthorRange, commit.Revert, commit.RevertMerge, + commit.RevertWithConflictSingleCommit, commit.Reword, commit.Search, commit.SetAuthor, From 3e5024480dfd9844c46081decce92b651f2569a9 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 30 Mar 2025 21:24:40 +0200 Subject: [PATCH 12/13] Check for conflicts after reverting a commit This way we get the usual menu for viewing the conflicts or aborting, like for rebases. --- pkg/gui/controllers/helpers/merge_and_rebase_helper.go | 2 ++ pkg/gui/controllers/local_commits_controller.go | 3 ++- .../tests/commit/revert_with_conflict_single_commit.go | 10 ++++------ 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 0e63c516f..3f2ced17e 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -138,6 +138,8 @@ var conflictStrings = []string{ "fix conflicts", "Resolve all conflicts manually", "Merge conflict in file", + "hint: after resolving the conflicts", + "CONFLICT (content):", } func isMergeConflictErr(errStr string) bool { diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 260f42cf7..0b1b81e72 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -868,7 +868,8 @@ func (self *LocalCommitsController) revert(commit *models.Commit) error { HandleConfirm: func() error { self.c.LogAction(self.c.Tr.Actions.RevertCommit) return self.c.WithWaitingStatusSync(self.c.Tr.RevertingStatus, func() error { - if err := self.c.Git().Commit.Revert(commit.Hash); err != nil { + result := self.c.Git().Commit.Revert(commit.Hash) + if err := self.c.Helpers().MergeAndRebase.CheckMergeOrRebase(result); err != nil { return err } return self.afterRevertCommit() diff --git a/pkg/integration/tests/commit/revert_with_conflict_single_commit.go b/pkg/integration/tests/commit/revert_with_conflict_single_commit.go index 9762e1f49..949bf4b34 100644 --- a/pkg/integration/tests/commit/revert_with_conflict_single_commit.go +++ b/pkg/integration/tests/commit/revert_with_conflict_single_commit.go @@ -33,11 +33,9 @@ var RevertWithConflictSingleCommit = NewIntegrationTest(NewIntegrationTestArgs{ Title(Equals("Revert commit")). Content(MatchesRegexp(`Are you sure you want to revert \w+?`)). Confirm() - t.ExpectPopup().Alert(). - Title(Equals("Error")). - // The exact error message is different on different git versions, - // but they all contain the word 'conflict' somewhere. - Content(Contains("conflict")). + t.ExpectPopup().Menu(). + Title(Equals("Conflicts!")). + Select(Contains("View conflicts")). Confirm() }). Lines( @@ -56,7 +54,7 @@ var RevertWithConflictSingleCommit = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Options().Content(Contains("View revert options: m")) t.Views().Information().Content(Contains("Reverting (Reset)")) - t.Views().Files().Focus(). + t.Views().Files().IsFocused(). Lines( Contains("UU myfile").IsSelected(), ). From 90db796f42404906d44eefc5ca7bfd1f92516de8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Tue, 11 Jun 2024 13:37:55 +0200 Subject: [PATCH 13/13] Add DisabledReason for rebase options when not rebasing or merging --- pkg/gui/controllers/global_controller.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/gui/controllers/global_controller.go b/pkg/gui/controllers/global_controller.go index 6fa6fefef..2d7f796a5 100644 --- a/pkg/gui/controllers/global_controller.go +++ b/pkg/gui/controllers/global_controller.go @@ -35,11 +35,12 @@ func (self *GlobalController) GetKeybindings(opts types.KeybindingsOpts) []*type OpensMenu: true, }, { - Key: opts.GetKey(opts.Config.Universal.CreateRebaseOptionsMenu), - Handler: opts.Guards.NoPopupPanel(self.c.Helpers().MergeAndRebase.CreateRebaseOptionsMenu), - Description: self.c.Tr.ViewMergeRebaseOptions, - Tooltip: self.c.Tr.ViewMergeRebaseOptionsTooltip, - OpensMenu: true, + Key: opts.GetKey(opts.Config.Universal.CreateRebaseOptionsMenu), + Handler: opts.Guards.NoPopupPanel(self.c.Helpers().MergeAndRebase.CreateRebaseOptionsMenu), + Description: self.c.Tr.ViewMergeRebaseOptions, + Tooltip: self.c.Tr.ViewMergeRebaseOptionsTooltip, + OpensMenu: true, + GetDisabledReason: self.canShowRebaseOptions, }, { Key: opts.GetKey(opts.Config.Universal.Refresh), @@ -191,3 +192,12 @@ func (self *GlobalController) escape() error { func (self *GlobalController) toggleWhitespace() error { return (&ToggleWhitespaceAction{c: self.c}).Call() } + +func (self *GlobalController) canShowRebaseOptions() *types.DisabledReason { + if self.c.Model().WorkingTreeStateAtLastCommitRefresh.None() { + return &types.DisabledReason{ + Text: self.c.Tr.NotMergingOrRebasing, + } + } + return nil +}