Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve run list doc with available --json fields #8934

Merged
merged 10 commits into from
May 8, 2024
10 changes: 10 additions & 0 deletions internal/docs/docs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -73,6 +75,14 @@ var printCmd = &cobra.Command{
Long: `an absolutely utterly useless command for testing.`,
}

var jsonCmd = &cobra.Command{
Use: "blah --json <fields>",
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",
Expand Down
13 changes: 13 additions & 0 deletions internal/docs/man.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand All @@ -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))
Expand Down
16 changes: 16 additions & 0 deletions internal/docs/man_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
13 changes: 13 additions & 0 deletions internal/docs/markdown.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
15 changes: 15 additions & 0 deletions internal/docs/markdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
63 changes: 63 additions & 0 deletions internal/text/text.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package text

import (
"fmt"
"math"
"net/url"
"regexp"
"slices"
"strings"
"time"

Expand Down Expand Up @@ -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()
}
92 changes: 92 additions & 0 deletions internal/text/text_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,95 @@ func TestFuzzyAgoAbbr(t *testing.T) {
assert.Equal(t, expected, fuzzy)
}
}

func TestFormatSlice(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: "<foo>, <bar>,\n<baz>, <foo>,\n<bar>, <baz>",
},
{
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))
})
}
}
4 changes: 4 additions & 0 deletions pkg/cmd/root/help.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"]})
}
Comment on lines +169 to 175
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realizing I don't know many commands where ARGUMENTS section would show up to say definitively if JSON fields should come before or after 🤔 ... my gut says JSON fields after arguments but will keep looking for an example as not the end of the world

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that arguments should be printed before the JSON fields, since they're more like an appendix section. Since this is already merged, I can submit another PR for that if you say so.

Expand Down
9 changes: 9 additions & 0 deletions pkg/cmdutil/json_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
49 changes: 49 additions & 0 deletions pkg/cmdutil/json_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down