cli-plugins: Reinstate deprecated `-h` short form of `--help`.

In the initial implementation I thought it would be good to not pass on the
deprecation to plugins (since they are new). However it turns out this causes
`docker helloworld -h` to print a spurious "pflag: help requested" line:

    $ docker helloworld -h
    pflag: help requested
    See 'docker helloworld --help'.

    Usage:	docker helloworld [OPTIONS] COMMAND

    A basic Hello World plugin for tests
    ...

Compared with:

    $ docker ps -h
    Flag shorthand -h has been deprecated, please use --help

    Usage:	docker ps [OPTIONS]

This is in essence because having the flag undefined hits a different path
within cobra, causing `c.execute()` to return early due to getting an error
(`flag.ErrHelp`) from `c.ParseFlags`, which launders the error through our
`FlagErrorFunc` which wraps it in a `StatusError` which in turn defeats an `if
err == flag.ErrHelp` check further up the call chain. If the flag is defined we
instead hit a path which returns a bare `flag.ErrHelp` without wrapping it.

I considered updating our `FlagErrorFunc` to not wrap `flag.ErrHelp` (and then
following the chain to the next thing) however while doing that I realised that
the code for `-h` (and `--help`) is deeply embedded into cobra (and its flags
library) such that actually using `-h` as a plugin argument meaning something
other than `help` is basically impossible/impractical. Therefore we may as well
have plugins behave identically to the monolithic CLI and support (deprecated)
the `-h` argument.

With this changed the help related blocks of `SetupRootCommand` and
`SetupPluginRootCommand` are now identical, so consolidate into
`setupCommonRootCommand`.

Tests are updated to check `-h` in a variety of scenarios, including the happy
case here.

Signed-off-by: Ian Campbell <ijc@docker.com>

cli/cobra.go

@@ -42,6 +42,10 @@ func setupCommonRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *p
 	rootCmd.SetFlagErrorFunc(FlagErrorFunc)
 	rootCmd.SetHelpCommand(helpCommand)
 
+	rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage")
+	rootCmd.PersistentFlags().MarkShorthandDeprecated("help", "please use --help")
+	rootCmd.PersistentFlags().Lookup("help").Hidden = true
+
 	return opts, flags, helpCommand
 }
 
@@ -52,20 +56,12 @@ func SetupRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *pflag.F
 
 	rootCmd.SetVersionTemplate("Docker version {{.Version}}\n")
 
-	rootCmd.PersistentFlags().BoolP("help", "h", false, "Print usage")
-	rootCmd.PersistentFlags().MarkShorthandDeprecated("help", "please use --help")
-	rootCmd.PersistentFlags().Lookup("help").Hidden = true
-
 	return opts, flags, helpCmd
 }
 
 // SetupPluginRootCommand sets default usage, help and error handling for a plugin root command.
 func SetupPluginRootCommand(rootCmd *cobra.Command) (*cliflags.ClientOptions, *pflag.FlagSet) {
 	opts, flags, _ := setupCommonRootCommand(rootCmd)
-
-	rootCmd.PersistentFlags().BoolP("help", "", false, "Print usage")
-	rootCmd.PersistentFlags().Lookup("help").Hidden = true
-
 	return opts, flags
 }
 

e2e/cli-plugins/run_test.go

@@ -9,6 +9,8 @@ import (
 	"gotest.tools/icmd"
 )
 
+const shortHFlagDeprecated = "Flag shorthand -h has been deprecated, please use --help\n"
+
 // TestRunNonexisting ensures correct behaviour when running a nonexistent plugin.
 func TestRunNonexisting(t *testing.T) {
 	run, _, cleanup := prepare(t)
@@ -50,6 +52,15 @@ func TestNonexistingHelp(t *testing.T) {
 		Out: "Usage:	docker [OPTIONS] COMMAND\n\nA self-sufficient runtime for containers",
 		Err: icmd.None,
 	})
+	// Short -h should be the same, modulo the deprecation message
+	exp := shortHFlagDeprecated + res.Stdout()
+	res = icmd.RunCmd(run("nonexistent", "-h"))
+	res.Assert(t, icmd.Expected{
+		ExitCode: 0,
+		// This should be identical to the --help case above
+		Out: exp,
+		Err: icmd.None,
+	})
 }
 
 // TestRunBad ensures correct behaviour when running an existent but invalid plugin
@@ -93,6 +104,15 @@ func TestBadHelp(t *testing.T) {
 		Out: "Usage:	docker [OPTIONS] COMMAND\n\nA self-sufficient runtime for containers",
 		Err: icmd.None,
 	})
+	// Short -h should be the same, modulo the deprecation message
+	exp := shortHFlagDeprecated + res.Stdout()
+	res = icmd.RunCmd(run("badmeta", "-h"))
+	res.Assert(t, icmd.Expected{
+		ExitCode: 0,
+		// This should be identical to the --help case above
+		Out: exp,
+		Err: icmd.None,
+	})
 }
 
 // TestRunGood ensures correct behaviour when running a valid plugin
@@ -137,6 +157,15 @@ func TestGoodHelp(t *testing.T) {
 	})
 	// This is the same golden file as `TestHelpGood`, above.
 	golden.Assert(t, res.Stdout(), "docker-help-helloworld.golden")
+	// Short -h should be the same, modulo the deprecation message
+	exp := shortHFlagDeprecated + res.Stdout()
+	res = icmd.RunCmd(run("-D", "helloworld", "-h"))
+	res.Assert(t, icmd.Expected{
+		ExitCode: 0,
+		// This should be identical to the --help case above
+		Out: exp,
+		Err: icmd.None,
+	})
 }
 
 // TestRunGoodSubcommand ensures correct behaviour when running a valid plugin with a subcommand