Bug in v3: Count is off by 1

[Skeeve]

Checklist

• [ x] Are you running the latest v3 release? The list of releases is here.
• [ x] Did you check the manual for your release? The v3 manual is here.
• [ x] Did you perform a search about this feature? Here's the GitHub guide about searching.

What problem did you observe?

When using cmd.Count("verbose") on my bool flag, it's value is always one too high

Test Program

package main

import (
	"context"
	"fmt"
	"os"

	"github.com/urfave/cli/v3"
)

func main() {
	cmd := &cli.Command{
		Name:                   "test",
		Usage:                  "Tool for testing",
		HideHelp:               true,
		UseShortOptionHandling: true,
		Flags: []cli.Flag{
			&cli.BoolFlag{
				Name:    "verbose",
				Aliases: []string{"v"},
				Usage:   "Be verbose",
			},
		},
		Action: tester,
	}

	if err := cmd.Run(context.Background(), []string{"prog", "-v"}); err != nil {
		fmt.Fprintln(os.Stderr, err)
		os.Exit(1)
	}
}


func tester(_ context.Context, cmd *cli.Command) (err error) {
	fmt.Printf(`verbosecount: %d`, cmd.Count("verbose"))
	return
}

Also on playground: https://play.golang.com/p/om6L1TD_6s5

[dearchap]

@Skeeve This looks very much like #1737 which was fixed in v2 but not ported to v3. Think you can make a PR ?

[Skeeve]

Think you can make a PR ?

I doubt it :(

I THINK the issue might be due to these two lines:

cli/flag_impl.go

Line 169 in c4cd0a5

f.count++

and

cli/flag_bool.go

Line 66 in c4cd0a5

*b.count = *b.count + 1

But I'm not so deep in understanding the source that I can tell for sure.

I'm also wondering: It seems, ALL flags are counted. Why is Count() only available for BoolfFlag? Sure, it's the only Flag wit Countable set, but what's the purpose hiding this info from the user, when it's already available?

[dearchap]

No one has requested count for non-bool flags. Not sure when I can get around to a fix though.

[Skeeve]

No one has requested count for non-bool flags. Not sure when I can get around to a fix though.

That "fix" (if it is a fix) should be easy. Just get rid of "if Countable" in the Count function.

[dearchap]

@Skeeve Would you like to submit a PR with the change ? I'll get glad to review it.

[Skeeve]

I would like to, but I can't. I have the slight feeling there is more wrong to it, than I first thought.

Here are my observations.

1. Count() returns the flag's countmember
2. Flag has baseBoolConfig which has a member Count
3. This Count is never incremented nor returned. At least I was unable to find it and I assume this should have been done.

I created a test command for counting bools.

I also thought about counting all flags. I think it only makes sense for the …SliceFlags as all the others usually are not used more than once.