Repository: alecthomas/kong
Pull request #146 (2021-02-18 07:49:17)

Add support for negatable boolean flags
Adds support for a new `negatable` struct tag, which will automatically set a boolean Struct flag value to `false` if the flag's name starts with `--no-`, even when the default is `true`. This idea is shamelessly stolen from the [`commander` npm library](https://github.com/tj/commander.js#other-option-types-negatable-boolean-and-booleanvalue)

For example:
```go
type Cmd struct {
  Sort bool `default:"true",negatable`
}
```

Calling
```sh
command
# or
command --sort
```
Will set `Sort` to `true`, but
```sh 
command --no-sort
```
Will set `Sort` to `false`.

## Why do this?

It avoids the situation of having to do:
```go
type Cmd struct {
  NoSort bool
}

func (c *Cmd) Run() error {
  if !c.NoSort {
    // Double negations are hard to read!
  }
  return nil
}
```

josephschmitt: Let me know if you think this needs unit tests. I couldn't find many in the model file. 
mickael-menu: This is a nice addition in my opinion!

However I would make it explicit by adding a `negatable` tag, and maybe showing it in the help output as `--[no-]sort`
alecthomas: Yes, I agree with Mickael on both points. 

Specifically re. the `negatable:""` tag: for example this would not make sense for `--help`.

Also, yes please add tests. They're mostly in `kong_test.go`.

alecthomas: Also, I'm not sure if you're aware, but `--sort=false` is supported by Kong.
alecthomas: I forgot to say, I think this is a great idea :)
josephschmitt: > Specifically re. the `negatable:""` tag: for example this would not make sense for `--help`.

The way it's set up, it's only for Flag's that are of a bool type. Is there any reason to not have it be automatic for all Boolean flags? Seems like a lot of extra decoration in someone's program, and then we also don't have to worry about docs saying only attach the tag to bool's.

> Also, yes please add tests. They're mostly in `kong_test.go`.

Yup there's a test in that file in my PR. 


josephschmitt: >maybe showing it in the help output as `--[no-]sort`

Oh great idea! I'll add it to help output

josephschmitt: > Also, I'm not sure if you're aware, but `--sort=false` is supported by Kong.

Yup! I just think the `--no-` prefix looks nicer 😅
mickael-menu: There are plenty of flags which would look weird with automatic `no` version: `--help`, `--quiet`, `--force`, `--version`, etc. One could argue that they are not "true" booleans. It's more the presence of the option that is tested than its value.
josephschmitt: > There are plenty of flags which would look weird with automatic `no` version: `--help`, `--quiet`, `--force`, `--version`, etc. One could argue that they are not "true" booleans. It's more the presence of the option that is tested than its value.

Gotcha, I'll add the tag, then.
josephschmitt: @mickael-menu @alecthomas updated!
josephschmitt: @alecthomas updated!
alecthomas: Apologies for the delay, I've had a bad cold for the last week or so.
josephschmitt: > Apologies for the delay, I've had a bad cold for the last week or so.

No worries! Hope you’re feeling better.

Patch:
diff --git a/README.md b/README.md
index f0b7a30..78341a1 100644
--- a/README.md
+++ b/README.md
@@ -440,6 +440,7 @@ Tag                    | Description
 `required`             | If present, flag/arg is required.
 `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
 `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.
diff --git a/context.go b/context.go
index 4d24161..554e8b1 100644
--- a/context.go
+++ b/context.go
@@ -626,7 +626,12 @@ func (c *Context) Apply() (string, error) {
 			panic("unsupported path ?!")
 		}
 		if value != nil {
-			value.Apply(c.getValue(value))
+			v := c.getValue(value)
+			if value.Flag != nil && value.Flag.Negated {
+				v.SetBool(!v.Bool())
+			}
+
+			value.Apply(v)
 		}
 	}
 
