From eaf3bf0971c552ef8c261b751aa0a80bfb0b6791 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Wed, 16 Oct 2024 18:01:04 +0200 Subject: [PATCH 1/3] Change NewRenderStringWith{out}ScrollTask to reuse the task key of the existing task This way it won't scroll to the top; we want this when entering the staging panel or the patch building panel by clicking into the view, and also when returning from these views by pressing escape. Note that there's a bug in this latter case: the focused panel still scrolls to the top when hitting escape, we will fix this in the next commit. Change it in the same way for NewRenderStringWithScrollTask, just for consistency, although it's not really necessary there. We use this function only for focusing the merge conflict view, and in that case we already have an empty task key before and after, so it doesn't change anything there. --- pkg/gui/tasks_adapter.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/gui/tasks_adapter.go b/pkg/gui/tasks_adapter.go index 62a53ae0b..545c45471 100644 --- a/pkg/gui/tasks_adapter.go +++ b/pkg/gui/tasks_adapter.go @@ -53,9 +53,7 @@ func (gui *Gui) newStringTaskWithoutScroll(view *gocui.View, str string) error { return nil } - // Using empty key so that on subsequent calls we won't reset the view's origin. - // Note this means that we will be scrolling back to the top if we're switching from a different key - if err := manager.NewTask(f, ""); err != nil { + if err := manager.NewTask(f, manager.GetTaskKey()); err != nil { return err } @@ -71,7 +69,7 @@ func (gui *Gui) newStringTaskWithScroll(view *gocui.View, str string, originX in return nil } - if err := manager.NewTask(f, ""); err != nil { + if err := manager.NewTask(f, manager.GetTaskKey()); err != nil { return err } From 8f3e59b78ed62c792d37843dbb4b32bf31ebbbd7 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 17 Oct 2024 09:41:33 +0200 Subject: [PATCH 2/3] Don't render staging view when it loses focus As far as I understand, this was needed back when the staging context was still responsible for rendering its highlight (as opposed to the gocui view, as it is today). It was necessary to call it with isFocused=false so that it removed the highlight. The isFocused bool is no longer used today (and we'll remove it in the next commit), so there's no need to render the view here any more. This fixes flickering when leaving the staging view by pressing escape. The reason is that in this case the patch state was already set to nil by the time we get here, so we would render an empty view for a brief moment. On top of that, it fixes unwanted scrolling to the top when leaving the staging view. The reason for this is that we have code in layout that scrolls views up if needed, e.g. because the window got taller or the view content got shorter (added in #3839). This kicked in because we emptied the view, and scrolled it all the way to the top, which we don't want. --- pkg/gui/controllers/staging_controller.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/pkg/gui/controllers/staging_controller.go b/pkg/gui/controllers/staging_controller.go index 769cba68a..4ae9a946b 100644 --- a/pkg/gui/controllers/staging_controller.go +++ b/pkg/gui/controllers/staging_controller.go @@ -132,8 +132,6 @@ func (self *StagingController) GetOnFocusLost() func(types.OnFocusLostOpts) { if opts.NewContextKey != self.otherContext.GetKey() { self.c.Views().Staging.Wrap = true self.c.Views().StagingSecondary.Wrap = true - self.c.Contexts().Staging.Render(false) - self.c.Contexts().StagingSecondary.Render(false) } } } From f08b3e9e1d6039b09990f2a7df9e5e216433c989 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 17 Oct 2024 09:29:36 +0200 Subject: [PATCH 3/3] Cleanup: remove isFocused parameter from GetContentToRender and related methods It became unused in f3eb180f75. --- pkg/gui/context/patch_explorer_context.go | 22 +++++++++---------- .../helpers/patch_building_helper.go | 2 +- pkg/gui/controllers/helpers/staging_helper.go | 4 ++-- .../controllers/patch_explorer_controller.go | 2 +- pkg/gui/patch_exploring/state.go | 2 +- pkg/gui/types/context.go | 8 +++---- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/pkg/gui/context/patch_explorer_context.go b/pkg/gui/context/patch_explorer_context.go index 330f0a557..6ecff8856 100644 --- a/pkg/gui/context/patch_explorer_context.go +++ b/pkg/gui/context/patch_explorer_context.go @@ -53,7 +53,7 @@ func NewPatchExplorerContext( func(selectedLineIdx int) error { ctx.GetMutex().Lock() defer ctx.GetMutex().Unlock() - ctx.NavigateTo(ctx.c.Context().IsCurrent(ctx), selectedLineIdx) + ctx.NavigateTo(selectedLineIdx) return nil }), ) @@ -79,15 +79,15 @@ func (self *PatchExplorerContext) GetIncludedLineIndices() []int { return self.getIncludedLineIndices() } -func (self *PatchExplorerContext) RenderAndFocus(isFocused bool) { - self.setContent(isFocused) +func (self *PatchExplorerContext) RenderAndFocus() { + self.setContent() self.FocusSelection() self.c.Render() } -func (self *PatchExplorerContext) Render(isFocused bool) { - self.setContent(isFocused) +func (self *PatchExplorerContext) Render() { + self.setContent() self.c.Render() } @@ -97,8 +97,8 @@ func (self *PatchExplorerContext) Focus() { self.c.Render() } -func (self *PatchExplorerContext) setContent(isFocused bool) { - self.GetView().SetContent(self.GetContentToRender(isFocused)) +func (self *PatchExplorerContext) setContent() { + self.GetView().SetContent(self.GetContentToRender()) } func (self *PatchExplorerContext) FocusSelection() { @@ -119,19 +119,19 @@ func (self *PatchExplorerContext) FocusSelection() { view.SetCursorY(endIdx - newOriginY) } -func (self *PatchExplorerContext) GetContentToRender(isFocused bool) string { +func (self *PatchExplorerContext) GetContentToRender() string { if self.GetState() == nil { return "" } - return self.GetState().RenderForLineIndices(isFocused, self.GetIncludedLineIndices()) + return self.GetState().RenderForLineIndices(self.GetIncludedLineIndices()) } -func (self *PatchExplorerContext) NavigateTo(isFocused bool, selectedLineIdx int) { +func (self *PatchExplorerContext) NavigateTo(selectedLineIdx int) { self.GetState().SetLineSelectMode() self.GetState().SelectLine(selectedLineIdx) - self.RenderAndFocus(isFocused) + self.RenderAndFocus() } func (self *PatchExplorerContext) GetMutex() *deadlock.Mutex { diff --git a/pkg/gui/controllers/helpers/patch_building_helper.go b/pkg/gui/controllers/helpers/patch_building_helper.go index b238d252c..63744167f 100644 --- a/pkg/gui/controllers/helpers/patch_building_helper.go +++ b/pkg/gui/controllers/helpers/patch_building_helper.go @@ -98,7 +98,7 @@ func (self *PatchBuildingHelper) RefreshPatchBuildingPanel(opts types.OnFocusOpt return } - mainContent := context.GetContentToRender(true) + mainContent := context.GetContentToRender() self.c.Contexts().CustomPatchBuilder.FocusSelection() diff --git a/pkg/gui/controllers/helpers/staging_helper.go b/pkg/gui/controllers/helpers/staging_helper.go index d7b202525..a6b870517 100644 --- a/pkg/gui/controllers/helpers/staging_helper.go +++ b/pkg/gui/controllers/helpers/staging_helper.go @@ -73,8 +73,8 @@ func (self *StagingHelper) RefreshStagingPanel(focusOpts types.OnFocusOpts) { mainState := mainContext.GetState() secondaryState := secondaryContext.GetState() - mainContent := mainContext.GetContentToRender(!secondaryFocused) - secondaryContent := secondaryContext.GetContentToRender(secondaryFocused) + mainContent := mainContext.GetContentToRender() + secondaryContent := secondaryContext.GetContentToRender() mainContext.GetMutex().Unlock() secondaryContext.GetMutex().Unlock() diff --git a/pkg/gui/controllers/patch_explorer_controller.go b/pkg/gui/controllers/patch_explorer_controller.go index 3e5e1539d..d84ff48a0 100644 --- a/pkg/gui/controllers/patch_explorer_controller.go +++ b/pkg/gui/controllers/patch_explorer_controller.go @@ -302,7 +302,7 @@ func (self *PatchExplorerController) withRenderAndFocus(f func() error) func() e return err } - self.context.RenderAndFocus(self.isFocused()) + self.context.RenderAndFocus() return nil }) } diff --git a/pkg/gui/patch_exploring/state.go b/pkg/gui/patch_exploring/state.go index 1535e1c0f..4c20b7a51 100644 --- a/pkg/gui/patch_exploring/state.go +++ b/pkg/gui/patch_exploring/state.go @@ -249,7 +249,7 @@ func (s *State) AdjustSelectedLineIdx(change int) { s.SelectLine(s.selectedLineIdx + change) } -func (s *State) RenderForLineIndices(isFocused bool, includedLineIndices []int) string { +func (s *State) RenderForLineIndices(includedLineIndices []int) string { includedLineIndicesSet := set.NewFromSlice(includedLineIndices) return s.patch.FormatView(patch.FormatViewOpts{ IncLineIndices: includedLineIndicesSet, diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 1b7037690..a2ee3425f 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -177,11 +177,11 @@ type IPatchExplorerContext interface { GetState() *patch_exploring.State SetState(*patch_exploring.State) GetIncludedLineIndices() []int - RenderAndFocus(isFocused bool) - Render(isFocused bool) + RenderAndFocus() + Render() Focus() - GetContentToRender(isFocused bool) string - NavigateTo(isFocused bool, selectedLineIdx int) + GetContentToRender() string + NavigateTo(selectedLineIdx int) GetMutex() *deadlock.Mutex IsPatchExplorerContext() // used for type switch }