@jasinner can you add some text explaining what you did and why? For example, is this just going to show the usage stuff w/o the full options list? And why did you change the exit code from non-zero to zero? Giving people some rationale behind your PR will help move it along.

I agree that the current stuff is lacking and I'm not sure why the more verbose stuff was removed.
For example:

$ docker build
docker: "build" requires 1 argument. See '/root/docker/bundles/1.5.0-dev/binary/docker build --help'.

isn't nearly as useful as what we used to show, which was the full --help output.

@duglin This change prints a short usage output, with the format of the command, when incorrect arguments are passed to a command. Like you said, currently at the moment the usage output is not very friendly as it only shows the number of argument required. This change not only shows that output, but the format of the command passed into the flags.subCmd method.

If users want the full, verbose output of the the flag.Usage command they can call --help.

I agree that the exit code on shortUsage should be 1. I will change it on Monday if required.

@jasinner yes please :)

Hi guys, I added some more commits. Please review.
When running 'docker run' without arguments output should be like this:

[jshepher@jshepher docker]$ bundles/1.5.0-dev/binary/docker-1.5.0-dev run 
docker: "run" requires a minimum of 1 argument. See 'bundles/1.5.0-dev/binary/docker-1.5.0-dev run --help'.

Usage: docker run IMAGE [COMMAND] [ARG...]

Run a command in a new container

When done, please merge your commits down to one.

@jasinner ping

@duglin The file utils/flags.go was removed from upstream master, so it was removed from my commit.
I did as you suggested, and called ShortUsage from the Usage method.
I think I managed to squash my commits down to 1 now too.

@jasinner I think you lost your "signed-off by" stuff during your squashing. Try doing:

git -s --amend --no-edit

and then pushing again to see if that fixes it.
Other than that I think it looks good

@duglin added signed-off by

thanks!
LGTM
/cc @tiborvass @LK4D4

As is here is the output from docker save:

docker: "save" requires a minimum of 1 argument. See 'docker save --help'.

Usage: docker save IMAGE [IMAGE...]

Save an image(s) to a tar archive (streamed to STDOUT by default)

And the output of docker save --help is litterally that same message minus the first line.
Can we get rid of the See 'docker save --help'. part, since it's the same exact output anyway?

Passing --help to save is not the same output, it shows all the arguments.

[jshepher@jshepher docker]$ bundles/1.5.0-dev/binary/docker-1.5.0-dev save
docker: "save" requires a minimum of 1 argument. See 'bundles/1.5.0-dev/binary/docker-1.5.0-dev save --help'.

Usage: docker save IMAGE [IMAGE...]

Save an image(s) to a tar archive (streamed to STDOUT by default)

with --help

[jshepher@jshepher docker]$ bundles/1.5.0-dev/binary/docker-1.5.0-dev save --help

Usage: docker save IMAGE [IMAGE...]

Save an image(s) to a tar archive (streamed to STDOUT by default)

  --help=false       Print usage
  -o, --output=      Write to an file, instead of STDOUT

Hi @duglin and @GordonTheTurtle I run gofmt, and attempted to fix up the signature. I hope I got everything correct, but let me know if there is anything outstanding. Jason

@jasinner you'll need to squash your commits down to one. That should make Gordon happy, assuming that one commit is signed.

ping @duglin I see this PR was updated, PTAL

@jasinner it looks good. Sorry for not mentioning this earlier, but I think we need a few testcases added. In particular, I think ones that:

• test that the short usage is shown on a missing param (just short, not full help)
• test that we get the correct exit code in both short and normal help output

@duglin do you maybe want to help with the test cases so we can get this into 1.7 or @jasinner do you think you will be able to make these changes

@jfrazelle I'll work on the testcases either tonight or tomorrow. if @jasinner doesn't respond by, say, Sunday maybe we can grab both sets of changes.

Or do you need it sooner?

thanks SGTM

On Fri, May 22, 2015 at 5:01 PM, Doug Davis notifications@github.com
wrote:

@jfrazelle https://github.com/jfrazelle I'll work on the testcases
either tonight or tomorrow. if @jasinner https://github.com/jasinner
doesn't respond by, say, Sunday maybe we can grab both sets of changes.

—
Reply to this email directly or view it on GitHub
#11858 (comment).

@jasinner I just did PR #13428 to continue this PR - mainly to add new testcases. I did however find some oddities in the way some things were done for some commands, so I fixed those too. Now we should be consistent. Hope you're ok with this. We're coming up on the deadline for v1.7 and we wanted to make sure this made it in.

This has been merged on master as part of #13428. Closing! 😄

@vieux @jfrazelle according to #11858 (comment) you wanted this in the 1.7 release?

ill cherry pick it

@duglin thanks for finishing this off. Happy to see it get merged.

On Sunday, 24 May 2015, Doug Davis notifications@github.com wrote:

@jasinner https://github.com/jasinner I just did PR #13428
#13428 to continue this PR -
mainly to add new testcases. I did however find some oddities in the way
some things were done for some commands, so I fixed those too. Now we
should be consistent. Hope you're ok with this. We're coming up on the
deadline for v1.7 and we wanted to make sure this made it in.

—
Reply to this email directly or view it on GitHub
#11858 (comment).