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

Different Args in "Action" and "Before" functions #92

Closed
igm opened this issue Jun 8, 2014 · 8 comments · Fixed by #170
Closed

Different Args in "Action" and "Before" functions #92

igm opened this issue Jun 8, 2014 · 8 comments · Fixed by #170
Assignees
Labels
kind/bug describes or fixes a bug

Comments

@igm
Copy link

igm commented Jun 8, 2014

I recently came across a problem when accessing Args from Context. I expected to get the same args in "Action" and "Before" functions, but "Action" has it's name at index 0 and arguments start from index 1, in "Before" arguments start from index 0.

An example to demonstrate: http://play.golang.org/p/CIrOdw3noq

Also looking at the examples https://github.com/codegangsta/cli#subcommands
I'd expect c.Args().First() to contain the first argument, not the command name itself. Is this desired behaviour, please?

@igm
Copy link
Author

igm commented Jun 9, 2014

It seems it is more inconsistent than that. If "Before" is nil, "Action" does not have its name at index 0.
example main.go: http://play.golang.org/p/dKDdOWqL8S

$ go run main.go foo 1 2 3 4
Args in Before: [1 2 3 4]
Args in Action: [foo 1 2 3 4]

$ go run main.go bar 1 2 3 4
Args in Action: [1 2 3 4]

@fatih
Copy link

fatih commented Jun 14, 2014

Confirmed, setting the "Before" function disables argument parsing of my defined flags. I removed Before functions completely.

@codegangsta codegangsta added the bug label Aug 2, 2014
@codegangsta
Copy link
Contributor

This is definitely a bug. Thanks for the report

@ghigt
Copy link
Contributor

ghigt commented Aug 2, 2014

This bug is due to these lines (#L211):

// Run default Action
if len(a.Commands) > 0 {
    a.Action(context)
} else {
    a.Action(ctx)
}

@codegangsta, I think we should remove this if statement. It is apparently not necessary.

EDIT: There is already a discussion about this issue in the responsible commit.

ghigt referenced this issue Aug 2, 2014
…ubcommands, display the CommandHelp instead of the SubcommandHelp
@codegangsta codegangsta added the status/claimed someone has claimed this issue label Jan 9, 2015
@jszwedko
Copy link
Contributor

jszwedko commented Jan 9, 2015

Addressed by #170

@cvik
Copy link

cvik commented Jan 27, 2015

Great, just found this bug as well and can confirm it. My research also found the lines above by ghigt.

The Ctx context haven't yet parsed the flags for the subcommand, but context context has and that one is used by the Before action, which makes it look like it works for Before, but not Action of the subcommand.

Any ETA when a fix can be merged to master?

@codegangsta codegangsta removed the status/claimed someone has claimed this issue label Jan 28, 2015
@jszwedko
Copy link
Contributor

@rawn merged in today, let me know if your issue persists.

@cvik
Copy link

cvik commented Feb 2, 2015

@jszwedko Works beautifully!

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

Successfully merging a pull request may close this issue.

6 participants