flag: make -help nicer.

- suppress the print that -help is not defined.
- return a special error code if -help is set
- do not change behavior if an explict "help" flag is defined.

R=golang-dev, dsymonds
CC=golang-dev
https://golang.org/cl/4641099

diff --git a/src/pkg/flag/flag.go b/src/pkg/flag/flag.go
index f9b852c0f74bb7e73dbb5003387ab2fae1ef0a11..01bbc3770080834ed444c3f6df470c1ce75d9728 100644 (file)
--- a/src/pkg/flag/flag.go
+++ b/src/pkg/flag/flag.go
@@ -66,6 +66,9 @@ import (
        "strconv"
 )
 
+// ErrHelp is the error returned if the flag -help is invoked but no such flag is defined.
+var ErrHelp = os.NewError("flag: help requested")
+
 // -- Bool Value
 type boolValue bool
 
@@ -568,12 +571,18 @@ func Var(value Value, name string, usage string) {
 func (f *FlagSet) failf(format string, a ...interface{}) os.Error {
        err := fmt.Errorf(format, a...)
        fmt.Fprintln(os.Stderr, err)
+       f.usage()
+       return err
+}
+
+// usage calls the Usage method for the flag set, or the usage function if
+// the flag set is commandLine.
+func (f *FlagSet) usage() {
        if f == commandLine {
                Usage()
        } else {
                f.Usage()
        }
-       return err
 }
 
 // parseOne parses one flag. It returns whether a flag was seen.
@@ -613,6 +622,10 @@ func (f *FlagSet) parseOne() (bool, os.Error) {
        m := f.formal
        flag, alreadythere := m[name] // BUG
        if !alreadythere {
+               if name == "help" || name == "h" { // special case for nice help message.
+                       f.usage()
+                       return false, ErrHelp
+               }
                return false, f.failf("flag provided but not defined: -%s", name)
        }
        if fv, ok := flag.Value.(*boolValue); ok { // special case: doesn't need an arg
@@ -645,6 +658,7 @@ func (f *FlagSet) parseOne() (bool, os.Error) {
 // Parse parses flag definitions from the argument list, which should not
 // include the command name.  Must be called after all flags in the FlagSet
 // are defined and before flags are accessed by the program.
+// The return value will be ErrHelp if -help was set but not defined.
 func (f *FlagSet) Parse(arguments []string) os.Error {
        f.args = arguments
        for {

diff --git a/src/pkg/flag/flag_test.go b/src/pkg/flag/flag_test.go
index fbd706921ef4df726820182c3036bb43501275ab..63d0a9fc8946915af20225268930361f0f3ae928 100644 (file)
--- a/src/pkg/flag/flag_test.go
+++ b/src/pkg/flag/flag_test.go
@@ -210,3 +210,46 @@ func TestChangingArgs(t *testing.T) {
                t.Fatalf("expected true subcmd true [args] got %v %v %v %v", *before, cmd, *after, args)
        }
 }
+
+// Test that -help invokes the usage message and returns ErrHelp.
+func TestHelp(t *testing.T) {
+       var helpCalled = false
+       fs := NewFlagSet("help test", ContinueOnError)
+       fs.Usage = func() { helpCalled = true }
+       var flag bool
+       fs.BoolVar(&flag, "flag", false, "regular flag")
+       // Regular flag invocation should work
+       err := fs.Parse([]string{"-flag=true"})
+       if err != nil {
+               t.Fatal("expected no error; got ", err)
+       }
+       if !flag {
+               t.Error("flag was not set by -flag")
+       }
+       if helpCalled {
+               t.Error("help called for regular flag")
+               helpCalled = false // reset for next test
+       }
+       // Help flag should work as expected.
+       err = fs.Parse([]string{"-help"})
+       if err == nil {
+               t.Fatal("error expected")
+       }
+       if err != ErrHelp {
+               t.Fatal("expected ErrHelp; got ", err)
+       }
+       if !helpCalled {
+               t.Fatal("help was not called")
+       }
+       // If we define a help flag, that should override.
+       var help bool
+       fs.BoolVar(&help, "help", false, "help flag")
+       helpCalled = false
+       err = fs.Parse([]string{"-help"})
+       if err != nil {
+               t.Fatal("expected no error for defined -help; got ", err)
+       }
+       if helpCalled {
+               t.Fatal("help was called; should not have been for defined help flag")
+       }
+}