-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Description
My urfave/cli version is
v2.25.3
Checklist
- Are you running the latest v2 release? The list of releases is here.
- Did you check the manual for your release? The v2 manual is here
- Did you perform a search about this problem? Here's the GitHub guide about searching.
Dependency Management
- My project is using go modules.
Describe the bug
The value of .Count for a BoolFlag is impacted by the number of aliases defined for that flag.
To reproduce
Reproducible program:
package main
import (
"fmt"
"os"
"github.com/urfave/cli/v2"
)
func main() {
verbosity := 0
app := &cli.App{
Flags: []cli.Flag{
&cli.BoolFlag{
Name: "verbose",
Aliases: []string{"v"},
Count: &verbosity,
},
},
Action: func(c *cli.Context) error {
fmt.Printf("Running with verbosity: %d\n", verbosity)
return nil
},
}
app.Run(os.Args)
}Observed behavior
$ go run main.go -v
Running with verbosity: 2Expected behavior
I would expect the count to be incremented only once, i.e.:
$ go run main.go -v
Running with verbosity: 1Additional context
Beyond the initial value, things are incremented as expected:
$ go run main.go -v -v
Running with verbosity: 3Want to fix this yourself?
I'd be happy to, though I don't think I have enough context to implement a suitable solution. The trace is:
- When we
.ApplyaBoolFlagwe copy thecountpointer for each aliasLine 138 in dc77f3c
value := newBoolValue(f.Value, dest, count) - And when we
.Setthe flag we increment this countLine 38 in dc77f3c
*b.count = *b.count + 1
So when we run a command:
- We parse the flags
Line 158 in acbbbf2
set, err := c.parseFlags(&a, cCtx) - Which requires normalizing
Line 384 in acbbbf2
if err := normalizeFlags(c.Flags, set); err != nil { - When normalizing we copy for each alias we didn't see
Line 229 in acbbbf2
copyFlag(name, ff, set) - Which in turn sets the value
Line 199 in acbbbf2
_ = set.Set(name, ff.Value.String())
Consequently the flag is .Set once per alias, so the count is incremented once per alias.
Maybe flag types could implement the copying logic themselves?
Run go version and paste its output here
$ go version
go version go1.20.4 linux/amd64
Run go env and paste its output here
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/user/.local/share/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/.local/share/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.4"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/user/src/cli/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build3085526498=/tmp/go-build -gno-record-gcc-switches"