Skip to content

V3: unknown flag parsing is broken from v2: flag provided but not defined #2072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MrNaif2018 opened this issue Mar 9, 2025 · 6 comments
Labels
area/v3 relates to / is being considered for v3 status/triage maintainers still need to look into this status/waiting-for-response Waiting for response from original requester

Comments

@MrNaif2018
Copy link
Contributor

Hi! Using the latest released urfave/cli v3.0.0-beta1, I get a different behaviour from v2

V2:

alex@alex-mac testcli % ./testcli -w test command arg1 arg2 --arg3 true --arg4 false
&[command arg1 arg2 --arg3 true --arg4 false]
alex@alex-mac testcli % ./testcli -w test command arg1 arg2 -- --arg3 true --arg4 false
&[command arg1 arg2 -- --arg3 true --arg4 false]

V3:

alex@alex-mac testcli % ./testcli -w test command arg1 arg2 --arg3 true --arg4 false
Incorrect Usage: flag provided but not defined: -arg3

flag provided but not defined: -arg3
alex@alex-mac testcli % ./testcli -w test command arg1 arg2 -- --arg3 true --arg4 false
&{[command arg1 arg2 -- --arg3 true --arg4 false]}

I rely on unknown flags parsing to parse all by-name arguments to external JSONRPC server. There's SkipFlagParsing but it seems to disable all functionality in general
Is there some way to allow to parse unknown flags in Action still?

Attaching example code to test in V2 and V3

V2 code
package main

import (
	"fmt"
	"os"

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

func main() {
	app := cli.NewApp()
	app.Name = "testcli"
	app.Version = "1.0.0"
	app.HideHelp = true
	app.Usage = "Call RPC methods from console"
	app.UsageText = "testcli method [args]"
	app.EnableBashCompletion = true
	app.Flags = []cli.Flag{
		&cli.BoolFlag{
			Name:    "help",
			Aliases: []string{"h"},
			Usage:   "show help",
		},
		&cli.StringFlag{
			Name:     "wallet",
			Aliases:  []string{"w"},
			Usage:    "specify wallet",
			Required: false,
		},
	}
	app.Action = func(c *cli.Context) error {
		args := c.Args()
		fmt.Println(args)
		return nil
	}
	if err := app.Run(os.Args); err != nil {
		fmt.Println(err)
	}
}
V3 code
package main

import (
	"context"
	"fmt"
	"os"

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

func main() {
	app := &cli.Command{
		Name:                  "testcli",
		Version:               "1.0.0",
		HideHelp:              true,
		Usage:                 "Call RPC methods from console",
		UsageText:             "testcli method [args]",
		EnableShellCompletion: true,
		Flags: []cli.Flag{
			&cli.BoolFlag{
				Name:    "help",
				Aliases: []string{"h"},
				Usage:   "show help",
			},
			&cli.StringFlag{
				Name:     "wallet",
				Aliases:  []string{"w"},
				Usage:    "specify wallet",
				Required: false,
			},
		},
		Action: func(ctx context.Context, cmd *cli.Command) error {
			fmt.Println(cmd.Args())
			return nil
		},
	}
	if err := app.Run(context.Background(), os.Args); err != nil {
		fmt.Println(err)
	}
}
@MrNaif2018 MrNaif2018 added area/v3 relates to / is being considered for v3 status/triage maintainers still need to look into this labels Mar 9, 2025
@dearchap
Copy link
Contributor

@MrNaif2018 Thanks for the examples and the output. v3 has support for intermingling flags and positions arguments in any order, thats why you see the behaviour in first example in v3. The second example uses "--" and so all flag parsing stops there. Currently I'm rewriting the parser for v3. See #2074 . In general though "--" is the correct way to stop flag/arg parsing so if you can use that you should keep doing it. The SkipFlagParsing was added to overcome the deficiencies in the golang parser we have been using so far. I'm thinking of getting rid of that flag entirely. Let me know what you think.

@MrNaif2018
Copy link
Contributor Author

Yeah, I agree that -- is the correct POSIX way, I haven't decided if I can allow that. Technically it's a breaking change in my CLI, but I just tell users which commands to run for troubleshooting anyway (:

There only concern I see is, sometimes it is required to use -- twice, not sure why. I was looking at cobra lib, and they haven't even solved it in any version of their lib, kudos to you that it worked in general (:
e.g. see here warewulf/warewulf#253

And in cobra lib they have two useful settings (one implemented, one not implemented):
It allows to whitelist unknown flags: https://github.com/spf13/pflag/blob/5ca813443bd2a4d9f46a253ea0407d23b3790713/flag.go#L129
Which means we can tell it to ignore them and still continue parsing
and second one which is implemented only in some fork, they expose a method to return a list of unknown flags.

So that could help implement this workflow if user sets whitelist unknown flag + adds their custom parsing

@dearchap
Copy link
Contributor

@MrNaif2018 Check #2074

@MrNaif2018
Copy link
Contributor Author

If I install from this PR, it will resolve the issue of parsing flags right without --? Or what exactly would it change

@dearchap
Copy link
Contributor

Not really but you should see a consistent behavior when using "--". Use just "--" without SkipFlagParsing and see how far you can go.

@dearchap
Copy link
Contributor

@MrNaif2018 Whats the behaviour you see with the latest code ?

@dearchap dearchap added the status/waiting-for-response Waiting for response from original requester label Mar 31, 2025
@aphilas aphilas mentioned this issue Apr 23, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v3 relates to / is being considered for v3 status/triage maintainers still need to look into this status/waiting-for-response Waiting for response from original requester
Projects
None yet
Development

No branches or pull requests

2 participants