diff --git a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go index 536c254dd35..be1d7b3a9de 100644 --- a/pkg/gui/controllers/helpers/merge_and_rebase_helper.go +++ b/pkg/gui/controllers/helpers/merge_and_rebase_helper.go @@ -18,6 +18,13 @@ import ( type MergeAndRebaseHelper struct { c *HelperCommon + + // Whether the "continue the rebase/merge?" prompt is currently on screen. + // We use this to auto-dismiss it if the operation stops being in the state + // that the prompt is offering to act on (e.g. it was continued or aborted + // externally), so the user isn't left with a stale prompt. Only accessed on + // the UI thread. + continueRebasePromptShowing bool } func NewMergeAndRebaseHelper( @@ -113,6 +120,7 @@ func (self *MergeAndRebaseHelper) genericMergeCommand(command string) error { Mode: types.ASYNC, CommitSelection: commitSelectionAfterMerge(success && selectHeadCommitOnSuccess), }) + self.RecordWhetherMergeOrRebaseStartedInLazygit() return err } result := self.c.Git().Rebase.GenericMergeOrRebaseAction(commandType, command) @@ -165,9 +173,21 @@ func isMergeConflictErr(errStr string) bool { return false } +// RecordWhetherMergeOrRebaseStartedInLazygit is called right after we run a +// merge/rebase/cherry-pick/revert step. If it left an operation in progress, +// that operation is one we started, which is what later lets us auto-prompt to +// continue it once its conflicts are resolved. If nothing is in progress +// anymore (the step completed or aborted the operation), we clear the flag. +func (self *MergeAndRebaseHelper) RecordWhetherMergeOrRebaseStartedInLazygit() { + self.c.State().GetRepoState().SetMergeOrRebaseStartedInLazygit( + self.c.Git().Status.WorkingTreeState().Any()) +} + func (self *MergeAndRebaseHelper) CheckMergeOrRebaseWithRefreshOptions(result error, refreshOptions types.RefreshOptions) error { self.c.Refresh(refreshOptions) + self.RecordWhetherMergeOrRebaseStartedInLazygit() + if result == nil { return nil } else if strings.Contains(result.Error(), "No changes - did you forget to use") { @@ -246,10 +266,17 @@ func (self *MergeAndRebaseHelper) AbortMergeOrRebaseWithConfirm() error { // PromptToContinueRebase asks the user if they want to continue the rebase/merge that's in progress func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { + self.continueRebasePromptShowing = true self.c.Confirm(types.ConfirmOpts{ Title: self.c.Tr.Continue, Prompt: fmt.Sprintf(self.c.Tr.ConflictsResolved, self.c.Git().Status.WorkingTreeState().CommandName()), + HandleClose: func() error { + self.continueRebasePromptShowing = false + return nil + }, HandleConfirm: func() error { + self.continueRebasePromptShowing = false + // 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 // conflicts, but after lazygit opened the prompt already. Ask again @@ -287,6 +314,27 @@ func (self *MergeAndRebaseHelper) PromptToContinueRebase() error { return nil } +// DismissContinueRebasePromptIfShowing closes the "continue the rebase/merge?" +// prompt if it's currently on screen. It's called when the operation is no +// longer in the state the prompt is offering to act on (e.g. it was continued +// or aborted outside lazygit, or new conflicts have appeared), so that the +// user isn't left with a prompt whose "continue" would now be wrong or fail. +// Must be called on the UI thread. +func (self *MergeAndRebaseHelper) DismissContinueRebasePromptIfShowing() { + if !self.continueRebasePromptShowing { + return + } + + self.continueRebasePromptShowing = false + + // Guard against popping something else: while our prompt is up no other + // popup can open, and confirming or closing it would have cleared the flag, + // so if it's set the confirmation context is ours. + if self.c.Context().Current() == self.c.Contexts().Confirmation { + self.c.Context().Pop() + } +} + func (self *MergeAndRebaseHelper) RebaseOntoRef(ref string) error { checkedOutBranch := self.c.Model().Branches[0] checkedOutBranchName := checkedOutBranch.Name diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 605309f9fc3..3dbd19674da 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -806,8 +806,31 @@ func (self *RefreshHelper) refreshStateFiles(background bool) error { } } - if self.c.Git().Status.WorkingTreeState().Any() && conflictFileCount == 0 && prevConflictFileCount > 0 { - self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() }) + repoState := self.c.State().GetRepoState() + workingTreeState := self.c.Git().Status.WorkingTreeState() + if workingTreeState.None() { + // No operation is in progress (any more), so forget that we started one. + // This also covers an operation that was finished or aborted externally. + repoState.SetMergeOrRebaseStartedInLazygit(false) + } + + if workingTreeState.Any() && conflictFileCount == 0 { + if prevConflictFileCount > 0 && repoState.GetMergeOrRebaseStartedInLazygit() { + // The conflicts of an operation we started have just been resolved + // (e.g. in the user's editor). Offer to continue it. We only do this + // for operations we started ourselves; prompting for one that was + // started outside lazygit (e.g. by a coding agent) would be confusing. + self.c.OnUIThread(func() error { return self.mergeAndRebaseHelper.PromptToContinueRebase() }) + } + } else { + // Either there's no operation in progress any more, or new conflicts have + // appeared. Either way, a "continue?" prompt we're showing is now stale + // (e.g. the operation was continued or aborted outside lazygit), so + // dismiss it rather than leave the user with a prompt that would fail. + self.c.OnUIThread(func() error { + self.mergeAndRebaseHelper.DismissContinueRebasePromptIfShowing() + return nil + }) } fileTreeViewModel.RWMutex.Lock() diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index dfc71d64214..08d560ce531 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -255,6 +255,14 @@ type GuiRepoState struct { CurrentPopupOpts *types.CreatePopupPanelOpts LastBackgroundFetchTime time.Time + + // Whether the rebase/merge/cherry-pick/revert that's currently in progress + // was started from within lazygit (as opposed to being started externally, + // e.g. in another terminal or by a coding agent). We only auto-prompt to + // continue such an operation once its conflicts are resolved if we started + // it ourselves; for an externally started one, popping up unbidden would be + // confusing. Reset whenever we observe that no operation is in progress. + mergeOrRebaseStartedInLazygit bool } var _ types.IRepoStateAccessor = new(GuiRepoState) @@ -283,6 +291,14 @@ func (self *GuiRepoState) SetCurrentPopupOpts(value *types.CreatePopupPanelOpts) self.CurrentPopupOpts = value } +func (self *GuiRepoState) GetMergeOrRebaseStartedInLazygit() bool { + return self.mergeOrRebaseStartedInLazygit +} + +func (self *GuiRepoState) SetMergeOrRebaseStartedInLazygit(value bool) { + self.mergeOrRebaseStartedInLazygit = value +} + func (self *GuiRepoState) GetScreenMode() types.ScreenMode { return self.ScreenMode } diff --git a/pkg/gui/gui_driver.go b/pkg/gui/gui_driver.go index 57425231ad9..fef33bf66b4 100644 --- a/pkg/gui/gui_driver.go +++ b/pkg/gui/gui_driver.go @@ -68,6 +68,15 @@ func (self *GuiDriver) FocusIn() { self.waitTillIdle() } +func (self *GuiDriver) PretendMergeOrRebaseStartedInLazygit() { + self.gui.onUIThread(func() error { + self.gui.State.SetMergeOrRebaseStartedInLazygit(true) + return nil + }) + + self.waitTillIdle() +} + // wait until lazygit is idle (i.e. all processing is done) before continuing func (self *GuiDriver) waitTillIdle() { <-self.isIdleChan diff --git a/pkg/gui/services/custom_commands/handler_creator.go b/pkg/gui/services/custom_commands/handler_creator.go index 19694c48178..1e321c2dec9 100644 --- a/pkg/gui/services/custom_commands/handler_creator.go +++ b/pkg/gui/services/custom_commands/handler_creator.go @@ -318,6 +318,10 @@ func (self *HandlerCreator) finalHandler(customCommand config.CustomCommand, ses if err != nil { if customCommand.After != nil && customCommand.After.CheckForConflicts { + // The custom command may have started a rebase/merge/etc.; if so, + // it's one we consider started in lazygit, so that we offer to + // continue it once its conflicts are resolved. + self.mergeAndRebaseHelper.RecordWhetherMergeOrRebaseStartedInLazygit() return self.mergeAndRebaseHelper.CheckForConflicts(err) } diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index b81fb15e135..7621f86861d 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -401,6 +401,8 @@ type IRepoStateAccessor interface { GetSearchState() *SearchState SetSplitMainPanel(bool) GetSplitMainPanel() bool + GetMergeOrRebaseStartedInLazygit() bool + SetMergeOrRebaseStartedInLazygit(bool) } // startup stages so we don't need to load everything at once diff --git a/pkg/integration/components/common.go b/pkg/integration/components/common.go index 2d62e9ea3c9..f338be52f74 100644 --- a/pkg/integration/components/common.go +++ b/pkg/integration/components/common.go @@ -38,6 +38,14 @@ func (self *Common) AbortMerge() { Confirm() } +// PretendMergeOrRebaseStartedInLazygit tells lazygit to treat the in-progress +// rebase/merge/etc. as one that it started, so that it will prompt to continue +// once the conflicts are resolved. Use it when a test sets up an operation by +// running git directly rather than through lazygit's UI. +func (self *Common) PretendMergeOrRebaseStartedInLazygit() { + self.t.gui.PretendMergeOrRebaseStartedInLazygit() +} + func (self *Common) AcknowledgeConflicts() { self.t.ExpectPopup().Menu(). Title(Equals("Conflicts!")). diff --git a/pkg/integration/components/test_test.go b/pkg/integration/components/test_test.go index e7d03ada968..e7fd0b66ad6 100644 --- a/pkg/integration/components/test_test.go +++ b/pkg/integration/components/test_test.go @@ -93,6 +93,8 @@ func (self *fakeGuiDriver) CheckAllToastsAcknowledged() {} func (self *fakeGuiDriver) Headless() bool { return false } +func (self *fakeGuiDriver) PretendMergeOrRebaseStartedInLazygit() {} + func TestManualFailure(t *testing.T) { test := NewIntegrationTest(NewIntegrationTestArgs{ Description: unitTestDescription, diff --git a/pkg/integration/tests/conflicts/continue_prompt_dismissed_when_resolved_externally.go b/pkg/integration/tests/conflicts/continue_prompt_dismissed_when_resolved_externally.go new file mode 100644 index 00000000000..e9b2ba3aa63 --- /dev/null +++ b/pkg/integration/tests/conflicts/continue_prompt_dismissed_when_resolved_externally.go @@ -0,0 +1,49 @@ +package conflicts + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var ContinuePromptDismissedWhenResolvedExternally = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "When the prompt to continue a merge is showing and the merge is then continued outside lazygit, dismiss the prompt", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shared.CreateMergeConflictFile(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + + // Resolve the conflict and refresh so lazygit prompts us to continue. + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU file").IsSelected(), + ). + Tap(func() { + t.Shell().UpdateFile("file", "resolved content") + }). + Press(keys.Universal.Refresh) + + t.ExpectPopup().Confirmation(). + Title(Equals("Continue")). + Content(Contains("All merge conflicts resolved. Continue the merge?")) + + // While the prompt is up, the merge is continued outside lazygit (e.g. by + // a coding agent). + t.Shell().ContinueMerge() + + // Simulate lazygit noticing the change (as it would on its next refresh or + // when the window regains focus); the stale prompt is dismissed. + t.FocusIn() + + t.Views().Files(). + IsFocused(). + IsEmpty() + + t.Views().Information().Content(DoesNotContain("Merging")) + }, +}) diff --git a/pkg/integration/tests/conflicts/merge_file_both.go b/pkg/integration/tests/conflicts/merge_file_both.go index 08357212530..eb1d0b192b9 100644 --- a/pkg/integration/tests/conflicts/merge_file_both.go +++ b/pkg/integration/tests/conflicts/merge_file_both.go @@ -55,6 +55,8 @@ var MergeFileBoth = NewIntegrationTest(NewIntegrationTestArgs{ Run: func(t *TestDriver, keys config.KeybindingConfig) { _, _, _, expected := testDataBoth() + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/conflicts/merge_file_current.go b/pkg/integration/tests/conflicts/merge_file_current.go index b809174465f..68cf990e500 100644 --- a/pkg/integration/tests/conflicts/merge_file_current.go +++ b/pkg/integration/tests/conflicts/merge_file_current.go @@ -54,6 +54,8 @@ var MergeFileCurrent = NewIntegrationTest(NewIntegrationTestArgs{ Run: func(t *TestDriver, keys config.KeybindingConfig) { _, _, _, expected := testDataCurrent() + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/conflicts/merge_file_incoming.go b/pkg/integration/tests/conflicts/merge_file_incoming.go index 8216b4a4d09..17667b296da 100644 --- a/pkg/integration/tests/conflicts/merge_file_incoming.go +++ b/pkg/integration/tests/conflicts/merge_file_incoming.go @@ -54,6 +54,8 @@ var MergeFileIncoming = NewIntegrationTest(NewIntegrationTestArgs{ Run: func(t *TestDriver, keys config.KeybindingConfig) { _, _, _, expected := testDataIncoming() + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/conflicts/pick_both_hunks_diff3.go b/pkg/integration/tests/conflicts/pick_both_hunks_diff3.go index 59a634c3b98..6e9a6eb9211 100644 --- a/pkg/integration/tests/conflicts/pick_both_hunks_diff3.go +++ b/pkg/integration/tests/conflicts/pick_both_hunks_diff3.go @@ -16,6 +16,8 @@ var PickBothHunksDiff3 = NewIntegrationTest(NewIntegrationTestArgs{ shared.CreateMergeConflictFile(shell) }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/conflicts/resolve_externally.go b/pkg/integration/tests/conflicts/resolve_externally.go index ab045f23397..bffb2d02338 100644 --- a/pkg/integration/tests/conflicts/resolve_externally.go +++ b/pkg/integration/tests/conflicts/resolve_externally.go @@ -15,6 +15,8 @@ var ResolveExternally = NewIntegrationTest(NewIntegrationTestArgs{ shared.CreateMergeConflictFile(shell) }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/conflicts/resolve_externally_started_merge_no_prompt.go b/pkg/integration/tests/conflicts/resolve_externally_started_merge_no_prompt.go new file mode 100644 index 00000000000..ec83f3f08f1 --- /dev/null +++ b/pkg/integration/tests/conflicts/resolve_externally_started_merge_no_prompt.go @@ -0,0 +1,41 @@ +package conflicts + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" + "github.com/jesseduffield/lazygit/pkg/integration/tests/shared" +) + +var ResolveExternallyStartedMergeNoPrompt = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "When a merge started outside lazygit has its conflicts resolved, don't prompt to continue it", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + // Start the merge by running git directly and never tell lazygit it was + // the one to start it, so from lazygit's point of view it was started + // externally (e.g. by a coding agent in another terminal). + shared.CreateMergeConflictFile(shell) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines( + Contains("UU file").IsSelected(), + ). + Tap(func() { + t.Shell().UpdateFile("file", "resolved content") + }). + Press(keys.Universal.Refresh) + + // No prompt to continue the merge appears; we stay in the files view + // with the conflict resolved and the merge still in progress. + t.Views().Files(). + IsFocused(). + Lines( + Contains("M file"), + ) + + t.Views().Information().Content(Contains("Merging")) + }, +}) diff --git a/pkg/integration/tests/conflicts/resolve_multiple_files.go b/pkg/integration/tests/conflicts/resolve_multiple_files.go index 7dd88e02c50..5a8f9447e11 100644 --- a/pkg/integration/tests/conflicts/resolve_multiple_files.go +++ b/pkg/integration/tests/conflicts/resolve_multiple_files.go @@ -15,6 +15,8 @@ var ResolveMultipleFiles = NewIntegrationTest(NewIntegrationTestArgs{ shared.CreateMergeConflictFiles(shell) }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go index 3deafb28852..30ae73e5498 100644 --- a/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go +++ b/pkg/integration/tests/conflicts/resolve_without_trailing_lf.go @@ -24,6 +24,8 @@ var ResolveWithoutTrailingLf = NewIntegrationTest(NewIntegrationTestArgs{ RunCommandExpectError([]string{"git", "merge", "--no-edit", "branch2"}) }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/file/discard_all_dir_changes.go b/pkg/integration/tests/file/discard_all_dir_changes.go index 6caa3d51993..d4c7c4d0f5a 100644 --- a/pkg/integration/tests/file/discard_all_dir_changes.go +++ b/pkg/integration/tests/file/discard_all_dir_changes.go @@ -73,6 +73,8 @@ var DiscardAllDirChanges = NewIntegrationTest(NewIntegrationTestArgs{ shell.RunShellCommand(`echo "renamed\nhaha" > dir/renamed2.txt && git add dir/renamed2.txt`) }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/file/discard_various_changes.go b/pkg/integration/tests/file/discard_various_changes.go index bc68fd218ec..453c8708c27 100644 --- a/pkg/integration/tests/file/discard_various_changes.go +++ b/pkg/integration/tests/file/discard_various_changes.go @@ -16,6 +16,8 @@ var DiscardVariousChanges = NewIntegrationTest(NewIntegrationTestArgs{ }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + type statusFile struct { status string label string 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 937c50114b8..16ecedd0418 100644 --- a/pkg/integration/tests/file/discard_various_changes_range_select.go +++ b/pkg/integration/tests/file/discard_various_changes_range_select.go @@ -16,6 +16,8 @@ var DiscardVariousChangesRangeSelect = NewIntegrationTest(NewIntegrationTestArgs }, Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Common().PretendMergeOrRebaseStartedInLazygit() + t.Views().Files(). IsFocused(). Lines( diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index a6105c646d4..74e4717139d 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -163,12 +163,14 @@ var tests = []*components.IntegrationTest{ config.NegativeRefspec, config.RemoteNamedStar, config.SidePanelsInPerRepoConfig, + conflicts.ContinuePromptDismissedWhenResolvedExternally, conflicts.Filter, conflicts.MergeFileBoth, conflicts.MergeFileCurrent, conflicts.MergeFileIncoming, conflicts.PickBothHunksDiff3, conflicts.ResolveExternally, + conflicts.ResolveExternallyStartedMergeNoPrompt, conflicts.ResolveMultipleFiles, conflicts.ResolveNoAutoStage, conflicts.ResolveNonTextualConflicts, diff --git a/pkg/integration/types/types.go b/pkg/integration/types/types.go index cd6102cdfca..34ce499ccf0 100644 --- a/pkg/integration/types/types.go +++ b/pkg/integration/types/types.go @@ -52,4 +52,9 @@ type GuiDriver interface { NextToast() *string CheckAllToastsAcknowledged() Headless() bool + // Record that the in-progress rebase/merge/etc. is to be treated as one + // that was started from within lazygit. Lets a test that starts an + // operation by running git directly (rather than through the UI) still get + // the "continue?" prompt when its conflicts are resolved. + PretendMergeOrRebaseStartedInLazygit() }