diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index 6257327e9..a61ad0013 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -43,13 +43,20 @@ func NewRecentReposHelper( } func (self *ReposHelper) EnterSubmodule(submodule *models.SubmoduleConfig) error { + // Check before pushing onto the repo-path stack, so a refused switch + // doesn't leave a stale entry there (which escape would later switch back + // to, needlessly reloading the current repo). + if self.switchRefusedBecauseBusy() { + return nil + } + wd, err := os.Getwd() if err != nil { return err } self.c.State().GetRepoPathStack().Push(wd) - return self.DispatchSwitchToRepo(submodule.FullPath(), context.NO_CONTEXT) + return self.switchTo(submodule.FullPath(), self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) } func (self *ReposHelper) getCurrentBranch(path string) string { @@ -129,10 +136,16 @@ func (self *ReposHelper) CreateRecentReposMenu() error { style.FgMagenta.Sprint(path), }, OnPress: func() error { + // Check before clearing the stack, so a refused switch doesn't + // forget the submodule breadcrumb (which would leave escape + // unable to return to the parent repo). + if self.switchRefusedBecauseBusy() { + return nil + } // if we were in a submodule, we want to forget about that stack of repos // so that hitting escape in the new repo does nothing self.c.State().GetRepoPathStack().Clear() - return self.DispatchSwitchToRepo(path, context.NO_CONTEXT) + return self.switchTo(path, self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) }, } }) @@ -140,17 +153,49 @@ func (self *ReposHelper) CreateRecentReposMenu() error { return self.c.Menu(types.CreateMenuOptions{Title: self.c.Tr.RecentRepos, Items: menuItems}) } -func (self *ReposHelper) DispatchSwitchToRepo(path string, contextKey types.ContextKey) error { - return self.DispatchSwitchTo(path, self.c.Tr.ErrRepositoryMovedOrDeleted, contextKey) +// SwitchToParentRepo switches back to the repo the current submodule was +// entered from (the top of the repo-path stack). Like the other callers that do +// work before switching, it checks for an in-flight operation *before* popping +// the stack, so a refused switch leaves the stack intact — otherwise the entry +// would be consumed and escape would no longer return to the parent once the +// operation finished. The caller must only call this when the stack is +// non-empty. +func (self *ReposHelper) SwitchToParentRepo() error { + if self.switchRefusedBecauseBusy() { + return nil + } + return self.switchTo(self.c.State().GetRepoPathStack().Pop(), self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) } -// DispatchSwitchTo switches lazygit to the repository (or worktree) at the -// given path. It runs synchronously on the UI thread: the switch swaps -// gui.State (in resetState) and reassigns gui.git and the process cwd, all of -// which the UI thread also reads, so doing it here rather than on a worker -// avoids racing those reads. The heavy data loading is still dispatched -// asynchronously by the refresh that onNewRepo kicks off. func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey types.ContextKey) error { + if self.switchRefusedBecauseBusy() { + return nil + } + return self.switchTo(path, errMsg, contextKey) +} + +// switchRefusedBecauseBusy reports (and shows a toast) whether a repo switch +// must be refused because a foreground git operation is in flight. Switching +// reassigns gui.git and the process cwd, so switching mid-operation would run +// the operation's remaining git commands against the wrong repo. Callers that +// do work before the switch (creating a worktree, recording the repo-path +// stack) check this up front, so they don't do that work only to have the +// switch refused; the switch itself (switchTo) is then unguarded. +func (self *ReposHelper) switchRefusedBecauseBusy() bool { + if self.c.GocuiGui().Busy() { + self.c.ErrorToast(self.c.Tr.CantSwitchWhileOperationInProgress) + return true + } + return false +} + +// switchTo switches lazygit to the repository (or worktree) at the given path. +// It runs synchronously on the UI thread: the switch swaps gui.State (in +// resetState) and reassigns gui.git and the process cwd, all of which the UI +// thread also reads, so doing it here rather than on a worker avoids racing +// those reads. The heavy data loading is still dispatched asynchronously by the +// refresh that onNewRepo kicks off. +func (self *ReposHelper) switchTo(path string, errMsg string, contextKey types.ContextKey) error { env.UnsetGitLocationEnvVars() originalPath, err := os.Getwd() if err != nil { diff --git a/pkg/gui/controllers/helpers/worktree_helper.go b/pkg/gui/controllers/helpers/worktree_helper.go index 2a189a752..abfd1c0f6 100644 --- a/pkg/gui/controllers/helpers/worktree_helper.go +++ b/pkg/gui/controllers/helpers/worktree_helper.go @@ -426,6 +426,13 @@ func (self *WorktreeHelper) promptForWorktreeLocation(dirName string, prompt str } func (self *WorktreeHelper) createWorktree(opts git_commands.NewWorktreeOpts, contextKey types.ContextKey) error { + // Check now, before we create the worktree, rather than when we come to + // switch to it afterwards: by then this operation's own waiting-status + // spinner would make Busy() true and refuse our own switch. + if self.reposHelper.switchRefusedBecauseBusy() { + return nil + } + return self.c.WithWaitingStatus(self.c.Tr.AddingWorktree, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.AddWorktree) if err := self.c.Git().Worktree.New(opts); err != nil { @@ -434,9 +441,11 @@ func (self *WorktreeHelper) createWorktree(opts git_commands.NewWorktreeOpts, co // The switch swaps gui.State and must run on the UI thread, but // we're on a worker here (creating the worktree is git work), so - // dispatch it rather than calling it directly. + // dispatch it. It's unguarded (switchTo, not DispatchSwitchTo) + // because we checked above and creating the worktree is now + // complete, so switching to it is safe. self.c.OnUIThread(func() error { - return self.reposHelper.DispatchSwitchTo(opts.Path, self.c.Tr.ErrWorktreeMovedOrRemoved, contextKey) + return self.reposHelper.switchTo(opts.Path, self.c.Tr.ErrWorktreeMovedOrRemoved, contextKey) }) return nil }) diff --git a/pkg/gui/controllers/quit_actions.go b/pkg/gui/controllers/quit_actions.go index 40ad6f7e3..9a7082542 100644 --- a/pkg/gui/controllers/quit_actions.go +++ b/pkg/gui/controllers/quit_actions.go @@ -2,7 +2,6 @@ package controllers import ( "github.com/jesseduffield/lazygit/pkg/gocui" - "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" ) @@ -81,9 +80,8 @@ func (self *QuitActions) Escape() error { } } - repoPathStack := self.c.State().GetRepoPathStack() - if !repoPathStack.IsEmpty() { - return self.c.Helpers().Repos.DispatchSwitchToRepo(repoPathStack.Pop(), context.NO_CONTEXT) + if !self.c.State().GetRepoPathStack().IsEmpty() { + return self.c.Helpers().Repos.SwitchToParentRepo() } if self.c.UserConfig().QuitOnTopLevelReturn { diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 0ee0a9e86..69ea7012f 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -769,6 +769,7 @@ type TranslationSet struct { ErrStageDirWithInlineMergeConflicts string ErrRepositoryMovedOrDeleted string ErrWorktreeMovedOrRemoved string + CantSwitchWhileOperationInProgress string CommandLog string ToggleShowCommandLog string FocusCommandLog string @@ -1921,6 +1922,7 @@ func EnglishTranslationSet() *TranslationSet { ErrRepositoryMovedOrDeleted: "Cannot find repo. It might have been moved or deleted ¯\\_(ツ)_/¯", CommandLog: "Command log", ErrWorktreeMovedOrRemoved: "Cannot find worktree. It might have been moved or removed ¯\\_(ツ)_/¯", + CantSwitchWhileOperationInProgress: "Can't switch repositories while an operation is in progress", ToggleShowCommandLog: "Toggle show/hide command log", FocusCommandLog: "Focus command log", CommandLogHeader: "You can hide/focus this panel by pressing '%s'\n",