Update search position (match x of y) when changing the selection in a list view

or in the staging view.

Author: stefanhaller

pkg/gui/context/list_context_trait.go

@@ -24,13 +24,21 @@ 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() {}
 
 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 {

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 {

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"),
 			)
 	},
 })