Several 'help' fixes #51
Conversation
c46be67 to
fd6b130
Compare
|
@fabianofranz This isn't passing the tests. Can you revisit it and make it pass. |
37b839e to
55a7e5d
Compare
|
@spf13 Good to go. |
| return true | ||
| } | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
Isn't this HasRunnableSubCommands() ? Why the change?
There was a problem hiding this comment.
Sorry, I wasn't aware of HasRunnableSubCommands(). Might have happened after I first created this PR. Reverted to the original behavior.
| @@ -370,6 +371,9 @@ func (c *Command) Find(arrs []string) (*Command, []string, error) { | |||
| // only accept a single prefix match - multiple matches would be ambiguous | |||
| if len(matches) == 1 { | |||
| return innerfind(matches[0], argsMinusX(args, argsWOflags[0])) | |||
| } else if len(matches) == 0 && len(args) > 0 && args[0] == "help" { | |||
| // special case help command | |||
| return innerfind(c, argsMinusX(append(args, "--help"), argsWOflags[0])) | |||
There was a problem hiding this comment.
So this is interesting. Before this change.
command help subcommand - works, shows help for subcommand
command subcommand help - fails, because 'help' is an unknown subsubcommand
Are we certain we want the later? (I personally like it)
There was a problem hiding this comment.
I changed this PR to revert that piece of code (on a separate commit so we don't loose it), since it may result in behavior not valid for every scenario.
The "help" string could be a valid argument that does not mean help. Using git add as an example:
git add help
Means I want to add the file called "help", which is perfectly plausible. I do not want the equivalent of git add --help here.
|
i still can't figure out how to write a test that is somehow 'wrong' can you help me figure out and example fabio? |
| @@ -798,7 +806,7 @@ func (c *Command) Flags() *flag.FlagSet { | |||
| c.flagErrorBuf = new(bytes.Buffer) | |||
| } | |||
| c.flags.SetOutput(c.flagErrorBuf) | |||
| c.PersistentFlags().BoolVarP(&c.helpFlagVal, "help", "h", false, "help for "+c.Name()) | |||
There was a problem hiding this comment.
So the change here is that you are calling c.mergePersistentFlags() here. (And then making sure mergePersistentFlags() won't bug later)
So is there a real bug that somehow we missed a meaningful and necessary c.mergePersistendFlags() call somewhere else? I really wish I knew how to trigger it...
Need to make sure
--helpis registered before callingpflagswhich tries to handle the help flag itself.Find()must be aware of the special commandhelpand consider the context it's being called in order to display the help for the proper command (instead of root).