From 7f62fe18b3e55a134c1c118fee2ee1db3e75cbf5 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Mon, 16 Feb 2026 22:43:11 +0900 Subject: [PATCH] Refactor filter --- filter/base.go | 30 +++++++++ filter/base_test.go | 144 ++++++++++++++++++++++++++++++++++++++++++++ filter/fuzzy.go | 27 +-------- filter/interface.go | 9 +++ filter/regexp.go | 40 +++--------- 5 files changed, 193 insertions(+), 57 deletions(-) create mode 100644 filter/base.go create mode 100644 filter/base_test.go diff --git a/filter/base.go b/filter/base.go new file mode 100644 index 0000000..22aa5dd --- /dev/null +++ b/filter/base.go @@ -0,0 +1,30 @@ +package filter + +import ( + "context" + + "github.com/peco/peco/line" + "github.com/peco/peco/pipeline" +) + +func (b *baseFilter) NewContext(ctx context.Context, query string) context.Context { + return newContext(ctx, query) +} + +func (b *baseFilter) BufSize() int { + return 0 +} + +func (b *baseFilter) Apply(ctx context.Context, lines []line.Line, out pipeline.ChanOutput) error { + return b.applyFn(ctx, lines, func(l line.Line) { + out.Send(ctx, l) + }) +} + +func (b *baseFilter) ApplyCollect(ctx context.Context, lines []line.Line) ([]line.Line, error) { + result := make([]line.Line, 0, len(lines)/2) + err := b.applyFn(ctx, lines, func(l line.Line) { + result = append(result, l) + }) + return result, err +} diff --git a/filter/base_test.go b/filter/base_test.go new file mode 100644 index 0000000..5a4fcdd --- /dev/null +++ b/filter/base_test.go @@ -0,0 +1,144 @@ +package filter + +import ( + "context" + "testing" + "time" + + "github.com/peco/peco/line" + "github.com/peco/peco/pipeline" + "github.com/stretchr/testify/require" +) + +// TestApplyAndApplyCollectConsistency verifies that Apply (channel-based) and +// ApplyCollect (slice-based) produce identical results for every built-in +// filter type. This is the key invariant that the baseFilter extraction must +// preserve. +func TestApplyAndApplyCollectConsistency(t *testing.T) { + lines := makeLines( + "hello world", + "hello tests", + "goodbye world", + "goodbye tests", + "fuzzy matching example", + "another line", + ) + + tests := []struct { + name string + filter Filter + query string + }{ + {"IgnoreCase", NewIgnoreCase(), "hello"}, + {"CaseSensitive", NewCaseSensitive(), "hello"}, + {"SmartCase", NewSmartCase(), "hello"}, + {"Regexp", NewRegexp(), "hello.*world"}, + {"IRegexp", NewIRegexp(), "HELLO"}, + {"Fuzzy", NewFuzzy(false), "hw"}, + {"FuzzyLongest", NewFuzzy(true), "hw"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx, cancel := context.WithTimeout( + tt.filter.NewContext(context.Background(), tt.query), + 10*time.Second, + ) + defer cancel() + + // Collect via Apply (channel path) + ch := make(chan interface{}, len(lines)+1) + err := tt.filter.Apply(ctx, lines, pipeline.ChanOutput(ch)) + require.NoError(t, err, "Apply should succeed") + close(ch) + + var applyResults []string + for v := range ch { + l, ok := v.(line.Line) + require.True(t, ok, "channel value should be line.Line") + applyResults = append(applyResults, l.DisplayString()) + } + + // Collect via ApplyCollect (direct slice path) + collector, ok := tt.filter.(Collector) + require.True(t, ok, "%s should implement Collector", tt.name) + + ctx2, cancel2 := context.WithTimeout( + tt.filter.NewContext(context.Background(), tt.query), + 10*time.Second, + ) + defer cancel2() + + collected, err := collector.ApplyCollect(ctx2, lines) + require.NoError(t, err, "ApplyCollect should succeed") + + var collectResults []string + for _, l := range collected { + collectResults = append(collectResults, l.DisplayString()) + } + + // They must produce exactly the same results + require.Equal(t, applyResults, collectResults, + "Apply and ApplyCollect should produce identical results") + require.NotEmpty(t, applyResults, + "test should produce at least one match (verify test data)") + }) + } +} + +// TestBufSizeDefaults verifies that built-in filters return the expected +// BufSize (0 for Regexp/Fuzzy, meaning "use config default"). +func TestBufSizeDefaults(t *testing.T) { + tests := []struct { + name string + filter Filter + expected int + }{ + {"IgnoreCase", NewIgnoreCase(), 0}, + {"CaseSensitive", NewCaseSensitive(), 0}, + {"SmartCase", NewSmartCase(), 0}, + {"Regexp", NewRegexp(), 0}, + {"IRegexp", NewIRegexp(), 0}, + {"Fuzzy", NewFuzzy(false), 0}, + {"FuzzyLongest", NewFuzzy(true), 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expected, tt.filter.BufSize()) + }) + } +} + +// TestNewContextStoresQuery verifies that NewContext stores the query and it +// can be retrieved by the filter's applyInternal. +func TestNewContextStoresQuery(t *testing.T) { + filters := []struct { + name string + filter Filter + }{ + {"IgnoreCase", NewIgnoreCase()}, + {"Fuzzy", NewFuzzy(false)}, + } + + for _, tt := range filters { + t.Run(tt.name, func(t *testing.T) { + ctx := tt.filter.NewContext(context.Background(), "test-query") + // The query should be stored in context — verify by running Apply + // with a line that matches "test-query" + ch := make(chan interface{}, 2) + lines := makeLines("this is a test-query line") + err := tt.filter.Apply(ctx, lines, pipeline.ChanOutput(ch)) + require.NoError(t, err) + close(ch) + + var results []line.Line + for v := range ch { + if l, ok := v.(line.Line); ok { + results = append(results, l) + } + } + require.Len(t, results, 1, "query stored by NewContext should be used by Apply") + }) + } +} diff --git a/filter/fuzzy.go b/filter/fuzzy.go index c2f0886..d3f1eb1 100644 --- a/filter/fuzzy.go +++ b/filter/fuzzy.go @@ -12,7 +12,6 @@ import ( "github.com/peco/peco/internal/util" "github.com/peco/peco/line" - "github.com/peco/peco/pipeline" ) // NewFuzzy builds a fuzzy-finder type of filter. @@ -25,17 +24,11 @@ import ( // 2. Earlier match // 3. Shorter line length func NewFuzzy(sortLongest bool) *Fuzzy { - return &Fuzzy{ + ff := &Fuzzy{ sortLongest: sortLongest, } -} - -func (ff Fuzzy) BufSize() int { - return 0 -} - -func (ff *Fuzzy) NewContext(ctx context.Context, query string) context.Context { - return newContext(ctx, query) + ff.applyFn = ff.applyInternal + return ff } func (ff Fuzzy) SupportsParallel() bool { @@ -194,20 +187,6 @@ LINE: return nil } -func (ff *Fuzzy) Apply(ctx context.Context, lines []line.Line, out pipeline.ChanOutput) error { - return ff.applyInternal(ctx, lines, func(l line.Line) { - out.Send(ctx, l) - }) -} - -func (ff *Fuzzy) ApplyCollect(ctx context.Context, lines []line.Line) ([]line.Line, error) { - result := make([]line.Line, 0, len(lines)/2) - err := ff.applyInternal(ctx, lines, func(l line.Line) { - result = append(result, l) - }) - return result, err -} - func popRune(s string) (string, rune, int) { r, n := utf8.DecodeRuneInString(s) return s[n:], r, n diff --git a/filter/interface.go b/filter/interface.go index ed50d82..6b5e671 100644 --- a/filter/interface.go +++ b/filter/interface.go @@ -47,11 +47,20 @@ type regexpQuery struct { lastUsed time.Time } +// baseFilter provides shared implementations of Apply, ApplyCollect, +// NewContext, and BufSize for filters that follow the applyInternal pattern. +// Filters embed this type and set applyFn to their type-specific matching logic. +type baseFilter struct { + applyFn func(ctx context.Context, lines []line.Line, emit func(line.Line)) error +} + type Fuzzy struct { + baseFilter sortLongest bool } type Regexp struct { + baseFilter factory *regexpQueryFactory flags regexpFlags quotemeta bool diff --git a/filter/regexp.go b/filter/regexp.go index 58b79eb..7793386 100644 --- a/filter/regexp.go +++ b/filter/regexp.go @@ -79,13 +79,9 @@ func termsToRegexps(terms []string, fullQuery string, flags regexpFlags, quoteme return regexps, nil } -func (rf *Regexp) NewContext(ctx context.Context, query string) context.Context { - return newContext(ctx, query) -} - // NewRegexp creates a new regexp based filter func NewRegexp() *Regexp { - return &Regexp{ + rf := &Regexp{ factory: ®expQueryFactory{ compiled: make(map[string]regexpQuery), threshold: time.Minute, @@ -95,24 +91,16 @@ func NewRegexp() *Regexp { name: "Regexp", outCh: pipeline.ChanOutput(make(chan interface{})), } + rf.applyFn = rf.applyInternal + return rf } // NewIRegexp creates a new case-insensitive regexp based filter func NewIRegexp() *Regexp { - return &Regexp{ - factory: ®expQueryFactory{ - compiled: make(map[string]regexpQuery), - threshold: time.Minute, - }, - flags: regexpFlagList(regexpFlagList{"i"}), - quotemeta: false, - name: "IRegexp", - outCh: pipeline.ChanOutput(make(chan interface{})), - } -} - -func (rf *Regexp) BufSize() int { - return 0 + rf := NewRegexp() + rf.flags = regexpFlagList([]string{"i"}) + rf.name = "IRegexp" + return rf } func (rf *Regexp) OutCh() <-chan interface{} { @@ -257,20 +245,6 @@ func (rf *Regexp) applyInternal(ctx context.Context, lines []line.Line, emit fun return nil } -func (rf *Regexp) Apply(ctx context.Context, lines []line.Line, out pipeline.ChanOutput) error { - return rf.applyInternal(ctx, lines, func(l line.Line) { - out.Send(ctx, l) - }) -} - -func (rf *Regexp) ApplyCollect(ctx context.Context, lines []line.Line) ([]line.Line, error) { - result := make([]line.Line, 0, len(lines)/2) - err := rf.applyInternal(ctx, lines, func(l line.Line) { - result = append(result, l) - }) - return result, err -} - func (rf *Regexp) SupportsParallel() bool { return true }