Skip to content
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

cli.Command: Argument order is changed depending on the flag set #355

Closed
brenol opened this issue Apr 20, 2016 · 5 comments
Closed

cli.Command: Argument order is changed depending on the flag set #355

brenol opened this issue Apr 20, 2016 · 5 comments
Assignees
Labels
kind/bug describes or fixes a bug
Milestone

Comments

@brenol
Copy link

brenol commented Apr 20, 2016

Hello!

I'm not sure if this is the expected behavior, however the following code:

package main

import (
    "fmt"
    "os"

    "github.com/codegangsta/cli"
)

func main() {
    app := cli.NewApp()

    app.Commands = []cli.Command{
        cli.Command{
            Name:  "update",
            Usage: "update a report",
            Flags: []cli.Flag{
                cli.StringFlag{
                    Name:  "filename",
                    Usage: "filename",
                },
            },
            Action: func(c *cli.Context) {
                fmt.Println(c.Args())
            },
        },
    }

    app.Run(os.Args)
}

Runs differently depending on the position of the arguments passed:

$ ./main update Arg1 Arg2 Arg3 --filename test.json
[Arg1 Arg2 Arg3]

$ ./main update Arg1 --filename test.json Arg2 Arg3
[Arg2 Arg3 Arg1]

Expected behavior for me would be that when running the second command, the following should output:

$ ./main update Arg1 --filename test.json Arg2 Arg3
[Arg1 Arg2 Arg3]

It seems that the fact that the flag is in the middle is breaking the Args() order.

Thanks

@jszwedko
Copy link
Contributor

@brenol definitely a bug, thank you for the report!

@jszwedko jszwedko added the kind/bug describes or fixes a bug label Apr 24, 2016
@jszwedko
Copy link
Contributor

This appears to be due to the argument reordering that is done to try to trick the stdlib flag parser to handling commands that have arguments specified before flags (the stdlib flag library stops flag parsing as soon as it encounters the first argument). At first blush, I think supporting this behavior would require much more complicated preprocessing of flags, but I'm open to reviewing a PR addressing this. I'm almost more inclined to remove the re-ordering feature as it appears to only work in very specific circumstances (if you specify all of your arguments before you specify flags).

https://github.com/codegangsta/cli/blob/master/command.go#L104-L116

@jszwedko jszwedko added this to the v2.0.0 milestone May 3, 2016
@jszwedko
Copy link
Contributor

jszwedko commented May 3, 2016

I think this supersedes #103 too -- my opinion is to remove the reordering.

@danslimmon
Copy link

Removing the ordering seems like a fine answer. I looked at this for a while today, and there's really no way to fix it without, like you said @jszwedko, a whole bunch of complicated flag preprocessing.

jszwedko added a commit that referenced this issue May 7, 2016
This was introduced by #36, but only worked in the specific case of all
arguments being passed before all flags. If the user mixed them, they
ended up with odd parsing behavior where the arguments were reordered
(causing #103 and #355).

Given the tradeoffs I think we should remove support for flag
reordering.

Fixes #103
Fixes #355
@codegangsta codegangsta added the status/claimed someone has claimed this issue label May 7, 2016
@jszwedko
Copy link
Contributor

Fixed by #398 (requires use of the v2 branch however as the change was not backwards compatible). Let me know what you think!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug describes or fixes a bug
Projects
None yet
Development

No branches or pull requests

4 participants