From ec4b8c974c81c2077fb0f8dcd6cfe0f627b228f9 Mon Sep 17 00:00:00 2001
From: Haim Ashkenazi <haim@babysnakes.org>
Date: Wed, 28 Feb 2018 17:04:17 +0200
Subject: [PATCH] zsh-completions: revised flags completion rendering + new
 features:

- If the flags are not bool the completion expects argument.
- You don't have to specify file extensions for file completion to
  work.
- Allow multiple occurrences of flag if type is stringArray.

Need to verify that these assumption are correct :)
---
 zsh_completions.go      | 79 ++++++++++++++++++++++++++++++-----------
 zsh_completions_test.go | 31 +++++++++++++---
 2 files changed, 85 insertions(+), 25 deletions(-)

diff --git a/zsh_completions.go b/zsh_completions.go
index dd23c45f..1d73e272 100644
--- a/zsh_completions.go
+++ b/zsh_completions.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"strings"
 	"text/template"
 
 	"github.com/spf13/pflag"
@@ -11,26 +12,11 @@ import (
 
 var (
 	funcMap = template.FuncMap{
-		"genZshFuncName": generateZshCompletionFuncName,
-		"extractFlags":   extractFlags,
-		"simpleFlag":     simpleFlag,
+		"genZshFuncName":              generateZshCompletionFuncName,
+		"extractFlags":                extractFlags,
+		"genFlagEntryForZshArguments": genFlagEntryForZshArguments,
 	}
 	zshCompletionText = `
-{{/* for pflag.Flag (specifically annotations) */}}
-{{define "flagAnnotations" -}}
-{{with index .Annotations "cobra_annotation_bash_completion_filename_extensions"}}:filename:_files{{end}}
-{{- end}}
-
-{{/* for pflag.Flag with short and long options */}}
-{{define "complexFlag" -}}
-"(-{{.Shorthand}} --{{.Name}})"{-{{.Shorthand}},--{{.Name}}}"[{{.Usage}}]{{template "flagAnnotations" .}}"
-{{- end}}
-
-{{/* for pflag.Flag with either short or long options */}}
-{{define "simpleFlag" -}}
-"{{with .Name}}--{{.}}{{else}}-{{.Shorthand}}{{end}}[{{.Usage}}]{{template "flagAnnotations" .}}"
-{{- end}}
-
 {{/* should accept Command (that contains subcommands) as parameter */}}
 {{define "argumentsC" -}}
 {{ $cmdPath := genZshFuncName .}}
@@ -38,7 +24,7 @@ function {{$cmdPath}} {
   local -a commands
 
   _arguments -C \{{- range extractFlags .}}
-    {{if simpleFlag .}}{{template "simpleFlag" .}}{{else}}{{template "complexFlag" .}}{{end}} \{{- end}}
+    {{genFlagEntryForZshArguments .}} \{{- end}}
     "1: :->cmnds" \
     "*::arg:->args"
 
@@ -66,7 +52,7 @@ function {{$cmdPath}} {
 {{define "arguments" -}}
 function {{genZshFuncName .}} {
 {{"  _arguments"}}{{range extractFlags .}} \
-    {{if simpleFlag .}}{{template "simpleFlag" .}}{{else}}{{template "complexFlag" .}}{{end -}}
+    {{genFlagEntryForZshArguments . -}}
 {{end}}
 }
 {{end}}
@@ -132,3 +118,56 @@ func extractFlags(c *Command) []*pflag.Flag {
 func simpleFlag(p *pflag.Flag) bool {
 	return p.Name == "" || p.Shorthand == ""
 }
+
+// genFlagEntryForZshArguments returns an entry that matches _arguments
+// zsh-completion parameters. It's too complicated to generate in a template.
+func genFlagEntryForZshArguments(f *pflag.Flag) string {
+	if f.Name == "" || f.Shorthand == "" {
+		return genFlagEntryForSingleOptionFlag(f)
+	}
+	return genFlagEntryForMultiOptionFlag(f)
+}
+
+func genFlagEntryForSingleOptionFlag(f *pflag.Flag) string {
+	var option, multiMark, extras string
+
+	if f.Value.Type() == "stringArray" {
+		multiMark = "*"
+	}
+
+	option = "--" + f.Name
+	if option == "--" {
+		option = "-" + f.Shorthand
+	}
+	extras = genZshFlagEntryExtras(f)
+
+	return fmt.Sprintf(`"%s%s[%s]%s"`, multiMark, option, f.Usage, extras)
+}
+
+func genFlagEntryForMultiOptionFlag(f *pflag.Flag) string {
+	var options, parenMultiMark, curlyMultiMark, extras string
+
+	if f.Value.Type() == "stringArray" {
+		parenMultiMark = "*"
+		curlyMultiMark = "\\*"
+	}
+
+	options = fmt.Sprintf(`"(%s-%s %s--%s)"{%s-%s,%s--%s}`,
+		parenMultiMark, f.Shorthand, parenMultiMark, f.Name, curlyMultiMark, f.Shorthand, curlyMultiMark, f.Name)
+	extras = genZshFlagEntryExtras(f)
+
+	return fmt.Sprintf(`%s"[%s]%s"`, options, f.Usage, extras)
+}
+
+func genZshFlagEntryExtras(f *pflag.Flag) string {
+	var extras string
+
+	_, pathSpecified := f.Annotations[BashCompFilenameExt]
+	if pathSpecified {
+		extras = ":filename:_files"
+	} else if !strings.HasPrefix(f.Value.Type(), "bool") {
+		extras = ":" // allow option variable without assisting
+	}
+
+	return extras
+}
diff --git a/zsh_completions_test.go b/zsh_completions_test.go
index c8d80c85..048c5e51 100644
--- a/zsh_completions_test.go
+++ b/zsh_completions_test.go
@@ -48,7 +48,7 @@ func TestGenZshCompletion(t *testing.T) {
 			},
 		},
 		{
-			name: "command with subcommands",
+			name: "command with subcommands and flags with values",
 			root: func() *Command {
 				r := &Command{
 					Use:  "rootcmd",
@@ -75,7 +75,7 @@ func TestGenZshCompletion(t *testing.T) {
 				`_arguments -C \\\n.*"--debug\[description]"`,
 				`function _rootcmd_subcmd1 {`,
 				`function _rootcmd_subcmd1 {`,
-				`_arguments \\\n.*"\(-o --option\)"{-o,--option}"\[option description]" \\\n`,
+				`_arguments \\\n.*"\(-o --option\)"{-o,--option}"\[option description]:" \\\n`,
 			},
 		},
 		{
@@ -88,20 +88,35 @@ func TestGenZshCompletion(t *testing.T) {
 					Run:   emptyRun,
 				}
 				r.Flags().StringVarP(&file, "config", "c", file, "config file")
-				r.MarkFlagFilename("config", "ext")
+				r.MarkFlagFilename("config")
 				return r
 			}(),
 			expectedExpressions: []string{
 				`\n +"\(-c --config\)"{-c,--config}"\[config file]:filename:_files"`,
 			},
 		},
+		{
+			name: "repeated variables both with and without value",
+			root: func() *Command {
+				r := genTestCommand("mycmd", true)
+				_ = r.Flags().StringArrayP("debug", "d", []string{}, "debug usage")
+				_ = r.Flags().StringArray("option", []string{}, "options")
+				return r
+			}(),
+			expectedExpressions: []string{
+				`"\*--option\[options]`,
+				`"\(\*-d \*--debug\)"{\\\*-d,\\\*--debug}`,
+			},
+		},
 	}
 
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
 			tc.root.Execute()
 			buf := new(bytes.Buffer)
-			tc.root.GenZshCompletion(buf)
+			if err := tc.root.GenZshCompletion(buf); err != nil {
+				t.Error(err)
+			}
 			output := buf.Bytes()
 
 			for _, expr := range tc.expectedExpressions {
@@ -173,7 +188,9 @@ func TestGenZshCompletionHidden(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			tc.root.Execute()
 			buf := new(bytes.Buffer)
-			tc.root.GenZshCompletion(buf)
+			if err := tc.root.GenZshCompletion(buf); err != nil {
+				t.Error(err)
+			}
 			output := buf.String()
 
 			for _, expr := range tc.expectedExpressions {
@@ -230,6 +247,7 @@ func constructLargeCommandHeirarchy() *Command {
 	var config, st1, st2 string
 	var long, debug bool
 	var in1, in2 int
+	var verbose []bool
 
 	r := genTestCommand("mycmd", false)
 	r.PersistentFlags().StringVarP(&config, "config", "c", config, "config usage")
@@ -238,6 +256,8 @@ func constructLargeCommandHeirarchy() *Command {
 	}
 	s1 := genTestCommand("sub1", true)
 	s1.Flags().BoolVar(&long, "long", long, "long descriptin")
+	s1.Flags().BoolSliceVar(&verbose, "verbose", verbose, "verbose description")
+	s1.Flags().StringArray("option", []string{}, "various options")
 	s2 := genTestCommand("sub2", true)
 	s2.PersistentFlags().BoolVar(&debug, "debug", debug, "debug description")
 	s3 := genTestCommand("sub3", true)
@@ -249,6 +269,7 @@ func constructLargeCommandHeirarchy() *Command {
 	s1_3 := genTestCommand("sub1sub3", true)
 	s1_3.Flags().IntVar(&in1, "int1", in1, "int1 descriptionn")
 	s1_3.Flags().IntVar(&in2, "int2", in2, "int2 descriptionn")
+	s1_3.Flags().StringArrayP("option", "O", []string{}, "more options")
 	s2_1 := genTestCommand("sub2sub1", true)
 	s2_2 := genTestCommand("sub2sub2", true)
 	s2_3 := genTestCommand("sub2sub3", true)