v2 bug: Do Not Make -v A Global Flag Or At Least Prevent Automatic Inclusion In Subcommands Of The Version Flag

[bonedaddy]

my urfave/cli version is

v2.1.1

Checklist

• Are you running the latest v2 release? The list of releases is here.

Dependency Management

• My project is using go modules.

Describe the bug

The -v flag is used as a shorthand for --version. For example if I have a binary called foo, foo -v can be used to print the version. Now lets say I have a subcommand bar with a verbose flag that has a v alias. Running this subcommand will cause a panic, with an error message indicating that the v flag is redefined. Because the version flag is reimported for all commands and subcommands.

To reproduce

Create a CLI binary that has a subcommand with any kind of flag that uses either v as the flag name, or as the flag alias. I would provide sample code but honestly im lazy and the project is closed-source lol

Observed behavior

Program crashes with a panic

Expected behavior

Allow me to invoke my -v aliased subcommands.

Additional context

Personally I don't think the -v aliased --version flag should be imported to subcommands. Really it should only be available when the CLI binary is invoked without any commands.

With a binary foo foo subcommand -v should not be invoking the --version flag. I can't think of anyone in the history of computers who would invoke -v with a subcommand to see the version of the CLI binary. It is significantly more keystrokes to get the version by foo subcommand -v than it is to get the version by foo -v.

Additionally it is extremely common for CLI binaries to use -v with subcommands to signify verbose output. That is actually how I discovered the bug because one of our subcommands is attempting to use -v to signify verbose output.

Currently the solution for this is to alias the verbose flag with vv instead of v, but I would really like to be able to use v because typically unix/linux CLI uses increasing counts of v (ie vv and vvv) to signify increasingly verbose logs.

Want to fix this yourself?

I would be willing to fix this myself but might need some guidance as to the part of the codebase to look at.

Run go version and paste its output here

go version go1.13.4 linux/amd64

Run go env and paste its output here

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/solidity/.cache/go-build"
GOENV="/home/solidity/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY="github.com/RTradeLtd/*"
GONOSUMDB="github.com/RTradeLtd/*"
GOOS="linux"
GOPATH="/home/solidity/go"
GOPRIVATE="github.com/RTradeLtd/*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/solidity/Code/RTradeLtd/TemporalX/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build521563016=/tmp/go-build -gno-record-gcc-switches"

[coilysiren]

There's a few things to unpack here, so I'm gonna go through them one by one

Running this subcommand will cause a panic, with an error message indicating that the v flag is redefined. Because the version flag is reimported for all commands and subcommands.

We should definitely overwrite rather than raising a panic here

With a binary foo foo subcommand -v should not be invoking the --version flag

There's a much nonzero chance that people are relying on foo subcommand -v outputting the version by default, so that behavior shouldn't change.

Currently the solution for this is to alias the verbose flag with vv instead of v, but I would really like to be able to use v because typically unix/linux CLI uses increasing counts of v (ie vv and vvv) to signify increasingly verbose logs.

See also #757 🙂

I would be willing to fix this myself but might need some guidance as to the part of the codebase to look at.

Offhand I don't know what area of the code to point you towards, but I would be very interested in a PR that changes -v flags on commands / subcommands to overwrite the existing version flag.

\ cc @urfave/cli for additional points of view here 🙏

[bonedaddy]

We should definitely overwrite rather than raising a panic here

Good idea I didn't realize this was possible

There's a much nonzero chance that people are relying on foo subcommand -v outputting the version by default, so that behavior shouldn't change.

Very true

See also #757 slightly_smiling_face

Thanks

Offhand I don't know what area of the code to point you towards, but I would be very interested in a PR that changes -v flags on commands / subcommands to overwrite the existing version flag.

Sounds good. If approved this will give me a fun thing to do on the plane ride back home tomorrow.

[Irioth]

There's a much nonzero chance that people are relying on foo subcommand -v outputting the version by default, so that behavior shouldn't change.

Version flag on subcommands doesn't work (at least in release v2.2.0)
I was quite surprised to see it attached to subcommand. So +1 for remove version flag from subcommands.