Add FlagSet.DisableBuiltinHelp toggle

This commit adds a toggle to disable the handling of --help and -h
flags.

Author: cornfeedhobo

README.md

@@ -22,6 +22,7 @@
   * [Unknown flags](#unknown-flags)
   * [Custom flag types in usage](#custom-flag-types-in-usage)
   * [Disable printing default value](#disable-printing-default-value)
+  * [Disable built-in help flags](#disable-built-in-help-flags)
 
 ## Installation
 
@@ -342,3 +343,14 @@ flag.Lookup("in").DisablePrintDefault = true
 ``` plain
   --in int   help message
 ```
+
+### Disable built-in help flags
+
+Normally pflag will handle `--help` and `-h` when the flags aren't explicitly defined.
+
+If for some reason there is a need to capture the error returned in this condition, it
+is possible to disable this built-in handling.
+
+``` go
+myFlagSet.DisableBuiltinHelp = true
+```

flag.go

@@ -54,6 +54,9 @@ type FlagSet struct {
 	// ParseErrorsWhitelist is used to configure a whitelist of errors
 	ParseErrorsWhitelist ParseErrorsWhitelist
 
+	// DisableBuiltinHelp toggles the built-in convention of handling -h and --help
+	DisableBuiltinHelp bool
+
 	name              string
 	parsed            bool
 	actual            map[NormalizedName]*Flag
@@ -983,7 +986,7 @@ func (f *FlagSet) parseLongArg(s string, args []string, fn parseFunc) (outArgs [
 
 	if !exists || (flag != nil && flag.ShorthandOnly) {
 		switch {
-		case !exists && name == "help":
+		case !exists && name == "help" && !f.DisableBuiltinHelp:
 			f.usage()
 			err = ErrHelp
 			return
@@ -1034,7 +1037,7 @@ func (f *FlagSet) parseSingleShortArg(shorthands string, args []string, fn parse
 	flag, exists := f.shorthands[char]
 	if !exists {
 		switch {
-		case char == 'h':
+		case char == 'h' && !f.DisableBuiltinHelp:
 			f.usage()
 			err = ErrHelp
 			return

flag_test.go

@@ -971,42 +971,130 @@ func TestChangingArgs(t *testing.T) {
 
 // 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")
+		t.Fatal("flag was not set by --flag")
 	}
 	if helpCalled {
-		t.Error("help called for regular flag")
+		t.Fatal("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")
+	for _, f := range []string{"--help", "-h", "-help", "-helpxyz", "-hxyz"} {
+		err = fs.Parse([]string{f})
+		if err == nil {
+			t.Fatalf("while passing %s, error expected\n", f)
+		}
+		if err != ErrHelp {
+			t.Fatalf("while passing %s, expected ErrHelp; got %s\n", f, err)
+		}
+		if !helpCalled {
+			t.Fatalf("while passing %s, help was not called\n", f)
+		}
+		helpCalled = false
+	}
+
+	// Help flag should not work when disabled
+	fs.DisableBuiltinHelp = true
+	for _, f := range []string{"--help", "-h"} {
+		err := fs.Parse([]string{f})
+		if err == nil {
+			t.Fatalf("while passing %s, error expected", f)
+		}
+		if err.Error() != "unknown flag: --help" &&
+			err.Error() != "unknown shorthand flag: 'h' in -h" {
+
+			t.Fatalf("while passing %s, unknown flag error expected, got %s\n", f, err)
+		}
+		if !helpCalled {
+			// Help should be triggered because this is an unknown, but not because the help flag was called
+			t.Fatalf("while passing %s, help was not called\n", f)
+		}
 	}
-	if err != ErrHelp {
-		t.Fatal("expected ErrHelp; got ", err)
+	helpCalled = false
+	// ... when disabled, any other shorthands should trigger an error
+	for _, f := range []string{"-help", "-helpxyz", "-hxyz"} {
+		err = fs.Parse([]string{f})
+		if err == nil {
+			t.Fatalf("while passing %s, error expected\n", f)
+		}
 	}
-	if !helpCalled {
-		t.Fatal("help was not called")
+	helpCalled = false
+	fs.ParseErrorsWhitelist.UnknownFlags = true
+	for _, f := range []string{"--help", "-h"} {
+		err := fs.Parse([]string{f})
+		t.Logf("help called: %v\n", helpCalled)
+		if err != nil {
+			t.Fatalf("while passing %s, error not expected, got %s\n", f, err)
+		}
+		if helpCalled {
+			// Help should be triggered because this is an unknown, but not because the help flag was called
+			t.Fatalf("while passing %s, help was not called\n", f)
+		}
 	}
+	fs.DisableBuiltinHelp = false
+
 	// If we define a help flag, that should override.
 	var help bool
-	fs.BoolVar(&help, "help", false, "help flag")
-	helpCalled = false
+	fs.BoolVarP(&help, "help", "h", false, "help flag")
 	err = fs.Parse([]string{"--help"})
 	if err != nil {
 		t.Fatal("expected no error for defined --help; got ", err)
 	}
+	if !help {
+		t.Fatal("help should be true for defined --help")
+	}
+	if helpCalled {
+		t.Fatal("help was called; should not have been for defined help flag")
+	}
+	help = false
+	// ... including the shorthand
+	err = fs.Parse([]string{"-h"})
+	if err != nil {
+		t.Fatal("expected no error for defined -h; got ", err)
+	}
+	if !help {
+		t.Fatal("help should be true for defined -h")
+	}
+	if helpCalled {
+		t.Fatal("help was called; should not have been for defined help flag")
+	}
+	help = false
+
+	// If we define a help flag, that should override when the built in help flag is disabled.
+	fs.DisableBuiltinHelp = true
+	err = fs.Parse([]string{"--help"})
+	if err != nil {
+		t.Fatal("expected no error for defined --help; got ", err)
+	}
+	if !help {
+		t.Fatal("help should be true for defined --help")
+	}
+	if helpCalled {
+		t.Fatal("help was called; should not have been for defined help flag")
+	}
+	help = false
+	// ... including the shorthand
+	err = fs.Parse([]string{"-h"})
+	if err != nil {
+		t.Fatal("expected no error for defined -h; got ", err)
+	}
+	if !help {
+		t.Fatal("help should be true for defined -h")
+	}
 	if helpCalled {
 		t.Fatal("help was called; should not have been for defined help flag")
 	}