Massive refactor to improve help formatting

coder · ammario · Mar 7, 2023 · Feb 25, 2023 · Feb 25, 2023 · Feb 25, 2023
commit 000466f57c93212394c575421d8706b5a00b0c0f
diff --git a/cli/bigcli/bigcli.go b/cli/bigcli/bigcli.go
@@ -6,3 +6,77 @@
 // cobra/viper. It's also a candidate to be broken out into its own open-source
 // library, so we avoid deep coupling with Coder concepts.
 package bigcli
+
+import (
+	"strings"
+
+	"golang.org/x/exp/maps"
+)
+
+// Group describes a hierarchy of groups that an option or command belongs to.
+type Group struct {
+	Parent      *Group
+	Name        string
+	Children    []Group
+	Description string
+}
+
+func (g *Group) AddChild(child Group) {
+	child.Parent = g
+	g.Children = append(g.Children, child)
+}
+
+// Ancestry returns the group and all of its parents, in order.
+func (g *Group) Ancestry() []Group {
+	groups := []Group{*g}
+	for p := g.Parent; p != nil; p = p.Parent {
+		// Prepend to the slice so that the order is correct.
+		groups = append([]Group{*p}, groups...)
+	}
+	return groups
+}
+
+func (g *Group) FullName() string {
+	var names []string
+	for _, g := range g.Ancestry() {
+		names = append(names, g.Name)
+	}
+	return strings.Join(names, " / ")
+}
+
+// Annotations is an arbitrary key-mapping used to extend the Option and Command types.
+// Its methods won't panic if the map is nil.
+type Annotations map[string]string
+
+// Mark sets a value on the attonations map, creating one
+// if it doesn't exist. Mark does not mutate the original and
+// returns a copy. It is suitable for chaining.
+func (a Annotations) Mark(key string, value string) Annotations {
+	var aa Annotations
+	if a != nil {
+		aa = maps.Clone(a)
+	} else {
+		aa = make(Annotations)
+	}
+	aa[key] = value
+	return aa
+}
+
+// IsSet returns true if the key is set in the annotations map.
+func (a Annotations) IsSet(key string) bool {
+	if a == nil {
+		return false
+	}
+	_, ok := a[key]
+	return ok
+}
+
+// Get retrieves a key from the map, returning false if the key is not found
+// or the map is nil.
+func (a Annotations) Get(key string) (string, bool) {
+	if a == nil {
+		return "", false
+	}
+	v, ok := a[key]
+	return v, ok
+}
diff --git a/cli/bigcli/command.go b/cli/bigcli/command.go
@@ -4,9 +4,8 @@ import "strings"
 
 // Command describes an executable command.
 type Command struct {
-	// Parents is a list of parent commands, with
-	// the root command at index 0.
-	Parents []*Command
+	// Parent is the direct parent of the command.
+	Parent *Command
 	// Children is a list of direct descendants.
 	Children []*Command
 	// Use is provided in form "command [flags] [args...]".
@@ -15,8 +14,9 @@ type Command struct {
 	Short string
 	// Long is a detailed description of the command,
 	// presented on its help page. It may contain examples.
-	Long    string
-	Options OptionSet
+	Long        string
+	Options     OptionSet
+	Annotations Annotations
 }
 
 // Name returns the first word in the Use string.
@@ -28,8 +28,9 @@ func (c *Command) Name() string {
 // as seen on the command line.
 func (c *Command) FullName() string {
 	var names []string
-	for _, p := range c.Parents {
-		names = append(names, p.Name())
+
+	if c.Parent != nil {
+		names = append(names, c.Parent.FullName())
 	}
 	names = append(names, c.Name())
 	return strings.Join(names, " ")
@@ -39,18 +40,9 @@ func (c *Command) FullName() string {
 // by the usage of its parents.
 func (c *Command) FullUsage() string {
 	var uses []string
-	for _, p := range c.Parents {
-		uses = append(uses, p.Use)
+	if c.Parent != nil {
+		uses = append(uses, c.Parent.FullUsage())
 	}
 	uses = append(uses, c.Use)
 	return strings.Join(uses, " ")
 }
-
-// Parent returns the direct parent of the command,
-// or nil if there are no parents.
-func (c *Command) Parent() *Command {
-	if len(c.Parents) == 0 {
-		return nil
-	}
-	return c.Parents[len(c.Parents)-1]
-}
diff --git a/cli/bigcli/option.go b/cli/bigcli/option.go
@@ -7,51 +7,13 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/iancoleman/strcase"
 	"github.com/spf13/pflag"
-	"golang.org/x/exp/maps"
 	"golang.org/x/xerrors"
 )
 
 // Disable is a sentinel value for Option.Flag, Option.Env, and Option.YAML to disable
 // features.
 const Disable = "-"
 
-// Annotations is an arbitrary key-mapping used to extend the Option type.
-// Its methods won't panic if the map is nil.
-type Annotations map[string]string
-
-// Mark sets a value on the attonations map, creating one
-// if it doesn't exist. Mark does not mutate the original and
-// returns a copy. It is suitable for chaining.
-func (a Annotations) Mark(key string, value string) Annotations {
-	var aa Annotations
-	if a != nil {
-		aa = maps.Clone(a)
-	} else {
-		aa = make(Annotations)
-	}
-	aa[key] = value
-	return aa
-}
-
-// IsSet returns true if the key is set in the annotations map.
-func (a Annotations) IsSet(key string) bool {
-	if a == nil {
-		return false
-	}
-	_, ok := a[key]
-	return ok
-}
-
-// Get retrieves a key from the map, returning false if the key is not found
-// or the map is nil.
-func (a Annotations) Get(key string) (string, bool) {
-	if a == nil {
-		return "", false
-	}
-	v, ok := a[key]
-	return v, ok
-}
-
 // Option is a configuration option for a CLI application.
 type Option struct {
 	Name        string
@@ -81,7 +43,7 @@ type Option struct {
 
 	// Group is a group hierarchy that helps organize this option in help, configs
 	// and other documentation.
-	Group []string
+	Group Group
 
 	// UseInstead is a list of options that should be used instead of this one.
 	// The field is used to generate a deprecation warning.

diff --git a/cli/bigcli/yaml.go b/cli/bigcli/yaml.go
@@ -1,7 +1,8 @@
 package bigcli
 
 import (
-	"github.com/iancoleman/strcase"
+	"strings"
+
 	"github.com/mitchellh/go-wordwrap"
 	"gopkg.in/yaml.v3"
 )
@@ -58,8 +59,8 @@ func (s OptionSet) ToYAML() (*yaml.Node, error) {
 			Value: opt.Value.String(),
 		}
 		var group []string
-		for _, g := range opt.Group {
-			group = append(group, strcase.ToSnake(g))
+		for _, g := range opt.Group.Ancestry() {
+			group = append(group, strings.ToLower(g.Name))
 		}
 		parent := deepMapNode(&root, group)
 		parent.Content = append(

diff --git a/cli/bigcli/yaml_test.go b/cli/bigcli/yaml_test.go
@@ -20,7 +20,7 @@ func TestOption_ToYAML(t *testing.T) {
 			Value:       &workspaceName,
 			Default:     "billie",
 			Description: "The workspace's name",
-			Group:       []string{"Names"},
+			Group:       bigcli.Group{Name: "Names"},
 		},
 	}
 

diff --git a/cli/server.go b/cli/server.go
@@ -133,10 +133,8 @@ func Server(newAPI func(context.Context, *coderd.Options) (*coderd.API, io.Close
 			// These parents and children will be moved once we convert the
 			// rest of the `cli` package to bigcli.
 			flagSet.Usage = usageFn(cmd.ErrOrStderr(), &bigcli.Command{
-				Parents: []*bigcli.Command{
-					{
-						Use: "coder",
-					},
+				Parent: &bigcli.Command{
+					Use: "coder",
 				},
 				Children: []*bigcli.Command{
 					{

diff --git a/cli/usage.go b/cli/usage.go
@@ -23,44 +23,6 @@ type optionGroup struct {
 	Options     bigcli.OptionSet
 }
 
-var optionGroupDescriptions = map[string]string{
-	"Networking": `
-Configure TLS, the wildcard access URL, bind addresses, access URLs, etc.
-`,
-	"Networking / DERP": `
-Most Coder deployments never have to think about DERP because all connections
-between workspaces and users are peer-to-peer. However, when Coder cannot establish
-a peer to peer connection, Coder uses a distributed relay network backed by
-Tailscale and WireGuard.
-`,
-	"Networking / TLS": `
-Configure TLS / HTTPS for your Coder deployment. If you're running
-Coder behind a TLS-terminating reverse proxy or are accessing Coder over a
-secure link, you can safely ignore these settings. 
-`,
-	`Introspection`: `
-Configure logging, tracing, and metrics exporting.
-`,
-	`oAuth2`: `
-Configure login and user-provisioning with GitHub via oAuth2.
-`,
-	`OIDC`: `
-Configure login and user-provisioning with OIDC.
-`,
-	`Telemetry`: `
-Telemetry is critical to our ability to improve Coder. We strip all personal
-information before sending data to our servers. Please only disable telemetry
-when required by your organization's security policy.
-`,
-	`Provisioning`: `
-Tune the behavior of the provisioner, which is responsible for creating,
-updating, and deleting workspace resources.
-`,
-	`Config`: `
-Use a YAML configuration file when your server launch become unwieldy.
-`,
-}
-
 const envPrefix = "CODER_"
 
 var usageTemplate = template.Must(
@@ -102,7 +64,8 @@ var usageTemplate = template.Must(
 			"isDeprecated": func(opt bigcli.Option) bool {
 				return len(opt.UseInstead) > 0
 			},
-			"formatDescription": func(s string) string {
+			"formatGroupDescription": func(s string) string {
+				s = strings.ReplaceAll(s, "\n", "")
 				s = "\n" + s + "\n"
 				s = wordwrap.WrapString(s, 60)
 				return s
@@ -115,10 +78,8 @@ var usageTemplate = template.Must(
 				}}
 
 				enterpriseGroup := optionGroup{
-					Name: "Enterprise",
-					Description: `
-These options are only available in the Enterprise Edition.
-`,
+					Name:        "Enterprise",
+					Description: `These options are only available in the Enterprise Edition.`,
 				}
 
 				// Sort options lexicographically.
@@ -136,13 +97,13 @@ These options are only available in the Enterprise Edition.
 						enterpriseGroup.Options = append(enterpriseGroup.Options, opt)
 						continue
 					}
-					if len(opt.Group) == 0 {
+					if len(opt.Group.Ancestry()) == 0 {
 						// Just add option to default group.
 						groups[0].Options = append(groups[0].Options, opt)
 						continue
 					}
 
-					groupName := strings.Join(opt.Group, " / ")
+					groupName := opt.Group.FullName()
 
 					for i, foundGroup := range groups {
 						if foundGroup.Name != groupName {
@@ -154,7 +115,7 @@ These options are only available in the Enterprise Edition.
 
 					groups = append(groups, optionGroup{
 						Name:        groupName,
-						Description: optionGroupDescriptions[groupName],
+						Description: opt.Group.Description,
 						Options:     bigcli.OptionSet{opt},
 					})
 				}

diff --git a/cli/usage.tpl b/cli/usage.tpl
@@ -6,19 +6,18 @@ usage: {{.FullUsage}}
 
 {{- range $index, $group := optionGroups . }}
 {{ with $group.Name }} {{- print $group.Name " Options" | prettyHeader }} {{ else -}} {{ prettyHeader "Options"}}{{- end -}}
-{{- with $group.Description }}
 {{ " " }}
-{{- . -}}
+{{- with $group.Description }}
+{{ formatGroupDescription . -}}
 {{ end }}
     {{- range $index, $option := $group.Options }}
     {{- with flagName $option }}
     --{{- . -}} {{ end }} {{- with $option.FlagShorthand }}, -{{- . -}} {{ end }}
     {{- with envName $option }}, ${{ . }} {{ end }}
     {{- with $option.Default }} (default: {{.}}) {{ end }}
         {{- with $option.Description }}
-            {{- "" }}
-            {{- $desc := formatDescription $option.Description }}
-{{- indent $desc 2}}
+            {{- $desc := wordWrap $option.Description 60 }}
+{{ indent $desc 2}}
 {{- if isDeprecated $option }} DEPRECATED {{ end }}
         {{- end -}}
     {{- end }}