Skip to content

Commit

Permalink
Make negatable flag name customisable (#439)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@camh-
camh- authored Sep 10, 2024
1 parent 7d84b95 commit 4ecb535
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 49 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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. |
Expand Down
7 changes: 7 additions & 0 deletions build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions context.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,13 +710,13 @@ func (c *Context) parseFlag(flags []*Flag, match string) (err error) {
candidates = append(candidates, alias)
}

neg := "--no-" + flag.Name
if !matched && !(match == neg && flag.Tag.Negatable) {
neg := negatableFlagName(flag.Name, 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))
Expand Down
33 changes: 14 additions & 19 deletions help.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
flagString += fmt.Sprintf("-%c, --[no-]%s", flag.Short, name)
} else {
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)
}
}
short = "-" + string(flag.Short) + ", "
} else if haveShort {
short = " "
}

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())
}
Expand Down
3 changes: 3 additions & 0 deletions help_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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."`
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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
Expand Down
93 changes: 71 additions & 22 deletions kong_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,9 @@ func TestTraceErrorPartiallySucceeds(t *testing.T) {
}

type commandWithNegatableFlag struct {
Flag bool `kong:"default='true',negatable"`
ran bool
Flag bool `kong:"default='true',negatable"`
Custom bool `kong:"default='true',negatable='standard'"`
ran bool
}

func (c *commandWithNegatableFlag) Run() error {
Expand All @@ -368,34 +369,64 @@ func (c *commandWithNegatableFlag) Run() error {

func TestNegatableFlag(t *testing.T) {
tests := []struct {
name string
args []string
expected bool
name string
args []string
expectedFlag bool
expectedCustom bool
}{
{
name: "no flag",
args: []string{"cmd"},
expected: true,
name: "no flag",
args: []string{"cmd"},
expectedFlag: true,
expectedCustom: true,
},
{
name: "boolean flag",
args: []string{"cmd", "--flag"},
expected: true,
name: "boolean flag",
args: []string{"cmd", "--flag"},
expectedFlag: true,
expectedCustom: true,
},
{
name: "inverted boolean flag",
args: []string{"cmd", "--flag=false"},
expected: false,
name: "custom boolean flag",
args: []string{"cmd", "--custom"},
expectedFlag: true,
expectedCustom: true,
},
{
name: "negated boolean flag",
args: []string{"cmd", "--no-flag"},
expected: false,
name: "inverted boolean flag",
args: []string{"cmd", "--flag=false"},
expectedFlag: false,
expectedCustom: true,
},
{
name: "inverted negated boolean flag",
args: []string{"cmd", "--no-flag=false"},
expected: true,
name: "custom inverted boolean flag",
args: []string{"cmd", "--custom=false"},
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 {
Expand All @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions negatable.go
View file
Original file line number Diff line number Diff line change
@@ -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
}
}
15 changes: 10 additions & 5 deletions tag.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 negatable && !isBool && !isBoolPtr {
return fmt.Errorf("negatable can only be set on booleans")
if t.Has("negatable") {
if !isBool && !isBoolPtr {
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)...)
Expand Down

0 comments on commit 4ecb535

Please sign in to comment.