From 18623d869bfb7aa7b87eda2c8dc7d1bf6149f316 Mon Sep 17 00:00:00 2001 From: Ilja Neumann Date: Sat, 3 Jun 2023 12:47:47 +0200 Subject: [PATCH] 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())) -}