Make negatable flag name customisable (#439)

* fix: Check if negatable duplicates another flag

Add a check for flags with the `negatable` option if the negative flag
conflicts with another tag, such as:

    Flag   bool `negatable:""`
    NoFlag bool

The flag `--no-flag` is ambiguous in this scenario.

* feat: Make negatable flag name customisable

Allow a value on the `negatable` tag to specify a flag name to use for
negation instead of using `--no-<flag-name>` as the flag.

e.g.

    Approve bool `default:"true",negatable:"deny"`

This example will allow `--deny` to set the `Approve` field to false.

README.md

@@ -571,6 +571,7 @@ Both can coexist with standard Tag parsing.
 | `optional:""`        | If present, flag/arg is optional.                                                                                                                                                                                                                                                                                              |
 | `hidden:""`          | If present, command or flag is hidden.                                                                                                                                                                                                                                                                                         |
 | `negatable:""`       | If present on a `bool` field, supports prefixing a flag with `--no-` to invert the default value                                                                                                                                                                                                                               |
+| `negatable:"X"`      | If present on a `bool` field, supports `--X` to invert the default value                                                                                                                                                                                                                                                       |
 | `format:"X"`         | Format for parsing input, if supported.                                                                                                                                                                                                                                                                                        |
 | `sep:"X"`            | Separator for sequences (defaults to ","). May be `none` to disable splitting.                                                                                                                                                                                                                                                 |
 | `mapsep:"X"`         | Separator for maps (defaults to ";"). May be `none` to disable splitting.                                                                                                                                                                                                                                                      |

build.go

@@ -315,6 +315,13 @@ func buildField(k *Kong, node *Node, v reflect.Value, ft reflect.StructField, fv
 			}
 			seenFlags["-"+string(tag.Short)] = true
 		}
