Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/xand tag #1

Merged
merged 6 commits into from
Aug 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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. |
| `xand:"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,
Xand: tag.Xand,
Hidden: tag.Hidden,
}
value.Flag = flag
Expand Down
74 changes: 72 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 := checkXorDuplicatedAndXandMissing(c.Path); err != nil {
return err
}

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

if len(missing) == 0 {
return nil
Expand Down Expand Up @@ -977,6 +1001,20 @@ func checkPassthroughArg(target reflect.Value) bool {
}
}

func checkXorDuplicatedAndXandMissing(paths []*Path) error {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't exactly see why we'd like to join the errors from checkXorDuplicates() and checkXandMising() here. Is it in case there are variables that are included in both Xor and Xand groups and we have to combine error messages from missing Xors and Xands?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I just thought it would be useful for the user to be told about both duplicated xor flag and missing xand flags in the same error message. So the user do not have to retry several times, and for example first get a message that there are two flags from the same xor group used, and then when fixing this get a new error message that there are missing xand flags and then needing to fix that. So from a user perspective I think it would be best to be told about both problems at the same time.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was slightly worried about breaking the single responsibility principle, but I think it makes sense in this case

errs := []string{}
if err := checkXorDuplicates(paths); err != nil {
errs = append(errs, fmt.Sprintf("%s", err))
}
if err := checkXandMissing(paths); err != nil {
errs = append(errs, fmt.Sprintf("%s", err))
}
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 +1033,38 @@ func checkXorDuplicates(paths []*Path) error {
return nil
}

func checkXandMissing(paths []*Path) error {
for _, path := range paths {
missingMsgs := []string{}
xandGroups := map[string][]*Flag{}
for _, flag := range path.Flags {
for _, xand := range flag.Xand {
xandGroups[xand] = append(xandGroups[xand], flag)
}
}
for _, flags := range xandGroups {
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 TestXand(t *testing.T) {
var cli struct {
Hello bool `xand:"another"`
One bool `xand:"group"`
Two string `xand:"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 TestXandChild(t *testing.T) {
var cli struct {
One bool `xand:"group"`
Cmd struct {
Two string `xand:"group"`
Three string `xand:"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 TestMultiXand(t *testing.T) {
var cli struct {
Hello bool `xand:"one,two"`
One bool `xand:"one"`
Two string `xand:"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(fmt.Sprintf("%s", err), ", ")
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 TestXorXand(t *testing.T) {
var cli struct {
Hello bool `xor:"one" xand:"two"`
One bool `xor:"one"`
Two string `xand:"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 TestXandRequired(t *testing.T) {
var cli struct {
One bool `xand:"one,two" required:""`
Two bool `xand:"one" required:""`
Three bool `xand:"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 TestXandRequiredMany(t *testing.T) {
var cli struct {
One bool `xand:"one" required:""`
Two bool `xand:"one" required:""`
Three bool `xand:"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
Xand []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
Xand []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 _, xand := range t.GetAll("xand") {
t.Xand = append(t.Xand, strings.FieldsFunc(xand, tagSplitFn)...)
}
t.Prefix = t.Get("prefix")
t.EnvPrefix = t.Get("envprefix")
t.Embed = t.Has("embed")
Expand Down