@@ -639,15 +644,19 @@ func (c *Context) parseFlag(flags []*Flag, match string) (err error) {
 	for _, flag := range flags {
 		long := "--" + flag.Name
 		short := "-" + string(flag.Short)
+		neg := "--no-" + flag.Name
 		candidates = append(candidates, long)
 		if flag.Short != 0 {
 			candidates = append(candidates, short)
 		}
-		if short != match && long != match {
+		if short != match && long != match && !(match == neg && flag.Tag.Negatable) {
 			continue
 		}
 		// Found a matching flag.
 		c.scan.Pop()
+		if match == neg && flag.Tag.Negatable {
+			flag.Negated = true
+		}
 		err := flag.Parse(c.scan, c.getValue(flag.Value))
 		if err != nil {
 			if e, ok := errors.Cause(err).(*expectedError); ok && e.token.InferredType().IsAny(FlagToken, ShortFlagToken) {
diff --git a/help.go b/help.go
index ab66ab0..d9f65bf 100644
--- a/help.go
+++ b/help.go
@@ -469,12 +469,24 @@ func formatFlag(haveShort bool, flag *Flag) string {
 	name := flag.Name
 	isBool := flag.IsBool()
 	if flag.Short != 0 {
-		flagString += fmt.Sprintf("-%c, --%s", flag.Short, name)
+		if isBool && flag.Tag.Negatable {
+			flagString += fmt.Sprintf("-%c, --[no-]%s", flag.Short, name)
+		} else {
+			flagString += fmt.Sprintf("-%c, --%s", flag.Short, name)
+		}
 	} else {
-		if haveShort {
-			flagString += fmt.Sprintf("    --%s", name)
+		if isBool && flag.Tag.Negatable {
+			if haveShort {
+				flagString = fmt.Sprintf("    --[no-]%s", name)
+			} else {
+				flagString = fmt.Sprintf("--[no-]%s", name)
+			}
 		} else {
-			flagString += fmt.Sprintf("--%s", name)
+			if haveShort {
+				flagString += fmt.Sprintf("    --%s", name)
+			} else {
+				flagString += fmt.Sprintf("--%s", name)
+			}
 		}
 	}
 	if !isBool {
diff --git a/model.go b/model.go
index c64cab3..4ce2456 100644
--- a/model.go
+++ b/model.go
@@ -369,6 +369,7 @@ type Flag struct {
 	Env         string
 	Short       rune
 	Hidden      bool
+	Negated     bool
 }
 
 func (f *Flag) String() string {
diff --git a/tag.go b/tag.go
index b780000..deffd86 100644
--- a/tag.go
+++ b/tag.go
@@ -33,6 +33,7 @@ type Tag struct {
 	Prefix      string // Optional prefix on anonymous structs. All sub-flags will have this prefix.
 	Embed       bool
 	Aliases     []string
+	Negatable   bool
 
 	// Storage for all tag keys for arbitrary lookups.
 	items map[string][]string
@@ -158,6 +159,11 @@ func parseTag(fv reflect.Value, ft reflect.StructField) *Tag {
 	t.Xor = t.Get("xor")
 	t.Prefix = t.Get("prefix")
 	t.Embed = t.Has("embed")
+	negatable := t.Has("negatable")
+	if negatable && ft.Type.Kind() != reflect.Bool {
+		fail("negatable can only be set on booleans")
+	}
+	t.Negatable = negatable
 	splitFn := func(r rune) bool {
 		return r == ',' || r == ' '
 	}


Test patch:
diff --git a/help_test.go b/help_test.go
index 6bf00b3..aa1a984 100644
--- a/help_test.go
+++ b/help_test.go
@@ -29,6 +29,7 @@ func TestHelp(t *testing.T) {
 		Slice    []string       `help:"A slice of strings." placeholder:"STR"`
 		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."`
 
 		One struct {
 			Flag string `help:"Nested flag."`
@@ -75,6 +76,7 @@ Flags:
       --slice=STR,...        A slice of strings.
       --map=KEY=VALUE;...    A map of strings to ints.
       --required             A required flag.
+  -s, --[no-]sort            Is sortable or not.
 
 Commands:
   one --required
@@ -115,6 +117,7 @@ Flags:
       --slice=STR,...        A slice of strings.
       --map=KEY=VALUE;...    A map of strings to ints.
       --required             A required flag.
+  -s, --[no-]sort            Is sortable or not.
 
       --flag=STRING          Nested flag under two.
       --required-two
diff --git a/kong_test.go b/kong_test.go
index 03ff1d6..9163141 100644
--- a/kong_test.go
+++ b/kong_test.go
@@ -356,6 +356,58 @@ func TestTraceErrorPartiallySucceeds(t *testing.T) {
 	require.Equal(t, "one", ctx.Command())
 }
 
+func TestNegatedBooleanFlag(t *testing.T) {
+	var cli struct {
+		Cmd struct {
+			Flag bool `kong:"default='true',negatable"`
+		} `kong:"cmd"`
+	}
+
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"cmd", "--no-flag"})
+	require.NoError(t, err)
+	require.Equal(t, false, cli.Cmd.Flag)
+}
+
+func TestInvertedNegatedBooleanFlag(t *testing.T) {
+	var cli struct {
+		Cmd struct {
+			Flag bool `kong:"default='true',negatable"`
+		} `kong:"cmd"`
+	}
+
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"cmd", "--no-flag=false"})
+	require.NoError(t, err)
+	require.Equal(t, true, cli.Cmd.Flag)
+}
+
+func TestExistingNoFlag(t *testing.T) {
+	var cli struct {
+		Cmd struct {
+			Flag   bool `kong:"default='true'"`
+			NoFlag string
+		} `kong:"cmd"`
+	}
+
+	p := mustNew(t, &cli)
+	_, err := p.Parse([]string{"cmd", "--no-flag=none"})
+	require.NoError(t, err)
+	require.Equal(t, true, cli.Cmd.Flag)
+	require.Equal(t, "none", cli.Cmd.NoFlag)
+}
+
+func TestInvalidNegatedNonBool(t *testing.T) {
+	var cli struct {
+		Cmd struct {
+			Flag string `kong:"negatable"`
+		} `kong:"cmd"`
+	}
+
+	_, err := kong.New(&cli)
+	require.Error(t, err)
+}
+
 type hookContext struct {
 	cmd    bool
 	values []string