Unknown flag parsing is broken from v2

[MrNaif2018]

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)
	}
}

[dearchap]

@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]

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]

@MrNaif2018 Check #2074

[MrNaif2018]

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

[dearchap]

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

[dearchap]

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

[MrNaif2018]

Really sorry for not replying! Now updated to the latest code, and if I do e.g.
./testcli -w test command arg1 arg2 --arg3 true --arg4 false
it would still tell
Incorrect Usage: flag provided but not defined: -arg3
And for the second test case of ./testcli -w test command arg1 arg2 -- --arg3 true --arg4 false
the output changed in a way that it no longer lists -- as a separate entry in array

To be honest in those months I got used to passing unknown flags after --, and I am fine with it. It just needs to be documented somewhere and it's all good.
If someone like me is doing the parsing and wants to convert unknown flags into keyword params, here's the code I use (JSON-RPC wrapper):

sl := args.Slice()[1:]
var params []interface{}
keyParams := map[string]interface{}{
    ...
}
i := 0
for i < len(sl) {
    if strings.HasPrefix(sl[i], "--") {
        if i+1 >= len(sl) {
            exitErr("Error: missing value for flag " + sl[i])
        }
        keyParams[sl[i][2:]] = sl[i+1]
        i += 1
    } else {
        params = append(params, sl[i])
    }
    i += 1
}
params = append(params, keyParams)