Automatic help when execute method not defined.

[dh3014]

my first concrete service looks like:

mb scheduled-job insert [arg1] [arg2] 

When I typed

mb

it automatically triggered HelpCommand and displayed help message, which is great since I then knew the application 'mb' has 2 commands 'scheduled-job' and 'help' as well as a brief description.

However when I further typed

 mb scheduled-job

it ends up showing nothing but a blank line. I trace the code found an ExecuteMethodNotDefinedException is thrown with no message, first I think I can add a helpful message when throwing this exception, second I thought it might be more consistent to catch this specific type of Exception in Application::runWithTry()

What I really want in the end, is the framework will show help message, with list of possible sub-commands. See if you think this is worth merging to the master branch.

If you agree then we can further discuss the detail a bit, my first glance is to add this feature in base Application::run(), one way is to catch ExecuteMethodNotDefinedException here then shows help message here if possible (if not possible, maybe just bubbles up the Exception so runWithTry() will display a helpful error message)

The other way is b4 we throw the Exception in Application::executeWrapper(), we check if showing help message is possible (sub-commands exist).

[c9s]

I like your proposal.

Actually that's included in our roadmap (throwing different exception to show corresponding help message).

The ExecuteMethodNotDefinedException is used in two case:

1. the developer forget to define the execute method , hence the command can't be executed. (that is, LogicException) and we should always throw the exception for this case. (or perhaps we can show a tutorial for the developer)
2. the command is a virtual command, it contains many sub-commands, so the execute method is not defined on purpose, in this case, we can do what you've suggested.

[dh3014]

I think case 1 is quite obvious.

Need some more details in case 2:

in CommandBase::executeWrapper(), current logic is like

if (!method_exists($this, 'execute'))
    throw new ExecuteMethodNotDefinedException($this);

We can make it

if (!method_exists($this, 'execute'))
    if ($this->hasCommands())
        $this->showHelpInstead();
    else
        throw new ExecuteMethodNotDefinedException($this);

The other way as mentioned earlier, we'll let ExecuteMethodNotDefinedException thrown as it was. But intercept it in Application::run() (will bubble up in no sub-commands possible)

try {
    $return = $last_cmd->executeWrapper($arguments);
} catch (ExecuteMethodNotFefinedException $e) {
    if ($last_cmd->hasCommands())
        $this->showHelpInstead();
    else
        throw $e;
}

Currently I prefer second position to add the logic in, since it's more consistent to place this logic in Application rather than in CommandBase, an intermediate exception interception is not that elegant, though.

[c9s]

Sounds great, I agree with the second solution. let's do it :)