Skip to content

Commit 3fcf037

Browse files
committed
review
1 parent 15ad85c commit 3fcf037

File tree

6 files changed

+85
-130
lines changed

6 files changed

+85
-130
lines changed

command.go

+28-44
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,6 @@ type Invocation struct {
203203
// Args is reduced into the remaining arguments after parsing flags
204204
// during Run.
205205
Args []string
206-
// CurWord is the word the terminal cursor is currently in
207-
CurWord string
208206

209207
// Environ is a list of environment variables. Use EnvsWithPrefix to parse
210208
// os.Environ.
@@ -297,7 +295,7 @@ func copyFlagSetWithout(fs *pflag.FlagSet, without string) *pflag.FlagSet {
297295
return fs2
298296
}
299297

300-
func (inv *Invocation) curWords() (prev string, cur string) {
298+
func (inv *Invocation) CurWords() (prev string, cur string) {
301299
if len(inv.Args) == 1 {
302300
cur = inv.Args[0]
303301
prev = ""
@@ -407,7 +405,7 @@ func (inv *Invocation) run(state *runState) error {
407405
// Outputted completions are not filtered based on the word under the cursor, as every shell we support does this already.
408406
// We only look at the current word to figure out handler to run, or what directory to inspect.
409407
if inv.IsCompletionMode() {
410-
for _, e := range inv.doCompletions() {
408+
for _, e := range inv.complete() {
411409
fmt.Fprintln(inv.Stdout, e)
412410
}
413411
return nil
@@ -590,24 +588,36 @@ func (inv *Invocation) with(fn func(*Invocation)) *Invocation {
590588
return &i2
591589
}
592590

593-
func (inv *Invocation) doCompletions() []string {
594-
prev, cur := inv.curWords()
595-
inv.CurWord = cur
596-
// If the current word is a flag set using `=`, use it's handler
597-
if strings.HasPrefix(cur, "--") && strings.Contains(cur, "=") {
598-
if out := inv.equalsFlagCompletions(cur); out != nil {
599-
return out
591+
func (inv *Invocation) complete() []string {
592+
prev, cur := inv.CurWords()
593+
594+
if strings.HasPrefix(cur, "--") {
595+
// If the current word is a flag set using `=`, use it's handler
596+
if strings.Contains(cur, "=") {
597+
words := strings.Split(cur, "=")
598+
flagName := words[0][2:]
599+
if out := inv.completeFlag(flagName); out != nil {
600+
for i, o := range out {
601+
out[i] = fmt.Sprintf("--%s=%s", flagName, o)
602+
}
603+
return out
604+
}
605+
} else if out := inv.Command.Options.ByFlag(cur[2:]); out != nil {
606+
// If the current word is a complete flag, auto-complete it so the
607+
// shell moves the cursor
608+
return []string{cur}
600609
}
601610
}
602611
// If the previous word is a flag, then we're writing it's value
603612
// and we should check it's handler
604613
if strings.HasPrefix(prev, "--") {
605-
if out := inv.flagCompletions(prev); out != nil {
614+
word := prev[2:]
615+
if out := inv.completeFlag(word); out != nil {
606616
return out
607617
}
608618
}
609-
// If the current word is the command, auto-complete it so the shell moves the cursor
610-
if inv.Command.Name() == inv.CurWord {
619+
// If the current word is the command, move the shell cursor
620+
if inv.Command.Name() == cur {
611621
return []string{inv.Command.Name()}
612622
}
613623
var completions []string
@@ -621,43 +631,17 @@ func (inv *Invocation) doCompletions() []string {
621631
return completions
622632
}
623633

624-
func (inv *Invocation) flagCompletions(word string) []string {
625-
return inv.doFlagCompletions("", word)
626-
}
627-
628-
func (inv *Invocation) equalsFlagCompletions(word string) []string {
629-
words := strings.Split(word, "=")
630-
word = words[0]
631-
if len(words) > 1 {
632-
inv.CurWord = words[1]
633-
} else {
634-
inv.CurWord = ""
635-
}
636-
prefix := word + "="
637-
return inv.doFlagCompletions(prefix, word)
638-
}
639-
640-
func (inv *Invocation) doFlagCompletions(prefix, word string) []string {
641-
opt := inv.Command.Options.ByFlag(word[2:])
634+
func (inv *Invocation) completeFlag(word string) []string {
635+
opt := inv.Command.Options.ByFlag(word)
642636
if opt == nil {
643637
return nil
644638
}
645639
if opt.CompletionHandler != nil {
646-
completions := opt.CompletionHandler(inv)
647-
out := make([]string, 0, len(completions))
648-
for _, completion := range completions {
649-
out = append(out, fmt.Sprintf("%s%s", prefix, completion))
650-
}
651-
return out
640+
return opt.CompletionHandler(inv)
652641
}
653642
val, ok := opt.Value.(*Enum)
654643
if ok {
655-
completions := val.Choices
656-
out := make([]string, 0, len(completions))
657-
for _, choice := range completions {
658-
out = append(out, fmt.Sprintf("%s%s", prefix, choice))
659-
}
660-
return out
644+
return val.Choices
661645
}
662646
return nil
663647
}

command_test.go

+4-5
Original file line numberDiff line numberDiff line change
@@ -154,11 +154,10 @@ func sampleCommand(t *testing.T) *serpent.Command {
154154
},
155155
Options: serpent.OptionSet{
156156
{
157-
Name: "extra",
158-
Flag: "extra",
159-
Description: "Extra files.",
160-
Value: serpent.StringArrayOf(&fileArr),
161-
CompletionHandler: completion.FileListHandler(nil),
157+
Name: "extra",
158+
Flag: "extra",
159+
Description: "Extra files.",
160+
Value: serpent.StringArrayOf(&fileArr),
162161
},
163162
},
164163
CompletionHandler: func(i *serpent.Invocation) []string {

completion/handlers.go

+30-52
Original file line numberDiff line numberDiff line change
@@ -13,65 +13,43 @@ import (
1313
// given filter func, which may be nil.
1414
func FileHandler(filter func(info os.FileInfo) bool) serpent.CompletionHandlerFunc {
1515
return func(inv *serpent.Invocation) []string {
16-
return listFiles(inv.CurWord, filter)
17-
}
18-
}
16+
var out []string
17+
_, word := inv.CurWords()
1918

20-
// FileListHandler returns a handler that completes a list of comma-separated,
21-
// file names, using the given filter func, which may be nil.
22-
func FileListHandler(filter func(info os.FileInfo) bool) serpent.CompletionHandlerFunc {
23-
return func(inv *serpent.Invocation) []string {
24-
curWord := strings.TrimLeft(inv.CurWord, `"`)
25-
if curWord == "" {
26-
return listFiles("", filter)
19+
dir, _ := filepath.Split(word)
20+
if dir == "" {
21+
dir = "."
2722
}
28-
parts := strings.Split(curWord, ",")
29-
out := listFiles(parts[len(parts)-1], filter)
30-
for i, s := range out {
31-
parts[len(parts)-1] = s
32-
out[i] = strings.Join(parts, ",")
23+
f, err := os.Open(dir)
24+
if err != nil {
25+
return out
3326
}
34-
return out
35-
}
36-
}
37-
38-
func listFiles(word string, filter func(info os.FileInfo) bool) []string {
39-
// Avoid reallocating for each of the first few files we see.
40-
out := make([]string, 0, 16)
41-
42-
dir, _ := filepath.Split(word)
43-
if dir == "" {
44-
dir = "."
45-
}
46-
f, err := os.Open(dir)
47-
if err != nil {
48-
return out
49-
}
50-
defer f.Close()
51-
if dir == "." {
52-
dir = ""
53-
}
54-
55-
infos, err := f.Readdir(0)
56-
if err != nil {
57-
return out
58-
}
59-
60-
for _, info := range infos {
61-
if filter != nil && !filter(info) {
62-
continue
27+
defer f.Close()
28+
if dir == "." {
29+
dir = ""
6330
}
6431

65-
var cur string
66-
if info.IsDir() {
67-
cur = fmt.Sprintf("%s%s%c", dir, info.Name(), os.PathSeparator)
68-
} else {
69-
cur = fmt.Sprintf("%s%s", dir, info.Name())
32+
infos, err := f.Readdir(0)
33+
if err != nil {
34+
return out
7035
}
7136

72-
if strings.HasPrefix(cur, word) {
73-
out = append(out, cur)
37+
for _, info := range infos {
38+
if filter != nil && !filter(info) {
39+
continue
40+
}
41+
42+
var cur string
43+
if info.IsDir() {
44+
cur = fmt.Sprintf("%s%s%c", dir, info.Name(), os.PathSeparator)
45+
} else {
46+
cur = fmt.Sprintf("%s%s", dir, info.Name())
47+
}
48+
49+
if strings.HasPrefix(cur, word) {
50+
out = append(out, cur)
51+
}
7452
}
53+
return out
7554
}
76-
return out
7755
}

completion_test.go

+20-26
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,16 @@ func TestCompletion(t *testing.T) {
8585
require.Equal(t, "--req-array\n--req-enum\n", io.Stdout.String())
8686
})
8787

88+
t.Run("NoOptDefValueFlag", func(t *testing.T) {
89+
t.Parallel()
90+
i := cmd().Invoke("--verbose", "")
91+
i.Environ.Set(serpent.CompletionModeEnv, "1")
92+
io := fakeIO(i)
93+
err := i.Run()
94+
require.NoError(t, err)
95+
require.Equal(t, "altfile\nfile\nrequired-flag\ntoupper\n--prefix\n", io.Stdout.String())
96+
})
97+
8898
t.Run("EnumOK", func(t *testing.T) {
8999
t.Parallel()
90100
i := cmd().Invoke("required-flag", "--req-enum", "")
@@ -105,6 +115,16 @@ func TestCompletion(t *testing.T) {
105115
require.Equal(t, "--req-enum=foo\n--req-enum=bar\n--req-enum=qux\n", io.Stdout.String())
106116
})
107117

118+
t.Run("EnumEqualsBeginQuotesOK", func(t *testing.T) {
119+
t.Parallel()
120+
i := cmd().Invoke("required-flag", "--req-enum", "--req-enum=\"")
121+
i.Environ.Set(serpent.CompletionModeEnv, "1")
122+
io := fakeIO(i)
123+
err := i.Run()
124+
require.NoError(t, err)
125+
require.Equal(t, "--req-enum=foo\n--req-enum=bar\n--req-enum=qux\n", io.Stdout.String())
126+
})
127+
108128
}
109129

110130
func TestFileCompletion(t *testing.T) {
@@ -179,31 +199,5 @@ func TestFileCompletion(t *testing.T) {
179199
require.Equal(t, len(files), len(output))
180200
}
181201
})
182-
t.Run(tc.name+"/List", func(t *testing.T) {
183-
t.Parallel()
184-
for _, path := range tc.paths {
185-
i := cmd().Invoke("altfile", "--extra", fmt.Sprintf(`"example.go,%s`, path))
186-
i.Environ.Set(serpent.CompletionModeEnv, "1")
187-
io := fakeIO(i)
188-
err := i.Run()
189-
require.NoError(t, err)
190-
output := strings.Split(io.Stdout.String(), "\n")
191-
output = output[:len(output)-1]
192-
for _, str := range output {
193-
parts := strings.Split(str, ",")
194-
require.Len(t, parts, 2)
195-
require.Equal(t, parts[0], "example.go")
196-
fileComp := parts[1]
197-
if strings.HasSuffix(fileComp, string(os.PathSeparator)) {
198-
require.DirExists(t, fileComp)
199-
} else {
200-
require.FileExists(t, fileComp)
201-
}
202-
}
203-
files, err := os.ReadDir(tc.realPath)
204-
require.NoError(t, err)
205-
require.Equal(t, len(files), len(output))
206-
}
207-
})
208202
}
209203
}

example/completetest/main.go

-3
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,6 @@ func main() {
108108
Flag: "extra",
109109
Description: "Extra files.",
110110
Value: serpent.StringArrayOf(&fileArr),
111-
CompletionHandler: completion.FileListHandler(func(info os.FileInfo) bool {
112-
return !info.IsDir()
113-
}),
114111
},
115112
},
116113
CompletionHandler: completion.FileHandler(nil),

option.go

+3
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,9 @@ func (optSet OptionSet) ByName(name string) *Option {
347347
}
348348

349349
func (optSet OptionSet) ByFlag(flag string) *Option {
350+
if flag == "" {
351+
return nil
352+
}
350353
for i := range optSet {
351354
opt := &optSet[i]
352355
if opt.Flag == flag || opt.FlagShorthand == flag {

0 commit comments

Comments
 (0)