Skip to content
This repository has been archived by the owner on May 16, 2018. It is now read-only.

Zend_Console_Getopt: Missing required parameter consumes next option as its parameter value #454

Merged
merged 4 commits into from
Nov 13, 2014

Conversation

a-lucas
Copy link
Contributor

@a-lucas a-lucas commented Nov 3, 2014

Hello here,

Following up on bug #377 (comment) ,
I created an easy fix for the moment.

I wanted to make the function checkRequiredArguments private, and automatically call it at the end of teh parse() function, but this breaks few tests because these tests assume a required value can be null.

2 tests are added :

testVerifyRequiredArgument() test the public function verifyRequiredArguments()

and testEmptyRequiredOption() shows the correct behavior.

@@ -730,6 +730,33 @@ public function parse()
$this->_parsed = true;
return $this;
}

public function checkRequiredArguments(){
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check the http://framework.zend.com/manual/1.12/en/coding-standard.coding-style.html

  • newline before method opening curly brace
  • space after if
  • space before statement opening curly brace
  • ...

@froschdesign
Copy link
Member

@a-lucas
Please add to each commit a clear and useful subject.

@froschdesign
Copy link
Member

@a-lucas
Sorry, but I can not find a signed CLA.

@a-lucas
Copy link
Contributor Author

a-lucas commented Nov 4, 2014

@froschdesign
I'm sorry for the unclear commit msg, it seems like I can't edit it. Can I?
I will get the CLA printed and scanned for you today.
I'll get the coding style fixed for you too.

@weierophinney
Copy link
Member

@froschdesign I've just now processed his CLA; all set to merge.

froschdesign added a commit that referenced this pull request Nov 13, 2014
Zend_Console_Getopt: Missing required parameter consumes next option as its parameter value
@froschdesign froschdesign merged commit dae96bd into zendframework:master Nov 13, 2014
@froschdesign
Copy link
Member

@a-lucas
Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants