help,usage and error messages and tests

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()))
-}