Skip to content

Commit

Permalink
Feature: Add xand tag (#442)
Browse files Browse the repository at this point in the history
* Feat: Add xand group and check for missing

* Fix: Split and combine err in TestMultiand for consistency

* Feat: Check missing required flags in xand groups

* Feat: Handle combined xor and xand

* Docs: Add info about combined xand and required use

* Docs: Fix language error in xand description

Co-authored-by: Stautis <thkrst@gmail.com>

* Feat: Rename xand to and

* Refactor: Switch from fmt.Sprintf to err.Error

* Refactor: Get requiredAndGroup map in separate function

---------

Co-authored-by: Stautis <thkrst@gmail.com>
  • Loading branch information
tinytyranid and Stautis authored Aug 8, 2024
1 parent 5f9c5cc commit ff6d5ba
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 14 deletions.
5 changes: 3 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ Tags can be in two forms:
Both can coexist with standard Tag parsing.

| Tag | Description |
| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
|----------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `cmd:""` | If present, struct is a command. |
| `arg:""` | If present, field is an argument. Required by default. |
| `env:"X,Y,..."` | Specify envars to use for default value. The envs are resolved in the declared order. The first value found is used. |
Expand All @@ -566,7 +566,7 @@ Both can coexist with standard Tag parsing.
| `default:"1"` | On a command, make it the default. |
| `default:"withargs"` | On a command, make it the default and allow args/flags from that command |
| `short:"X"` | Short name, if flag. |
| `aliases:"X,Y"` | One or more aliases (for cmd or flag). |
| `aliases:"X,Y"` | One or more aliases (for cmd or flag). |
| `required:""` | If present, flag/arg is required. |
| `optional:""` | If present, flag/arg is optional. |
| `hidden:""` | If present, command or flag is hidden. |
Expand All @@ -577,6 +577,7 @@ Both can coexist with standard Tag parsing.
| `enum:"X,Y,..."` | Set of valid values allowed for this flag. An enum field must be `required` or have a valid `default`. |
| `group:"X"` | Logical group for a flag or command. |
| `xor:"X,Y,..."` | Exclusive OR groups for flags. Only one flag in the group can be used which is restricted within the same command. When combined with `required`, at least one of the `xor` group will be required. |
| `and:"X,Y,..."` | Exclsuive AND groups for flags. All flags in the group must be used in the same command. When combined with `required`, all flags in the group will be required. |
| `prefix:"X"` | Prefix for all sub-flags. |
| `envprefix:"X"` | Envar prefix for all sub-flags. |
| `set:"K=V"` | Set a variable for expansion by child elements. Multiples can occur. |
Expand Down
1 change: 1 addition & 0 deletions build.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ func buildField(k *Kong, node *Node, v reflect.Value, ft reflect.StructField, fv
Envs: tag.Envs,
Group: buildGroupForKey(k, tag.Group),
Xor: tag.Xor,
And: tag.And,
Hidden: tag.Hidden,
}
value.Flag = flag
Expand Down
79 changes: 77 additions & 2 deletions context.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ func (c *Context) Validate() error { //nolint: gocyclo
if err := checkMissingPositionals(positionals, node.Positional); err != nil {
return err
}
if err := checkXorDuplicates(c.Path); err != nil {
if err := checkXorDuplicatedAndAndMissing(c.Path); err != nil {
return err
}

Expand Down Expand Up @@ -831,23 +831,35 @@ func (c *Context) PrintUsage(summary bool) error {
func checkMissingFlags(flags []*Flag) error {
xorGroupSet := map[string]bool{}
xorGroup := map[string][]string{}
andGroupSet := map[string]bool{}
andGroup := map[string][]string{}
missing := []string{}
andGroupRequired := getRequiredAndGroupMap(flags)
for _, flag := range flags {
for _, and := range flag.And {
flag.Required = andGroupRequired[and]
}
if flag.Set {
for _, xor := range flag.Xor {
xorGroupSet[xor] = true
}
for _, and := range flag.And {
andGroupSet[and] = true
}
}
if !flag.Required || flag.Set {
continue
}
if len(flag.Xor) > 0 {
if len(flag.Xor) > 0 || len(flag.And) > 0 {
for _, xor := range flag.Xor {
if xorGroupSet[xor] {
continue
}
xorGroup[xor] = append(xorGroup[xor], flag.Summary())
}
for _, and := range flag.And {
andGroup[and] = append(andGroup[and], flag.Summary())
}
} else {
missing = append(missing, flag.Summary())
}
Expand All @@ -857,6 +869,11 @@ func checkMissingFlags(flags []*Flag) error {
missing = append(missing, strings.Join(flags, " or "))
}
}
for _, flags := range andGroup {
if len(flags) > 1 {
missing = append(missing, strings.Join(flags, " and "))
}
}

if len(missing) == 0 {
return nil
Expand All @@ -867,6 +884,18 @@ func checkMissingFlags(flags []*Flag) error {
return fmt.Errorf("missing flags: %s", strings.Join(missing, ", "))
}

func getRequiredAndGroupMap(flags []*Flag) map[string]bool {
andGroupRequired := map[string]bool{}
for _, flag := range flags {
for _, and := range flag.And {
if flag.Required {
andGroupRequired[and] = true
}
}
}
return andGroupRequired
}

func checkMissingChildren(node *Node) error {
missing := []string{}

Expand Down Expand Up @@ -977,6 +1006,20 @@ func checkPassthroughArg(target reflect.Value) bool {
}
}

func checkXorDuplicatedAndAndMissing(paths []*Path) error {
errs := []string{}
if err := checkXorDuplicates(paths); err != nil {
errs = append(errs, err.Error())
}
if err := checkAndMissing(paths); err != nil {
errs = append(errs, err.Error())
}
if len(errs) > 0 {
return fmt.Errorf(strings.Join(errs, ", "))
}
return nil
}

func checkXorDuplicates(paths []*Path) error {
for _, path := range paths {
seen := map[string]*Flag{}
Expand All @@ -995,6 +1038,38 @@ func checkXorDuplicates(paths []*Path) error {
return nil
}

func checkAndMissing(paths []*Path) error {
for _, path := range paths {
missingMsgs := []string{}
andGroups := map[string][]*Flag{}
for _, flag := range path.Flags {
for _, and := range flag.And {
andGroups[and] = append(andGroups[and], flag)
}
}
for _, flags := range andGroups {
oneSet := false
notSet := []*Flag{}
flagNames := []string{}
for _, flag := range flags {
flagNames = append(flagNames, flag.Name)
if flag.Set {
oneSet = true
} else {
notSet = append(notSet, flag)
}
}
if len(notSet) > 0 && oneSet {
missingMsgs = append(missingMsgs, fmt.Sprintf("--%s must be used together", strings.Join(flagNames, " and --")))
}
}
if len(missingMsgs) > 0 {
return fmt.Errorf("%s", strings.Join(missingMsgs, ", "))
}
}
return nil
}

func findPotentialCandidates(needle string, haystack []string, format string, args ...interface{}) error {
if len(haystack) == 0 {
return fmt.Errorf(format, args...)
Expand Down
10 changes: 0 additions & 10 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,15 +1,5 @@
github.com/alecthomas/assert/v2 v2.4.1 h1:mwPZod/d35nlaCppr6sFP0rbCL05WH9fIo7lvsf47zo=
github.com/alecthomas/assert/v2 v2.4.1/go.mod h1:fw5suVxB+wfYJ3291t0hRTqtGzFYdSwstnRQdaQx2DM=
github.com/alecthomas/assert/v2 v2.5.0 h1:OJKYg53BQx06/bMRBSPDCO49CbCDNiUQXwdoNrt6x5w=
github.com/alecthomas/assert/v2 v2.5.0/go.mod h1:fw5suVxB+wfYJ3291t0hRTqtGzFYdSwstnRQdaQx2DM=
github.com/alecthomas/assert/v2 v2.6.0 h1:o3WJwILtexrEUk3cUVal3oiQY2tfgr/FHWiz/v2n4FU=
github.com/alecthomas/assert/v2 v2.6.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
github.com/alecthomas/assert/v2 v2.8.1 h1:YCxnYR6jjpfnEK5AK5SysALKdUEBPGH4Y7As6tBnDw0=
github.com/alecthomas/assert/v2 v2.8.1/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
github.com/alecthomas/assert/v2 v2.10.0 h1:jjRCHsj6hBJhkmhznrCzoNpbA3zqy0fYiUcYZP/GkPY=
github.com/alecthomas/assert/v2 v2.10.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
github.com/alecthomas/repr v0.3.0 h1:NeYzUPfjjlqHY4KtzgKJiWd6sVq2eNUPTi34PiFGjY8=
github.com/alecthomas/repr v0.3.0/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4=
github.com/alecthomas/repr v0.4.0 h1:GhI2A8MACjfegCPVq9f1FLvIBS+DrQ2KQBFZP1iFzXc=
github.com/alecthomas/repr v0.4.0/go.mod h1:Fr0507jx4eOXV7AlPV6AVZLYrLIuIeSOWtW57eE/O/4=
github.com/hexops/gotextdiff v1.0.3 h1:gitA9+qJrrTCsiCl7+kh75nPqQt1cx4ZkudSTLoUqJM=
Expand Down
109 changes: 109 additions & 0 deletions kong_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"errors"
"fmt"
"sort"
"strings"
"testing"

Expand Down Expand Up @@ -919,6 +920,21 @@ func TestXor(t *testing.T) {
assert.NoError(t, err)
}

func TestAnd(t *testing.T) {
var cli struct {
Hello bool `and:"another"`
One bool `and:"group"`
Two string `and:"group"`
}
p := mustNew(t, &cli)
_, err := p.Parse([]string{"--hello", "--one"})
assert.EqualError(t, err, "--one and --two must be used together")

p = mustNew(t, &cli)
_, err = p.Parse([]string{"--one", "--two=hi", "--hello"})
assert.NoError(t, err)
}

func TestXorChild(t *testing.T) {
var cli struct {
One bool `xor:"group"`
Expand All @@ -936,6 +952,23 @@ func TestXorChild(t *testing.T) {
assert.Error(t, err, "--two and --three can't be used together")
}

func TestAndChild(t *testing.T) {
var cli struct {
One bool `and:"group"`
Cmd struct {
Two string `and:"group"`
Three string `and:"group"`
} `cmd`
}
p := mustNew(t, &cli)
_, err := p.Parse([]string{"--one", "cmd", "--two=hi", "--three=hello"})
assert.NoError(t, err)

p = mustNew(t, &cli)
_, err = p.Parse([]string{"--two=hi", "cmd"})
assert.Error(t, err, "--two and --three must be used together")
}

func TestMultiXor(t *testing.T) {
var cli struct {
Hello bool `xor:"one,two"`
Expand All @@ -952,6 +985,47 @@ func TestMultiXor(t *testing.T) {
assert.EqualError(t, err, "--hello and --two can't be used together")
}

func TestMultiAnd(t *testing.T) {
var cli struct {
Hello bool `and:"one,two"`
One bool `and:"one"`
Two string `and:"two"`
}

p := mustNew(t, &cli)
_, err := p.Parse([]string{"--hello"})
// Split and combine error so messages always will be in the same order
// when testing
missingMsgs := strings.Split(err.Error(), ", ")
sort.Strings(missingMsgs)
err = fmt.Errorf("%s", strings.Join(missingMsgs, ", "))
assert.EqualError(t, err, "--hello and --one must be used together, --hello and --two must be used together")

p = mustNew(t, &cli)
_, err = p.Parse([]string{"--two=foo"})
assert.EqualError(t, err, "--hello and --two must be used together")
}

func TestXorAnd(t *testing.T) {
var cli struct {
Hello bool `xor:"one" and:"two"`
One bool `xor:"one"`
Two string `and:"two"`
}

p := mustNew(t, &cli)
_, err := p.Parse([]string{"--hello"})
assert.EqualError(t, err, "--hello and --two must be used together")

p = mustNew(t, &cli)
_, err = p.Parse([]string{"--one"})
assert.NoError(t, err)

p = mustNew(t, &cli)
_, err = p.Parse([]string{"--hello", "--one"})
assert.EqualError(t, err, "--hello and --one can't be used together, --hello and --two must be used together")
}

func TestXorRequired(t *testing.T) {
var cli struct {
One bool `xor:"one,two" required:""`
Expand All @@ -972,6 +1046,26 @@ func TestXorRequired(t *testing.T) {
assert.EqualError(t, err, "missing flags: --four, --one or --three, --one or --two")
}

func TestAndRequired(t *testing.T) {
var cli struct {
One bool `and:"one,two" required:""`
Two bool `and:"one" required:""`
Three bool `and:"two"`
Four bool `required:""`
}
p := mustNew(t, &cli)
_, err := p.Parse([]string{"--one", "--two", "--three"})
assert.EqualError(t, err, "missing flags: --four")

p = mustNew(t, &cli)
_, err = p.Parse([]string{"--four"})
assert.EqualError(t, err, "missing flags: --one and --three, --one and --two")

p = mustNew(t, &cli)
_, err = p.Parse([]string{})
assert.EqualError(t, err, "missing flags: --four, --one and --three, --one and --two")
}

func TestXorRequiredMany(t *testing.T) {
var cli struct {
One bool `xor:"one" required:""`
Expand All @@ -991,6 +1085,21 @@ func TestXorRequiredMany(t *testing.T) {
assert.EqualError(t, err, "missing flags: --one or --two or --three")
}

func TestAndRequiredMany(t *testing.T) {
var cli struct {
One bool `and:"one" required:""`
Two bool `and:"one" required:""`
Three bool `and:"one" required:""`
}
p := mustNew(t, &cli)
_, err := p.Parse([]string{})
assert.EqualError(t, err, "missing flags: --one and --two and --three")

p = mustNew(t, &cli)
_, err = p.Parse([]string{"--three"})
assert.EqualError(t, err, "missing flags: --one and --two")
}

func TestEnumSequence(t *testing.T) {
var cli struct {
State []string `enum:"a,b,c" default:"a"`
Expand Down
1 change: 1 addition & 0 deletions model.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ type Flag struct {
*Value
Group *Group // Logical grouping when displaying. May also be used by configuration loaders to group options logically.
Xor []string
And []string
PlaceHolder string
Envs []string
Aliases []string
Expand Down
4 changes: 4 additions & 0 deletions tag.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type Tag struct {
Enum string
Group string
Xor []string
And []string
Vars Vars
Prefix string // Optional prefix on anonymous structs. All sub-flags will have this prefix.
EnvPrefix string
Expand Down Expand Up @@ -249,6 +250,9 @@ func hydrateTag(t *Tag, typ reflect.Type) error { //nolint: gocyclo
for _, xor := range t.GetAll("xor") {
t.Xor = append(t.Xor, strings.FieldsFunc(xor, tagSplitFn)...)
}
for _, and := range t.GetAll("and") {
t.And = append(t.And, strings.FieldsFunc(and, tagSplitFn)...)
}
t.Prefix = t.Get("prefix")
t.EnvPrefix = t.Get("envprefix")
t.Embed = t.Has("embed")
Expand Down

0 comments on commit ff6d5ba

Please sign in to comment.