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

"Needs" behaviour #50

Open
phil043 opened this issue Jul 15, 2015 · 2 comments
Open

"Needs" behaviour #50

phil043 opened this issue Jul 15, 2015 · 2 comments

Comments

@phil043
Copy link

phil043 commented Jul 15, 2015

Needs behaves a bit strangely and should interact with "requires" but doesn't. It's actually acting as a defacto "requires". Even if a "needed" command isn't "required" Commando will still throw an exception if it is specified as needed by an unused argument.

E.g

$cmd
->option("a");

$cmd->option("b")
    ->needs("c")

$cmd->option("c");

The following command should run:

filename.php --a --b --c

Only this should fail

filename.php --a --b

But this also (incorrectly) fails:

filename.php --a

In the last one the "needs" clause says that "b needs c" even though c isn't a required argument and b wasn't even specified.

The fix in my local branch is to replace the current "needs" block in Command.php with the following:

    // See if our options have what they require
            foreach ($keyvals as $key => $value) {
                $option = $this->getOption($key);
                if(!is_null($option->getValue()) || $option->isRequired()){
                    $needs = $option->hasNeeds($this->options);
                    if ($needs !== true) {
                        throw new \InvalidArgumentException(
                            'Option "'.$option->getName().'" does not have required option(s): '.implode(', ', $needs)
                        );
                    }
                }
            }

But I'd like to know if the current behaviour of "needs" also acting as a defacto "requires" is actually the desired behaviour before I upload it and submit a pull request.

@mancioshell
Copy link

+1 for this behaviour

@klobyone
Copy link

Yup, just ran into this as well.

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

No branches or pull requests

3 participants