Browse code

Merge pull request #13428 from duglin/niceHelp2

Nice help2

David Calavera authored on 2015/05/29 03:09:02
Showing 10 changed files
... ...
@@ -7,7 +7,6 @@ import (
7 7
 	"fmt"
8 8
 	"io"
9 9
 	"net/http"
10
-	"os"
11 10
 	"path/filepath"
12 11
 	"reflect"
13 12
 	"strings"
... ...
@@ -97,7 +96,7 @@ func (cli *DockerCli) Cmd(args ...string) error {
97 97
 	if len(args) > 0 {
98 98
 		method, exists := cli.getMethod(args[0])
99 99
 		if !exists {
100
-			return fmt.Errorf("docker: '%s' is not a docker command. See 'docker --help'.", args[0])
100
+			return fmt.Errorf("docker: '%s' is not a docker command.\nSee 'docker --help'.", args[0])
101 101
 		}
102 102
 		return method(args[1:]...)
103 103
 	}
... ...
@@ -117,18 +116,19 @@ func (cli *DockerCli) Subcmd(name, signature, description string, exitOnError bo
117 117
 		errorHandling = flag.ContinueOnError
118 118
 	}
119 119
 	flags := flag.NewFlagSet(name, errorHandling)
120
+	if signature != "" {
121
+		signature = " " + signature
122
+	}
120 123
 	flags.Usage = func() {
124
+		flags.ShortUsage()
125
+		flags.PrintDefaults()
126
+	}
127
+	flags.ShortUsage = func() {
121 128
 		options := ""
122
-		if signature != "" {
123
-			signature = " " + signature
124
-		}
125 129
 		if flags.FlagCountUndeprecated() > 0 {
126 130
 			options = " [OPTIONS]"
127 131
 		}
128
-		fmt.Fprintf(cli.out, "\nUsage: docker %s%s%s\n\n%s\n\n", name, options, signature, description)
129
-		flags.SetOutput(cli.out)
130
-		flags.PrintDefaults()
131
-		os.Exit(0)
132
+		fmt.Fprintf(flags.Out(), "\nUsage: docker %s%s%s\n\n%s\n", name, options, signature, description)
132 133
 	}
133 134
 	return flags
134 135
 }
... ...
@@ -145,6 +145,7 @@ func (cli *DockerCli) CmdCreate(args ...string) error {
145 145
 	config, hostConfig, cmd, err := runconfig.Parse(cmd, args)
146 146
 	if err != nil {
147 147
 		cmd.ReportError(err.Error(), true)
148
+		os.Exit(1)
148 149
 	}
149 150
 	if config.Image == "" {
150 151
 		cmd.Usage()
... ...
@@ -15,7 +15,7 @@ import (
15 15
 func (cli *DockerCli) CmdInfo(args ...string) error {
16 16
 	cmd := cli.Subcmd("info", "", "Display system-wide information", true)
17 17
 	cmd.Require(flag.Exact, 0)
18
-	cmd.ParseFlags(args, false)
18
+	cmd.ParseFlags(args, true)
19 19
 
20 20
 	rdr, _, err := cli.call("GET", "/info", nil, nil)
21 21
 	if err != nil {
... ...
@@ -16,7 +16,7 @@ func (cli *DockerCli) CmdLogout(args ...string) error {
16 16
 	cmd := cli.Subcmd("logout", "[SERVER]", "Log out from a Docker registry, if no server is\nspecified \""+registry.IndexServerAddress()+"\" is the default.", true)
17 17
 	cmd.Require(flag.Max, 1)
18 18
 
19
-	cmd.ParseFlags(args, false)
19
+	cmd.ParseFlags(args, true)
20 20
 	serverAddress := registry.IndexServerAddress()
21 21
 	if len(cmd.Args()) > 0 {
22 22
 		serverAddress = cmd.Arg(0)
... ...
@@ -12,7 +12,7 @@ import (
12 12
 func (cli *DockerCli) CmdPause(args ...string) error {
13 13
 	cmd := cli.Subcmd("pause", "CONTAINER [CONTAINER...]", "Pause all processes within a container", true)
14 14
 	cmd.Require(flag.Min, 1)
15
-	cmd.ParseFlags(args, false)
15
+	cmd.ParseFlags(args, true)
16 16
 
17 17
 	var errNames []string
18 18
 	for _, name := range cmd.Args() {
... ...
@@ -57,6 +57,7 @@ func (cli *DockerCli) CmdRun(args ...string) error {
57 57
 	// just in case the Parse does not exit
58 58
 	if err != nil {
59 59
 		cmd.ReportError(err.Error(), true)
60
+		os.Exit(1)
60 61
 	}
61 62
 
62 63
 	if len(hostConfig.Dns) > 0 {
... ...
@@ -12,7 +12,7 @@ import (
12 12
 func (cli *DockerCli) CmdUnpause(args ...string) error {
13 13
 	cmd := cli.Subcmd("unpause", "CONTAINER [CONTAINER...]", "Unpause all processes within a container", true)
14 14
 	cmd.Require(flag.Min, 1)
15
-	cmd.ParseFlags(args, false)
15
+	cmd.ParseFlags(args, true)
16 16
 
17 17
 	var errNames []string
18 18
 	for _, name := range cmd.Args() {
... ...
@@ -20,7 +20,7 @@ func (cli *DockerCli) CmdVersion(args ...string) error {
20 20
 	cmd := cli.Subcmd("version", "", "Show the Docker version information.", true)
21 21
 	cmd.Require(flag.Exact, 0)
22 22
 
23
-	cmd.ParseFlags(args, false)
23
+	cmd.ParseFlags(args, true)
24 24
 
25 25
 	if dockerversion.VERSION != "" {
26 26
 		fmt.Fprintf(cli.out, "Client version: %s\n", dockerversion.VERSION)
... ...
@@ -93,6 +93,8 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) {
93 93
 		// Skip first line, its "Commands:"
94 94
 		cmds := []string{}
95 95
 		for _, cmd := range strings.Split(out[i:], "\n")[1:] {
96
+			var stderr string
97
+
96 98
 			// Stop on blank line or non-idented line
97 99
 			if cmd == "" || !unicode.IsSpace(rune(cmd[0])) {
98 100
 				break
... ...
@@ -102,12 +104,24 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) {
102 102
 			cmd = strings.Split(strings.TrimSpace(cmd), " ")[0]
103 103
 			cmds = append(cmds, cmd)
104 104
 
105
+			// Check the full usage text
105 106
 			helpCmd := exec.Command(dockerBinary, cmd, "--help")
106 107
 			helpCmd.Env = newEnvs
107
-			out, ec, err := runCommandWithOutput(helpCmd)
108
+			out, stderr, ec, err = runCommandWithStdoutStderr(helpCmd)
109
+			if len(stderr) != 0 {
110
+				c.Fatalf("Error on %q help. non-empty stderr:%q", cmd, stderr)
111
+			}
112
+			if strings.HasSuffix(out, "\n\n") {
113
+				c.Fatalf("Should not have blank line on %q\nout:%q", cmd, out)
114
+			}
115
+			if !strings.Contains(out, "--help=false") {
116
+				c.Fatalf("Should show full usage on %q\nout:%q", cmd, out)
117
+			}
108 118
 			if err != nil || ec != 0 {
109 119
 				c.Fatalf("Error on %q help: %s\nexit code:%d", cmd, out, ec)
110 120
 			}
121
+
122
+			// Check each line for lots of stuff
111 123
 			lines := strings.Split(out, "\n")
112 124
 			for _, line := range lines {
113 125
 				if len(line) > 80 {
... ...
@@ -142,6 +156,77 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) {
142 142
 				}
143 143
 
144 144
 			}
145
+
146
+			// For each command make sure we generate an error
147
+			// if we give a bad arg
148
+			dCmd := exec.Command(dockerBinary, cmd, "--badArg")
149
+			out, stderr, ec, err = runCommandWithStdoutStderr(dCmd)
150
+			if len(out) != 0 || len(stderr) == 0 || ec == 0 || err == nil {
151
+				c.Fatalf("Bad results from 'docker %s --badArg'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", cmd, ec, out, stderr, err)
152
+			}
153
+			// Be really picky
154
+			if strings.HasSuffix(stderr, "\n\n") {
155
+				c.Fatalf("Should not have a blank line at the end of 'docker rm'\n%s", stderr)
156
+			}
157
+
158
+			// Now make sure that each command will print a short-usage
159
+			// (not a full usage - meaning no opts section) if we
160
+			// are missing a required arg or pass in a bad arg
161
+
162
+			// These commands will never print a short-usage so don't test
163
+			noShortUsage := map[string]string{
164
+				"images": "",
165
+				"login":  "",
166
+				"logout": "",
167
+			}
168
+
169
+			if _, ok := noShortUsage[cmd]; !ok {
170
+				// For each command run it w/o any args. It will either return
171
+				// valid output or print a short-usage
172
+				var dCmd *exec.Cmd
173
+				var stdout, stderr string
174
+				var args []string
175
+
176
+				// skipNoArgs are ones that we don't want to try w/o
177
+				// any args. Either because it'll hang the test or
178
+				// lead to incorrect test result (like false negative).
179
+				// Whatever the reason, skip trying to run w/o args and
180
+				// jump to trying with a bogus arg.
181
+				skipNoArgs := map[string]string{
182
+					"events": "",
183
+					"load":   "",
184
+				}
185
+
186
+				ec = 0
187
+				if _, ok := skipNoArgs[cmd]; !ok {
188
+					args = []string{cmd}
189
+					dCmd = exec.Command(dockerBinary, args...)
190
+					stdout, stderr, ec, err = runCommandWithStdoutStderr(dCmd)
191
+				}
192
+
193
+				// If its ok w/o any args then try again with an arg
194
+				if ec == 0 {
195
+					args = []string{cmd, "badArg"}
196
+					dCmd = exec.Command(dockerBinary, args...)
197
+					stdout, stderr, ec, err = runCommandWithStdoutStderr(dCmd)
198
+				}
199
+
200
+				if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil {
201
+					c.Fatalf("Bad output from %q\nstdout:%q\nstderr:%q\nec:%d\nerr:%q", args, stdout, stderr, ec, err)
202
+				}
203
+				// Should have just short usage
204
+				if !strings.Contains(stderr, "\nUsage: ") {
205
+					c.Fatalf("Missing short usage on %q\nstderr:%q", args, stderr)
206
+				}
207
+				// But shouldn't have full usage
208
+				if strings.Contains(stderr, "--help=false") {
209
+					c.Fatalf("Should not have full usage on %q\nstderr:%q", args, stderr)
210
+				}
211
+				if strings.HasSuffix(stderr, "\n\n") {
212
+					c.Fatalf("Should not have a blank line on %q\nstderr:%q", args, stderr)
213
+				}
214
+			}
215
+
145 216
 		}
146 217
 
147 218
 		expected := 39
... ...
@@ -153,31 +238,92 @@ func (s *DockerSuite) TestHelpTextVerify(c *check.C) {
153 153
 
154 154
 }
155 155
 
156
-func (s *DockerSuite) TestHelpErrorStderr(c *check.C) {
157
-	// If we had a generic CLI test file this one shoudl go in there
156
+func (s *DockerSuite) TestHelpExitCodesHelpOutput(c *check.C) {
157
+	// Test to make sure the exit code and output (stdout vs stderr) of
158
+	// various good and bad cases are what we expect
158 159
 
159
-	cmd := exec.Command(dockerBinary, "boogie")
160
-	out, ec, err := runCommandWithOutput(cmd)
161
-	if err == nil || ec == 0 {
162
-		c.Fatalf("Boogie command should have failed")
160
+	// docker : stdout=all, stderr=empty, rc=0
161
+	cmd := exec.Command(dockerBinary)
162
+	stdout, stderr, ec, err := runCommandWithStdoutStderr(cmd)
163
+	if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil {
164
+		c.Fatalf("Bad results from 'docker'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err)
165
+	}
166
+	// Be really pick
167
+	if strings.HasSuffix(stdout, "\n\n") {
168
+		c.Fatalf("Should not have a blank line at the end of 'docker'\n%s", stdout)
163 169
 	}
164 170
 
165
-	expected := "docker: 'boogie' is not a docker command. See 'docker --help'.\n"
166
-	if out != expected {
167
-		c.Fatalf("Bad output from boogie\nGot:%s\nExpected:%s", out, expected)
171
+	// docker help: stdout=all, stderr=empty, rc=0
172
+	cmd = exec.Command(dockerBinary, "help")
173
+	stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd)
174
+	if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil {
175
+		c.Fatalf("Bad results from 'docker help'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err)
176
+	}
177
+	// Be really pick
178
+	if strings.HasSuffix(stdout, "\n\n") {
179
+		c.Fatalf("Should not have a blank line at the end of 'docker help'\n%s", stdout)
168 180
 	}
169 181
 
170
-	cmd = exec.Command(dockerBinary, "rename", "foo", "bar")
171
-	out, ec, err = runCommandWithOutput(cmd)
172
-	if err == nil || ec == 0 {
173
-		c.Fatalf("Rename should have failed")
182
+	// docker --help: stdout=all, stderr=empty, rc=0
183
+	cmd = exec.Command(dockerBinary, "--help")
184
+	stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd)
185
+	if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil {
186
+		c.Fatalf("Bad results from 'docker --help'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err)
187
+	}
188
+	// Be really pick
189
+	if strings.HasSuffix(stdout, "\n\n") {
190
+		c.Fatalf("Should not have a blank line at the end of 'docker --help'\n%s", stdout)
174 191
 	}
175 192
 
176
-	expected = `Error response from daemon: no such id: foo
177
-Error: failed to rename container named foo
178
-`
179
-	if out != expected {
180
-		c.Fatalf("Bad output from rename\nGot:%s\nExpected:%s", out, expected)
193
+	// docker inspect busybox: stdout=all, stderr=empty, rc=0
194
+	// Just making sure stderr is empty on valid cmd
195
+	cmd = exec.Command(dockerBinary, "inspect", "busybox")
196
+	stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd)
197
+	if len(stdout) == 0 || len(stderr) != 0 || ec != 0 || err != nil {
198
+		c.Fatalf("Bad results from 'docker inspect busybox'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err)
199
+	}
200
+	// Be really pick
201
+	if strings.HasSuffix(stdout, "\n\n") {
202
+		c.Fatalf("Should not have a blank line at the end of 'docker inspect busyBox'\n%s", stdout)
203
+	}
204
+
205
+	// docker rm: stdout=empty, stderr=all, rc!=0
206
+	// testing the min arg error msg
207
+	cmd = exec.Command(dockerBinary, "rm")
208
+	stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd)
209
+	if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil {
210
+		c.Fatalf("Bad results from 'docker rm'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err)
211
+	}
212
+	// Should not contain full help text but should contain info about
213
+	// # of args and Usage line
214
+	if !strings.Contains(stderr, "requires a minimum") {
215
+		c.Fatalf("Missing # of args text from 'docker rm'\nstderr:%s", stderr)
216
+	}
217
+
218
+	// docker rm NoSuchContainer: stdout=empty, stderr=all, rc=0
219
+	// testing to make sure no blank line on error
220
+	cmd = exec.Command(dockerBinary, "rm", "NoSuchContainer")
221
+	stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd)
222
+	if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil {
223
+		c.Fatalf("Bad results from 'docker rm NoSuchContainer'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err)
224
+	}
225
+	// Be really picky
226
+	if strings.HasSuffix(stderr, "\n\n") {
227
+		c.Fatalf("Should not have a blank line at the end of 'docker rm'\n%s", stderr)
228
+	}
229
+
230
+	// docker BadCmd: stdout=empty, stderr=all, rc=0
231
+	cmd = exec.Command(dockerBinary, "BadCmd")
232
+	stdout, stderr, ec, err = runCommandWithStdoutStderr(cmd)
233
+	if len(stdout) != 0 || len(stderr) == 0 || ec == 0 || err == nil {
234
+		c.Fatalf("Bad results from 'docker BadCmd'\nec:%d\nstdout:%s\nstderr:%s\nerr:%q", ec, stdout, stderr, err)
235
+	}
236
+	if stderr != "docker: 'BadCmd' is not a docker command.\nSee 'docker --help'.\n" {
237
+		c.Fatalf("Unexcepted output for 'docker badCmd'\nstderr:%s", stderr)
238
+	}
239
+	// Be really picky
240
+	if strings.HasSuffix(stderr, "\n\n") {
241
+		c.Fatalf("Should not have a blank line at the end of 'docker rm'\n%s", stderr)
181 242
 	}
182 243
 
183 244
 }
... ...
@@ -289,7 +289,8 @@ type FlagSet struct {
289 289
 	// Usage is the function called when an error occurs while parsing flags.
290 290
 	// The field is a function (not a method) that may be changed to point to
291 291
 	// a custom error handler.
292
-	Usage func()
292
+	Usage      func()
293
+	ShortUsage func()
293 294
 
294 295
 	name             string
295 296
 	parsed           bool
... ...
@@ -511,6 +512,12 @@ func (f *FlagSet) PrintDefaults() {
511 511
 	if runtime.GOOS != "windows" && home == "/" {
512 512
 		home = ""
513 513
 	}
514
+
515
+	// Add a blank line between cmd description and list of options
516
+	if f.FlagCount() > 0 {
517
+		fmt.Fprintln(writer, "")
518
+	}
519
+
514 520
 	f.VisitAll(func(flag *Flag) {
515 521
 		format := "  -%s=%s"
516 522
 		names := []string{}
... ...
@@ -564,6 +571,12 @@ var Usage = func() {
564 564
 	PrintDefaults()
565 565
 }
566 566
 
567
+// Usage prints to standard error a usage message documenting the standard command layout
568
+// The function is a variable that may be changed to point to a custom function.
569
+var ShortUsage = func() {
570
+	fmt.Fprintf(CommandLine.output, "Usage of %s:\n", os.Args[0])
571
+}
572
+
567 573
 // FlagCount returns the number of flags that have been defined.
568 574
 func (f *FlagSet) FlagCount() int { return len(sortFlags(f.formal)) }
569 575
 
... ...
@@ -1067,12 +1080,15 @@ func (cmd *FlagSet) ParseFlags(args []string, withHelp bool) error {
1067 1067
 		return err
1068 1068
 	}
1069 1069
 	if help != nil && *help {
1070
+		cmd.SetOutput(os.Stdout)
1070 1071
 		cmd.Usage()
1071
-		// just in case Usage does not exit
1072 1072
 		os.Exit(0)
1073 1073
 	}
1074 1074
 	if str := cmd.CheckArgs(); str != "" {
1075
+		cmd.SetOutput(os.Stderr)
1075 1076
 		cmd.ReportError(str, withHelp)
1077
+		cmd.ShortUsage()
1078
+		os.Exit(1)
1076 1079
 	}
1077 1080
 	return nil
1078 1081
 }
... ...
@@ -1080,13 +1096,12 @@ func (cmd *FlagSet) ParseFlags(args []string, withHelp bool) error {
1080 1080
 func (cmd *FlagSet) ReportError(str string, withHelp bool) {
1081 1081
 	if withHelp {
1082 1082
 		if os.Args[0] == cmd.Name() {
1083
-			str += ". See '" + os.Args[0] + " --help'"
1083
+			str += ".\nSee '" + os.Args[0] + " --help'"
1084 1084
 		} else {
1085
-			str += ". See '" + os.Args[0] + " " + cmd.Name() + " --help'"
1085
+			str += ".\nSee '" + os.Args[0] + " " + cmd.Name() + " --help'"
1086 1086
 		}
1087 1087
 	}
1088
-	fmt.Fprintf(cmd.Out(), "docker: %s\n", str)
1089
-	os.Exit(1)
1088
+	fmt.Fprintf(cmd.Out(), "docker: %s.\n", str)
1090 1089
 }
1091 1090
 
1092 1091
 // Parsed reports whether f.Parse has been called.