Merge pull request #175 from alexflint/bracketing-positionalsv1.4.3

Fix bracketing for non-required positionals in usage

3 files changed, 96 insertions, 20 deletions

diff --git a/example_test.go b/example_test.go
index 20b9225..fd64777 100644
--- a/example_test.go
+++ b/example_test.go
@@ -154,7 +154,7 @@ func Example_helpText() {
 	os.Args = split("./example --help")
 
 	var args struct {
-		Input    string   `arg:"positional"`
+		Input    string   `arg:"positional,required"`
 		Output   []string `arg:"positional"`
 		Verbose  bool     `arg:"-v" help:"verbosity level"`
 		Dataset  string   `help:"dataset to use"`
@@ -188,7 +188,7 @@ func Example_helpPlaceholder() {
 	os.Args = split("./example --help")
 
 	var args struct {
-		Input    string   `arg:"positional" placeholder:"SRC"`
+		Input    string   `arg:"positional,required" placeholder:"SRC"`
 		Output   []string `arg:"positional" placeholder:"DST"`
 		Optimize int      `arg:"-O" help:"optimization level" placeholder:"LEVEL"`
 		MaxJobs  int      `arg:"-j" help:"maximum number of simultaneous jobs" placeholder:"N"`
@@ -259,7 +259,7 @@ func Example_helpTextWhenUsingSubcommand() {
 	os.Args = split("./example get --help")
 
 	type getCmd struct {
-		Item string `arg:"positional" help:"item to fetch"`
+		Item string `arg:"positional,required" help:"item to fetch"`
 	}
 
 	type listCmd struct {
@@ -389,7 +389,7 @@ func Example_errorText() {
 	os.Args = split("./example --optimize INVALID")
 
 	var args struct {
-		Input    string   `arg:"positional"`
+		Input    string   `arg:"positional,required"`
 		Output   []string `arg:"positional"`
 		Verbose  bool     `arg:"-v" help:"verbosity level"`
 		Dataset  string   `help:"dataset to use"`
diff --git a/usage.go b/usage.go
index 860dc15..e936811 100644
--- a/usage.go
+++ b/usage.go
@@ -124,22 +124,32 @@ func (p *Parser) writeUsageForSubcommand(w io.Writer, cmd *command) {
 		}
 	}
 
-	// write the positional component of the usage message
+	// When we parse positionals, we check that:
+	//  1. required positionals come before non-required positionals
+	//  2. there is at most one multiple-value positional
+	//  3. if there is a multiple-value positional then it comes after all other positionals
+	// Here we merely print the usage string, so we do not explicitly re-enforce those rules
+
+	// write the positionals in following form:
+	//    REQUIRED1 REQUIRED2
+	//    REQUIRED1 REQUIRED2 [OPTIONAL1 [OPTIONAL2]]
+	//    REQUIRED1 REQUIRED2 REPEATED [REPEATED ...]
+	//    REQUIRED1 REQUIRED2 [REPEATEDOPTIONAL [REPEATEDOPTIONAL ...]]
+	//    REQUIRED1 REQUIRED2 [OPTIONAL1 [REPEATEDOPTIONAL [REPEATEDOPTIONAL ...]]]
+	var closeBrackets int
 	for _, spec := range positionals {
-		// prefix with a space
 		fmt.Fprint(w, " ")
+		if !spec.required {
+			fmt.Fprint(w, "[")
+			closeBrackets += 1
+		}
 		if spec.cardinality == multiple {
-			if !spec.required {
-				fmt.Fprint(w, "[")
-			}
 			fmt.Fprintf(w, "%s [%s ...]", spec.placeholder, spec.placeholder)
-			if !spec.required {
-				fmt.Fprint(w, "]")
-			}
 		} else {
 			fmt.Fprint(w, spec.placeholder)
 		}
 	}
+	fmt.Fprint(w, strings.Repeat("]", closeBrackets))
 
 	// if the program supports subcommands, give a hint to the user about their existence
 	if len(cmd.subcommands) > 0 {
diff --git a/usage_test.go b/usage_test.go
index f790e34..1744536 100644
--- a/usage_test.go
+++ b/usage_test.go
@@ -59,7 +59,7 @@ Options:
 `
 
 	var args struct {
-		Input    string       `arg:"positional"`
+		Input    string       `arg:"positional,required"`
 		Output   []string     `arg:"positional" help:"list of outputs"`
 		Name     string       `help:"name to use"`
 		Value    int          `help:"secret value"`
@@ -141,10 +141,10 @@ func TestUsageCannotMarshalToString(t *testing.T) {
 }
 
 func TestUsageLongPositionalWithHelp_legacyForm(t *testing.T) {
-	expectedUsage := "Usage: example VERYLONGPOSITIONALWITHHELP"
+	expectedUsage := "Usage: example [VERYLONGPOSITIONALWITHHELP]"
 
 	expectedHelp := `
-Usage: example VERYLONGPOSITIONALWITHHELP
+Usage: example [VERYLONGPOSITIONALWITHHELP]
 
 Positional arguments:
   VERYLONGPOSITIONALWITHHELP
@@ -170,10 +170,10 @@ Options:
 }
 
 func TestUsageLongPositionalWithHelp_newForm(t *testing.T) {
-	expectedUsage := "Usage: example VERYLONGPOSITIONALWITHHELP"
+	expectedUsage := "Usage: example [VERYLONGPOSITIONALWITHHELP]"
 
 	expectedHelp := `
-Usage: example VERYLONGPOSITIONALWITHHELP
+Usage: example [VERYLONGPOSITIONALWITHHELP]
 
 Positional arguments:
   VERYLONGPOSITIONALWITHHELP
@@ -285,8 +285,74 @@ Options:
 	assert.Equal(t, expectedUsage, strings.TrimSpace(usage.String()))
 }
 
+func TestUsageForRequiredPositionals(t *testing.T) {
+	expectedUsage := "Usage: example REQUIRED1 REQUIRED2\n"
+	var args struct {
+		Required1 string `arg:"positional,required"`
+		Required2 string `arg:"positional,required"`
+	}
+
+	p, err := NewParser(Config{Program: "example"}, &args)
+	require.NoError(t, err)
+
+	var usage bytes.Buffer
+	p.WriteUsage(&usage)
+	assert.Equal(t, expectedUsage, usage.String())
+}
+
+func TestUsageForMixedPositionals(t *testing.T) {
+	expectedUsage := "Usage: example REQUIRED1 REQUIRED2 [OPTIONAL1 [OPTIONAL2]]\n"
+	var args struct {
+		Required1 string `arg:"positional,required"`
+		Required2 string `arg:"positional,required"`
+		Optional1 string `arg:"positional"`
+		Optional2 string `arg:"positional"`
+	}
+
+	p, err := NewParser(Config{Program: "example"}, &args)
+	require.NoError(t, err)
+
+	var usage bytes.Buffer
+	p.WriteUsage(&usage)
+	assert.Equal(t, expectedUsage, usage.String())
+}
+
+func TestUsageForRepeatedPositionals(t *testing.T) {
+	expectedUsage := "Usage: example REQUIRED1 REQUIRED2 REPEATED [REPEATED ...]\n"
+	var args struct {
+		Required1 string   `arg:"positional,required"`
+		Required2 string   `arg:"positional,required"`
+		Repeated  []string `arg:"positional,required"`
+	}
+
+	p, err := NewParser(Config{Program: "example"}, &args)
+	require.NoError(t, err)
+
+	var usage bytes.Buffer
+	p.WriteUsage(&usage)
+	assert.Equal(t, expectedUsage, usage.String())
+}
+
+func TestUsageForMixedAndRepeatedPositionals(t *testing.T) {
+	expectedUsage := "Usage: example REQUIRED1 REQUIRED2 [OPTIONAL1 [OPTIONAL2 [REPEATED [REPEATED ...]]]]\n"
+	var args struct {
+		Required1 string   `arg:"positional,required"`
+		Required2 string   `arg:"positional,required"`
+		Optional1 string   `arg:"positional"`
+		Optional2 string   `arg:"positional"`
+		Repeated  []string `arg:"positional"`
+	}
+
+	p, err := NewParser(Config{Program: "example"}, &args)
+	require.NoError(t, err)
+
+	var usage bytes.Buffer
+	p.WriteUsage(&usage)
+	assert.Equal(t, expectedUsage, usage.String())
+}
+
 func TestRequiredMultiplePositionals(t *testing.T) {
-	expectedUsage := "Usage: example REQUIREDMULTIPLE [REQUIREDMULTIPLE ...]"
+	expectedUsage := "Usage: example REQUIREDMULTIPLE [REQUIREDMULTIPLE ...]\n"
 
 	expectedHelp := `
 Usage: example REQUIREDMULTIPLE [REQUIREDMULTIPLE ...]
@@ -301,7 +367,7 @@ Options:
 		RequiredMultiple []string `arg:"positional,required" help:"required multiple positional"`
 	}
 
-	p, err := NewParser(Config{}, &args)
+	p, err := NewParser(Config{Program: "example"}, &args)
 	require.NoError(t, err)
 
 	var help bytes.Buffer
@@ -310,7 +376,7 @@ Options:
 
 	var usage bytes.Buffer
 	p.WriteUsage(&usage)
-	assert.Equal(t, expectedUsage, strings.TrimSpace(usage.String()))
+	assert.Equal(t, expectedUsage, usage.String())
 }
 
 func TestUsageWithNestedSubcommands(t *testing.T) {