Autoparsing on get option causes later option rules not to trip

[kael-shipman]

Given this script:

$cmd = new \Commando\Command();

// Define an option
$cmd->option("t");

// If that option is a certain value, then define another option (this causes autoparsing)
if ($cmd['t'] == 'something') {
    // do something, like perhaps define other options
}

// Now define a required option. Because we've already parsed the options, the rules
// for this option won't cause an error when really they should.
$cmd->option("e")
    ->required()
    ->must(function($t) {
        return preg_match("/^[^0-9]+$/", $t);
    }); 

// Try to echo the required 'e' option. Script should not get to this point, but does,
// and also doesn't throw an error here.
echo $cmd['e'];

Expected Behavior

• With option 'e' not given: should show an error and stop execution when required called on option 'e'.
• With option 'e' given, but not matching regexp: should show an error and stop execution when must called on option 'e'.
• With option 'e' given and correct, should echo the value of option 'e'.

Actual Behavior

Does not show any errors at all, and outputs any value given for option 'e'.

[kael-shipman]

@nategood I'll take this and PR it to you if you'll accept the spec.

[NeoVance]

All of the options would be parsed again every time an option operator was called. The current library architecture does not allow for this without performance impact, I believe. You can manually call Commando's parse function at any time to execute options that were added after the initial parse.

When accessing the command as an array the parse is executed, and a flag is set to keep the execution from happening again.

[kael-shipman]

Ok, that's fair. It seems like it would be easy enough to unset the parsed flag when adding new options, though. Does that sound like a reasonable solution? Something like....

    public function __call($name, $arguments)
    {
        // ...
        $option = call_user_func_array(array($this, "_$name"), $arguments);
        $this->parsed = false;
        return $this;
    }

That way we can avoid incurring the parse penalty on every change, but we can make sure that new changes trigger future parsing in the same way that initial changes do.

[NeoVance]

The only problem with this idea, is that it will make validation and other rules like needs a nightmare to deal with. Idk

[NeoVance]

It would be up to the user to manage error handling, at that point. Are you talking about only on new options, or when modifying an existing option? I rather like the idea of the user controlling when parsing occurs better. There is already confusion with the behavior around parsing, and I think this will add to that, personally.

[kael-shipman]

Yes, maybe that's the way to go: remove auto-parsing entirely and state clearly in the documentation that you must call parse explicitly when you're done setting up options. This would allow the behavior that I'm seeking without incurring unnecessary performance penalties and without making it impossible to know whether and when a parse has occurred.

[NeoVance]

Right. I would want some input from @nategood on the subject. Either there needs to be a sane way to invalidate the currently parsed options and re-parse (automatically) or parsing should just be a manual step that is called explicitly as needed by the user.

I personally think the manual option is the best way to go. Automatic parsing in the current state of the library can cause a lot of headaches around rules/needs/must etc...in all but the most basic commands.

For instance I like to parse before setting a must, which allows me to dynamically assign rules for the current command based on the options that are in the command. Unfortunately I have to remember to A) make sure that all of the options are set up beforehand. B) make sure there is no other validation that could stop the script when parsing, and C) remember to re-parse with all validation in place later.