From 0280e6e5911d4198f69eae9c876722fdf6149b89 Mon Sep 17 00:00:00 2001 From: Pablo Diaz Date: Fri, 20 May 2022 17:35:02 +0200 Subject: [PATCH 1/7] ignores short and long parameters --- example_test.go | 16 ++++++++++++++++ parse.go | 5 ----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/example_test.go b/example_test.go index 5272393..acfacad 100644 --- a/example_test.go +++ b/example_test.go @@ -496,3 +496,19 @@ func Example_allSupportedTypes() { // output: } + +func Example_envVarOnly() { + os.Args = split("./example") + _ = os.Setenv("NUM_WORKERS", "my_key") + + defer os.Unsetenv("NUM_WORKERS") + + var args struct { + AuthKey string `arg:"-,--,env:NUM_WORKERS"` + } + + MustParse(&args) + + fmt.Println(args.AuthKey) + // output: my_key +} diff --git a/parse.go b/parse.go index be77924..e5df067 100644 --- a/parse.go +++ b/parse.go @@ -360,11 +360,6 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { case strings.HasPrefix(key, "--"): spec.long = key[2:] case strings.HasPrefix(key, "-"): - if len(key) != 2 { - errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", - t.Name(), field.Name)) - return false - } spec.short = key[1:] case key == "required": spec.required = true From c3cac76438ca36d9291d2a26f3ba558ef929d588 Mon Sep 17 00:00:00 2001 From: Pablo Diaz Date: Sat, 21 May 2022 17:24:45 +0200 Subject: [PATCH 2/7] added tests and fixed usage --- example_test.go | 32 +++++++++++++++++++++++++++++--- parse.go | 2 +- usage.go | 21 ++++++++++++++++++++- usage_test.go | 29 +++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/example_test.go b/example_test.go index acfacad..6216e18 100644 --- a/example_test.go +++ b/example_test.go @@ -499,12 +499,12 @@ func Example_allSupportedTypes() { func Example_envVarOnly() { os.Args = split("./example") - _ = os.Setenv("NUM_WORKERS", "my_key") + _ = os.Setenv("AUTH_KEY", "my_key") - defer os.Unsetenv("NUM_WORKERS") + defer os.Unsetenv("AUTH_KEY") var args struct { - AuthKey string `arg:"-,--,env:NUM_WORKERS"` + AuthKey string `arg:"-,--,env:AUTH_KEY"` } MustParse(&args) @@ -512,3 +512,29 @@ func Example_envVarOnly() { fmt.Println(args.AuthKey) // output: my_key } + +func Example_envVarOnlyShouldIgnoreFlag() { + os.Args = split("./example --=my_key") + + var args struct { + AuthKey string `arg:"-,--,env:AUTH_KEY"` + } + + err := Parse(&args) + + fmt.Println(err) + // output: unknown argument --=my_key +} + +func Example_envVarOnlyShouldIgnoreShortFlag() { + os.Args = split("./example -=my_key") + + var args struct { + AuthKey string `arg:"-,--,env:AUTH_KEY"` + } + + err := Parse(&args) + + fmt.Println(err) + // output: unknown argument -=my_key +} diff --git a/parse.go b/parse.go index e5df067..aee60e7 100644 --- a/parse.go +++ b/parse.go @@ -656,7 +656,7 @@ func (p *Parser) process(args []string) error { // lookup the spec for this option (note that the "specs" slice changes as // we expand subcommands so it is better not to use a map) spec := findOption(specs, opt) - if spec == nil { + if spec == nil || opt == "" { return fmt.Errorf("unknown argument %s", arg) } wasPresent[spec] = true diff --git a/usage.go b/usage.go index 43d6231..f032149 100644 --- a/usage.go +++ b/usage.go @@ -84,6 +84,11 @@ func (p *Parser) writeUsageForSubcommand(w io.Writer, cmd *command) { ancestors = append(ancestors, ancestor.name) ancestor = ancestor.parent } + for _, spec := range cmd.specs { + if spec.short == "" && spec.long == "" { + ancestors = append(ancestors, spec.env+"="+strings.ToLower(spec.env)+"_value") + } + } // print the beginning of the usage string fmt.Fprint(w, "Usage:") @@ -208,7 +213,7 @@ func (p *Parser) WriteHelpForSubcommand(w io.Writer, subcommand ...string) error // writeHelp writes the usage string for the given subcommand func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) { - var positionals, longOptions, shortOptions []*spec + var positionals, longOptions, shortOptions, envOnlyOptions []*spec for _, spec := range cmd.specs { switch { case spec.positional: @@ -217,6 +222,8 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) { longOptions = append(longOptions, spec) case spec.short != "": shortOptions = append(shortOptions, spec) + case spec.short == "" && spec.long == "": + envOnlyOptions = append(envOnlyOptions, spec) } } @@ -275,6 +282,14 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) { }) } + // write the list of environment only variables + if len(shortOptions)+len(longOptions) > 0 || cmd.parent == nil { + fmt.Fprint(w, "\nEnvironment variables:\n") + for _, spec := range envOnlyOptions { + p.printEnvOnlyVar(w, spec) + } + } + // write the list of subcommands if len(cmd.subcommands) > 0 { fmt.Fprint(w, "\nCommands:\n") @@ -301,6 +316,10 @@ func (p *Parser) printOption(w io.Writer, spec *spec) { } } +func (p *Parser) printEnvOnlyVar(w io.Writer, spec *spec) { + printTwoCols(w, spec.env, "", "", "") +} + // lookupCommand finds a subcommand based on a sequence of subcommand names. The // first string should be a top-level subcommand, the next should be a child // subcommand of that subcommand, and so on. If no strings are given then the diff --git a/usage_test.go b/usage_test.go index 69feac2..d5e150f 100644 --- a/usage_test.go +++ b/usage_test.go @@ -646,3 +646,32 @@ Options: p.WriteHelp(&help) assert.Equal(t, expectedHelp[1:], help.String()) } + +func TestFailEnvOnly(t *testing.T) { + expectedUsage := "Usage: AUTH_KEY=auth_key_value example [--arg ARG]" + + expectedHelp := ` +Usage: AUTH_KEY=auth_key_value example [--arg ARG] + +Options: + --arg ARG, -a ARG [env: MY_ARG] + --help, -h display this help and exit + +Environment variables: + AUTH_KEY +` + var args struct { + ArgParam string `arg:"-a,--arg,env:MY_ARG"` + AuthKey string `arg:"-,--,env:AUTH_KEY"` + } + p, err := NewParser(Config{Program: "example"}, &args) + assert.NoError(t, err) + + var help bytes.Buffer + p.WriteHelp(&help) + assert.Equal(t, expectedHelp[1:], help.String()) + + var usage bytes.Buffer + p.WriteUsage(&usage) + assert.Equal(t, expectedUsage, strings.TrimSpace(usage.String())) +} From 5f10667949b09f9a43c241d969eea8d3ba9456c0 Mon Sep 17 00:00:00 2001 From: Pablo Diaz Date: Sat, 21 May 2022 17:44:32 +0200 Subject: [PATCH 3/7] fixed tests --- example_test.go | 6 +++--- parse.go | 5 +++++ usage.go | 5 +++-- usage_test.go | 14 +++++++++----- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/example_test.go b/example_test.go index 6216e18..4bd7632 100644 --- a/example_test.go +++ b/example_test.go @@ -504,7 +504,7 @@ func Example_envVarOnly() { defer os.Unsetenv("AUTH_KEY") var args struct { - AuthKey string `arg:"-,--,env:AUTH_KEY"` + AuthKey string `arg:"--,env:AUTH_KEY"` } MustParse(&args) @@ -517,7 +517,7 @@ func Example_envVarOnlyShouldIgnoreFlag() { os.Args = split("./example --=my_key") var args struct { - AuthKey string `arg:"-,--,env:AUTH_KEY"` + AuthKey string `arg:"--,env:AUTH_KEY"` } err := Parse(&args) @@ -530,7 +530,7 @@ func Example_envVarOnlyShouldIgnoreShortFlag() { os.Args = split("./example -=my_key") var args struct { - AuthKey string `arg:"-,--,env:AUTH_KEY"` + AuthKey string `arg:"--,env:AUTH_KEY"` } err := Parse(&args) diff --git a/parse.go b/parse.go index aee60e7..a85419f 100644 --- a/parse.go +++ b/parse.go @@ -360,6 +360,11 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { case strings.HasPrefix(key, "--"): spec.long = key[2:] case strings.HasPrefix(key, "-"): + if len(key) != 2 { + errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", + t.Name(), field.Name)) + return false + } spec.short = key[1:] case key == "required": spec.required = true diff --git a/usage.go b/usage.go index f032149..fb5da75 100644 --- a/usage.go +++ b/usage.go @@ -84,9 +84,10 @@ func (p *Parser) writeUsageForSubcommand(w io.Writer, cmd *command) { ancestors = append(ancestors, ancestor.name) ancestor = ancestor.parent } + // Print environment only variables for _, spec := range cmd.specs { if spec.short == "" && spec.long == "" { - ancestors = append(ancestors, spec.env+"="+strings.ToLower(spec.env)+"_value") + ancestors = append(ancestors, "["+spec.env+"="+strings.ToLower(spec.env)+"_value"+"]") } } @@ -283,7 +284,7 @@ func (p *Parser) writeHelpForSubcommand(w io.Writer, cmd *command) { } // write the list of environment only variables - if len(shortOptions)+len(longOptions) > 0 || cmd.parent == nil { + if len(envOnlyOptions) > 0 { fmt.Fprint(w, "\nEnvironment variables:\n") for _, spec := range envOnlyOptions { p.printEnvOnlyVar(w, spec) diff --git a/usage_test.go b/usage_test.go index d5e150f..5ac7757 100644 --- a/usage_test.go +++ b/usage_test.go @@ -544,14 +544,18 @@ Options: } func TestUsageWithEnvOptions(t *testing.T) { - expectedUsage := "Usage: example [-s SHORT]" + expectedUsage := "Usage: [CUSTOM=custom_value] [ENVONLY=envonly_value] example [-s SHORT]" expectedHelp := ` -Usage: example [-s SHORT] +Usage: [CUSTOM=custom_value] [ENVONLY=envonly_value] example [-s SHORT] Options: -s SHORT [env: SHORT] --help, -h display this help and exit + +Environment variables: + ENVONLY + CUSTOM ` var args struct { Short string `arg:"--,-s,env"` @@ -648,10 +652,10 @@ Options: } func TestFailEnvOnly(t *testing.T) { - expectedUsage := "Usage: AUTH_KEY=auth_key_value example [--arg ARG]" + expectedUsage := "Usage: [AUTH_KEY=auth_key_value] example [--arg ARG]" expectedHelp := ` -Usage: AUTH_KEY=auth_key_value example [--arg ARG] +Usage: [AUTH_KEY=auth_key_value] example [--arg ARG] Options: --arg ARG, -a ARG [env: MY_ARG] @@ -662,7 +666,7 @@ Environment variables: ` var args struct { ArgParam string `arg:"-a,--arg,env:MY_ARG"` - AuthKey string `arg:"-,--,env:AUTH_KEY"` + AuthKey string `arg:"--,env:AUTH_KEY"` } p, err := NewParser(Config{Program: "example"}, &args) assert.NoError(t, err) From ccf62e0ffccbae98aeaf60d7fbe76ccc924de11e Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Sat, 3 Jun 2023 03:14:14 +0200 Subject: [PATCH 4/7] don't print env-vars in usage line --- usage.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/usage.go b/usage.go index fb5da75..c76644f 100644 --- a/usage.go +++ b/usage.go @@ -84,12 +84,6 @@ func (p *Parser) writeUsageForSubcommand(w io.Writer, cmd *command) { ancestors = append(ancestors, ancestor.name) ancestor = ancestor.parent } - // Print environment only variables - for _, spec := range cmd.specs { - if spec.short == "" && spec.long == "" { - ancestors = append(ancestors, "["+spec.env+"="+strings.ToLower(spec.env)+"_value"+"]") - } - } // print the beginning of the usage string fmt.Fprint(w, "Usage:") From b928a1839ae3502fc6fef8d7743dd78f2c772c8a Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Sat, 3 Jun 2023 09:50:42 +0200 Subject: [PATCH 5/7] Parse env-only vars --- parse.go | 35 ++++++++++++++++++++++++++++++----- usage_test.go | 8 ++++---- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/parse.go b/parse.go index a85419f..2eb14e3 100644 --- a/parse.go +++ b/parse.go @@ -54,9 +54,10 @@ type spec struct { separate bool // if true, each slice and map entry will have its own --flag help string // the help text for this option env string // the name of the environment variable for this option, or empty for none - defaultValue reflect.Value // default value for this option - defaultString string // default value for this option, in string form to be displayed in help text - placeholder string // name of the data in help + envOnly bool + defaultValue reflect.Value // default value for this option + defaultString string // default value for this option, in string form to be displayed in help text + placeholder string // name of the data in help } // command represents a named subcommand, or the top-level command @@ -343,7 +344,9 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { // Look at the tag var isSubcommand bool // tracks whether this field is a subcommand - for _, key := range strings.Split(tag, ",") { + kvPairs := strings.Split(tag, ",") + + for _, key := range kvPairs { if key == "" { continue } @@ -360,7 +363,7 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { case strings.HasPrefix(key, "--"): spec.long = key[2:] case strings.HasPrefix(key, "-"): - if len(key) != 2 { + if len(key) > 2 { errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", t.Name(), field.Name)) return false @@ -415,6 +418,12 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { spec.placeholder = strings.ToUpper(spec.field.Name) } + if spec.env == "" && emptyLongAndShort(kvPairs) { + errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", + t.Name(), field.Name)) + return false + } + // if this is a subcommand then we've done everything we need to do if isSubcommand { return false @@ -788,6 +797,22 @@ func isFlag(s string) bool { return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != "" } +func emptyLongAndShort(kv []string) bool { + var noShort, noLong bool + for _, key := range kv { + if key == "-" { + noShort = true + } + + if key == "--" { + noLong = true + } + } + + return noShort && noLong + +} + // val returns a reflect.Value corresponding to the current value for the // given path func (p *Parser) val(dest path) reflect.Value { diff --git a/usage_test.go b/usage_test.go index 5ac7757..0debb4a 100644 --- a/usage_test.go +++ b/usage_test.go @@ -544,10 +544,10 @@ Options: } func TestUsageWithEnvOptions(t *testing.T) { - expectedUsage := "Usage: [CUSTOM=custom_value] [ENVONLY=envonly_value] example [-s SHORT]" + expectedUsage := "Usage: example [-s SHORT]" expectedHelp := ` -Usage: [CUSTOM=custom_value] [ENVONLY=envonly_value] example [-s SHORT] +Usage: example [-s SHORT] Options: -s SHORT [env: SHORT] @@ -652,10 +652,10 @@ Options: } func TestFailEnvOnly(t *testing.T) { - expectedUsage := "Usage: [AUTH_KEY=auth_key_value] example [--arg ARG]" + expectedUsage := "Usage: example [--arg ARG]" expectedHelp := ` -Usage: [AUTH_KEY=auth_key_value] example [--arg ARG] +Usage: example [--arg ARG] Options: --arg ARG, -a ARG [env: MY_ARG] From 18623d869bfb7aa7b87eda2c8dc7d1bf6149f316 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Sat, 3 Jun 2023 12:47:47 +0200 Subject: [PATCH 6/7] help,usage and error messages and tests --- parse.go | 12 ++++++++- parse_test.go | 26 +++++++++++++++++++ usage.go | 13 +++++++++- usage_test.go | 70 ++++++++++++++++++++++++++++----------------------- 4 files changed, 88 insertions(+), 33 deletions(-) diff --git a/parse.go b/parse.go index 2eb14e3..a16485b 100644 --- a/parse.go +++ b/parse.go @@ -418,10 +418,14 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { spec.placeholder = strings.ToUpper(spec.field.Name) } - if spec.env == "" && emptyLongAndShort(kvPairs) { + noFormSpecs := emptyLongAndShort(kvPairs) + + if spec.env == "" && noFormSpecs { errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", t.Name(), field.Name)) return false + } else if spec.env != "" && noFormSpecs { + spec.envOnly = true } // if this is a subcommand then we've done everything we need to do @@ -759,10 +763,16 @@ func (p *Parser) process(args []string) error { } if spec.required { + if spec.envOnly { + msg := fmt.Sprintf("environment variable %s is required", spec.env) + return errors.New(msg) + } + msg := fmt.Sprintf("%s is required", name) if spec.env != "" { msg += " (or environment variable " + spec.env + ")" } + return errors.New(msg) } diff --git a/parse_test.go b/parse_test.go index d368b17..77a034f 100644 --- a/parse_test.go +++ b/parse_test.go @@ -227,6 +227,14 @@ func TestRequiredWithEnv(t *testing.T) { require.Error(t, err, "--foo is required (or environment variable FOO)") } +func TestRequiredWithEnvOnly(t *testing.T) { + var args struct { + Foo string `arg:"required,--,-,env:FOO"` + } + _, err := parseWithEnv("", []string{}, &args) + require.Error(t, err, "environment variable FOO is required") +} + func TestShortFlag(t *testing.T) { var args struct { Foo string `arg:"-f"` @@ -845,6 +853,24 @@ func TestDefaultValuesIgnored(t *testing.T) { assert.Equal(t, "", args.Foo) } +func TestRequiredEnvironmentOnlyVariableIsMissing(t *testing.T) { + var args struct { + Foo string `arg:"required,--,env:FOO"` + } + + _, err := parseWithEnv("", []string{""}, &args) + assert.Error(t, err) +} + +func TestOptionalEnvironmentOnlyVariable(t *testing.T) { + var args struct { + Foo string `arg:"env:FOO"` + } + + _, err := parseWithEnv("", []string{}, &args) + assert.NoError(t, err) +} + func TestEnvironmentVariableInSubcommandIgnored(t *testing.T) { var args struct { Sub *struct { diff --git a/usage.go b/usage.go index c76644f..0498910 100644 --- a/usage.go +++ b/usage.go @@ -312,7 +312,18 @@ func (p *Parser) printOption(w io.Writer, spec *spec) { } func (p *Parser) printEnvOnlyVar(w io.Writer, spec *spec) { - printTwoCols(w, spec.env, "", "", "") + ways := make([]string, 0, 2) + if spec.required { + ways = append(ways, "Required.") + } else { + ways = append(ways, "Optional.") + } + + if spec.help != "" { + ways = append(ways, spec.help) + } + + printTwoCols(w, spec.env, strings.Join(ways, " "), spec.defaultString, "") } // lookupCommand finds a subcommand based on a sequence of subcommand names. The diff --git a/usage_test.go b/usage_test.go index 0debb4a..1a64ad4 100644 --- a/usage_test.go +++ b/usage_test.go @@ -56,6 +56,10 @@ Options: --testenv TESTENV, -a TESTENV [env: TEST_ENV] --file FILE, -f FILE File with mandatory extension [default: scratch.txt] --help, -h display this help and exit + +Environment variables: + API_KEY Required. Only via env-var for security reasons + TRACE Optional. Record low-level trace ` var args struct { @@ -70,6 +74,8 @@ Options: Values []float64 `help:"Values"` Workers int `arg:"-w,env:WORKERS" help:"number of workers to start" default:"10"` TestEnv string `arg:"-a,env:TEST_ENV"` + ApiKey string `arg:"required,-,--,env:API_KEY" help:"Only via env-var for security reasons"` + Trace bool `arg:"-,--,env" help:"Record low-level trace"` File *NameDotName `arg:"-f" help:"File with mandatory extension"` } args.Name = "Foo Bar" @@ -554,12 +560,14 @@ Options: --help, -h display this help and exit Environment variables: - ENVONLY - CUSTOM + ENVONLY Optional. + ENVONLY2 Optional. + CUSTOM Optional. ` var args struct { Short string `arg:"--,-s,env"` EnvOnly string `arg:"--,env"` + EnvOnly2 string `arg:"--,-,env"` EnvOnlyOverriden string `arg:"--,env:CUSTOM"` } @@ -575,6 +583,35 @@ Environment variables: assert.Equal(t, expectedUsage, strings.TrimSpace(usage.String())) } +func TestEnvOnlyArgs(t *testing.T) { + expectedUsage := "Usage: example [--arg ARG]" + + expectedHelp := ` +Usage: example [--arg ARG] + +Options: + --arg ARG, -a ARG [env: MY_ARG] + --help, -h display this help and exit + +Environment variables: + AUTH_KEY Required. +` + var args struct { + ArgParam string `arg:"-a,--arg,env:MY_ARG"` + AuthKey string `arg:"required,--,env:AUTH_KEY"` + } + p, err := NewParser(Config{Program: "example"}, &args) + assert.NoError(t, err) + + var help bytes.Buffer + p.WriteHelp(&help) + assert.Equal(t, expectedHelp[1:], help.String()) + + var usage bytes.Buffer + p.WriteUsage(&usage) + assert.Equal(t, expectedUsage, strings.TrimSpace(usage.String())) +} + func TestFail(t *testing.T) { var stdout bytes.Buffer var exitCode int @@ -650,32 +687,3 @@ Options: p.WriteHelp(&help) assert.Equal(t, expectedHelp[1:], help.String()) } - -func TestFailEnvOnly(t *testing.T) { - expectedUsage := "Usage: example [--arg ARG]" - - expectedHelp := ` -Usage: example [--arg ARG] - -Options: - --arg ARG, -a ARG [env: MY_ARG] - --help, -h display this help and exit - -Environment variables: - AUTH_KEY -` - var args struct { - ArgParam string `arg:"-a,--arg,env:MY_ARG"` - AuthKey string `arg:"--,env:AUTH_KEY"` - } - p, err := NewParser(Config{Program: "example"}, &args) - assert.NoError(t, err) - - var help bytes.Buffer - p.WriteHelp(&help) - assert.Equal(t, expectedHelp[1:], help.String()) - - var usage bytes.Buffer - p.WriteUsage(&usage) - assert.Equal(t, expectedUsage, strings.TrimSpace(usage.String())) -} From 259c83fd5aeb44fbb67a183cf09b4ca16d9b30e2 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Thu, 29 Jun 2023 21:26:34 +0200 Subject: [PATCH 7/7] Remove usage of additional envOnly struct variable --- parse.go | 38 +++++--------------------------------- 1 file changed, 5 insertions(+), 33 deletions(-) diff --git a/parse.go b/parse.go index a16485b..63cfab3 100644 --- a/parse.go +++ b/parse.go @@ -54,10 +54,9 @@ type spec struct { separate bool // if true, each slice and map entry will have its own --flag help string // the help text for this option env string // the name of the environment variable for this option, or empty for none - envOnly bool - defaultValue reflect.Value // default value for this option - defaultString string // default value for this option, in string form to be displayed in help text - placeholder string // name of the data in help + defaultValue reflect.Value // default value for this option + defaultString string // default value for this option, in string form to be displayed in help text + placeholder string // name of the data in help } // command represents a named subcommand, or the top-level command @@ -344,9 +343,8 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { // Look at the tag var isSubcommand bool // tracks whether this field is a subcommand - kvPairs := strings.Split(tag, ",") - for _, key := range kvPairs { + for _, key := range strings.Split(tag, ",") { if key == "" { continue } @@ -418,16 +416,6 @@ func cmdFromStruct(name string, dest path, t reflect.Type) (*command, error) { spec.placeholder = strings.ToUpper(spec.field.Name) } - noFormSpecs := emptyLongAndShort(kvPairs) - - if spec.env == "" && noFormSpecs { - errs = append(errs, fmt.Sprintf("%s.%s: short arguments must be one character only", - t.Name(), field.Name)) - return false - } else if spec.env != "" && noFormSpecs { - spec.envOnly = true - } - // if this is a subcommand then we've done everything we need to do if isSubcommand { return false @@ -763,7 +751,7 @@ func (p *Parser) process(args []string) error { } if spec.required { - if spec.envOnly { + if spec.short == "" && spec.long == "" { msg := fmt.Sprintf("environment variable %s is required", spec.env) return errors.New(msg) } @@ -807,22 +795,6 @@ func isFlag(s string) bool { return strings.HasPrefix(s, "-") && strings.TrimLeft(s, "-") != "" } -func emptyLongAndShort(kv []string) bool { - var noShort, noLong bool - for _, key := range kv { - if key == "-" { - noShort = true - } - - if key == "--" { - noLong = true - } - } - - return noShort && noLong - -} - // val returns a reflect.Value corresponding to the current value for the // given path func (p *Parser) val(dest path) reflect.Value {