From 1e88ec55fcc83fb23c9982155c1ad8d591b012f4 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 17:54:21 +0100 Subject: [PATCH 01/10] Add `FormatSlice` function Signed-off-by: Babak K. Shandiz --- internal/text/text.go | 63 ++++++++++++++++++++++++++ internal/text/text_test.go | 92 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+) diff --git a/internal/text/text.go b/internal/text/text.go index 9ecfad93b..c14071c32 100644 --- a/internal/text/text.go +++ b/internal/text/text.go @@ -2,8 +2,10 @@ package text import ( "fmt" + "math" "net/url" "regexp" + "slices" "strings" "time" @@ -81,3 +83,64 @@ func RemoveDiacritics(value string) string { func PadRight(maxWidth int, s string) string { return text.PadRight(maxWidth, s) } + +// FormatSlice concatenates elements of the given string slice into a +// well-formatted, possibly multiline, string with specific line length limit. +// Elements can be optionally surrounded by custom strings (e.g., quotes or +// brackets). If the lineLength argument is non-positive, no line length limit +// will be applied. +func FormatSlice(values []string, lineLength uint, indent uint, prependWith string, appendWith string, sort bool) string { + if lineLength <= 0 { + lineLength = math.MaxInt + } + + sortedValues := values + if sort { + sortedValues = slices.Clone(values) + slices.Sort(sortedValues) + } + + pre := strings.Repeat(" ", int(indent)) + if len(sortedValues) == 0 { + return pre + } else if len(sortedValues) == 1 { + return pre + sortedValues[0] + } + + builder := strings.Builder{} + currentLineLength := 0 + sep := "," + ws := " " + + for i := 0; i < len(sortedValues); i++ { + v := prependWith + sortedValues[i] + appendWith + isLast := i == -1+len(sortedValues) + + if currentLineLength == 0 { + builder.WriteString(pre) + builder.WriteString(v) + currentLineLength += len(v) + if !isLast { + builder.WriteString(sep) + currentLineLength += len(sep) + } + } else { + if !isLast && currentLineLength+len(ws)+len(v)+len(sep) > int(lineLength) || + isLast && currentLineLength+len(ws)+len(v) > int(lineLength) { + currentLineLength = 0 + builder.WriteString("\n") + i-- + continue + } + + builder.WriteString(ws) + builder.WriteString(v) + currentLineLength += len(ws) + len(v) + if !isLast { + builder.WriteString(sep) + currentLineLength += len(sep) + } + } + } + return builder.String() +} diff --git a/internal/text/text_test.go b/internal/text/text_test.go index 98f16da5d..6d76d4872 100644 --- a/internal/text/text_test.go +++ b/internal/text/text_test.go @@ -54,3 +54,95 @@ func TestFuzzyAgoAbbr(t *testing.T) { assert.Equal(t, expected, fuzzy) } } + +func TestFormatSliceDoc(t *testing.T) { + tests := []struct { + name string + values []string + indent uint + lineLength uint + prependWith string + appendWith string + sort bool + wants string + }{ + { + name: "empty", + lineLength: 10, + values: []string{}, + wants: "", + }, + { + name: "empty with indent", + lineLength: 10, + indent: 2, + values: []string{}, + wants: " ", + }, + { + name: "single", + lineLength: 10, + values: []string{"foo"}, + wants: "foo", + }, + { + name: "single with indent", + lineLength: 10, + indent: 2, + values: []string{"foo"}, + wants: " foo", + }, + { + name: "long single with indent", + lineLength: 10, + indent: 2, + values: []string{"some-long-value"}, + wants: " some-long-value", + }, + { + name: "exact line length", + lineLength: 4, + values: []string{"a", "b"}, + wants: "a, b", + }, + { + name: "values longer than line length", + lineLength: 4, + values: []string{"long-value", "long-value"}, + wants: "long-value,\nlong-value", + }, + { + name: "zero line length (no wrapping expected)", + lineLength: 0, + values: []string{"foo", "bar"}, + wants: "foo, bar", + }, + { + name: "simple", + lineLength: 10, + values: []string{"foo", "bar", "baz", "foo", "bar", "baz"}, + wants: "foo, bar,\nbaz, foo,\nbar, baz", + }, + { + name: "simple, surrounded", + lineLength: 13, + prependWith: "<", + appendWith: ">", + values: []string{"foo", "bar", "baz", "foo", "bar", "baz"}, + wants: ", ,\n, ,\n, ", + }, + { + name: "sort", + lineLength: 99, + sort: true, + values: []string{"c", "b", "a"}, + wants: "a, b, c", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wants, FormatSlice(tt.values, tt.lineLength, tt.indent, tt.prependWith, tt.appendWith, tt.sort)) + }) + } +} From 506378cc21e648f70a8a5f7ab7dd331dc57e8b2f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 17:54:59 +0100 Subject: [PATCH 02/10] Add `help:json-fields` annotation Signed-off-by: Babak K. Shandiz --- pkg/cmdutil/json_flags.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/cmdutil/json_flags.go b/pkg/cmdutil/json_flags.go index bc3c242fe..440e0e0b8 100644 --- a/pkg/cmdutil/json_flags.go +++ b/pkg/cmdutil/json_flags.go @@ -83,6 +83,15 @@ func AddJSONFlags(cmd *cobra.Command, exportTarget *Exporter, fields []string) { } return e }) + + if len(fields) == 0 { + return + } + + if cmd.Annotations == nil { + cmd.Annotations = map[string]string{} + } + cmd.Annotations["help:json-fields"] = strings.Join(fields, ",") } func checkJSONFlags(cmd *cobra.Command) (*jsonExporter, error) { From a3ff7791dbcdd8ae464f1d5cd63a50f544fb4a7a Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 17:55:19 +0100 Subject: [PATCH 03/10] Add tests to verify proper `help:json-fields` annotations Signed-off-by: Babak K. Shandiz --- pkg/cmdutil/json_flags_test.go | 49 ++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/pkg/cmdutil/json_flags_test.go b/pkg/cmdutil/json_flags_test.go index 16dd87900..63c63aa00 100644 --- a/pkg/cmdutil/json_flags_test.go +++ b/pkg/cmdutil/json_flags_test.go @@ -119,6 +119,55 @@ func TestAddJSONFlags(t *testing.T) { } } +// TestAddJSONFlagsSetsAnnotations asserts that `AddJSONFlags` function adds the +// appropriate annotation to the command, which could later be used by doc +// generator functions. +func TestAddJSONFlagsSetsAnnotations(t *testing.T) { + tests := []struct { + name string + cmd *cobra.Command + jsonFields []string + expectedAnnotations map[string]string + }{ + { + name: "empty set of fields", + cmd: &cobra.Command{}, + jsonFields: []string{}, + expectedAnnotations: nil, + }, + { + name: "empty set of fields, with existing annotations", + cmd: &cobra.Command{Annotations: map[string]string{"foo": "bar"}}, + jsonFields: []string{}, + expectedAnnotations: map[string]string{"foo": "bar"}, + }, + { + name: "no other annotations", + cmd: &cobra.Command{}, + jsonFields: []string{"few", "json", "fields"}, + expectedAnnotations: map[string]string{ + "help:json-fields": "few,json,fields", + }, + }, + { + name: "with existing annotations (ensure no overwrite)", + cmd: &cobra.Command{Annotations: map[string]string{"foo": "bar"}}, + jsonFields: []string{"few", "json", "fields"}, + expectedAnnotations: map[string]string{ + "foo": "bar", + "help:json-fields": "few,json,fields", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + AddJSONFlags(tt.cmd, nil, tt.jsonFields) + assert.Equal(t, tt.expectedAnnotations, tt.cmd.Annotations) + }) + } +} + func TestAddFormatFlags(t *testing.T) { tests := []struct { name string From 9f9d0415da31582a979b195f7f8920a582788d51 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 17:55:46 +0100 Subject: [PATCH 04/10] Output `JSON FIELDS` Signed-off-by: Babak K. Shandiz --- pkg/cmd/root/help.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index 1586445e1..7180a3152 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -166,6 +166,10 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, args []string) { if inheritedFlagUsages != "" { helpEntries = append(helpEntries, helpEntry{"INHERITED FLAGS", dedent(inheritedFlagUsages)}) } + if _, ok := command.Annotations["help:json-fields"]; ok { + fields := strings.Split(command.Annotations["help:json-fields"], ",") + helpEntries = append(helpEntries, helpEntry{"JSON FIELDS", text.FormatSlice(fields, 80, 0, "", "", true)}) + } if _, ok := command.Annotations["help:arguments"]; ok { helpEntries = append(helpEntries, helpEntry{"ARGUMENTS", command.Annotations["help:arguments"]}) } From 2b304a5e84a09b527f72078eb5a13411513a3037 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 17:56:37 +0100 Subject: [PATCH 05/10] Output `JSON Fields` section Signed-off-by: Babak K. Shandiz --- internal/docs/man.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/docs/man.go b/internal/docs/man.go index 1643fdd98..41d3371db 100644 --- a/internal/docs/man.go +++ b/internal/docs/man.go @@ -10,6 +10,7 @@ import ( "strings" "time" + "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/root" "github.com/cpuguy83/go-md2man/v2/md2man" "github.com/spf13/cobra" @@ -178,6 +179,17 @@ func manPrintOptions(buf *bytes.Buffer, command *cobra.Command) { } } +func manPrintJSONFields(buf *bytes.Buffer, command *cobra.Command) { + raw, ok := command.Annotations["help:json-fields"] + if !ok { + return + } + + buf.WriteString("# JSON FIELDS\n") + buf.WriteString(text.FormatSlice(strings.Split(raw, ","), 0, 0, "`", "`", true)) + buf.WriteString("\n") +} + func genMan(cmd *cobra.Command, header *GenManHeader) []byte { cmd.InitDefaultHelpCmd() cmd.InitDefaultHelpFlag() @@ -195,6 +207,7 @@ func genMan(cmd *cobra.Command, header *GenManHeader) []byte { } } manPrintOptions(buf, cmd) + manPrintJSONFields(buf, cmd) if len(cmd.Example) > 0 { buf.WriteString("# EXAMPLE\n") buf.WriteString(fmt.Sprintf("```\n%s\n```\n", cmd.Example)) From b445adae20c66dd6dddf90c4fa768a0a14f4597b Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 17:57:12 +0100 Subject: [PATCH 06/10] Add `jsonCmd` test command variable Signed-off-by: Babak K. Shandiz --- internal/docs/docs_test.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/internal/docs/docs_test.go b/internal/docs/docs_test.go index ad1b32631..06e924224 100644 --- a/internal/docs/docs_test.go +++ b/internal/docs/docs_test.go @@ -26,6 +26,8 @@ func init() { printCmd.Flags().IntP("intthree", "i", 345, "help message for flag intthree") printCmd.Flags().BoolP("boolthree", "b", true, "help message for flag boolthree") + jsonCmd.Flags().StringSlice("json", nil, "help message for flag json") + echoCmd.AddCommand(timesCmd, echoSubCmd, deprecatedCmd) rootCmd.AddCommand(printCmd, echoCmd, dummyCmd) } @@ -73,6 +75,14 @@ var printCmd = &cobra.Command{ Long: `an absolutely utterly useless command for testing.`, } +var jsonCmd = &cobra.Command{ + Use: "blah --json ", + Short: "View details in JSON", + Annotations: map[string]string{ + "help:json-fields": "foo,bar,baz", + }, +} + var dummyCmd = &cobra.Command{ Use: "dummy [action]", Short: "Performs a dummy action", From 5b99a4a2ece135a97e73f2dc7648932cc862b53f Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 17:57:50 +0100 Subject: [PATCH 07/10] Add test to verify JSON fields section is rendered Signed-off-by: Babak K. Shandiz --- internal/docs/man_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/docs/man_test.go b/internal/docs/man_test.go index fb595d17d..287f356ed 100644 --- a/internal/docs/man_test.go +++ b/internal/docs/man_test.go @@ -98,6 +98,22 @@ func TestGenManSeeAlso(t *testing.T) { } } +func TestGenManJSONFields(t *testing.T) { + buf := new(bytes.Buffer) + header := &GenManHeader{} + if err := renderMan(jsonCmd, header, buf); err != nil { + t.Fatal(err) + } + + output := buf.String() + + checkStringContains(t, output, translate(jsonCmd.Name())) + checkStringContains(t, output, "JSON FIELDS") + checkStringContains(t, output, "foo") + checkStringContains(t, output, "bar") + checkStringContains(t, output, "baz") +} + func TestManPrintFlagsHidesShortDeprecated(t *testing.T) { c := &cobra.Command{} c.Flags().StringP("foo", "f", "default", "Foo flag") From ad71f8cf9f62ddd095728d64dccfabacfc3bf4b4 Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 18:00:54 +0100 Subject: [PATCH 08/10] Output `JSON Fields` section in Markdown Signed-off-by: Babak K. Shandiz --- internal/docs/markdown.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/internal/docs/markdown.go b/internal/docs/markdown.go index 8a193d0b6..19899110d 100644 --- a/internal/docs/markdown.go +++ b/internal/docs/markdown.go @@ -8,12 +8,24 @@ import ( "path/filepath" "strings" + "github.com/cli/cli/v2/internal/text" "github.com/cli/cli/v2/pkg/cmd/root" "github.com/spf13/cobra" "github.com/spf13/cobra/doc" "github.com/spf13/pflag" ) +func printJSONFields(w io.Writer, cmd *cobra.Command) { + raw, ok := cmd.Annotations["help:json-fields"] + if !ok { + return + } + + fmt.Fprint(w, "### JSON Fields\n\n") + fmt.Fprint(w, text.FormatSlice(strings.Split(raw, ","), 0, 0, "`", "`", true)) + fmt.Fprint(w, "\n\n") +} + func printOptions(w io.Writer, cmd *cobra.Command) error { flags := cmd.NonInheritedFlags() flags.SetOutput(w) @@ -135,6 +147,7 @@ func genMarkdownCustom(cmd *cobra.Command, w io.Writer, linkHandler func(string) if err := printOptions(w, cmd); err != nil { return err } + printJSONFields(w, cmd) fmt.Fprint(w, "{% endraw %}\n") if len(cmd.Example) > 0 { From 05a87b966452e858087809e67805d4194097ce2e Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 18:01:12 +0100 Subject: [PATCH 09/10] Add test to verify JSON fields section is rendered in Markdown Signed-off-by: Babak K. Shandiz --- internal/docs/markdown_test.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/docs/markdown_test.go b/internal/docs/markdown_test.go index 0bf65ddf3..0710cad71 100644 --- a/internal/docs/markdown_test.go +++ b/internal/docs/markdown_test.go @@ -70,6 +70,21 @@ func TestGenMdNoHiddenParents(t *testing.T) { checkStringOmits(t, output, "Options inherited from parent commands") } +func TestGenMdJSONFields(t *testing.T) { + buf := new(bytes.Buffer) + if err := genMarkdownCustom(jsonCmd, buf, nil); err != nil { + t.Fatal(err) + } + output := buf.String() + + checkStringContains(t, output, jsonCmd.Long) + checkStringContains(t, output, jsonCmd.Example) + checkStringContains(t, output, "JSON Fields") + checkStringContains(t, output, "`foo`") + checkStringContains(t, output, "`bar`") + checkStringContains(t, output, "`baz`") +} + func TestGenMdTree(t *testing.T) { c := &cobra.Command{Use: "do [OPTIONS] arg1 arg2"} tmpdir, err := os.MkdirTemp("", "test-gen-md-tree") From 4ac966f030c7c33f87f0899b5c8809a912c2d45a Mon Sep 17 00:00:00 2001 From: "Babak K. Shandiz" Date: Mon, 6 May 2024 18:35:54 +0100 Subject: [PATCH 10/10] Fix test function name Signed-off-by: Babak K. Shandiz --- internal/text/text_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/text/text_test.go b/internal/text/text_test.go index 6d76d4872..46566bbf8 100644 --- a/internal/text/text_test.go +++ b/internal/text/text_test.go @@ -55,7 +55,7 @@ func TestFuzzyAgoAbbr(t *testing.T) { } } -func TestFormatSliceDoc(t *testing.T) { +func TestFormatSlice(t *testing.T) { tests := []struct { name string values []string