From 116367eb07351125754aef01f7d3da152b018710 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Fri, 6 Feb 2015 14:17:34 -0800 Subject: [PATCH] Fix for help when $HOME is / estesp noticed that when $HOME is / the ~ substitutions messes up becuase it tries to replace all paths that start with "/" with "~". This fixes it so that it will only replace it when $HOME isn't "/". Signed-off-by: Doug Davis --- integration-cli/docker_cli_help_test.go | 154 +++++++++++++++--------- pkg/homedir/homedir.go | 14 ++- pkg/mflag/flag.go | 12 +- 3 files changed, 118 insertions(+), 62 deletions(-) diff --git a/integration-cli/docker_cli_help_test.go b/integration-cli/docker_cli_help_test.go index 6a1e9a7249..2690b628bb 100644 --- a/integration-cli/docker_cli_help_test.go +++ b/integration-cli/docker_cli_help_test.go @@ -1,7 +1,9 @@ package main import ( + "os" "os/exec" + "runtime" "strings" "testing" "unicode" @@ -9,62 +11,44 @@ import ( "github.com/docker/docker/pkg/homedir" ) -func TestMainHelpWidth(t *testing.T) { +func TestHelpWidth(t *testing.T) { // Make sure main help text fits within 80 chars and that - // on non-windows system we use ~ when possible (to shorten things) + // on non-windows system we use ~ when possible (to shorten things). + // Test for HOME set to its default value and set to "/" on linux + // Yes on windows setting up an array and looping (right now) isn't + // necessary because we just have one value, but we'll need the + // array/loop on linux so we might as well set it up so that we can + // test any number of home dirs later on and all we need to do is + // modify the array - the rest of the testing infrastructure should work + homes := []string{homedir.Get()} - home := homedir.Get() - helpCmd := exec.Command(dockerBinary, "help") - out, ec, err := runCommandWithOutput(helpCmd) - if err != nil || ec != 0 { - t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec) - } - lines := strings.Split(out, "\n") - for _, line := range lines { - if len(line) > 80 { - t.Fatalf("Line is too long(%d chars):\n%s", len(line), line) - - } - if home != "" && strings.Contains(line, home) { - t.Fatalf("Line should use '%q' instead of %q:\n%s", homedir.GetShortcutString(), home, line) - } - } - logDone("help - verify main width") -} - -func TestCmdHelpWidth(t *testing.T) { - // Make sure main help text fits within 80 chars and that - // on non-windows system we use ~ when possible (to shorten things) - - home := homedir.Get() - // Pull the list of commands from the "Commands:" section of docker help - helpCmd := exec.Command(dockerBinary, "help") - out, ec, err := runCommandWithOutput(helpCmd) - if err != nil || ec != 0 { - t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec) - } - i := strings.Index(out, "Commands:") - if i < 0 { - t.Fatalf("Missing 'Commands:' in:\n%s", out) + // Non-Windows machines need to test for this special case of $HOME + if runtime.GOOS != "windows" { + homes = append(homes, "/") } - // Grab all chars starting at "Commands:" - // Skip first line, its "Commands:" - count := 0 - cmds := "" - for _, command := range strings.Split(out[i:], "\n")[1:] { - // Stop on blank line or non-idented line - if command == "" || !unicode.IsSpace(rune(command[0])) { + homeKey := homedir.Key() + baseEnvs := os.Environ() + + // Remove HOME env var from list so we can add a new value later. + for i, env := range baseEnvs { + if strings.HasPrefix(env, homeKey+"=") { + baseEnvs = append(baseEnvs[:i], baseEnvs[i+1:]...) break } + } - // Grab just the first word of each line - command = strings.Split(strings.TrimSpace(command), " ")[0] + for _, home := range homes { + // Dup baseEnvs and add our new HOME value + newEnvs := make([]string, len(baseEnvs)+1) + copy(newEnvs, baseEnvs) + newEnvs[len(newEnvs)-1] = homeKey + "=" + home - count++ - cmds = cmds + "\n" + command + scanForHome := runtime.GOOS != "windows" && home != "/" - helpCmd := exec.Command(dockerBinary, command, "--help") + // Check main help text to make sure its not over 80 chars + helpCmd := exec.Command(dockerBinary, "help") + helpCmd.Env = newEnvs out, ec, err := runCommandWithOutput(helpCmd) if err != nil || ec != 0 { t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec) @@ -72,19 +56,75 @@ func TestCmdHelpWidth(t *testing.T) { lines := strings.Split(out, "\n") for _, line := range lines { if len(line) > 80 { - t.Fatalf("Help for %q is too long(%d chars):\n%s", command, len(line), line) + t.Fatalf("Line is too long(%d chars):\n%s", len(line), line) } - if home != "" && strings.Contains(line, home) { - t.Fatalf("Help for %q should use home shortcut instead of %q on:\n%s", command, home, line) + if scanForHome && strings.Contains(line, `=`+home) { + t.Fatalf("Line should use '%q' instead of %q:\n%s", homedir.GetShortcutString(), home, line) } + if runtime.GOOS != "windows" { + i := strings.Index(line, homedir.GetShortcutString()) + if i >= 0 && i != len(line)-1 && line[i+1] != '/' { + t.Fatalf("Main help should not have used home shortcut:\n%s", line) + } + } + } + + // Make sure each cmd's help text fits within 80 chars and that + // on non-windows system we use ~ when possible (to shorten things). + // Pull the list of commands from the "Commands:" section of docker help + helpCmd = exec.Command(dockerBinary, "help") + helpCmd.Env = newEnvs + out, ec, err = runCommandWithOutput(helpCmd) + if err != nil || ec != 0 { + t.Fatalf("docker help should have worked\nout:%s\nec:%d", out, ec) + } + i := strings.Index(out, "Commands:") + if i < 0 { + t.Fatalf("Missing 'Commands:' in:\n%s", out) + } + + // Grab all chars starting at "Commands:" + // Skip first line, its "Commands:" + cmds := []string{} + for _, cmd := range strings.Split(out[i:], "\n")[1:] { + // Stop on blank line or non-idented line + if cmd == "" || !unicode.IsSpace(rune(cmd[0])) { + break + } + + // Grab just the first word of each line + cmd = strings.Split(strings.TrimSpace(cmd), " ")[0] + cmds = append(cmds, cmd) + + helpCmd := exec.Command(dockerBinary, cmd, "--help") + helpCmd.Env = newEnvs + out, ec, err := runCommandWithOutput(helpCmd) + if err != nil || ec != 0 { + t.Fatalf("Error on %q help: %s\nexit code:%d", cmd, out, ec) + } + lines := strings.Split(out, "\n") + for _, line := range lines { + if len(line) > 80 { + t.Fatalf("Help for %q is too long(%d chars):\n%s", cmd, + len(line), line) + } + if scanForHome && strings.Contains(line, `"`+home) { + t.Fatalf("Help for %q should use ~ instead of %q on:\n%s", + cmd, home, line) + } + i := strings.Index(line, "~") + if i >= 0 && i != len(line)-1 && line[i+1] != '/' { + t.Fatalf("Help for %q should not have used ~:\n%s", cmd, line) + } + } + } + + expected := 39 + if len(cmds) != expected { + t.Fatalf("Wrong # of cmds(%d), it should be: %d\nThe list:\n%q", + len(cmds), expected, cmds) } } - expected := 39 - if count != expected { - t.Fatalf("Wrong # of commands (%d), it should be: %d\nThe list:\n%s", - len(cmds), expected, cmds) - } - - logDone("help - cmd widths") + logDone("help - widths") } diff --git a/pkg/homedir/homedir.go b/pkg/homedir/homedir.go index 79d4431fab..3ffb297559 100644 --- a/pkg/homedir/homedir.go +++ b/pkg/homedir/homedir.go @@ -5,14 +5,20 @@ import ( "runtime" ) +// Key returns the env var name for the user's home dir based on +// the platform being run on +func Key() string { + if runtime.GOOS == "windows" { + return "USERPROFILE" + } + return "HOME" +} + // Get returns the home directory of the current user with the help of // environment variables depending on the target operating system. // Returned path should be used with "path/filepath" to form new paths. func Get() string { - if runtime.GOOS == "windows" { - return os.Getenv("USERPROFILE") - } - return os.Getenv("HOME") + return os.Getenv(Key()) } // GetShortcutString returns the string that is shortcut to user's home directory diff --git a/pkg/mflag/flag.go b/pkg/mflag/flag.go index edb17a2857..d02c7b1e01 100644 --- a/pkg/mflag/flag.go +++ b/pkg/mflag/flag.go @@ -86,6 +86,7 @@ import ( "fmt" "io" "os" + "runtime" "sort" "strconv" "strings" @@ -505,7 +506,16 @@ func Set(name, value string) error { // otherwise, the default values of all defined flags in the set. func (f *FlagSet) PrintDefaults() { writer := tabwriter.NewWriter(f.Out(), 20, 1, 3, ' ', 0) - home := homedir.Get() + var home string + if runtime.GOOS != "windows" { + // Only do this on non-windows systems + home = homedir.Get() + + // Don't substitute when HOME is / + if home == "/" { + home = "" + } + } f.VisitAll(func(flag *Flag) { format := " -%s=%s" names := []string{}