cli: public commands API

internals/cli/cli.go

@@ -49,70 +49,50 @@ var (
 // the pebble client.
 const defaultPebbleDir = "/var/lib/pebble/default"
 
-type options struct {
-	Version func() `long:"version"`
-}
+// ErrExtraArgs is returned  if extra arguments to a command are found
+var ErrExtraArgs = fmt.Errorf("too many arguments for command")
 
-type argDesc struct {
-	name string
-	desc string
-}
+// CmdInfo holds information needed by the CLI to execute commands and
+// populate entries in the help manual.
+type CmdInfo struct {
+	// Name of the command
+	Name string
 
-var optionsData options
+	// Summary is a single-line help string that will be displayed
+	// in the full Pebble help manual (i.e. help --all)
+	Summary string
 
-// ErrExtraArgs is returned  if extra arguments to a command are found
-var ErrExtraArgs = fmt.Errorf("too many arguments for command")
+	// Description contains exhaustive documentation about the command,
+	// that will be reflected in the specific help manual for the
+	// command, and in the Pebble man page.
+	Description string
+
+	// Builder is a function that creates a new instance of the command
+	// struct containing an Execute(args []string) implementation.
+	Builder func() flags.Commander
 
-// cmdInfo holds information needed to call parser.AddCommand(...).
-type cmdInfo struct {
-	name, shortHelp, longHelp string
-	builder                   func() flags.Commander
-	hidden                    bool
-	optDescs                  map[string]string
-	argDescs                  []argDesc
-	alias                     string
-	extra                     func(*flags.Command)
+	// ArgsHelp (optional) contains help about the command-line arguments
+	// (including options) supported by the command.
+	//
+	//  map[string]string{
+	//      "--long-option": "my very long option",
+	//      "-v": "verbose output",
+	//      "<change-id>": "named positional argument"
+	//  }
+	ArgsHelp map[string]string
+
+	// Whether to pass all arguments after the first non-option as remaining
+	// command line arguments. This is equivalent to strict POSIX processing.
+	PassAfterNonOption bool
 }
 
 // commands holds information about all non-debug commands.
-var commands []*cmdInfo
-
-// debugCommands holds information about all debug commands.
-var debugCommands []*cmdInfo
+var commands []*CmdInfo
 
-// addCommand replaces parser.addCommand() in a way that is compatible with
+// AddCommand replaces parser.addCommand() in a way that is compatible with
 // re-constructing a pristine parser.
-func addCommand(name, shortHelp, longHelp string, builder func() flags.Commander, optDescs map[string]string, argDescs []argDesc) *cmdInfo {
-	info := &cmdInfo{
-		name:      name,
-		shortHelp: shortHelp,
-		longHelp:  longHelp,
-		builder:   builder,
-		optDescs:  optDescs,
-		argDescs:  argDescs,
-	}
+func AddCommand(info *CmdInfo) {
 	commands = append(commands, info)
-	return info
-}
-
-// addDebugCommand replaces parser.addCommand() in a way that is
-// compatible with re-constructing a pristine parser. It is meant for
-// adding debug commands.
-func addDebugCommand(name, shortHelp, longHelp string, builder func() flags.Commander, optDescs map[string]string, argDescs []argDesc) *cmdInfo {
-	info := &cmdInfo{
-		name:      name,
-		shortHelp: shortHelp,
-		longHelp:  longHelp,
-		builder:   builder,
-		optDescs:  optDescs,
-		argDescs:  argDescs,
-	}
-	debugCommands = append(debugCommands, info)
-	return info
-}
-
-type parserSetter interface {
-	setParser(*flags.Parser)
 }
 
 func lintDesc(cmdName, optName, desc, origDesc string) {
@@ -157,148 +137,120 @@ func fixupArg(optName string) string {
 	return optName
 }
 
-type clientSetter interface {
-	setClient(*client.Client)
-}
-
 type clientMixin struct {
 	client *client.Client
 }
 
+type clientSetter interface {
+	setClient(*client.Client)
+}
+
 func (ch *clientMixin) setClient(cli *client.Client) {
 	ch.client = cli
 }
 
+type parserSetter interface {
+	setParser(*flags.Parser)
+}
+
+type options struct {
+	Version func() `long:"version"`
+}
+
 // Parser creates and populates a fresh parser.
 // Since commands have local state a fresh parser is required to isolate tests
 // from each other.
 func Parser(cli *client.Client) *flags.Parser {
-	optionsData.Version = func() {
-		printVersions(cli)
-		panic(&exitStatus{0})
+	// Implement --version by default on every command
+	defaultOptions := options{
+		Version: func() {
+			printVersions(cli)
+			panic(&exitStatus{0})
+		},
 	}
-	flagopts := flags.Options(flags.PassDoubleDash)
-	parser := flags.NewParser(&optionsData, flagopts)
+
+	flagOpts := flags.Options(flags.PassDoubleDash)
+	parser := flags.NewParser(&defaultOptions, flagOpts)
 	parser.ShortDescription = "Tool to interact with pebble"
 	parser.LongDescription = longPebbleDescription
-	// hide the unhelpful "[OPTIONS]" from help output
-	parser.Usage = ""
+
+	// Hide the global --version option on every command
 	if version := parser.FindOptionByLongName("version"); version != nil {
 		version.Description = "Print the version and exit"
 		version.Hidden = true
 	}
-	// add --help like what go-flags would do for us, but hidden
+
+	// Add --help like what go-flags would do for us, but hidden
 	addHelp(parser)
 
 	// Add all regular commands
 	for _, c := range commands {
-		obj := c.builder()
+		obj := c.Builder()
 		if x, ok := obj.(clientSetter); ok {
 			x.setClient(cli)
 		}
 		if x, ok := obj.(parserSetter); ok {
 			x.setParser(parser)
 		}
 
-		cmd, err := parser.AddCommand(c.name, c.shortHelp, strings.TrimSpace(c.longHelp), obj)
+		cmd, err := parser.AddCommand(c.Name, c.Summary, strings.TrimSpace(c.Description), obj)
 		if err != nil {
-			logger.Panicf("cannot add command %q: %v", c.name, err)
+			logger.Panicf("cannot add command %q: %v", c.Name, err)
 		}
-		cmd.Hidden = c.hidden
-		if c.alias != "" {
-			cmd.Aliases = append(cmd.Aliases, c.alias)
+		cmd.PassAfterNonOption = c.PassAfterNonOption
+
+		optionsHelp := map[string]string{}
+		positionalArgsHelp := map[string]string{}
+
+		for specifier, help := range c.ArgsHelp {
+			isLongOption := strings.HasPrefix(specifier, "--")
+			isShortOption := utf8.RuneCountInString(specifier) == 2 && strings.HasPrefix(specifier, "-")
+			if isShortOption || isLongOption {
+				optionsHelp[specifier] = help
+			} else if strings.HasPrefix(specifier, "<") && strings.HasSuffix(specifier, ">") {
+				// This is a positional argument
+				positionalArgsHelp[specifier] = help
+			} else {
+				logger.Panicf("invalid help specifier: %#v %#v", c.Name, strings.HasPrefix(specifier, "-"))
+			}
 		}
 
+		hasAnyOptionHelp := len(optionsHelp) > 0
+		hasAnyPositionalHelp := len(positionalArgsHelp) > 0
+
+		// Check either all or none opts/positional argument descriptions are set
 		opts := cmd.Options()
-		if c.optDescs != nil && len(opts) != len(c.optDescs) {
-			logger.Panicf("wrong number of option descriptions for %s: expected %d, got %d", c.name, len(opts), len(c.optDescs))
-		}
-		for _, opt := range opts {
-			name := opt.LongName
-			if name == "" {
-				name = string(opt.ShortName)
-			}
-			desc, ok := c.optDescs[name]
-			if !(c.optDescs == nil || ok) {
-				logger.Panicf("%s missing description for %s", c.name, name)
-			}
-			lintDesc(c.name, name, desc, opt.Description)
-			if desc != "" {
-				opt.Description = desc
-			}
+		if hasAnyOptionHelp && len(opts) != len(optionsHelp) {
+			logger.Panicf("wrong number of option descriptions for %s: expected %d, got %d", c.Name, len(opts), len(optionsHelp))
 		}
-
 		args := cmd.Args()
-		if c.argDescs != nil && len(args) != len(c.argDescs) {
-			logger.Panicf("wrong number of argument descriptions for %s: expected %d, got %d", c.name, len(args), len(c.argDescs))
-		}
-		for i, arg := range args {
-			name, desc := arg.Name, ""
-			if c.argDescs != nil {
-				name = c.argDescs[i].name
-				desc = c.argDescs[i].desc
-			}
-			lintArg(c.name, name, desc, arg.Description)
-			name = fixupArg(name)
-			arg.Name = name
-			arg.Description = desc
-		}
-		if c.extra != nil {
-			c.extra(cmd)
-		}
-	}
-	// Add the debug command
-	debugCommand, err := parser.AddCommand("debug", shortDebugHelp, longDebugHelp, &cmdDebug{})
-	debugCommand.Hidden = true
-	if err != nil {
-		logger.Panicf("cannot add command %q: %v", "debug", err)
-	}
-	// Add all the sub-commands of the debug command
-	for _, c := range debugCommands {
-		obj := c.builder()
-		if x, ok := obj.(clientSetter); ok {
-			x.setClient(cli)
-		}
-		cmd, err := debugCommand.AddCommand(c.name, c.shortHelp, strings.TrimSpace(c.longHelp), obj)
-		if err != nil {
-			logger.Panicf("cannot add debug command %q: %v", c.name, err)
-		}
-		cmd.Hidden = c.hidden
-		opts := cmd.Options()
-		if c.optDescs != nil && len(opts) != len(c.optDescs) {
-			logger.Panicf("wrong number of option descriptions for %s: expected %d, got %d", c.name, len(opts), len(c.optDescs))
+		if hasAnyPositionalHelp && len(args) != len(positionalArgsHelp) {
+			logger.Panicf("wrong number of argument descriptions for %s: expected %d, got %d", c.Name, len(args), len(positionalArgsHelp))
 		}
+
 		for _, opt := range opts {
-			name := opt.LongName
-			if name == "" {
-				name = string(opt.ShortName)
-			}
-			desc, ok := c.optDescs[name]
-			if !(c.optDescs == nil || ok) {
-				logger.Panicf("%s missing description for %s", c.name, name)
-			}
-			lintDesc(c.name, name, desc, opt.Description)
-			if desc != "" {
-				opt.Description = desc
+			if description, ok := optionsHelp["--"+opt.LongName]; ok {
+				lintDesc(c.Name, opt.LongName, description, opt.Description)
+				opt.Description = description
+			} else if description, ok := optionsHelp["-"+string(opt.ShortName)]; ok {
+				lintDesc(c.Name, string(opt.ShortName), description, opt.Description)
+				opt.Description = description
+			} else if hasAnyOptionHelp {
+				logger.Panicf("%s missing description for %s", c.Name, opt)
 			}
 		}
 
-		args := cmd.Args()
-		if c.argDescs != nil && len(args) != len(c.argDescs) {
-			logger.Panicf("wrong number of argument descriptions for %s: expected %d, got %d", c.name, len(args), len(c.argDescs))
-		}
-		for i, arg := range args {
-			name, desc := arg.Name, ""
-			if c.argDescs != nil {
-				name = c.argDescs[i].name
-				desc = c.argDescs[i].desc
+		for _, arg := range args {
+			if description, ok := positionalArgsHelp[arg.Name]; ok {
+				lintArg(c.Name, arg.Name, description, arg.Description)
+				arg.Name = fixupArg(arg.Name)
+				arg.Description = description
+			} else if hasAnyPositionalHelp {
+				logger.Panicf("%s missing description for %s", c.Name, arg.Name)
 			}
-			lintArg(c.name, name, desc, arg.Description)
-			name = fixupArg(name)
-			arg.Name = name
-			arg.Description = desc
 		}
 	}
+
 	return parser
 }
 
@@ -362,7 +314,7 @@ func Run() error {
 						sug = "pebble help " + x.Name
 					}
 				}
-				return fmt.Errorf("unknown command %q, see '%s'.", sub, sug)
+				return fmt.Errorf("unknown command %q, see '%s'", sub, sug)
 			}
 		}
 

internals/cli/cmd_add.go

@@ -19,7 +19,6 @@ import (
 	"io/ioutil"
 
 	"github.com/canonical/go-flags"
-
 	"github.com/canonical/pebble/client"
 )
 
@@ -32,17 +31,22 @@ type cmdAdd struct {
 	} `positional-args:"yes"`
 }
 
-var addDescs = map[string]string{
-	"combine": `Combine the new layer with an existing layer that has the given label (default is to append)`,
-}
-
-var shortAddHelp = "Dynamically add a layer to the plan's layers"
-var longAddHelp = `
+func init() {
+	AddCommand(&CmdInfo{
+		Name:    "add",
+		Summary: "Dynamically add a layer to the plan's layers",
+		Description: `
 The add command reads the plan's layer YAML from the path specified and
 appends a layer with the given label to the plan's layers. If --combine
 is specified, combine the layer with an existing layer that has the given
 label (or append if the label is not found).
-`
+`,
+		ArgsHelp: map[string]string{
+			"--combine": "Combine the new layer with an existing layer that has the given label (default is to append)",
+		},
+		Builder: func() flags.Commander { return &cmdAdd{} },
+	})
+}
 
 func (cmd *cmdAdd) Execute(args []string) error {
 	if len(args) > 0 {
@@ -65,7 +69,3 @@ func (cmd *cmdAdd) Execute(args []string) error {
 		cmd.Positional.Label, cmd.Positional.LayerPath)
 	return nil
 }
-
-func init() {
-	addCommand("add", shortAddHelp, longAddHelp, func() flags.Commander { return &cmdAdd{} }, addDescs, nil)
-}