Skip to content

Commit

Permalink
Update usage and help to (almost) match the existing docker behaviour
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Nephin <[email protected]>
  • Loading branch information
dnephin committed May 31, 2016
1 parent 69264be commit 667dcb0
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 23 deletions.
11 changes: 11 additions & 0 deletions api/client/volume/cmd.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,27 @@
package volume

import (
"fmt"

"github.com/spf13/cobra"

"github.com/docker/docker/api/client"
"github.com/docker/docker/cli"
)

// NewVolumeCommand returns a cobra command for `volume` subcommands
func NewVolumeCommand(dockerCli *client.DockerCli) *cobra.Command {
cmd := &cobra.Command{
Use: "volume",
Short: "Manage Docker volumes",
// TODO: remove once cobra is patched to handle this
RunE: func(cmd *cobra.Command, args []string) error {
fmt.Fprintf(dockerCli.Err(), "\n%s", cmd.UsageString())
if len(args) > 0 {
return cli.StatusError{StatusCode: 1}
}
return nil
},
}
cmd.AddCommand(
newCreateCommand(dockerCli),
Expand Down
9 changes: 8 additions & 1 deletion api/client/volume/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"golang.org/x/net/context"

"github.com/docker/docker/api/client"
"github.com/docker/docker/cli"
"github.com/docker/docker/opts"
runconfigopts "github.com/docker/docker/runconfig/opts"
"github.com/docker/engine-api/types"
Expand All @@ -20,12 +21,18 @@ type createOptions struct {
}

func newCreateCommand(dockerCli *client.DockerCli) *cobra.Command {
var opts createOptions
opts := createOptions{
driverOpts: *opts.NewMapOpts(nil, nil),
}

cmd := &cobra.Command{
Use: "create",
Short: "Create a volume",
RunE: func(cmd *cobra.Command, args []string) error {
// TODO: remove once cobra is patched to handle this
if err := cli.AcceptsNoArgs(args, cmd); err != nil {
return err
}
return runCreate(dockerCli, opts)
},
}
Expand Down
5 changes: 5 additions & 0 deletions api/client/volume/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"golang.org/x/net/context"

"github.com/docker/docker/api/client"
"github.com/docker/docker/cli"
"github.com/docker/engine-api/types"
"github.com/docker/engine-api/types/filters"
"github.com/spf13/cobra"
Expand All @@ -34,6 +35,10 @@ func newListCommand(dockerCli *client.DockerCli) *cobra.Command {
Aliases: []string{"list"},
Short: "List volumes",
RunE: func(cmd *cobra.Command, args []string) error {
// TODO: remove once cobra is patched to handle this
if err := cli.AcceptsNoArgs(args, cmd); err != nil {
return err
}
return runList(dockerCli, opts)
},
}
Expand Down
28 changes: 18 additions & 10 deletions cli/cobraadaptor/adaptor.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,21 @@ type CobraAdaptor struct {

// NewCobraAdaptor returns a new handler
func NewCobraAdaptor(clientFlags *cliflags.ClientFlags) CobraAdaptor {
var rootCmd = &cobra.Command{
Use: "docker",
}
rootCmd.SetUsageTemplate(usageTemplate)

stdin, stdout, stderr := term.StdStreams()
dockerCli := client.NewDockerCli(stdin, stdout, stderr, clientFlags)

var rootCmd = &cobra.Command{
Use: "docker",
SilenceUsage: true,
SilenceErrors: true,
}
rootCmd.SetUsageTemplate(usageTemplate)
rootCmd.SetHelpTemplate(helpTemplate)
rootCmd.SetOutput(stdout)
rootCmd.AddCommand(
volume.NewVolumeCommand(dockerCli),
)

return CobraAdaptor{
rootCmd: rootCmd,
dockerCli: dockerCli,
Expand Down Expand Up @@ -64,20 +68,24 @@ func (c CobraAdaptor) Command(name string) func(...string) error {
return nil
}

var usageTemplate = `Usage: {{if .Runnable}}{{if .HasFlags}}{{appendIfNotPresent .UseLine "[OPTIONS]"}}{{else}}{{.UseLine}}{{end}}{{end}}{{if .HasSubCommands}}{{ .CommandPath}} COMMAND {{end}}{{if gt .Aliases 0}}
var usageTemplate = `Usage: {{if not .HasSubCommands}}{{if .HasLocalFlags}}{{appendIfNotPresent .UseLine "[OPTIONS]"}}{{else}}{{.UseLine}}{{end}}{{end}}{{if .HasSubCommands}}{{ .CommandPath}} COMMAND{{end}}
{{with or .Long .Short }}{{. | trim}}{{end}}{{if gt .Aliases 0}}
Aliases:
{{.NameAndAliases}}
{{end}}{{if .HasExample}}
{{.NameAndAliases}}{{end}}{{if .HasExample}}
Examples:
{{ .Example }}{{end}}{{ if .HasLocalFlags}}
{{ .Example }}{{end}}{{if .HasFlags}}
Options:
{{.LocalFlags.FlagUsages | trimRightSpace}}{{end}}{{ if .HasAvailableSubCommands}}
{{.Flags.FlagUsages | trimRightSpace}}{{end}}{{ if .HasAvailableSubCommands}}
Commands:{{range .Commands}}{{if .IsAvailableCommand}}
{{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{ if .HasSubCommands }}
Run '{{.CommandPath}} COMMAND --help' for more information on a command.{{end}}
`

var helpTemplate = `
{{if or .Runnable .HasSubCommands}}{{.UsageString}}{{end}}`
14 changes: 14 additions & 0 deletions cli/required.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,17 @@ func MinRequiredArgs(args []string, min int, cmd *cobra.Command) error {
cmd.Short,
)
}

// AcceptsNoArgs returns an error message if there are args
func AcceptsNoArgs(args []string, cmd *cobra.Command) error {
if len(args) == 0 {
return nil
}

return fmt.Errorf(
"\"%s\" accepts no argument(s).\n\nUsage: %s\n\n%s",
cmd.CommandPath(),
cmd.UseLine(),
cmd.Short,
)
}
1 change: 0 additions & 1 deletion cli/usage.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ var DockerCommandUsage = []Command{
{"unpause", "Unpause all processes within a container"},
{"update", "Update configuration of one or more containers"},
{"version", "Show the Docker version information"},
{"volume", "Manage Docker volumes"},
{"wait", "Block until a container stops, then print its exit code"},
}

Expand Down
9 changes: 5 additions & 4 deletions integration-cli/docker_cli_help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,8 @@ func testCommand(cmd string, newEnvs []string, scanForHome bool, home string) er
// If a line starts with 4 spaces then assume someone
// added a multi-line description for an option and we need
// to flag it
if strings.HasPrefix(line, " ") {
if strings.HasPrefix(line, " ") &&
!strings.HasPrefix(strings.TrimLeft(line, " "), "--") {
return fmt.Errorf("Help for %q should not have a multi-line option", cmd)
}

Expand All @@ -260,7 +261,7 @@ func testCommand(cmd string, newEnvs []string, scanForHome bool, home string) er

// Options should NOT end with a space
if strings.HasSuffix(line, " ") {
return fmt.Errorf("Help for %q should not end with a space", cmd)
return fmt.Errorf("Help for %q should not end with a space: %s", cmd, line)
}

}
Expand Down Expand Up @@ -326,8 +327,8 @@ func testCommand(cmd string, newEnvs []string, scanForHome bool, home string) er
return fmt.Errorf("Bad output from %q\nstdout:%q\nstderr:%q\nec:%d\nerr:%q\n", args, out, stderr, ec, err)
}
// Should have just short usage
if !strings.Contains(stderr, "\nUsage:\t") {
return fmt.Errorf("Missing short usage on %q\n", args)
if !strings.Contains(stderr, "\nUsage:") {
return fmt.Errorf("Missing short usage on %q\n:%#v", args, stderr)
}
// But shouldn't have full usage
if strings.Contains(stderr, "--help=false") {
Expand Down
14 changes: 7 additions & 7 deletions integration-cli/docker_cli_volume_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (s *DockerSuite) TestVolumeCliCreateOptionConflict(c *check.C) {
c.Assert(err, check.NotNil, check.Commentf("volume create exception name already in use with another driver"))
c.Assert(out, checker.Contains, "A volume named test already exists")

out, _ = dockerCmd(c, "volume", "inspect", "--format='{{ .Driver }}'", "test")
out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Driver }}", "test")
_, _, err = dockerCmdWithError("volume", "create", "--name", "test", "--driver", strings.TrimSpace(out))
c.Assert(err, check.IsNil)
}
Expand All @@ -39,11 +39,11 @@ func (s *DockerSuite) TestVolumeCliInspect(c *check.C) {

out, _ := dockerCmd(c, "volume", "create")
name := strings.TrimSpace(out)
out, _ = dockerCmd(c, "volume", "inspect", "--format='{{ .Name }}'", name)
out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Name }}", name)
c.Assert(strings.TrimSpace(out), check.Equals, name)

dockerCmd(c, "volume", "create", "--name", "test")
out, _ = dockerCmd(c, "volume", "inspect", "--format='{{ .Name }}'", "test")
out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Name }}", "test")
c.Assert(strings.TrimSpace(out), check.Equals, "test")
}

Expand Down Expand Up @@ -212,7 +212,7 @@ func (s *DockerSuite) TestVolumeCliRm(c *check.C) {
func (s *DockerSuite) TestVolumeCliNoArgs(c *check.C) {
out, _ := dockerCmd(c, "volume")
// no args should produce the cmd usage output
usage := "Usage: docker volume [OPTIONS] [COMMAND]"
usage := "Usage: docker volume COMMAND"
c.Assert(out, checker.Contains, usage)

// invalid arg should error and show the command usage on stderr
Expand All @@ -224,7 +224,7 @@ func (s *DockerSuite) TestVolumeCliNoArgs(c *check.C) {
_, stderr, _, err = runCommandWithStdoutStderr(exec.Command(dockerBinary, "volume", "--no-such-flag"))
c.Assert(err, check.NotNil, check.Commentf(stderr))
c.Assert(stderr, checker.Contains, usage)
c.Assert(stderr, checker.Contains, "flag provided but not defined: --no-such-flag")
c.Assert(stderr, checker.Contains, "unknown flag: --no-such-flag")
}

func (s *DockerSuite) TestVolumeCliInspectTmplError(c *check.C) {
Expand Down Expand Up @@ -268,7 +268,7 @@ func (s *DockerSuite) TestVolumeCliCreateLabel(c *check.C) {
out, _, err := dockerCmdWithError("volume", "create", "--label", testLabel+"="+testValue, "--name", testVol)
c.Assert(err, check.IsNil)

out, _ = dockerCmd(c, "volume", "inspect", "--format='{{ .Labels."+testLabel+" }}'", testVol)
out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Labels."+testLabel+" }}", testVol)
c.Assert(strings.TrimSpace(out), check.Equals, testValue)
}

Expand All @@ -295,7 +295,7 @@ func (s *DockerSuite) TestVolumeCliCreateLabelMultiple(c *check.C) {
c.Assert(err, check.IsNil)

for k, v := range testLabels {
out, _ = dockerCmd(c, "volume", "inspect", "--format='{{ .Labels."+k+" }}'", testVol)
out, _ = dockerCmd(c, "volume", "inspect", "--format={{ .Labels."+k+" }}", testVol)
c.Assert(strings.TrimSpace(out), check.Equals, v)
}
}
5 changes: 5 additions & 0 deletions opts/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ func (opts *ListOpts) Len() int {
return len((*opts.values))
}

// Type returns a string name for this Option type
func (opts *ListOpts) Type() string {
return "list"
}

// NamedOption is an interface that list and map options
// with names implement.
type NamedOption interface {
Expand Down

0 comments on commit 667dcb0

Please sign in to comment.