From 57991c1da899bea7fd553d9f976e015b0c72519f Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 17 May 2025 11:35:40 +0200 Subject: [PATCH 1/3] Fix crash when clicking in the status view The click handler of MainViewController was registered as a global handler, so it was used when a side panel was focused that doesn't have a SwitchToFocusedMainViewController attached (e.g. Status, Worktrees, or Submodules). This handler would then push the main view context, but with the code that is meant only for toggling between the main view pair contexts, i.e. with taking over the parentContext from the otherContext, which doesn't have one at that point. This would later lead to a crash in onClick because the parentContext was nil. Fix this by splitting the click handler in two, one for when it already has the focus, and one for toggling from the other view, and make these focus specific. --- pkg/gui/controllers/main_view_controller.go | 41 +++++++++++---------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index e6b209286..8ccbcc6af 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -56,21 +56,16 @@ func (self *MainViewController) GetKeybindings(opts types.KeybindingsOpts) []*ty func (self *MainViewController) GetMouseKeybindings(opts types.KeybindingsOpts) []*gocui.ViewMouseBinding { return []*gocui.ViewMouseBinding{ { - ViewName: self.context.GetViewName(), - Key: gocui.MouseLeft, - Handler: func(opts gocui.ViewMouseBindingOpts) error { - if self.isFocused() { - return self.onClick(opts) - } - - self.context.SetParentContext(self.otherContext.GetParentContext()) - self.c.Context().Push(self.context, types.OnFocusOpts{ - ClickedWindowName: self.context.GetWindowName(), - ClickedViewLineIdx: opts.Y, - }) - - return nil - }, + ViewName: self.context.GetViewName(), + Key: gocui.MouseLeft, + Handler: self.onClickInAlreadyFocusedView, + FocusedView: self.context.GetViewName(), + }, + { + ViewName: self.context.GetViewName(), + Key: gocui.MouseLeft, + Handler: self.onClickInOtherViewOfMainViewPair, + FocusedView: self.otherContext.GetViewName(), }, } } @@ -93,7 +88,7 @@ func (self *MainViewController) escape() error { return nil } -func (self *MainViewController) onClick(opts gocui.ViewMouseBindingOpts) error { +func (self *MainViewController) onClickInAlreadyFocusedView(opts gocui.ViewMouseBindingOpts) error { parentCtx := self.context.GetParentContext() if parentCtx.GetOnClickFocusedMainView() != nil { return parentCtx.GetOnClickFocusedMainView()(self.context.GetViewName(), opts.Y) @@ -101,6 +96,16 @@ func (self *MainViewController) onClick(opts gocui.ViewMouseBindingOpts) error { return nil } +func (self *MainViewController) onClickInOtherViewOfMainViewPair(opts gocui.ViewMouseBindingOpts) error { + self.context.SetParentContext(self.otherContext.GetParentContext()) + self.c.Context().Push(self.context, types.OnFocusOpts{ + ClickedWindowName: self.context.GetWindowName(), + ClickedViewLineIdx: opts.Y, + }) + + return nil +} + func (self *MainViewController) openSearch() error { if manager := self.c.GetViewBufferManagerForView(self.context.GetView()); manager != nil { manager.ReadToEnd(func() { @@ -112,7 +117,3 @@ func (self *MainViewController) openSearch() error { return nil } - -func (self *MainViewController) isFocused() bool { - return self.c.Context().Current().GetKey() == self.context.GetKey() -} From 12ed50464b668a37644caaa78576a2faca3f8e5b Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 17 May 2025 11:47:49 +0200 Subject: [PATCH 2/3] Cleanup: pass target context to focusMainView directly It's a bit silly to pass a window name and then call a function to get the corresponding context, when we can simply pass the context directly. --- .../switch_to_focused_main_view_controller.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pkg/gui/controllers/switch_to_focused_main_view_controller.go b/pkg/gui/controllers/switch_to_focused_main_view_controller.go index cb03f5e15..3c08388b9 100644 --- a/pkg/gui/controllers/switch_to_focused_main_view_controller.go +++ b/pkg/gui/controllers/switch_to_focused_main_view_controller.go @@ -60,19 +60,18 @@ func (self *SwitchToFocusedMainViewController) Context() types.Context { } func (self *SwitchToFocusedMainViewController) onClickMain(opts gocui.ViewMouseBindingOpts) error { - return self.focusMainView("main") + return self.focusMainView(self.c.Contexts().Normal) } func (self *SwitchToFocusedMainViewController) onClickSecondary(opts gocui.ViewMouseBindingOpts) error { - return self.focusMainView("secondary") + return self.focusMainView(self.c.Contexts().NormalSecondary) } func (self *SwitchToFocusedMainViewController) handleFocusMainView() error { - return self.focusMainView("main") + return self.focusMainView(self.c.Contexts().Normal) } -func (self *SwitchToFocusedMainViewController) focusMainView(mainViewName string) error { - mainViewContext := self.c.Helpers().Window.GetContextForWindow(mainViewName) +func (self *SwitchToFocusedMainViewController) focusMainView(mainViewContext types.Context) error { mainViewContext.SetParentContext(self.context) if context, ok := mainViewContext.(types.ISearchableContext); ok { context.ClearSearchString() From bbd17abc43a7d0b8866b7129fd3fd1d650c70962 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 22 May 2025 08:19:13 +0200 Subject: [PATCH 3/3] Add ContextMgr.NextInStack and use it to access side panel of focused main view This way we don't have to abuse the parent context mechanism, which isn't meant for this purpose. --- pkg/gui/context.go | 16 ++++++++++++++++ pkg/gui/controllers/context_lines_controller.go | 4 +++- pkg/gui/controllers/main_view_controller.go | 8 +++----- .../rename_similarity_threshold_controller.go | 4 +++- .../switch_to_focused_main_view_controller.go | 1 - pkg/gui/types/context.go | 1 + pkg/gui/view_helpers.go | 6 +++--- 7 files changed, 29 insertions(+), 11 deletions(-) diff --git a/pkg/gui/context.go b/pkg/gui/context.go index ca5727d38..45b2bdf34 100644 --- a/pkg/gui/context.go +++ b/pkg/gui/context.go @@ -357,3 +357,19 @@ func (self *ContextMgr) CurrentPopup() []types.Context { return context.GetKind() == types.TEMPORARY_POPUP || context.GetKind() == types.PERSISTENT_POPUP }) } + +func (self *ContextMgr) NextInStack(c types.Context) types.Context { + self.RLock() + defer self.RUnlock() + + for i := range self.ContextStack { + if self.ContextStack[i].GetKey() == c.GetKey() { + if i == 0 { + return nil + } + return self.ContextStack[i-1] + } + } + + panic("context not in stack") +} diff --git a/pkg/gui/controllers/context_lines_controller.go b/pkg/gui/controllers/context_lines_controller.go index 8e35d5103..aec202aeb 100644 --- a/pkg/gui/controllers/context_lines_controller.go +++ b/pkg/gui/controllers/context_lines_controller.go @@ -131,7 +131,9 @@ func (self *ContextLinesController) currentSidePanel() types.Context { currentContext := self.c.Context().CurrentStatic() if currentContext.GetKey() == context.NORMAL_MAIN_CONTEXT_KEY || currentContext.GetKey() == context.NORMAL_SECONDARY_CONTEXT_KEY { - return currentContext.GetParentContext() + if sidePanelContext := self.c.Context().NextInStack(currentContext); sidePanelContext != nil { + return sidePanelContext + } } return currentContext diff --git a/pkg/gui/controllers/main_view_controller.go b/pkg/gui/controllers/main_view_controller.go index 8ccbcc6af..5dfdb3233 100644 --- a/pkg/gui/controllers/main_view_controller.go +++ b/pkg/gui/controllers/main_view_controller.go @@ -76,7 +76,6 @@ func (self *MainViewController) Context() types.Context { func (self *MainViewController) togglePanel() error { if self.otherContext.GetView().Visible { - self.otherContext.SetParentContext(self.context.GetParentContext()) self.c.Context().Push(self.otherContext, types.OnFocusOpts{}) } @@ -89,15 +88,14 @@ func (self *MainViewController) escape() error { } func (self *MainViewController) onClickInAlreadyFocusedView(opts gocui.ViewMouseBindingOpts) error { - parentCtx := self.context.GetParentContext() - if parentCtx.GetOnClickFocusedMainView() != nil { - return parentCtx.GetOnClickFocusedMainView()(self.context.GetViewName(), opts.Y) + sidePanelContext := self.c.Context().NextInStack(self.context) + if sidePanelContext != nil && sidePanelContext.GetOnClickFocusedMainView() != nil { + return sidePanelContext.GetOnClickFocusedMainView()(self.context.GetViewName(), opts.Y) } return nil } func (self *MainViewController) onClickInOtherViewOfMainViewPair(opts gocui.ViewMouseBindingOpts) error { - self.context.SetParentContext(self.otherContext.GetParentContext()) self.c.Context().Push(self.context, types.OnFocusOpts{ ClickedWindowName: self.context.GetWindowName(), ClickedViewLineIdx: opts.Y, diff --git a/pkg/gui/controllers/rename_similarity_threshold_controller.go b/pkg/gui/controllers/rename_similarity_threshold_controller.go index 88c611723..790c005a9 100644 --- a/pkg/gui/controllers/rename_similarity_threshold_controller.go +++ b/pkg/gui/controllers/rename_similarity_threshold_controller.go @@ -106,7 +106,9 @@ func (self *RenameSimilarityThresholdController) currentSidePanel() types.Contex currentContext := self.c.Context().CurrentStatic() if currentContext.GetKey() == context.NORMAL_MAIN_CONTEXT_KEY || currentContext.GetKey() == context.NORMAL_SECONDARY_CONTEXT_KEY { - return currentContext.GetParentContext() + if sidePanelContext := self.c.Context().NextInStack(currentContext); sidePanelContext != nil { + return sidePanelContext + } } return currentContext diff --git a/pkg/gui/controllers/switch_to_focused_main_view_controller.go b/pkg/gui/controllers/switch_to_focused_main_view_controller.go index 3c08388b9..1973e3d63 100644 --- a/pkg/gui/controllers/switch_to_focused_main_view_controller.go +++ b/pkg/gui/controllers/switch_to_focused_main_view_controller.go @@ -72,7 +72,6 @@ func (self *SwitchToFocusedMainViewController) handleFocusMainView() error { } func (self *SwitchToFocusedMainViewController) focusMainView(mainViewContext types.Context) error { - mainViewContext.SetParentContext(self.context) if context, ok := mainViewContext.(types.ISearchableContext); ok { context.ClearSearchString() } diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index b75d97fb6..1c9759486 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -300,6 +300,7 @@ type IContextMgr interface { CurrentStatic() Context CurrentSide() Context CurrentPopup() []Context + NextInStack(context Context) Context IsCurrent(c Context) bool IsCurrentOrParent(c Context) bool ForEach(func(Context)) diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 383550d91..081b2a426 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -152,9 +152,9 @@ func (gui *Gui) postRefreshUpdate(c types.Context) { // just don't rerender the view while searching, on the assumption that users will probably // either search or change their data, but not both at the same time. if !currentCtx.GetView().IsSearching() { - parentCtx := currentCtx.GetParentContext() - if parentCtx.GetKey() == c.GetKey() { - parentCtx.HandleRenderToMain() + sidePanelContext := gui.State.ContextMgr.NextInStack(currentCtx) + if sidePanelContext != nil && sidePanelContext.GetKey() == c.GetKey() { + sidePanelContext.HandleRenderToMain() } } }