Add support for --version flag

[nmiyake]

Fixes #282

[nmiyake]

Implementation of proposal at #282 (comment).

Looking for feedback on over-all approach. If it looks reasonable to be accepted, then I can update PR to address any implementation issue and add documentation.

[n10v]

Thank you for your contribution! It's really good!
Just fix the two things I wrote below.

I'm also interested in opinion of @eparis about this PR.

[n10v]

Why do we need here .Use?
What do you think about printing only a version? Just {{.Version}}.

[nmiyake]

Can certainly do that as well, but for many applications the version output takes the form {{appName}} version {{version}}, so thought that it would be a sensible default. Examples:

➜ go version
go version go1.9.2 darwin/amd64
➜ git --version
git version 2.6.3

[n10v]

Okay. But then use .Name instead of .Use, because .Use can be in format command [path] [flags].

[nmiyake]

Done

[n10v]

Correct to // InitDefaultVersionFlag ....

[nmiyake]

Done

[n10v]

One more thing I want to discuss is, if it's better to add version subcommand instead of flag. Many cobra applications (at least hugo, docker, gopherjs, rclone) use namely subcommand.

[dnephin]

docker has both, --version is for a short single line version (as it is in this PR), and the version subcommand has a multi-line output with a bunch of versions for both client and server.

A default version subcommand would be difficult because it would break any applications that accept only positional args and no subcommands.

[nmiyake]

Agree that version subcommand makes sense as well -- common applications like docker and git support both:

➜ git version
git version 2.6.3
➜ git --version
git version 2.6.3
➜ docker version
Client:
 Version:      17.11.0-ce
 API version:  1.34
 Go version:   go1.8.4
 Git commit:   1caf76c
 Built:        Mon Nov 20 18:30:18 2017
 OS/Arch:      darwin/amd64

Server:
 Version:      17.11.0-ce
 API version:  1.34 (minimum version 1.12)
 Go version:   go1.8.5
➜ docker --version
Docker version 17.11.0-ce, build 1caf76c

@dnephin agree with the back-compat concern in the general case, but this can be mitigated by only adding the command if the root command has no subcommands -- this is the same behavior used for the help command.

I've updated the PR to add the subcommand behavior as well.

[nmiyake]

Added support for version subcommand. Behavior is as follows:

• Like help, if the root command has no subcommands, this subcommand is not added
• For backcompat, the subcommand is only added if the Version property is non-empty
• For backcompat, if a version subcommand has already been added, the default one will not be added

The one point of customization that I think might be nice to expose is the ability to remove the version subcommand entirely if it is not wanted (for example, if the author only wants the flag but not the subcommand). Right now, I don't believe this is possible for help either. Will file a separate issue to discuss that.

[nmiyake]

@BoGeM what are the next steps here for discussion/deciding a path forward?

[n10v]

The question is, should we add both subcommand and version. There is a lot of question to implemented version subcommand (e.g. is there a possibility to use only flag or only subcommand?), so IMHO it's better to leave only version flag for now.

[nmiyake]

Makes sense -- also agree given that users can always add a version command themselves, whereas the --version flag implementation requires library-level support to be done properly. Will update PR to be only flag again.

[n10v]

--version flag implementation requires library-level support to be done properly

Actually, that is also not so hard to implement it by yourself, but having already integrated --version flag is a nice and convenient feature IMHO.

[nmiyake]

Updated PR to be flag implementation only

[n10v]

Okay. The code and tests are great, so I would like to merge it.

[n10v]

Thank you for your contribution!

[benkeil]

The current implementation does not work with the default run method (like usage).

rootCmd := &cobra.Command{Use: "root", Version: "1.0.0"}
fmt.Println(executeCommand(rootCmd, "--version"))

[nmiyake]

@benkeil I believe this is expected behavior. Per https://github.com/spf13/cobra/blob/master/command.go#L679, if a command isn't runnable, the expected behavior is to run the "help" command. In your example above, rootCmd is not runnable, so attempting to run the command will always execute the "help" behavior.

If you make the root command runnable by providing a Run or RunE function:

rootCmd := &Command{Use: "root", Version: "1.0.0", Run: func(cmd *Command, args []string){}}

Then the --version flag will work as expected. The same is true if you add any subcommands to rootCmd.

[benkeil]

That’s right, but then the —version flag makes no sense, because I must always implement it by myself.

[n10v]

@benkeil No, you must not. The version flag works only if you have Run function in your command. In Run function normally you have logic of your command/application, not implementation of version flag.

[nmiyake]

Actually, after some further testing locally, I think I understand the concern -- if an application has a root command that has no Run defined but does have subcommands, the --version flag does not work, when I would expect it to actually work in that case. Fix is fairly trivial -- will put up PR with fix and updated test for review/comment.

[nmiyake]

PR is at #595. This fix should work for the standard case. I'm not sure if there are any concerns for cases where a non-root (intermediate) command has a non-blank version, but the linked PR properly encapsulates the logic that I meant to implement for this flag.

[benkeil]

Thank you.