Skip to content

Commit

Permalink
Don't search the whole file more than once
Browse files Browse the repository at this point in the history
Should improve the performance situation reported here:
#209
  • Loading branch information
walles committed May 13, 2024
1 parent e6c69c2 commit edb3e51
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 10 deletions.
10 changes: 5 additions & 5 deletions m/pager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ func TestFindFirstHitSimple(t *testing.T) {

pager.searchPattern = toPattern("AB")

hit := pager.findFirstHit(linenumbers.LineNumber{}, false)
hit := pager.findFirstHit(linenumbers.LineNumber{}, nil, false)
assert.Assert(t, hit.internalDontTouch.lineNumber.IsZero())
assert.Equal(t, hit.internalDontTouch.deltaScreenLines, 0)
}
Expand All @@ -312,7 +312,7 @@ func TestFindFirstHitAnsi(t *testing.T) {

pager.searchPattern = toPattern("AB")

hit := pager.findFirstHit(linenumbers.LineNumber{}, false)
hit := pager.findFirstHit(linenumbers.LineNumber{}, nil, false)
assert.Assert(t, hit.internalDontTouch.lineNumber.IsZero())
assert.Equal(t, hit.internalDontTouch.deltaScreenLines, 0)
}
Expand All @@ -326,7 +326,7 @@ func TestFindFirstHitNoMatch(t *testing.T) {

pager.searchPattern = toPattern("this pattern should not be found")

hit := pager.findFirstHit(linenumbers.LineNumber{}, false)
hit := pager.findFirstHit(linenumbers.LineNumber{}, nil, false)
assert.Assert(t, hit == nil)
}

Expand All @@ -340,7 +340,7 @@ func TestFindFirstHitNoMatchBackwards(t *testing.T) {
pager.searchPattern = toPattern("this pattern should not be found")
theEnd := *linenumbers.LineNumberFromLength(reader.GetLineCount())

hit := pager.findFirstHit(theEnd, true)
hit := pager.findFirstHit(theEnd, nil, true)
assert.Assert(t, hit == nil)
}

Expand Down Expand Up @@ -682,7 +682,7 @@ func benchmarkSearch(b *testing.B, highlighted bool) {
b.ResetTimer()

// This test will search through all the N copies we made of our file
hit := pager.findFirstHit(linenumbers.LineNumber{}, false)
hit := pager.findFirstHit(linenumbers.LineNumber{}, nil, false)

if hit != nil {
panic(fmt.Errorf("This test is meant to scan the whole file without finding anything"))
Expand Down
15 changes: 10 additions & 5 deletions m/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@ func (p *Pager) scrollToSearchHits() {
return
}

firstHitPosition := p.findFirstHit(*p.scrollPosition.lineNumber(p), false)
firstHitPosition := p.findFirstHit(*p.scrollPosition.lineNumber(p), nil, false)
if firstHitPosition == nil {
// Try again from the top
firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, false)
firstHitPosition = p.findFirstHit(linenumbers.LineNumber{}, p.scrollPosition.lineNumber(p), false)
}
if firstHitPosition == nil {
// No match, give up
Expand All @@ -35,7 +35,7 @@ func (p *Pager) scrollToSearchHits() {
// scrollPosition for searching.
//
// FIXME: We should take startPosition.deltaScreenLines into account as well!
func (p *Pager) findFirstHit(startPosition linenumbers.LineNumber, backwards bool) *scrollPosition {
func (p *Pager) findFirstHit(startPosition linenumbers.LineNumber, beforePosition *linenumbers.LineNumber, backwards bool) *scrollPosition {
searchPosition := startPosition
for {
line := p.reader.GetLine(searchPosition)
Expand All @@ -58,6 +58,11 @@ func (p *Pager) findFirstHit(startPosition linenumbers.LineNumber, backwards boo
searchPosition = searchPosition.NonWrappingAdd(-1)
} else {
searchPosition = searchPosition.NonWrappingAdd(1)

if beforePosition != nil && searchPosition == *beforePosition {
// No match, give up
return nil
}
}
}
}
Expand Down Expand Up @@ -105,7 +110,7 @@ func (p *Pager) scrollToNextSearchHit() {
panic(fmt.Sprint("Unknown search mode when finding next: ", p.mode))
}

firstHitPosition := p.findFirstHit(firstSearchPosition, false)
firstHitPosition := p.findFirstHit(firstSearchPosition, nil, false)
if firstHitPosition == nil {
p.mode = PagerModeNotFound{pager: p}
return
Expand Down Expand Up @@ -144,7 +149,7 @@ func (p *Pager) scrollToPreviousSearchHit() {
panic(fmt.Sprint("Unknown search mode when finding previous: ", p.mode))
}

firstHitPosition := p.findFirstHit(firstSearchPosition, true)
firstHitPosition := p.findFirstHit(firstSearchPosition, nil, true)
if firstHitPosition == nil {
p.mode = PagerModeNotFound{pager: p}
return
Expand Down

0 comments on commit edb3e51

Please sign in to comment.