Merge branch 'filter-shares-search-history'

Fixes: #466

Author: walles

internal/history-navigator.go

@@ -0,0 +1,57 @@
+package internal
+
+// HistoryNavigator lets an InputBox-driven prompt (search, filter, ...) cycle
+// up and down through a shared SearchHistory without losing whatever text the
+// user is in the middle of typing.
+type HistoryNavigator struct {
+	history       *SearchHistory
+	index         int
+	lastTypedText string
+}
+
+// NewHistoryNavigator returns a HistoryNavigator positioned past the end of
+// history's entries, i.e. it will show whatever the user types rather than a
+// history entry until Move is called.
+func NewHistoryNavigator(history *SearchHistory) *HistoryNavigator {
+	return &HistoryNavigator{
+		history: history,
+		index:   len(history.entries),
+	}
+}
+
+// TextEdited must be called whenever the user changes the input box text by
+// hand, so that arrowing away and back finds that text again.
+func (n *HistoryNavigator) TextEdited(text string) {
+	n.index = len(n.history.entries)
+	n.lastTypedText = text
+}
+
+// Move steps the history index by delta (negative towards older entries,
+// positive towards newer), clamping at both ends. It returns the text that
+// should now be shown, and whether there was anything to move to at all (an
+// empty history has nothing to navigate).
+func (n *HistoryNavigator) Move(delta int) (string, bool) {
+	if len(n.history.entries) == 0 {
+		return "", false
+	}
+
+	n.index += delta
+	if n.index < 0 {
+		n.index = 0
+	}
+	if n.index > len(n.history.entries) {
+		n.index = len(n.history.entries)
+	}
+
+	if n.index == len(n.history.entries) {
+		return n.lastTypedText, true
+	}
+
+	return n.history.entries[n.index], true
+}
+
+// Commit adds text as a new history entry. Call this when the user submits
+// or aborts the prompt.
+func (n *HistoryNavigator) Commit(text string) {
+	n.history.addEntry(text)
+}

internal/history-navigator_test.go

@@ -0,0 +1,110 @@
+package internal
+
+import (
+	"testing"
+
+	"gotest.tools/v3/assert"
+)
+
+func TestHistoryNavigator_MoveCyclesThroughEntriesNewestFirst(t *testing.T) {
+	history := &SearchHistory{entries: []string{"a", "b", "c"}}
+	navigator := NewHistoryNavigator(history)
+
+	text, ok := navigator.Move(-1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "c", text)
+
+	text, ok = navigator.Move(-1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "b", text)
+
+	text, ok = navigator.Move(-1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "a", text)
+}
+
+func TestHistoryNavigator_MoveClampsAtOldestEntry(t *testing.T) {
+	history := &SearchHistory{entries: []string{"a", "b"}}
+	navigator := NewHistoryNavigator(history)
+
+	navigator.Move(-1)
+	navigator.Move(-1)
+	text, ok := navigator.Move(-1)
+
+	assert.Assert(t, ok)
+	assert.Equal(t, "a", text)
+}
+
+func TestHistoryNavigator_MoveClampsAtNewestEntry(t *testing.T) {
+	history := &SearchHistory{entries: []string{"a", "b"}}
+	navigator := NewHistoryNavigator(history)
+
+	navigator.Move(-1)
+	navigator.Move(1)
+	text, ok := navigator.Move(1)
+
+	assert.Assert(t, ok)
+	assert.Equal(t, "", text)
+}
+
+func TestHistoryNavigator_MoveOnEmptyHistoryDoesNothing(t *testing.T) {
+	history := &SearchHistory{}
+	navigator := NewHistoryNavigator(history)
+
+	_, ok := navigator.Move(-1)
+
+	assert.Assert(t, !ok)
+}
+
+func TestHistoryNavigator_TypedTextSurvivesUpThenDown(t *testing.T) {
+	history := &SearchHistory{entries: []string{"a", "b"}}
+	navigator := NewHistoryNavigator(history)
+
+	navigator.TextEdited("typing this")
+
+	text, ok := navigator.Move(-1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "b", text)
+
+	text, ok = navigator.Move(1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "typing this", text)
+}
+
+func TestHistoryNavigator_MoveStepsThroughEveryEntryBothWays(t *testing.T) {
+	history := &SearchHistory{entries: []string{"a", "b", "c"}}
+	navigator := NewHistoryNavigator(history)
+
+	text, ok := navigator.Move(-1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "c", text)
+
+	text, ok = navigator.Move(-1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "b", text)
+
+	text, ok = navigator.Move(-1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "a", text)
+
+	text, ok = navigator.Move(1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "b", text)
+
+	text, ok = navigator.Move(1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "c", text)
+
+	text, ok = navigator.Move(1)
+	assert.Assert(t, ok)
+	assert.Equal(t, "", text)
+}
+
+func TestHistoryNavigator_CommitAddsEntryToHistory(t *testing.T) {
+	history := &SearchHistory{entries: []string{"a"}}
+	navigator := NewHistoryNavigator(history)
+
+	navigator.Commit("b")
+
+	assert.DeepEqual(t, []string{"a", "b"}, history.entries)
+}