+		if tag.Negatable != "" {
+			negFlag := negatableFlagName(value.Name, tag.Negatable)
+			if seenFlags[negFlag] {
+				return failField(v, ft, "duplicate negation flag %s", negFlag)
+			}
+			seenFlags[negFlag] = true
+		}
 		flag := &Flag{
 			Value:       value,
 			Aliases:     tag.Aliases,

context.go

@@ -710,13 +710,13 @@ func (c *Context) parseFlag(flags []*Flag, match string) (err error) {
 			candidates = append(candidates, alias)
 		}
 
-		neg := "--no-" + flag.Name
+		neg := negatableFlagName(flag.Name, flag.Tag.Negatable)
-		if !matched && !(match == neg && flag.Tag.Negatable) {
+		if !matched && match != neg {
 			continue
 		}
 		// Found a matching flag.
 		c.scan.Pop()
-		if match == neg && flag.Tag.Negatable {
+		if match == neg && flag.Tag.Negatable != "" {
 			flag.Negated = true
 		}
 		err := flag.Parse(c.scan, c.getValue(flag.Value))

help.go

@@ -491,27 +491,22 @@ func formatFlag(haveShort bool, flag *Flag) string {
 	name := flag.Name
 	isBool := flag.IsBool()
 	isCounter := flag.IsCounter()
+
+	short := ""
 	if flag.Short != 0 {
-		if isBool && flag.Tag.Negatable {
+		short = "-" + string(flag.Short) + ", "
-			flagString += fmt.Sprintf("-%c, --[no-]%s", flag.Short, name)
+	} else if haveShort {
-		} else {
+		short = "    "
-			flagString += fmt.Sprintf("-%c, --%s", flag.Short, name)
-		}
-	} else {
-		if isBool && flag.Tag.Negatable {
-			if haveShort {
-				flagString = fmt.Sprintf("    --[no-]%s", name)
-			} else {
-				flagString = fmt.Sprintf("--[no-]%s", name)
-			}
-		} else {
-			if haveShort {
-				flagString += fmt.Sprintf("    --%s", name)
-			} else {
-				flagString += fmt.Sprintf("--%s", name)
-			}
-		}
 	}
+
+	if isBool && flag.Tag.Negatable == negatableDefault {
+		name = "[no-]" + name
+	} else if isBool && flag.Tag.Negatable != "" {
+		name += "/" + flag.Tag.Negatable
+	}
+
+	flagString += fmt.Sprintf("%s--%s", short, name)
+
 	if !isBool && !isCounter {
 		flagString += fmt.Sprintf("=%s", flag.FormatPlaceHolder())
 	}

help_test.go

@@ -71,6 +71,7 @@ func TestHelp(t *testing.T) {
 		Map      map[string]int `help:"A map of strings to ints."`
 		Required bool           `required help:"A required flag."`
 		Sort     bool           `negatable short:"s" help:"Is sortable or not."`
+		Approve  bool           `negatable:"deny" help:"Approve or deny message."`
 
 		One struct {
 			Flag string `help:"Nested flag."`
@@ -118,6 +119,7 @@ Flags:
       --map=KEY=VALUE;...    A map of strings to ints.
       --required             A required flag.
   -s, --[no-]sort            Is sortable or not.
+      --approve/deny         Approve or deny message.
 
 Commands:
   one --required [flags]
@@ -159,6 +161,7 @@ Flags:
       --map=KEY=VALUE;...    A map of strings to ints.
       --required             A required flag.
   -s, --[no-]sort            Is sortable or not.
+      --approve/deny         Approve or deny message.
 
       --flag=STRING          Nested flag under two.
       --required-two

kong_test.go

@@ -357,8 +357,9 @@ func TestTraceErrorPartiallySucceeds(t *testing.T) {
 }
 
 type commandWithNegatableFlag struct {
-	Flag bool `kong:"default='true',negatable"`
+	Flag   bool `kong:"default='true',negatable"`
-	ran  bool
+	Custom bool `kong:"default='true',negatable='standard'"`
+	ran    bool
 }
 
 func (c *commandWithNegatableFlag) Run() error {
@@ -368,34 +369,64 @@ func (c *commandWithNegatableFlag) Run() error {
 
 func TestNegatableFlag(t *testing.T) {
 	tests := []struct {
-		name     string
+		name           string
-		args     []string
+		args           []string
-		expected bool
+		expectedFlag   bool
+		expectedCustom bool
 	}{
 		{
-			name:     "no flag",
+			name:           "no flag",
-			args:     []string{"cmd"},
+			args:           []string{"cmd"},
-			expected: true,
+			expectedFlag:   true,
+			expectedCustom: true,
 		},
 		{
-			name:     "boolean flag",
+			name:           "boolean flag",
-			args:     []string{"cmd", "--flag"},
+			args:           []string{"cmd", "--flag"},
-			expected: true,
+			expectedFlag:   true,
+			expectedCustom: true,
 		},
 		{
-			name:     "inverted boolean flag",
+			name:           "custom boolean flag",
-			args:     []string{"cmd", "--flag=false"},
+			args:           []string{"cmd", "--custom"},
-			expected: false,
+			expectedFlag:   true,
+			expectedCustom: true,
 		},
 		{
-			name:     "negated boolean flag",
+			name:           "inverted boolean flag",
-			args:     []string{"cmd", "--no-flag"},
+			args:           []string{"cmd", "--flag=false"},
-			expected: false,
+			expectedFlag:   false,
+			expectedCustom: true,
 		},
 		{
-			name:     "inverted negated boolean flag",
+			name:           "custom inverted boolean flag",
-			args:     []string{"cmd", "--no-flag=false"},
+			args:           []string{"cmd", "--custom=false"},
-			expected: true,
+			expectedFlag:   true,
+			expectedCustom: false,
+		},
+		{
+			name:           "negated boolean flag",
+			args:           []string{"cmd", "--no-flag"},
+			expectedFlag:   false,
+			expectedCustom: true,
+		},
+		{
+			name:           "custom negated boolean flag",
+			args:           []string{"cmd", "--standard"},
+			expectedFlag:   true,
+			expectedCustom: false,
+		},
+		{
+			name:           "inverted negated boolean flag",
+			args:           []string{"cmd", "--no-flag=false"},
+			expectedFlag:   true,
+			expectedCustom: true,
+		},
+		{
+			name:           "inverted custom negated boolean flag",
+			args:           []string{"cmd", "--standard=false"},
+			expectedFlag:   true,
+			expectedCustom: true,
 		},
 	}
 	for _, tt := range tests {
@@ -408,16 +439,34 @@ func TestNegatableFlag(t *testing.T) {
 			p := mustNew(t, &cli)
 			kctx, err := p.Parse(tt.args)
 			assert.NoError(t, err)
-			assert.Equal(t, tt.expected, cli.Cmd.Flag)
+			assert.Equal(t, tt.expectedFlag, cli.Cmd.Flag)
+			assert.Equal(t, tt.expectedCustom, cli.Cmd.Custom)
 
 			err = kctx.Run()
 			assert.NoError(t, err)
-			assert.Equal(t, tt.expected, cli.Cmd.Flag)
+			assert.Equal(t, tt.expectedFlag, cli.Cmd.Flag)
+			assert.Equal(t, tt.expectedCustom, cli.Cmd.Custom)
 			assert.True(t, cli.Cmd.ran)
 		})
 	}
 }
 
+func TestDuplicateNegatableLong(t *testing.T) {
+	cli2 := struct {
+		NoFlag bool
+		Flag   bool `negatable:""` // negation duplicates NoFlag
+	}{}
+	_, err := kong.New(&cli2)
+	assert.EqualError(t, err, "<anonymous struct>.Flag: duplicate negation flag --no-flag")
+
+	cli3 := struct {
+		One bool
+		Two bool `negatable:"one"` // negation duplicates Flag2
+	}{}
+	_, err = kong.New(&cli3)
+	assert.EqualError(t, err, "<anonymous struct>.Two: duplicate negation flag --one")
+}
+
 func TestExistingNoFlag(t *testing.T) {
 	var cli struct {
 		Cmd struct {

negatable.go

@@ -0,0 +1,19 @@
+package kong
+
+// negatableDefault is a placeholder value for the Negatable tag to indicate
+// the negated flag is --no-<flag-name>. This is needed as at the time of
+// parsing a tag, the field's flag name is not yet known.
+const negatableDefault = "_"
+
+// negatableFlagName returns the name of the flag for a negatable field, or
+// an empty string if the field is not negatable.
+func negatableFlagName(name, negation string) string {
+	switch negation {
+	case "":
+		return ""
+	case negatableDefault:
+		return "--no-" + name
+	default:
+		return "--" + negation
+	}
+}

tag.go

@@ -38,7 +38,7 @@ type Tag struct {
 	EnvPrefix   string
 	Embed       bool
 	Aliases     []string
-	Negatable   bool
+	Negatable   string
 	Passthrough bool
 
 	// Storage for all tag keys for arbitrary lookups.
@@ -256,11 +256,16 @@ func hydrateTag(t *Tag, typ reflect.Type) error { //nolint: gocyclo
 	t.Prefix = t.Get("prefix")
 	t.EnvPrefix = t.Get("envprefix")
 	t.Embed = t.Has("embed")
-	negatable := t.Has("negatable")
+	if t.Has("negatable") {
-	if negatable && !isBool && !isBoolPtr {
+		if !isBool && !isBoolPtr {
-		return fmt.Errorf("negatable can only be set on booleans")
+			return fmt.Errorf("negatable can only be set on booleans")
+		}
+		negatable := t.Get("negatable")
+		if negatable == "" {
+			negatable = negatableDefault // placeholder for default negation of --no-<flag>
+		}
+		t.Negatable = negatable
 	}
-	t.Negatable = negatable
 	aliases := t.Get("aliases")
 	if len(aliases) > 0 {
 		t.Aliases = append(t.Aliases, strings.FieldsFunc(aliases, tagSplitFn)...)