From 4dfa4e8aa2d96215f6a3bc59221de62f3dfaa3b6 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 10 Jul 2025 20:03:50 +0200 Subject: [PATCH] Allow more than one controller to attach OnFocus/OnFocusLost functions Trying to do this would previously have the second one silently overwrite the first one's. We don't currently have this in lazygit, but I ran into the situation once during development, and it can lead to bugs that are hard to diagnose. Instead of holding a list of functions, we could also have added a panic in case the function was set already; this would have been good enough for the current state, and enough to catch mistakes early in the future. However, I decided to allow multiple controllers to attach these functions, because I can't see a reason not to. --- pkg/gui/context/base_context.go | 12 +++++++----- pkg/gui/context/simple_context.go | 8 ++++---- pkg/gui/controllers.go | 2 +- pkg/gui/types/context.go | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/pkg/gui/context/base_context.go b/pkg/gui/context/base_context.go index d988da356..b254d043d 100644 --- a/pkg/gui/context/base_context.go +++ b/pkg/gui/context/base_context.go @@ -18,8 +18,8 @@ type BaseContext struct { onClickFn func() error onClickFocusedMainViewFn onClickFocusedMainViewFn onRenderToMainFn func() - onFocusFn onFocusFn - onFocusLostFn onFocusLostFn + onFocusFns []onFocusFn + onFocusLostFns []onFocusLostFn focusable bool transient bool @@ -135,9 +135,11 @@ func (self *BaseContext) AddMouseKeybindingsFn(fn types.MouseKeybindingsFn) { self.mouseKeybindingsFns = append(self.mouseKeybindingsFns, fn) } -func (self *BaseContext) ClearAllBindingsFn() { +func (self *BaseContext) ClearAllAttachedControllerFunctions() { self.keybindingsFns = nil self.mouseKeybindingsFns = nil + self.onFocusFns = nil + self.onFocusLostFns = nil } func (self *BaseContext) AddOnClickFn(fn func() error) { @@ -168,13 +170,13 @@ func (self *BaseContext) AddOnRenderToMainFn(fn func()) { func (self *BaseContext) AddOnFocusFn(fn onFocusFn) { if fn != nil { - self.onFocusFn = fn + self.onFocusFns = append(self.onFocusFns, fn) } } func (self *BaseContext) AddOnFocusLostFn(fn onFocusLostFn) { if fn != nil { - self.onFocusLostFn = fn + self.onFocusLostFns = append(self.onFocusLostFns, fn) } } diff --git a/pkg/gui/context/simple_context.go b/pkg/gui/context/simple_context.go index db8160ea0..a5b051b93 100644 --- a/pkg/gui/context/simple_context.go +++ b/pkg/gui/context/simple_context.go @@ -37,8 +37,8 @@ func (self *SimpleContext) HandleFocus(opts types.OnFocusOpts) { self.GetViewTrait().SetHighlight(true) } - if self.onFocusFn != nil { - self.onFocusFn(opts) + for _, fn := range self.onFocusFns { + fn(opts) } if self.onRenderToMainFn != nil { @@ -49,8 +49,8 @@ func (self *SimpleContext) HandleFocus(opts types.OnFocusOpts) { func (self *SimpleContext) HandleFocusLost(opts types.OnFocusLostOpts) { self.GetViewTrait().SetHighlight(false) self.view.SetOriginX(0) - if self.onFocusLostFn != nil { - self.onFocusLostFn(opts) + for _, fn := range self.onFocusLostFns { + fn(opts) } } diff --git a/pkg/gui/controllers.go b/pkg/gui/controllers.go index abbb62155..dec0ab23a 100644 --- a/pkg/gui/controllers.go +++ b/pkg/gui/controllers.go @@ -21,7 +21,7 @@ func (gui *Gui) Helpers() *helpers.Helpers { // the lower in the list the keybindings will appear. func (gui *Gui) resetHelpersAndControllers() { for _, context := range gui.Contexts().Flatten() { - context.ClearAllBindingsFn() + context.ClearAllAttachedControllerFunctions() } helperCommon := gui.c diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index d1b0ff4d4..6d5f5d573 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -88,7 +88,7 @@ type IBaseContext interface { AddKeybindingsFn(KeybindingsFn) AddMouseKeybindingsFn(MouseKeybindingsFn) - ClearAllBindingsFn() + ClearAllAttachedControllerFunctions() // This is a bit of a hack at the moment: we currently only set an onclick function so that // our list controller can come along and wrap it in a list-specific click handler.