Skip to content

Conversation

@eparis
Copy link
Collaborator

@eparis eparis commented Apr 3, 2015

The special case code to handle a runnable root command had some
problems. It was noticed that if you created a runnable root and a
subcommand. And the subcommand was then executed with both a valid and
invalid flag, the error message was about the valid flag being invalid.
For example

./command subcommand --goodflag=10 --badflag=10

Would fail and tell you that --goodflag was an invalid flag. Instead if
we just do away with the special Command.execute() for the root command
the parser for subcommand is what prints the error and it gets it
right...

@eparis
Copy link
Collaborator Author

eparis commented Apr 3, 2015

@deads2k

#79

What do you think about solving the above like this?

@deads2k
Copy link
Contributor

deads2k commented Apr 6, 2015

No objection. I was just trying for minimal change.

command.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stripping leading empty arguments? Is this related to the bad flag message problem or just something else you found on the way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It related to the fact that root commands have special code to ignore it later. So I'm just ripping them out for all commands.

@deads2k
Copy link
Contributor

deads2k commented Apr 6, 2015

It's difficult to read the diff and determine complete fidelity of error reporting with the existing code, but the new version is more readable.

lgtm.

eparis added 2 commits April 6, 2015 15:42
The special case code to handle a runnable root command had some
problems.  It was noticed that if you created a runnable root and a
subcommand.  And the subcommand was then executed with both a valid and
invalid flag, the error message was about the valid flag being invalid.
For example

./command subcommand --goodflag=10 --badflag=10

Would fail and tell you that --goodflag was an invalid flag. Instead if
we just do away with the special Command.execute() for the root command
the parser for subcommand is what prints the error and it gets it
right...
@eparis eparis force-pushed the subcommand-invalid-flag branch from 901b899 to bd0f8a8 Compare April 6, 2015 19:49
eparis added a commit that referenced this pull request Apr 6, 2015
Stop special casing runnable root commands
@eparis eparis merged commit 29e27b1 into spf13:master Apr 6, 2015
@eparis eparis deleted the subcommand-invalid-flag branch August 12, 2015 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants