Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Set custom prefix filter on predictor #100

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Set custom prefix filter on predictor #100

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5b02316
custom prefix filtering
WillAbides Jul 6, 2019
f4f0aa2
remove extra entries from gocomplete/go.sum
WillAbides Jul 11, 2019
f51d684
fix formatting for code comments
WillAbides Jul 11, 2019
0d8a15e
formatting fix for PrefixFilter check
WillAbides Jul 11, 2019
0aa33eb
unexport defaultPrefixFilter
WillAbides Jul 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions command.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ func (c *Command) predict(a Args) (options []string, only bool) {
// if last completed word is a global flag that we need to complete
if predictor, ok := c.GlobalFlags[a.LastCompleted]; ok && predictor != nil {
Log("Predicting according to global flag %s", a.LastCompleted)
return predictor.Predict(a), true
return predictAndFilterPrefix(predictor, a), true
}

options = append(options, c.GlobalFlags.Predict(a)...)
options = append(options, predictAndFilterPrefix(c.GlobalFlags, a)...)

// if a sub command was entered, we won't add the parent command
// completions and we return here.
Expand All @@ -98,13 +98,13 @@ func (c *Command) predict(a Args) (options []string, only bool) {
// if last completed word is a command flag that we need to complete
if predictor, ok := c.Flags[a.LastCompleted]; ok && predictor != nil {
Log("Predicting according to flag %s", a.LastCompleted)
return predictor.Predict(a), true
return predictAndFilterPrefix(predictor, a), true
}

options = append(options, c.Sub.Predict(a)...)
options = append(options, c.Flags.Predict(a)...)
options = append(options, predictAndFilterPrefix(c.Sub, a)...)
options = append(options, predictAndFilterPrefix(c.Flags, a)...)
if c.Args != nil {
options = append(options, c.Args.Predict(a)...)
options = append(options, predictAndFilterPrefix(c.Args, a)...)
}

return
Expand Down
11 changes: 1 addition & 10 deletions complete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"io"
"os"
"strconv"
"strings"

"github.com/posener/complete/cmd"
)
Expand Down Expand Up @@ -69,15 +68,7 @@ func (c *Complete) Complete() bool {
options := c.Command.Predict(a)
Log("Options: %s", options)

// filter only options that match the last argument
matches := []string{}
for _, option := range options {
if strings.HasPrefix(option, a.Last) {
matches = append(matches, option)
}
}
Log("Matches: %s", matches)
c.output(matches)
c.output(options)
return true
}

Expand Down
51 changes: 42 additions & 9 deletions complete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func TestCompleter_Complete(t *testing.T) {
initTests()

c := Command{
cmd := Command{
Sub: Commands{
"sub1": {
Flags: Flags{
Expand All @@ -28,6 +28,18 @@ func TestCompleter_Complete(t *testing.T) {
},
Args: PredictFiles("*.md"),
},
"permissiveSub": {
Args: &PrefixFilteringPredictor{
Predictor: PredictSet("aaa", "bbb", "Aab"),
PrefixFilterFunc: PermissivePrefixFilter,
},
},
"caseInsensitiveSub": {
Args: &PrefixFilteringPredictor{
Predictor: PredictSet("aaa", "bbb", "Aab", "åaa"),
PrefixFilterFunc: CaseInsensitivePrefixFilter,
},
},
},
Flags: Flags{
"-o": PredictFiles("*.txt"),
Expand All @@ -37,7 +49,8 @@ func TestCompleter_Complete(t *testing.T) {
"-global1": PredictAnything,
},
}
cmp := New("cmd", c)

cmp := New("cmd", cmd)

tests := []struct {
line string
Expand All @@ -47,7 +60,27 @@ func TestCompleter_Complete(t *testing.T) {
{
line: "cmd ",
point: -1,
want: []string{"sub1", "sub2"},
want: []string{"sub1", "sub2", "permissiveSub", "caseInsensitiveSub"},
},
{
line: "cmd permissiveSub ",
point: -1,
want: []string{"aaa", "bbb", "Aab"},
},
{
line: "cmd permissiveSub a",
point: -1,
want: []string{"aaa", "bbb", "Aab"},
},
{
line: "cmd caseInsensitiveSub ",
point: -1,
want: []string{"aaa", "bbb", "Aab", "åaa"},
},
{
line: "cmd caseInsensitiveSub a",
point: -1,
want: []string{"aaa", "Aab"},
},
{
line: "cmd -",
Expand All @@ -57,7 +90,7 @@ func TestCompleter_Complete(t *testing.T) {
{
line: "cmd -h ",
point: -1,
want: []string{"sub1", "sub2"},
want: []string{"sub1", "sub2", "permissiveSub", "caseInsensitiveSub"},
},
{
line: "cmd -global1 ", // global1 is known follow flag
Expand Down Expand Up @@ -142,7 +175,7 @@ func TestCompleter_Complete(t *testing.T) {
{
line: "cmd -no-such-flag ",
point: -1,
want: []string{"sub1", "sub2"},
want: []string{"sub1", "sub2", "permissiveSub", "caseInsensitiveSub"},
},
{
line: "cmd -no-such-flag -",
Expand All @@ -157,7 +190,7 @@ func TestCompleter_Complete(t *testing.T) {
{
line: "cmd no-such-command ",
point: -1,
want: []string{"sub1", "sub2"},
want: []string{"sub1", "sub2", "permissiveSub", "caseInsensitiveSub"},
},
{
line: "cmd -o ",
Expand Down Expand Up @@ -212,12 +245,12 @@ func TestCompleter_Complete(t *testing.T) {
{
line: "cmd -o ./readme.md ",
point: -1,
want: []string{"sub1", "sub2"},
want: []string{"sub1", "sub2", "permissiveSub", "caseInsensitiveSub"},
},
{
line: "cmd -o=./readme.md ",
point: -1,
want: []string{"sub1", "sub2"},
want: []string{"sub1", "sub2", "permissiveSub", "caseInsensitiveSub"},
},
{
line: "cmd -o sub2 -flag3 ",
Expand Down Expand Up @@ -256,7 +289,7 @@ func TestCompleter_Complete(t *testing.T) {
line: "cmd -o ",
// ^
point: 4,
want: []string{"sub1", "sub2"},
want: []string{"sub1", "sub2", "permissiveSub", "caseInsensitiveSub"},
},
}

Expand Down
2 changes: 2 additions & 0 deletions gocomplete/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/U
github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
github.com/hashicorp/go-multierror v1.0.0 h1:iVjPR7a6H0tWELX5NxNe7bYopibicUzc7uPribsnS6o=
github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
WillAbides marked this conversation as resolved.
Show resolved Hide resolved
18 changes: 17 additions & 1 deletion predict.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ func PredictOr(predictors ...Predictor) Predictor {
if p == nil {
continue
}
prediction = append(prediction, p.Predict(a)...)
prediction = append(prediction, predictAndFilterPrefix(p, a)...)
}
return
})
Expand All @@ -39,3 +39,19 @@ var PredictNothing Predictor
// PredictAnything expects something, but nothing particular, such as a number
// or arbitrary name.
var PredictAnything = PredictFunc(func(Args) []string { return nil })

func predictAndFilterPrefix(p Predictor, a Args) []string {
options := p.Predict(a)
prefixerFunc := DefaultPrefixFilter
prefixFilter, ok := p.(PrefixFilter)
WillAbides marked this conversation as resolved.
Show resolved Hide resolved
if ok {
prefixerFunc = prefixFilter.FilterPrefix
}
matches := make([]string, 0, len(options))
for _, option := range options {
if prefixerFunc(option, a.Last) {
matches = append(matches, option)
}
}
return matches
}
35 changes: 35 additions & 0 deletions predict_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,46 @@ package complete
import (
"fmt"
"os"
"reflect"
"sort"
"strings"
"testing"
)

func Test_predictAndFilterPrefix(t *testing.T) {
t.Parallel()
initTests()

t.Run("default prefix filter", func(t *testing.T) {
predictor := PredictSet("a", "ab", "b", "c")
args := Args{
Last: "a",
}
want := []string{"a", "ab"}
got := predictAndFilterPrefix(predictor, args)
if !reflect.DeepEqual(want, got) {
t.Errorf("unexpected result\nwant: %v\ngot: %v", want, got)
}
})

t.Run("permissive filter", func(t *testing.T) {
predictor := PredictSet("a", "ab", "b", "c")
predictor = &PrefixFilteringPredictor{
Predictor: predictor,
PrefixFilterFunc: PermissivePrefixFilter,
}

args := Args{
Last: "a",
}
want := []string{"a", "ab", "b", "c"}
got := predictAndFilterPrefix(predictor, args)
if !reflect.DeepEqual(want, got) {
t.Errorf("unexpected result\nwant: %v\ngot: %v", want, got)
}
})
}

func TestPredicate(t *testing.T) {
t.Parallel()
initTests()
Expand Down
48 changes: 48 additions & 0 deletions prefixfilter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package complete

import (
"strings"
)

//PrefixFilter filters a predictor's options based on the prefix
WillAbides marked this conversation as resolved.
Show resolved Hide resolved
type PrefixFilter interface {
FilterPrefix(str, prefix string) bool
}

//PrefixFilteringPredictor is a Predictor that also implements PrefixFilter
type PrefixFilteringPredictor struct {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this struct definition is not necessary?
If anyone wants to use the new interface, They will just make sure that the predictor also implements the new interface?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be useful in instances where somebody wants to use an existing Predictor with a non-default PrefixFilter.

An example would be if I want to use a SetPredictor with a case insensitive matcher I would do something like:

&complete.PrefixFilteringPredictor{
	PrefixFilterFunc: complete.CaseInsensitivePrefixFilter,
	Predictor:        complete.PredictSet("foo", "bar", "baz", )
}

Without it, I end up implementing PrefixFilteringPredictor in my own codebase. Admittedly it's pretty simple to implement though.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following https://github.com/posener/complete/pull/100/files#r301385506, this won't be needed.

Predictor Predictor
PrefixFilterFunc func(s, prefix string) bool
}

func (p *PrefixFilteringPredictor) Predict(a Args) []string {
if p.Predictor == nil {
return []string{}
}
return p.Predictor.Predict(a)
}

func (p *PrefixFilteringPredictor) FilterPrefix(str, prefix string) bool {
if p.PrefixFilterFunc == nil {
return DefaultPrefixFilter(str, prefix)
}
return p.PrefixFilterFunc(str, prefix)
}

//DefaultPrefixFilter is the PrefixFilter used when none is set
func DefaultPrefixFilter(s, prefix string) bool {
WillAbides marked this conversation as resolved.
Show resolved Hide resolved
return strings.HasPrefix(s, prefix)
}

//PermissivePrefixFilter always returns true
func PermissivePrefixFilter(_, _ string) bool {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MatchAny?
Maybe this is also not needed?

return true
}

//CaseInsensitivePrefixFilter ignores case differences between the prefix and tested string
func CaseInsensitivePrefixFilter(s, prefix string) bool {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Consider a different API where this is a factory method that accepts a Predictor and returns a MatchPredictor: it attributes it with case insensitivity.
WDYT?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I like that suggestion if I understand it correctly. I'm assuming that MatchPredictor would be an interface that includes both Predictor and PrefixFilter (that brings up a naming question which I'm ignoring in this comment to stay focused on this part).

I'll work on an API change that looks like that.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you understood correctly.
This will also enables us dropping PrefixFilteringPredictor.

if len(prefix) > len(s) {
return false
}
return strings.EqualFold(prefix, s[:len(prefix)])
}
Loading