cli: public commands API

internals/cli/cli.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"os/user"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"unicode"
 	"unicode/utf8"
@@ -49,70 +50,62 @@ 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
+
+	// When set, the command will be a subcommand of the `debug` command.
+	// Do not set this flag manually, use AddDebugCommand instead.
+	debug bool
 }
 
-// commands holds information about all non-debug commands.
-var commands []*cmdInfo
-
-// debugCommands holds information about all debug commands.
-var debugCommands []*cmdInfo
-
-// 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,
-	}
+// commands holds information about all the regular Pebble commands.
+var commands []*CmdInfo
+
+// debugCommands holds information about all the subcommands of the `pebble debug` command.
+var debugCommands []*CmdInfo
+
+// AddCommand adds a command to the top-level parser.
+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,
-	}
+// AddDebugCommand adds a subcommand to the `debug` command.
+func AddDebugCommand(info *CmdInfo) {
+	info.debug = true
 	debugCommands = append(debugCommands, info)
-	return info
-}
-
-type parserSetter interface {
-	setParser(*flags.Parser)
 }
 
 func lintDesc(cmdName, optName, desc, origDesc string) {
@@ -169,134 +162,108 @@ func (ch *clientMixin) setClient(cli *client.Client) {
 	ch.client = cli
 }
 
+type parserSetter interface {
+	setParser(*flags.Parser)
+}
+
+type defaultOptions struct {
+	Version func() `long:"version" hidden:"yes" description:"Print the version and exit"`
+}
+
 // 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
+	defaultOpts := defaultOptions{
+		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(&defaultOpts, flagOpts)
 	parser.ShortDescription = "Tool to interact with pebble"
 	parser.LongDescription = longPebbleDescription
-	// hide the unhelpful "[OPTIONS]" from help output
-	parser.Usage = ""
-	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()
+	// Regular expressions for positional and flag arguments
+	positionalRegexp := regexp.MustCompile(`^<[\w-]+>$`)
+	flagRegexp := regexp.MustCompile(`^-(\w|-[\w]+(-?[\w]+)*)$`)
+
+	// Create debug command
+	debugCmd, err := parser.AddCommand("debug", cmdDebugSummary, cmdDebugDescription, &cmdDebug{})
+	debugCmd.Hidden = true
+	if err != nil {
+		logger.Panicf("cannot add command %q: %v", "debug", err)
+	}
+
+	// Add all commands
+	for _, c := range append(commands, debugCommands...) {
+		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)
-		if err != nil {
-			logger.Panicf("cannot add command %q: %v", c.name, err)
-		}
-		cmd.Hidden = c.hidden
-		if c.alias != "" {
-			cmd.Aliases = append(cmd.Aliases, c.alias)
+		// Target either the `debug` command or the top-level parser depending on this command
+		// being or not marked as debug.
+		var target *flags.Command
+		if c.debug {
+			target = debugCmd
+		} else {
+			target = parser.Command
 		}
 
-		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))
+		cmd, err := target.AddCommand(c.Name, c.Summary, strings.TrimSpace(c.Description), obj)
+		if err != nil {
+			logger.Panicf("cannot add command %q: %v", c.Name, err)
 		}
-		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
+		cmd.PassAfterNonOption = c.PassAfterNonOption
+
+		// Extract help for flags and positional arguments from ArgsHelp
+		flagHelp := map[string]string{}
+		positionalHelp := map[string]string{}
+
+		for specifier, help := range c.ArgsHelp {
+			if flagRegexp.MatchString(specifier) {
+				flagHelp[specifier] = help
+			} else if positionalRegexp.MatchString(specifier) {
+				positionalHelp[specifier] = help
+			} else {
+				logger.Panicf("invalid help specifier from %q: %q", c.Name, specifier)
 			}
 		}
 
-		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
+		// Make sure all 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))
+		if len(opts) != len(flagHelp) {
+			logger.Panicf("wrong number of flag descriptions for %q: expected %d, got %d", c.Name, len(opts), len(flagHelp))
 		}
 		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 := flagHelp["--"+opt.LongName]; ok {
+				lintDesc(c.Name, opt.LongName, description, opt.Description)
+				opt.Description = description
+			} else if description, ok := flagHelp["-"+string(opt.ShortName)]; ok {
+				lintDesc(c.Name, string(opt.ShortName), description, opt.Description)
+				opt.Description = description
+			} else if !opt.Hidden {
+				logger.Panicf("%q missing description for %q", 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 := positionalHelp[arg.Name]; ok {
+				lintArg(c.Name, arg.Name, description, arg.Description)
+				arg.Name = fixupArg(arg.Name)
+				arg.Description = description
 			}
-			lintArg(c.name, name, desc, arg.Description)
-			name = fixupArg(name)
-			arg.Name = name
-			arg.Description = desc
 		}
 	}
 	return parser
@@ -362,7 +329,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,10 +19,17 @@ import (
 	"io/ioutil"
 
 	"github.com/canonical/go-flags"
-
 	"github.com/canonical/pebble/client"
 )
 
+const cmdAddSummary = "Dynamically add a layer to the plan's layers"
+const cmdAddDescription = `
+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).
+`
+
 type cmdAdd struct {
 	clientMixin
 	Combine    bool `long:"combine"`
@@ -32,18 +39,18 @@ 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)`,
+func init() {
+	AddCommand(&CmdInfo{
+		Name:        "add",
+		Summary:     cmdAddSummary,
+		Description: cmdAddDescription,
+		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{} },
+	})
 }
 
-var shortAddHelp = "Dynamically add a layer to the plan's layers"
-var longAddHelp = `
-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).
-`
-
 func (cmd *cmdAdd) Execute(args []string) error {
 	if len(args) > 0 {
 		return ErrExtraArgs
@@ -65,7 +72,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)
-}