From f19f1031c032a23ba12f3c18289555547fa9669f Mon Sep 17 00:00:00 2001
From: Cam Hutchison <camh@xdna.net>
Date: Tue, 2 Jun 2020 08:17:42 +1000
Subject: [PATCH] Format help placeholders with slice/map separators

Use the specified separators for a flag in the placeholders printed in
the help message for slices and maps. Do not print the separator and
ellipsis if the separator is disabled (`"none"`).

Previously the default separator was printed with an ellipsis, ignoring
the `sep` and `mapsep` tags.

Add tests for `Flag.FormatPlaceHolder()`.
---
 model.go      |  9 ++++++---
 model_test.go | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 3 deletions(-)

diff --git a/model.go b/model.go
index 8023dce..bd9eec4 100644
--- a/model.go
+++ b/model.go
@@ -372,8 +372,8 @@ func (f *Flag) String() string {
 // FormatPlaceHolder formats the placeholder string for a Flag.
 func (f *Flag) FormatPlaceHolder() string {
 	tail := ""
-	if f.Value.IsSlice() {
-		tail += ",..."
+	if f.Value.IsSlice() && f.Value.Tag.Sep != -1 {
+		tail += string(f.Value.Tag.Sep) + "..."
 	}
 	if f.Default != "" {
 		if f.Value.Target.Kind() == reflect.String {
@@ -385,7 +385,10 @@ func (f *Flag) FormatPlaceHolder() string {
 		return f.PlaceHolder + tail
 	}
 	if f.Value.IsMap() {
-		return "KEY=VALUE;..."
+		if f.Value.Tag.MapSep != -1 {
+			tail = string(f.Value.Tag.MapSep) + "..."
+		}
+		return "KEY=VALUE" + tail
 	}
 	return strings.ToUpper(f.Name) + tail
 }
diff --git a/model_test.go b/model_test.go
index b128aaf..9910704 100644
--- a/model_test.go
+++ b/model_test.go
@@ -25,3 +25,44 @@ func TestModelApplicationCommands(t *testing.T) {
 	}
 	require.Equal(t, []string{"one two", "one three <four>"}, actual)
 }
+
+func TestFormatPlaceHolder(t *testing.T) {
+	var cli struct {
+		String                  string
+		DefaultInt              int    `default:"42"`
+		DefaultStr              string `default:"hello"`
+		Placeholder             string `placeholder:"world"`
+		SliceSep                []string
+		SliceNoSep              []string `sep:"none"`
+		SliceDefault            []string `default:"hello"`
+		SlicePlaceholder        []string `placeholder:"world"`
+		SliceDefaultPlaceholder []string `default:"hello" placeholder:"world"`
+		MapSep                  map[string]string
+		MapNoSep                map[string]string `mapsep:"none"`
+		MapDefault              map[string]string `mapsep:"none" default:"hello"`
+		MapPlaceholder          map[string]string `mapsep:"none" placeholder:"world"`
+	}
+	tests := map[string]string{
+		"help":                      "HELP",
+		"string":                    "STRING",
+		"default-int":               "42",
+		"default-str":               `"hello"`,
+		"placeholder":               "world",
+		"slice-sep":                 "SLICE-SEP,...",
+		"slice-no-sep":              "SLICE-NO-SEP",
+		"slice-default":             "hello,...",
+		"slice-placeholder":         "world,...",
+		"slice-default-placeholder": "hello,...",
+		"map-sep":                   "KEY=VALUE;...",
+		"map-no-sep":                "KEY=VALUE",
+		"map-default":               "hello",
+		"map-placeholder":           "world",
+	}
+
+	p := mustNew(t, &cli)
+	for _, flag := range p.Model.Flags {
+		want, ok := tests[flag.Name]
+		require.Truef(t, ok, "unknown flag name: %s", flag.Name)
+		require.Equal(t, want, flag.FormatPlaceHolder())
+	}
+}