diff --git a/api/client/cli.go b/api/client/cli.go index c264e3802b..f51472b98c 100644 --- a/api/client/cli.go +++ b/api/client/cli.go @@ -8,7 +8,6 @@ import ( "io" "net" "net/http" - "os" "path/filepath" "reflect" "strings" @@ -98,7 +97,7 @@ func (cli *DockerCli) Cmd(args ...string) error { if len(args) > 0 { method, exists := cli.getMethod(args[0]) if !exists { - return fmt.Errorf("docker: '%s' is not a docker command. See 'docker --help'.", args[0]) + return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'.", args[0]) } return method(args[1:]...) } @@ -124,15 +123,13 @@ func (cli *DockerCli) Subcmd(name, signature, description string, exitOnError bo flags.Usage = func() { flags.ShortUsage() flags.PrintDefaults() - os.Exit(0) } flags.ShortUsage = func() { options := "" if flags.FlagCountUndeprecated() > 0 { options = " [OPTIONS]" } - fmt.Fprintf(cli.out, "\nUsage: docker %s%s%s\n\n%s\n\n", name, options, signature, description) - flags.SetOutput(cli.out) + fmt.Fprintf(flags.Out(), "\nUsage: docker %s%s%s\n\n%s\n", name, options, signature, description) } return flags } diff --git a/api/client/info.go b/api/client/info.go index 06a6f0ec54..f32548e24d 100644 --- a/api/client/info.go +++ b/api/client/info.go @@ -15,7 +15,7 @@ import ( func (cli *DockerCli) CmdInfo(args ...string) error { cmd := cli.Subcmd("info", "", "Display system-wide information", true) cmd.Require(flag.Exact, 0) - cmd.ParseFlags(args, false) + cmd.ParseFlags(args, true) rdr, _, err := cli.call("GET", "/info", nil, nil) if err != nil { diff --git a/api/client/logout.go b/api/client/logout.go index 74d0c278fa..a571af2cff 100644 --- a/api/client/logout.go +++ b/api/client/logout.go @@ -16,7 +16,7 @@ func (cli *DockerCli) CmdLogout(args ...string) error { cmd := cli.Subcmd("logout", "[SERVER]", "Log out from a Docker registry, if no server is\nspecified \""+registry.IndexServerAddress()+"\" is the default.", true) cmd.Require(flag.Max, 1) - cmd.ParseFlags(args, false) + cmd.ParseFlags(args, true) serverAddress := registry.IndexServerAddress() if len(cmd.Args()) > 0 { serverAddress = cmd.Arg(0) diff --git a/api/client/pause.go b/api/client/pause.go index 2f8f3c83f1..ee690c77fa 100644 --- a/api/client/pause.go +++ b/api/client/pause.go @@ -12,7 +12,7 @@ import ( func (cli *DockerCli) CmdPause(args ...string) error { cmd := cli.Subcmd("pause", "CONTAINER [CONTAINER...]", "Pause all processes within a container", true) cmd.Require(flag.Min, 1) - cmd.ParseFlags(args, false) + cmd.ParseFlags(args, true) var errNames []string for _, name := range cmd.Args() { diff --git a/api/client/unpause.go b/api/client/unpause.go index dceeb23af9..428dfec998 100644 --- a/api/client/unpause.go +++ b/api/client/unpause.go @@ -12,7 +12,7 @@ import ( func (cli *DockerCli) CmdUnpause(args ...string) error { cmd := cli.Subcmd("unpause", "CONTAINER [CONTAINER...]", "Unpause all processes within a container", true) cmd.Require(flag.Min, 1) - cmd.ParseFlags(args, false) + cmd.ParseFlags(args, true) var errNames []string for _, name := range cmd.Args() { diff --git a/api/client/version.go b/api/client/version.go index 4e06a6c8ad..bc215c7423 100644 --- a/api/client/version.go +++ b/api/client/version.go @@ -20,7 +20,7 @@ func (cli *DockerCli) CmdVersion(args ...string) error { cmd := cli.Subcmd("version", "", "Show the Docker version information.", true) cmd.Require(flag.Exact, 0) - cmd.ParseFlags(args, false) + cmd.ParseFlags(args, true) if dockerversion.VERSION != "" { fmt.Fprintf(cli.out, "Client version: %s\n", dockerversion.VERSION) diff --git a/integration-cli/docker_cli_help_test.go b/integration-cli/docker_cli_help_test.go index 86b0b3bbc7..da57601c15 100644 --- a/integration-cli/docker_cli_help_test.go +++ b/integration-cli/docker_cli_help_test.go @@ -93,6 +93,8 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) { // Skip first line, its "Commands:" cmds := []string{} for _, cmd := range strings.Split(out[i:], "\n")[1:] { + var stderr string + // Stop on blank line or non-idented line if cmd == "" || !unicode.IsSpace(rune(cmd[0])) { break @@ -102,12 +104,24 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) { cmd = strings.Split(strings.TrimSpace(cmd), " ")[0] cmds = append(cmds, cmd) + // Check the full usage text helpCmd := exec.Command(dockerBinary, cmd, "--help") helpCmd.Env = newEnvs - out, ec, err := runCommandWithOutput(helpCmd) + out, stderr, ec, err = runCommandWithStdoutStderr(helpCmd) + if len(stderr) != 0 { + c.Fatalf("Error on %q help. non-empty stderr:%q", cmd, stderr) + } + if strings.HasSuffix(out, "\n\n") { + c.Fatalf("Should not have blank line on %q\nout:%q", cmd, out) + } + if !strings.Contains(out, "--help=false") { + c.Fatalf("Should show full usage on %q\nout:%q", cmd, out) + } if err != nil || ec != 0 { c.Fatalf("Error on %q help: %s\nexit code:%d", cmd, out, ec) } + + // Check each line for lots of stuff lines := strings.Split(out, "\n") for _, line := range lines { if len(line) > 80 { @@ -142,6 +156,77 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) { } } + + // For each command make sure we generate an error + // if we give a bad arg + dCmd := exec.Command(dockerBinary, cmd, "--badArg") + out, stderr, ec, err = runCommandWithStdoutStderr(dCmd) + if len(out) != 0 || len(stderr) == 0 || ec == 0 || err == nil { + c.Fatalf("Bad results from 'docker %s --badArg'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", cmd, ec, out, stderr, err) + } + // Be really picky + if strings.HasSuffix(stderr, "\n\n") { + c.Fatalf("Should not have a blank line at the end of 'docker rm'\n%s", stderr) + } + + // Now make sure that each command will print a short-usage + // (not a full usage - meaning no opts section) if we + // are missing a required arg or pass in a bad arg + + // These commands will never print a short-usage so don't test + noShortUsage := map[string]string{ + "images": "", + "login": "", + "logout": "", + } + + if _, ok := noShortUsage[cmd]; !ok { + // For each command run it w/o any args. It will either return + // valid output or print a short-usage + var dCmd *exec.Cmd + var stdout, stderr string + var args []string + + // skipNoArgs are ones that we don't want to try w/o + // any args. Either because it'll hang the test or + // lead to incorrect test result (like false negative). + // Whatever the reason, skip trying to run w/o args and + // jump to trying with a bogus arg. + skipNoArgs := map[string]string{ + "events": "", + "load": "", + } + + ec = 0 + if _, ok := skipNoArgs[cmd]; !ok { + args = []string{cmd} + dCmd = exec.Command(dockerBinary, args...) + stdout, stderr, ec, err = runCommandWithStdoutStderr(dCmd) + } + + // If its ok w/o any args then try again with an arg + if ec == 0 { + args = []string{cmd, "badArg"} + dCmd = exec.Command(dockerBinary, args...) + stdout, stderr, ec, err = runCommandWithStdoutStderr(dCmd) + } + + if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil { + c.Fatalf("Bad output from %q\nstdout:%q\nstderr:%q\nec:%d\nerr:%q", args, stdout, stderr, ec, err) + } + // Should have just short usage + if !strings.Contains(stderr, "\nUsage: ") { + c.Fatalf("Missing short usage on %q\nstderr:%q", args, stderr) + } + // But shouldn't have full usage + if strings.Contains(stderr, "--help=false") { + c.Fatalf("Should not have full usage on %q\nstderr:%q", args, stderr) + } + if strings.HasSuffix(stderr, "\n\n") { + c.Fatalf("Should not have a blank line on %q\nstderr:%q", args, stderr) + } + } + } expected := 39 @@ -153,31 +238,92 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) { } -func (s *DockerSuite) TestHelpErrorStderr(c *check.C) { - // If we had a generic CLI test file this one shoudl go in there +func (s *DockerSuite) TestHelpExitCodesHelpOutput(c *check.C) { + // Test to make sure the exit code and output (stdout vs stderr) of + // various good and bad cases are what we expect - cmd := exec.Command(dockerBinary, "boogie") - out, ec, err := runCommandWithOutput(cmd) - if err == nil || ec == 0 { - c.Fatalf("Boogie command should have failed") + // docker : stdout=all, stderr=empty, rc=0 + cmd := exec.Command(dockerBinary) + stdout, stderr, ec, err := runCommandWithStdoutStderr(cmd) + if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil { + c.Fatalf("Bad results from 'docker'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err) + } + // Be really pick + if strings.HasSuffix(stdout, "\n\n") { + c.Fatalf("Should not have a blank line at the end of 'docker'\n%s", stdout) } - expected := "docker: 'boogie' is not a docker command. See 'docker --help'.\n" - if out != expected { - c.Fatalf("Bad output from boogie\nGot:%s\nExpected:%s", out, expected) + // docker help: stdout=all, stderr=empty, rc=0 + cmd = exec.Command(dockerBinary, "help") + stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd) + if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil { + c.Fatalf("Bad results from 'docker help'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err) + } + // Be really pick + if strings.HasSuffix(stdout, "\n\n") { + c.Fatalf("Should not have a blank line at the end of 'docker help'\n%s", stdout) } - cmd = exec.Command(dockerBinary, "rename", "foo", "bar") - out, ec, err = runCommandWithOutput(cmd) - if err == nil || ec == 0 { - c.Fatalf("Rename should have failed") + // docker --help: stdout=all, stderr=empty, rc=0 + cmd = exec.Command(dockerBinary, "--help") + stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd) + if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil { + c.Fatalf("Bad results from 'docker --help'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err) + } + // Be really pick + if strings.HasSuffix(stdout, "\n\n") { + c.Fatalf("Should not have a blank line at the end of 'docker --help'\n%s", stdout) } - expected = `Error response from daemon: no such id: foo -Error: failed to rename container named foo -` - if out != expected { - c.Fatalf("Bad output from rename\nGot:%s\nExpected:%s", out, expected) + // docker inspect busybox: stdout=all, stderr=empty, rc=0 + // Just making sure stderr is empty on valid cmd + cmd = exec.Command(dockerBinary, "inspect", "busybox") + stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd) + if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil { + c.Fatalf("Bad results from 'docker inspect busybox'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err) + } + // Be really pick + if strings.HasSuffix(stdout, "\n\n") { + c.Fatalf("Should not have a blank line at the end of 'docker inspect busyBox'\n%s", stdout) + } + + // docker rm: stdout=empty, stderr=all, rc!=0 + // testing the min arg error msg + cmd = exec.Command(dockerBinary, "rm") + stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd) + if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil { + c.Fatalf("Bad results from 'docker rm'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err) + } + // Should not contain full help text but should contain info about + // # of args and Usage line + if !strings.Contains(stderr, "requires a minimum") { + c.Fatalf("Missing # of args text from 'docker rm'\nstderr:%s", stderr) + } + + // docker rm NoSuchContainer: stdout=empty, stderr=all, rc=0 + // testing to make sure no blank line on error + cmd = exec.Command(dockerBinary, "rm", "NoSuchContainer") + stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd) + if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil { + c.Fatalf("Bad results from 'docker rm NoSuchContainer'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err) + } + // Be really picky + if strings.HasSuffix(stderr, "\n\n") { + c.Fatalf("Should not have a blank line at the end of 'docker rm'\n%s", stderr) + } + + // docker BadCmd: stdout=empty, stderr=all, rc=0 + cmd = exec.Command(dockerBinary, "BadCmd") + stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd) + if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil { + c.Fatalf("Bad results from 'docker BadCmd'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err) + } + if stderr != "docker: 'BadCmd' is not a docker command.\nSee 'docker --help'.\n" { + c.Fatalf("Unexcepted output for 'docker badCmd'\nstderr:%s", stderr) + } + // Be really picky + if strings.HasSuffix(stderr, "\n\n") { + c.Fatalf("Should not have a blank line at the end of 'docker rm'\n%s", stderr) } } diff --git a/pkg/mflag/flag.go b/pkg/mflag/flag.go index 2cead9ed38..9626a2f6b7 100644 --- a/pkg/mflag/flag.go +++ b/pkg/mflag/flag.go @@ -512,6 +512,12 @@ func (f *FlagSet) PrintDefaults() { if runtime.GOOS != "windows" && home == "/" { home = "" } + + // Add a blank line between cmd description and list of options + if f.FlagCount() > 0 { + fmt.Fprintln(writer, "") + } + f.VisitAll(func(flag *Flag) { format := " -%s=%s" names := []string{} @@ -1074,11 +1080,12 @@ func (cmd *FlagSet) ParseFlags(args []string, withHelp bool) error { return err } if help != nil && *help { + cmd.SetOutput(os.Stdout) cmd.Usage() - // just in case Usage does not exit os.Exit(0) } if str := cmd.CheckArgs(); str != "" { + cmd.SetOutput(os.Stderr) cmd.ReportError(str, withHelp) cmd.ShortUsage() os.Exit(1) @@ -1089,9 +1096,9 @@ func (cmd *FlagSet) ParseFlags(args []string, withHelp bool) error { func (cmd *FlagSet) ReportError(str string, withHelp bool) { if withHelp { if os.Args[0] == cmd.Name() { - str += ". See '" + os.Args[0] + " --help'" + str += ".\nSee '" + os.Args[0] + " --help'" } else { - str += ". See '" + os.Args[0] + " " + cmd.Name() + " --help'" + str += ".\nSee '" + os.Args[0] + " " + cmd.Name() + " --help'" } } fmt.Fprintf(cmd.Out(), "docker: %s.\n", str)