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

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 {

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")
+	}
+}