From ae66f720f56842d72f9c482d4da824d318e202ec Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Thu, 27 Jul 2023 18:51:27 +1000 Subject: [PATCH] Update repo switch logic We now always re-use the state of the repo if we're returning to it, and we always reset the windows to their default tabs. We reset to default tabs because it's easy to implement. If people want to: * have tab states be retained when switching * have tab states specific to the current repo retained when switching back Then we'll need to revisit this --- pkg/gui/controllers/helpers/repos_helper.go | 14 +++---- .../controllers/helpers/worktree_helper.go | 2 +- pkg/gui/controllers/quit_actions.go | 2 +- pkg/gui/gui.go | 39 ++++++++++--------- pkg/gui/layout.go | 10 ++++- pkg/integration/tests/test_list.go | 2 +- ...d_window_focus.go => reset_window_tabs.go} | 13 +++++-- 7 files changed, 49 insertions(+), 33 deletions(-) rename pkg/integration/tests/worktree/{retained_window_focus.go => reset_window_tabs.go} (63%) diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index 5ea42222d..c39997c5b 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -20,7 +20,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/utils" ) -type onNewRepoFn func(startArgs appTypes.StartArgs, reuseState bool, contextKey types.ContextKey) error +type onNewRepoFn func(startArgs appTypes.StartArgs, contextKey types.ContextKey) error // helps switch back and forth between repos type ReposHelper struct { @@ -48,7 +48,7 @@ func (self *ReposHelper) EnterSubmodule(submodule *models.SubmoduleConfig) error } self.c.State().GetRepoPathStack().Push(wd) - return self.DispatchSwitchToRepo(submodule.Path, true, context.NO_CONTEXT) + return self.DispatchSwitchToRepo(submodule.Path, context.NO_CONTEXT) } func (self *ReposHelper) getCurrentBranch(path string) string { @@ -131,7 +131,7 @@ func (self *ReposHelper) CreateRecentReposMenu() error { // 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, false, context.NO_CONTEXT) + return self.DispatchSwitchToRepo(path, context.NO_CONTEXT) }, } }) @@ -139,11 +139,11 @@ func (self *ReposHelper) CreateRecentReposMenu() error { return self.c.Menu(types.CreateMenuOptions{Title: self.c.Tr.RecentRepos, Items: menuItems}) } -func (self *ReposHelper) DispatchSwitchToRepo(path string, reuse bool, contextKey types.ContextKey) error { - return self.DispatchSwitchTo(path, reuse, self.c.Tr.ErrRepositoryMovedOrDeleted, contextKey) +func (self *ReposHelper) DispatchSwitchToRepo(path string, contextKey types.ContextKey) error { + return self.DispatchSwitchTo(path, self.c.Tr.ErrRepositoryMovedOrDeleted, contextKey) } -func (self *ReposHelper) DispatchSwitchTo(path string, reuse bool, errMsg string, contextKey types.ContextKey) error { +func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey types.ContextKey) error { return self.c.WithWaitingStatus(self.c.Tr.Switching, func(gocui.Task) error { env.UnsetGitDirEnvs() originalPath, err := os.Getwd() @@ -180,6 +180,6 @@ func (self *ReposHelper) DispatchSwitchTo(path string, reuse bool, errMsg string self.c.Mutexes().RefreshingFilesMutex.Lock() defer self.c.Mutexes().RefreshingFilesMutex.Unlock() - return self.onNewRepo(appTypes.StartArgs{}, reuse, contextKey) + return self.onNewRepo(appTypes.StartArgs{}, contextKey) }) } diff --git a/pkg/gui/controllers/helpers/worktree_helper.go b/pkg/gui/controllers/helpers/worktree_helper.go index aa4cea5e1..f122ac01e 100644 --- a/pkg/gui/controllers/helpers/worktree_helper.go +++ b/pkg/gui/controllers/helpers/worktree_helper.go @@ -182,7 +182,7 @@ func (self *WorktreeHelper) Switch(path string, contextKey types.ContextKey) err self.c.LogAction(self.c.Tr.SwitchToWorktree) - return self.reposHelper.DispatchSwitchTo(path, true, self.c.Tr.ErrWorktreeMovedOrRemoved, contextKey) + return self.reposHelper.DispatchSwitchTo(path, self.c.Tr.ErrWorktreeMovedOrRemoved, contextKey) } func (self *WorktreeHelper) Remove(worktree *models.Worktree, force bool) error { diff --git a/pkg/gui/controllers/quit_actions.go b/pkg/gui/controllers/quit_actions.go index a7301d5b1..dd23448ec 100644 --- a/pkg/gui/controllers/quit_actions.go +++ b/pkg/gui/controllers/quit_actions.go @@ -78,7 +78,7 @@ func (self *QuitActions) Escape() error { repoPathStack := self.c.State().GetRepoPathStack() if !repoPathStack.IsEmpty() { - return self.c.Helpers().Repos.DispatchSwitchToRepo(repoPathStack.Pop(), true, context.NO_CONTEXT) + return self.c.Helpers().Repos.DispatchSwitchToRepo(repoPathStack.Pop(), context.NO_CONTEXT) } if self.c.UserConfig.QuitOnTopLevelReturn { diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 03cf74217..ec4ab77c5 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -276,7 +276,7 @@ func (self *GuiRepoState) GetSplitMainPanel() bool { return self.SplitMainPanel } -func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, reuseState bool, contextKey types.ContextKey) error { +func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, contextKey types.ContextKey) error { var err error gui.git, err = commands.NewGitCommand( gui.Common, @@ -289,7 +289,7 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, reuseState bool, context return err } - contextToPush := gui.resetState(startArgs, reuseState) + contextToPush := gui.resetState(startArgs) gui.resetHelpersAndControllers() @@ -324,26 +324,26 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, reuseState bool, context // it gets a bit confusing to land back in the status panel when visiting a repo // you've already switched from. There's no doubt some easy way to make the UX // optimal for all cases but I'm too lazy to think about what that is right now -func (gui *Gui) resetState(startArgs appTypes.StartArgs, reuseState bool) types.Context { +func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context { currentDir, err := os.Getwd() + if err != nil { + gui.c.Log.Error(err) + } - if reuseState { - if err == nil { - if state := gui.RepoStateMap[Repo(currentDir)]; state != nil { - gui.State = state - gui.State.ViewsSetup = false + if state := gui.RepoStateMap[Repo(currentDir)]; state != nil { + gui.State = state + gui.State.ViewsSetup = false - // setting this to nil so we don't get stuck based on a popup that was - // previously opened - gui.Mutexes.PopupMutex.Lock() - gui.State.CurrentPopupOpts = nil - gui.Mutexes.PopupMutex.Unlock() + contextTree := gui.State.Contexts + gui.State.WindowViewNameMap = initialWindowViewNameMap(contextTree) - return gui.c.CurrentContext() - } - } else { - gui.c.Log.Error(err) - } + // setting this to nil so we don't get stuck based on a popup that was + // previously opened + gui.Mutexes.PopupMutex.Lock() + gui.State.CurrentPopupOpts = nil + gui.Mutexes.PopupMutex.Unlock() + + return gui.c.CurrentContext() } contextTree := gui.contextTree() @@ -351,6 +351,7 @@ func (gui *Gui) resetState(startArgs appTypes.StartArgs, reuseState bool) types. initialScreenMode := initialScreenMode(startArgs, gui.Config) gui.State = &GuiRepoState{ + ViewsSetup: false, Model: &types.Model{ CommitFiles: nil, Files: make([]*models.File, 0), @@ -656,7 +657,7 @@ func (gui *Gui) Run(startArgs appTypes.StartArgs) error { } // onNewRepo must be called after g.SetManager because SetManager deletes keybindings - if err := gui.onNewRepo(startArgs, false, context.NO_CONTEXT); err != nil { + if err := gui.onNewRepo(startArgs, context.NO_CONTEXT); err != nil { return err } diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index ec192527b..f9daaa5b6 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -175,6 +175,10 @@ func (gui *Gui) prepareView(viewName string) (*gocui.View, error) { } func (gui *Gui) onInitialViewsCreationForRepo() error { + if err := gui.onRepoViewReset(); err != nil { + return err + } + // hide any popup views. This only applies when we've just switched repos for _, viewName := range gui.popupViewNames() { view, err := gui.g.View(viewName) @@ -201,7 +205,7 @@ func (gui *Gui) popupViewNames() []string { }) } -func (gui *Gui) onInitialViewsCreation() error { +func (gui *Gui) onRepoViewReset() error { // now we order the views (in order of bottom first) for _, view := range gui.orderedViews() { if _, err := gui.g.SetViewOnTop(view.Name()); err != nil { @@ -228,6 +232,10 @@ func (gui *Gui) onInitialViewsCreation() error { } gui.g.Mutexes.ViewsMutex.Unlock() + return nil +} + +func (gui *Gui) onInitialViewsCreation() error { if !gui.c.UserConfig.DisableStartupPopups { storedPopupVersion := gui.c.GetAppState().StartupPopupVersion if storedPopupVersion < StartupPopupVersion { diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 4060f1368..361fa2064 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -231,6 +231,6 @@ var tests = []*components.IntegrationTest{ worktree.ForceRemoveWorktree, worktree.Rebase, worktree.RemoveWorktreeFromBranch, - worktree.RetainedWindowFocus, + worktree.ResetWindowTabs, worktree.WorktreeInRepo, } diff --git a/pkg/integration/tests/worktree/retained_window_focus.go b/pkg/integration/tests/worktree/reset_window_tabs.go similarity index 63% rename from pkg/integration/tests/worktree/retained_window_focus.go rename to pkg/integration/tests/worktree/reset_window_tabs.go index d518d6d1a..c24373be8 100644 --- a/pkg/integration/tests/worktree/retained_window_focus.go +++ b/pkg/integration/tests/worktree/reset_window_tabs.go @@ -5,8 +5,15 @@ import ( . "github.com/jesseduffield/lazygit/pkg/integration/components" ) -var RetainedWindowFocus = NewIntegrationTest(NewIntegrationTestArgs{ - Description: "Verify that the focused context in each window is retained when switching worktrees", +// This is verifying logic that is subject to change (we're just doing the easiest approach) +// There are two other UX flows we could have: +// 1) associate window tab states with the repo, so that when you switch back to a repo you get the same window tab states +// 2) retain the same window tab states when switching repos +// Option 1 is straightforward, but option 2 is harder because you'd need to deactivate any views containing dependent +// content e.g. the sub-commits view. + +var ResetWindowTabs = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Verify that window tabs are reset whenever switching repos", ExtraCmdArgs: []string{}, Skip: false, SetupConfig: func(config *config.AppConfig) {}, @@ -39,7 +46,7 @@ var RetainedWindowFocus = NewIntegrationTest(NewIntegrationTestArgs{ // navigate back to the branches window Press(keys.Universal.NextBlock) - t.Views().Remotes(). + t.Views().Branches(). IsFocused() }, })