Merge pull request #84 from zimmski/fix-required-flag-completion

fix required flag completion
diff --git a/completion.go b/completion.go
index fea0e3a..76a2808 100644
--- a/completion.go
+++ b/completion.go
@@ -275,7 +275,7 @@
 	return ret
 }
 
-func (c *completion) Execute(args []string) error {
+func (c *completion) execute(args []string) {
 	ret := c.complete(args)
 
 	if c.ShowDescriptions && len(ret) > 1 {
@@ -301,6 +301,4 @@
 			fmt.Println(v.Item)
 		}
 	}
-
-	return nil
 }
diff --git a/completion_test.go b/completion_test.go
index 908e421..7dc916c 100644
--- a/completion_test.go
+++ b/completion_test.go
@@ -1,6 +1,9 @@
 package flags
 
 import (
+	"bytes"
+	"io"
+	"os"
 	"path"
 	"path/filepath"
 	"reflect"
@@ -33,9 +36,10 @@
 }
 
 var completionTestOptions struct {
-	Verbose bool `short:"v" long:"verbose"`
-	Debug   bool `short:"d" long:"debug"`
-	Version bool `long:"version"`
+	Verbose  bool `short:"v" long:"verbose"`
+	Debug    bool `short:"d" long:"debug"`
+	Version  bool `long:"version"`
+	Required bool `long:"required" required:"true"`
 
 	AddCommand struct {
 		Positional struct {
@@ -59,16 +63,20 @@
 	} `command:"rename"`
 }
 
-func TestCompletion(t *testing.T) {
+type completionTest struct {
+	Args      []string
+	Completed []string
+}
+
+var completionTests []completionTest
+
+func init() {
 	_, sourcefile, _, _ := runtime.Caller(0)
-	sourcedir := filepath.Join(filepath.SplitList(path.Dir(sourcefile))...)
+	completionTestSourcedir := filepath.Join(filepath.SplitList(path.Dir(sourcefile))...)
 
-	excompl := []string{filepath.Join(sourcedir, "completion.go"), filepath.Join(sourcedir, "completion_test.go")}
+	completionTestFilename := []string{filepath.Join(completionTestSourcedir, "completion.go"), filepath.Join(completionTestSourcedir, "completion_test.go")}
 
-	tests := []struct {
-		Args      []string
-		Completed []string
-	}{
+	completionTests = []completionTest{
 		{
 			// Short names
 			[]string{"-"},
@@ -84,7 +92,7 @@
 		{
 			// Long names
 			[]string{"--"},
-			[]string{"--debug", "--verbose", "--version"},
+			[]string{"--debug", "--required", "--verbose", "--version"},
 		},
 
 		{
@@ -107,60 +115,60 @@
 
 		{
 			// Positional filename
-			[]string{"add", filepath.Join(sourcedir, "completion")},
-			excompl,
+			[]string{"add", filepath.Join(completionTestSourcedir, "completion")},
+			completionTestFilename,
 		},
 
 		{
 			// Multiple positional filename (1 arg)
-			[]string{"add-multi", filepath.Join(sourcedir, "completion")},
-			excompl,
+			[]string{"add-multi", filepath.Join(completionTestSourcedir, "completion")},
+			completionTestFilename,
 		},
 		{
 			// Multiple positional filename (2 args)
-			[]string{"add-multi", filepath.Join(sourcedir, "completion.go"), filepath.Join(sourcedir, "completion")},
-			excompl,
+			[]string{"add-multi", filepath.Join(completionTestSourcedir, "completion.go"), filepath.Join(completionTestSourcedir, "completion")},
+			completionTestFilename,
 		},
 		{
 			// Multiple positional filename (3 args)
-			[]string{"add-multi", filepath.Join(sourcedir, "completion.go"), filepath.Join(sourcedir, "completion.go"), filepath.Join(sourcedir, "completion")},
-			excompl,
+			[]string{"add-multi", filepath.Join(completionTestSourcedir, "completion.go"), filepath.Join(completionTestSourcedir, "completion.go"), filepath.Join(completionTestSourcedir, "completion")},
+			completionTestFilename,
 		},
 
 		{
 			// Flag filename
-			[]string{"rm", "-f", path.Join(sourcedir, "completion")},
-			excompl,
+			[]string{"rm", "-f", path.Join(completionTestSourcedir, "completion")},
+			completionTestFilename,
 		},
 
 		{
 			// Flag short concat last filename
-			[]string{"rm", "-of", path.Join(sourcedir, "completion")},
-			excompl,
+			[]string{"rm", "-of", path.Join(completionTestSourcedir, "completion")},
+			completionTestFilename,
 		},
 
 		{
 			// Flag concat filename
-			[]string{"rm", "-f" + path.Join(sourcedir, "completion")},
-			[]string{"-f" + excompl[0], "-f" + excompl[1]},
+			[]string{"rm", "-f" + path.Join(completionTestSourcedir, "completion")},
+			[]string{"-f" + completionTestFilename[0], "-f" + completionTestFilename[1]},
 		},
 
 		{
 			// Flag equal concat filename
-			[]string{"rm", "-f=" + path.Join(sourcedir, "completion")},
-			[]string{"-f=" + excompl[0], "-f=" + excompl[1]},
+			[]string{"rm", "-f=" + path.Join(completionTestSourcedir, "completion")},
+			[]string{"-f=" + completionTestFilename[0], "-f=" + completionTestFilename[1]},
 		},
 
 		{
 			// Flag concat long filename
-			[]string{"rm", "--filename=" + path.Join(sourcedir, "completion")},
-			[]string{"--filename=" + excompl[0], "--filename=" + excompl[1]},
+			[]string{"rm", "--filename=" + path.Join(completionTestSourcedir, "completion")},
+			[]string{"--filename=" + completionTestFilename[0], "--filename=" + completionTestFilename[1]},
 		},
 
 		{
 			// Flag long filename
-			[]string{"rm", "--filename", path.Join(sourcedir, "completion")},
-			excompl,
+			[]string{"rm", "--filename", path.Join(completionTestSourcedir, "completion")},
+			completionTestFilename,
 		},
 
 		{
@@ -169,11 +177,13 @@
 			[]string{"hello universe"},
 		},
 	}
+}
 
+func TestCompletion(t *testing.T) {
 	p := NewParser(&completionTestOptions, Default)
 	c := &completion{parser: p}
 
-	for _, test := range tests {
+	for _, test := range completionTests {
 		ret := c.complete(test.Args)
 		items := make([]string, len(ret))
 
@@ -186,3 +196,44 @@
 		}
 	}
 }
+
+func TestParserCompletion(t *testing.T) {
+	os.Setenv("GO_FLAGS_COMPLETION", "1")
+
+	for _, test := range completionTests {
+		tmp := os.Stdout
+
+		r, w, _ := os.Pipe()
+		os.Stdout = w
+
+		out := make(chan string)
+
+		go func() {
+			var buf bytes.Buffer
+
+			io.Copy(&buf, r)
+
+			out <- buf.String()
+		}()
+
+		p := NewParser(&completionTestOptions, None)
+
+		_, err := p.ParseArgs(test.Args)
+
+		w.Close()
+
+		os.Stdout = tmp
+
+		if err != nil {
+			t.Fatalf("Unexpected error: %s", err)
+		}
+
+		got := strings.Split(strings.Trim(<-out, "\n"), "\n")
+
+		if !reflect.DeepEqual(got, test.Completed) {
+			t.Errorf("Expected: %#v\nGot: %#v", test.Completed, got)
+		}
+	}
+
+	os.Setenv("GO_FLAGS_COMPLETION", "")
+}
diff --git a/examples/bash-completion b/examples/bash-completion
index 070ae40..974f52a 100644
--- a/examples/bash-completion
+++ b/examples/bash-completion
@@ -2,7 +2,7 @@
 	args=("${COMP_WORDS[@]:1:$COMP_CWORD}")
 
 	local IFS=$'\n'
-	COMPREPLY=($(GO_FLAGS_COMPLETION=1 ${COMP_WORDS[0]} __complete -- "${args[@]}"))
+	COMPREPLY=($(GO_FLAGS_COMPLETION=1 ${COMP_WORDS[0]} "${args[@]}"))
 	return 1
 }
 
diff --git a/flags.go b/flags.go
index 923379b..a425c11 100644
--- a/flags.go
+++ b/flags.go
@@ -196,13 +196,12 @@
 and it is up to the user to make sure there are no negative side effects (for
 example from init functions).
 
-Completion works by setting the environment variable
-`GO_FLAGS_COMPLETION=1`, which enables a builtin flags command (named
-`__complete`) which can be used to output a list of completions for the
-passed arguments. The basic invocation to complete a set of arguments is
-therefore:
+Setting the environment variable `GO_FLAGS_COMPLETION=1` enables completion
+by replacing the argument parsing routine with the completion routine which
+outputs completions for the passed arguments. The basic invocation to
+complete a set of arguments is therefore:
 
-    GO_FLAGS_COMPLETION=1 ./completion-example __complete -- arg1 arg2 arg3
+    GO_FLAGS_COMPLETION=1 ./completion-example arg1 arg2 arg3
 
 where `completion-example` is the binary, `arg1` and `arg2` are
 the current arguments, and `arg3` (the last argument) is the argument
@@ -220,7 +219,7 @@
 
         # Call completion (note that the first element of COMP_WORDS is
         # the executable itself)
-        COMPREPLY=($(GO_FLAGS_COMPLETION=1 ${COMP_WORDS[0]} __complete -- "${args[@]}"))
+        COMPREPLY=($(GO_FLAGS_COMPLETION=1 ${COMP_WORDS[0]} "${args[@]}"))
         return 0
     }
 
diff --git a/parser.go b/parser.go
index 9156b5d..0ad010d 100644
--- a/parser.go
+++ b/parser.go
@@ -117,12 +117,6 @@
 
 	p.Command.parent = p
 
-	if len(os.Getenv("GO_FLAGS_COMPLETION")) != 0 {
-		p.AddCommand("__complete", "completion", "automatic flags completion", &completion{parser: p})
-
-		p.Options |= PassDoubleDash
-	}
-
 	return p
 }
 
@@ -154,6 +148,14 @@
 		p.addHelpGroups(p.showBuiltinHelp)
 	}
 
+	if len(os.Getenv("GO_FLAGS_COMPLETION")) != 0 {
+		comp := &completion{parser: p}
+
+		comp.execute(args)
+
+		return nil, nil
+	}
+
 	s := &parseState{
 		args:    args,
 		retargs: make([]string, 0, len(args)),
@@ -226,15 +228,15 @@
 	var reterr error
 
 	if s.err != nil {
-		reterr = p.printError(s.err)
+		reterr = s.err
 	} else if len(s.command.commands) != 0 && !s.command.SubcommandsOptional {
-		reterr = p.printError(s.estimateCommand())
+		reterr = s.estimateCommand()
 	} else if cmd, ok := s.command.data.(Commander); ok {
-		reterr = p.printError(cmd.Execute(s.retargs))
+		reterr = cmd.Execute(s.retargs)
 	}
 
 	if reterr != nil {
-		return append([]string{s.arg}, s.args...), reterr
+		return append([]string{s.arg}, s.args...), p.printError(reterr)
 	}
 
 	return s.retargs, nil