internal/pagermode-filter.go

@@ -7,13 +7,17 @@ import (
 )
 
 type PagerModeFilter struct {
-	pager    *Pager
-	inputBox *InputBox
+	pager                 *Pager
+	initialScrollPosition scrollPosition // Pager position before filtering started
+	inputBox              *InputBox
+	history               *HistoryNavigator
 }
 
-func NewPagerModeFilter(p *Pager) *PagerModeFilter {
+func NewPagerModeFilter(p *Pager, initialScrollPosition scrollPosition) *PagerModeFilter {
 	m := &PagerModeFilter{
-		pager: p,
+		pager:                 p,
+		initialScrollPosition: initialScrollPosition,
+		history:               NewHistoryNavigator(p.searchHistory),
 	}
 	m.inputBox = &InputBox{
 		accept: INPUTBOX_ACCEPT_ALL,
@@ -35,21 +39,36 @@ func (m *PagerModeFilter) updateFilterPattern(text string) {
 
 func (m *PagerModeFilter) onKey(key twin.KeyCode) {
 	if m.inputBox.handleKey(key) {
+		m.history.TextEdited(m.inputBox.text)
 		return
 	}
 
 	switch key {
 	case twin.KeyEnter:
+		m.history.Commit(m.inputBox.text)
 		m.pager.mode = PagerModeViewing{pager: m.pager}
 
 	case twin.KeyEscape:
+		m.history.Commit(m.inputBox.text)
 		m.pager.mode = PagerModeViewing{pager: m.pager}
 		m.pager.filter = search.Search{}
 		m.pager.search.Clear()
+		m.pager.scrollPosition = m.initialScrollPosition
 
-	case twin.KeyUp, twin.KeyDown, twin.KeyPgUp, twin.KeyPgDown:
-		viewing := PagerModeViewing{pager: m.pager}
-		viewing.onKey(key)
+	case twin.KeyUp:
+		if text, ok := m.history.Move(-1); ok {
+			m.inputBox.setText(text)
+		}
+
+	case twin.KeyDown:
+		if text, ok := m.history.Move(1); ok {
+			m.inputBox.setText(text)
+		}
+
+	case twin.KeyPgUp, twin.KeyPgDown:
+		m.history.Commit(m.inputBox.text)
+		m.pager.mode = PagerModeViewing{pager: m.pager}
+		m.pager.mode.onKey(key)
 
 	default:
 		log.Debugf("Unhandled filter key event %v", key)
@@ -58,4 +77,5 @@ func (m *PagerModeFilter) onKey(key twin.KeyCode) {
 
 func (m *PagerModeFilter) onRune(char rune) {
 	m.inputBox.handleRune(char)
+	m.history.TextEdited(m.inputBox.text)
 }

internal/pagermode-filter_test.go

@@ -0,0 +1,99 @@
+package internal
+
+import (
+	"testing"
+
+	"github.com/walles/moor/v2/internal/linemetadata"
+	"github.com/walles/twin"
+	"gotest.tools/v3/assert"
+)
+
+// Ref: https://github.com/walles/moor/issues/466
+func TestPagerModeFilter_UpArrowShowsMostRecentSearchHistoryEntry(t *testing.T) {
+	pager := createThreeLinesPager(t)
+	pager.searchHistory.entries = []string{"b", "d"}
+
+	filterMode := NewPagerModeFilter(pager, pager.scrollPosition)
+	filterMode.onKey(twin.KeyUp)
+
+	assert.Equal(t, "d", filterMode.inputBox.text)
+}
+
+func TestPagerModeFilter_DownArrowStepsBackTowardsNewestEntry(t *testing.T) {
+	pager := createThreeLinesPager(t)
+	pager.searchHistory.entries = []string{"b", "d"}
+
+	filterMode := NewPagerModeFilter(pager, pager.scrollPosition)
+	filterMode.onKey(twin.KeyUp)
+	filterMode.onKey(twin.KeyUp)
+	filterMode.onKey(twin.KeyDown)
+
+	assert.Equal(t, "d", filterMode.inputBox.text)
+}
+
+func TestPagerModeFilter_EnterPersistsTypedTextToSearchHistory(t *testing.T) {
+	pager := createThreeLinesPager(t)
+	pager.searchHistory = &SearchHistory{} // No file backing, keep this test disk-free
+
+	filterMode := NewPagerModeFilter(pager, pager.scrollPosition)
+	filterMode.inputBox.setText("abc")
+	filterMode.onKey(twin.KeyEnter)
+
+	assert.DeepEqual(t, []string{"abc"}, pager.searchHistory.entries)
+}
+
+// Search and filter modes each get their own HistoryNavigator, but both wrap
+// the same *SearchHistory on the pager, so an entry committed from one mode
+// must be visible when navigating history from the other.
+//
+// Ref: https://github.com/walles/moor/issues/466
+func TestPagerModeFilter_SharesSearchHistoryWithSearchMode(t *testing.T) {
+	pager := createThreeLinesPager(t)
+	pager.searchHistory = &SearchHistory{} // No file backing, keep this test disk-free
+
+	filterMode := NewPagerModeFilter(pager, pager.scrollPosition)
+	filterMode.inputBox.setText("from filter")
+	filterMode.onKey(twin.KeyEnter)
+
+	searchMode := NewPagerModeSearch(pager, SearchDirectionForward, pager.scrollPosition)
+	searchMode.onKey(twin.KeyUp)
+
+	assert.Equal(t, "from filter", searchMode.inputBox.text)
+}
+
+// Pressing ESC should return the view to wherever it was scrolled to before
+// filtering started, the same way search's ESC does, even though filtering
+// out non-matching lines may have moved the pager's scroll position while
+// typing.
+//
+// Ref: https://github.com/walles/moor/issues/466
+func TestPagerModeFilter_EscapeRestoresScrollPositionFromBeforeFiltering(t *testing.T) {
+	pager := createThreeLinesPager(t)
+	pager.scrollPosition = NewScrollPositionFromIndex(linemetadata.IndexFromZeroBased(4), "before filtering")
+	initialIndex := pager.lineIndex().Index()
+
+	filterMode := NewPagerModeFilter(pager, pager.scrollPosition)
+	filterMode.onRune('a') // Filters down to only the line containing "a"
+
+	// This mirrors what a real redraw does, and confirms filtering really did
+	// move the scroll position as a side effect.
+	assert.Assert(t, pager.lineIndex().Index() != initialIndex, "Filtering should have moved the scroll position")
+
+	filterMode.onKey(twin.KeyEscape)
+
+	assert.Equal(t, initialIndex, pager.lineIndex().Index())
+}
+
+// Ref: https://github.com/walles/moor/issues/466
+func TestPagerModeFilter_PgDownPersistsTypedTextAndSwitchesToViewing(t *testing.T) {
+	pager := createThreeLinesPager(t)
+	pager.searchHistory = &SearchHistory{} // No file backing, keep this test disk-free
+
+	filterMode := NewPagerModeFilter(pager, pager.scrollPosition)
+	pager.mode = filterMode
+	filterMode.inputBox.setText("abc")
+	filterMode.onKey(twin.KeyPgDown)
+
+	assert.DeepEqual(t, []string{"abc"}, pager.searchHistory.entries)
+	assert.Equal(t, "Viewing", modeName(pager))
+}