diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 720af133e..98833fdb2 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/pkg/gui/context/list_context_trait.go @@ -24,6 +24,10 @@ type ListContextTrait struct { // If renderOnlyVisibleLines is true, needRerenderVisibleLines indicates whether we need to // rerender the visible lines e.g. because the scroll position changed needRerenderVisibleLines bool + + // true if we're inside the OnSearchSelect call; in that case we don't want to update the search + // result index. + inOnSearchSelect bool } func (self *ListContextTrait) IsListContext() {} @@ -31,6 +35,10 @@ func (self *ListContextTrait) IsListContext() {} func (self *ListContextTrait) FocusLine(scrollIntoView bool) { self.Context.FocusLine(scrollIntoView) + // Need to capture this in a local variable because by the time the AfterLayout function runs, + // the field will have been reset to false already + inOnSearchSelect := self.inOnSearchSelect + // Doing this at the end of the layout function because we need the view to be // resized before we focus the line, otherwise if we're in accordion mode // the view could be squashed and won't know how to adjust the cursor/origin. @@ -40,6 +48,9 @@ func (self *ListContextTrait) FocusLine(scrollIntoView bool) { self.GetViewTrait().FocusPoint( self.ModelIndexToViewIndex(self.list.GetSelectedLineIdx()), scrollIntoView) + if !inOnSearchSelect { + self.GetView().SetNearestSearchPosition() + } selectRangeIndex, isSelectingRange := self.list.GetRangeStartIdx() if isSelectingRange { @@ -119,7 +130,9 @@ func (self *ListContextTrait) HandleRender() { func (self *ListContextTrait) OnSearchSelect(selectedLineIdx int) { self.GetList().SetSelection(self.ViewIndexToModelIndex(selectedLineIdx)) + self.inOnSearchSelect = true self.HandleFocus(types.OnFocusOpts{}) + self.inOnSearchSelect = false } func (self *ListContextTrait) IsItemVisible(item types.HasUrn) bool { diff --git a/pkg/gui/context/patch_explorer_context.go b/pkg/gui/context/patch_explorer_context.go index c747aa88f..082800127 100644 --- a/pkg/gui/context/patch_explorer_context.go +++ b/pkg/gui/context/patch_explorer_context.go @@ -16,6 +16,10 @@ type PatchExplorerContext struct { getIncludedLineIndices func() []int c *ContextCommon mutex deadlock.Mutex + + // true if we're inside the OnSelectItem callback; in that case we don't want to update the + // search result index. + inOnSelectItemCallback bool } var ( @@ -53,7 +57,9 @@ func NewPatchExplorerContext( ctx.GetView().SetOnSelectItem(func(selectedLineIdx int) { ctx.GetMutex().Lock() defer ctx.GetMutex().Unlock() + ctx.inOnSelectItemCallback = true ctx.NavigateTo(selectedLineIdx) + ctx.inOnSelectItemCallback = false }) ctx.SetHandleRenderFunc(ctx.OnViewWidthChanged) @@ -111,6 +117,10 @@ func (self *PatchExplorerContext) FocusSelection() { // As far as the view is concerned, we are always selecting a range view.SetRangeSelectStart(startIdx) view.SetCursorY(endIdx - newOriginY) + + if !self.inOnSelectItemCallback { + view.SetNearestSearchPosition() + } } func (self *PatchExplorerContext) GetContentToRender() string { diff --git a/pkg/integration/tests/commit/search.go b/pkg/integration/tests/commit/search.go index 24754b517..5439a1b32 100644 --- a/pkg/integration/tests/commit/search.go +++ b/pkg/integration/tests/commit/search.go @@ -11,6 +11,10 @@ var Search = NewIntegrationTest(NewIntegrationTestArgs{ Skip: false, SetupConfig: func(config *config.AppConfig) {}, SetupRepo: func(shell *Shell) { + // Creating a branch avoids that searching for 't' will unexpectedly match the first commit + // (since it finds it in the extra info line, which is "HEAD -> master") + shell.NewBranch("branch") + shell.EmptyCommit("one") shell.EmptyCommit("two") shell.EmptyCommit("three") @@ -103,6 +107,54 @@ var Search = NewIntegrationTest(NewIntegrationTestArgs{ Contains("three"), Contains("two"), Contains("one").IsSelected(), + ). + NavigateToLine(Contains("three")). + Tap(func() { + t.Views().Search().IsVisible().Content(Contains("matches for 'o' (1 of 3)")) + }). + Press("N"). + Tap(func() { + t.Views().Search().IsVisible().Content(Contains("matches for 'o' (1 of 3)")) + }). + Lines( + Contains("four").IsSelected(), + Contains("three"), + Contains("two"), + Contains("one"), + ). + Press(keys.Universal.StartSearch). + Tap(func() { + t.ExpectSearch(). + Type("t"). + Confirm() + + t.Views().Search().IsVisible().Content(Contains("matches for 't' (1 of 2)")) + }). + Lines( + Contains("four"), + Contains("three").IsSelected(), + Contains("two"), + Contains("one"), + ). + SelectPreviousItem(). + Tap(func() { + t.Views().Search().IsVisible().Content(Contains("matches for 't' (1 of 2)")) + }). + Lines( + Contains("four").IsSelected(), + Contains("three"), + Contains("two"), + Contains("one"), + ). + Press("n"). + Tap(func() { + t.Views().Search().IsVisible().Content(Contains("matches for 't' (1 of 2)")) + }). + Lines( + Contains("four"), + Contains("three").IsSelected(), + Contains("two"), + Contains("one"), ) }, })