Making boolean switches behave.

markstory · markstory · commit 10090696a019 · 2010-10-13T23:18:26.000-04:00
Boolean switches always show up in the parsed options.  When left undefined they insert a false, and when included they insert a true.  This makes working with them require less checks.
diff --git a/cake/console/console_option_parser.php b/cake/console/console_option_parser.php
@@ -445,8 +445,15 @@ public function parse($argv, $command = null) {
 			}
 		}
 		foreach ($this->_options as $option) {
-			if ($option->defaultValue() !== null && !isset($params[$option->name()]) && !$option->isBoolean()) {
-				$params[$option->name()] = $option->defaultValue();
+			$name = $option->name();
+			$isBoolean = $option->isBoolean();
+			$default = $option->defaultValue();
+	
+			if ($default !== null && !isset($params[$name]) && !$isBoolean) {
+				$params[$name] = $default;
+			}
+			if ($isBoolean && !isset($params[$name])) {
+				$params[$name] = false;
 			}
 		}
 		return array($params, $args);
diff --git a/cake/console/libs/shell.php b/cake/console/libs/shell.php
@@ -331,7 +331,7 @@ public function runCommand($command, $argv) {
 		if (($isTask || $isMethod || $isMain) && $command !== 'execute' ) {
 			$this->startup();
 		}
-		if (isset($this->params['help'])) {
+		if (!empty($this->params['help'])) {
 			return $this->out($this->OptionParser->help($command));
 		}
 
diff --git a/cake/console/libs/testsuite.php b/cake/console/libs/testsuite.php
@@ -104,7 +104,9 @@ protected function parseArgs() {
  */
 	protected function runnerOptions() {
 		$options = array();
-		foreach ($this->params as $param => $value) {
+		$params = $this->params;
+		unset($params['help']);
+		foreach ($params as $param => $value) {
 			$options[] = '--' . $param;
 			if (is_string($value)) {
 				$options[] = $value;
diff --git a/cake/tests/cases/console/console_option_parser.test.php b/cake/tests/cases/console/console_option_parser.test.php
@@ -28,7 +28,7 @@ class ConsoleOptionParserTest extends CakeTestCase {
  * @return void
  */
 	function testDescription() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$result = $parser->description('A test');
 
 		$this->assertEquals($parser, $result, 'Setting description is not chainable');
@@ -44,7 +44,7 @@ function testDescription() {
  * @return void
  */
 	function testEpilog() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$result = $parser->epilog('A test');
 
 		$this->assertEquals($parser, $result, 'Setting epilog is not chainable');
@@ -60,7 +60,7 @@ function testEpilog() {
  * @return void
  */
 	function testAddOptionReturnSelf() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$result = $parser->addOption('test');
 		$this->assertEquals($parser, $result, 'Did not return $this from addOption');
 	}
@@ -71,12 +71,12 @@ function testAddOptionReturnSelf() {
  * @return void
  */
 	function testAddOptionLong() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('test', array(
 			'short' => 't'
 		));
 		$result = $parser->parse(array('--test', 'value'));
-		$this->assertEquals(array('test' => 'value'), $result[0], 'Long parameter did not parse out');
+		$this->assertEquals(array('test' => 'value', 'help' => false), $result[0], 'Long parameter did not parse out');
 	}
 
 /**
@@ -85,12 +85,12 @@ function testAddOptionLong() {
  * @return void
  */
 	function testAddOptionLongEquals() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('test', array(
 			'short' => 't'
 		));
 		$result = $parser->parse(array('--test=value'));
-		$this->assertEquals(array('test' => 'value'), $result[0], 'Long parameter did not parse out');
+		$this->assertEquals(array('test' => 'value', 'help' => false), $result[0], 'Long parameter did not parse out');
 	}
 
 /**
@@ -99,19 +99,19 @@ function testAddOptionLongEquals() {
  * @return void
  */
 	function testAddOptionDefault() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('test', array(
 			'default' => 'default value',
 		));
 		$result = $parser->parse(array('--test'));
-		$this->assertEquals(array('test' => 'default value'), $result[0], 'Default value did not parse out');
+		$this->assertEquals(array('test' => 'default value', 'help' => false), $result[0], 'Default value did not parse out');
 		
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('test', array(
 			'default' => 'default value',
 		));
 		$result = $parser->parse(array());
-		$this->assertEquals(array('test' => 'default value'), $result[0], 'Default value did not parse out');
+		$this->assertEquals(array('test' => 'default value', 'help' => false), $result[0], 'Default value did not parse out');
 	}
 
 /**
@@ -120,12 +120,32 @@ function testAddOptionDefault() {
  * @return void
  */
 	function testAddOptionShort() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('test', array(
 			'short' => 't'
 		));
 		$result = $parser->parse(array('-t', 'value'));
-		$this->assertEquals(array('test' => 'value'), $result[0], 'Short parameter did not parse out');
+		$this->assertEquals(array('test' => 'value', 'help' => false), $result[0], 'Short parameter did not parse out');
+	}
+
+/**
+ * test adding and using boolean options.
+ *
+ * @return void
+ */
+	function testAddOptionBoolean() {
+		$parser = new ConsoleOptionParser('test', false);
+		$parser->addOption('test', array(
+			'boolean' => true,
+		));
+
+		$result = $parser->parse(array('--test', 'value'));
+		$expected = array(array('test' => true, 'help' => false), array('value'));
+		$this->assertEquals($expected, $result);
+		
+		$result = $parser->parse(array('value'));
+		$expected = array(array('test' => false, 'help' => false), array('value'));
+		$this->assertEquals($expected, $result);
 	}
 
 /**
@@ -134,13 +154,13 @@ function testAddOptionShort() {
  * @return void
  */
 	function testAddOptionMultipleShort() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('test', array('short' => 't'))
 			->addOption('file', array('short' => 'f'))
 			->addOption('output', array('short' => 'o'));
 
 		$result = $parser->parse(array('-o', '-t', '-f'));
-		$expected = array('file' => true, 'test' => true, 'output' => true);
+		$expected = array('file' => true, 'test' => true, 'output' => true, 'help' => false);
 		$this->assertEquals($expected, $result[0], 'Short parameter did not parse out');
 
 		$result = $parser->parse(array('-otf'));
@@ -153,13 +173,13 @@ function testAddOptionMultipleShort() {
  * @return void
  */
 	function testMultipleOptions() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('test')
 			->addOption('connection')
 			->addOption('table', array('short' => 't'));
 
 		$result = $parser->parse(array('--test', 'value', '-t', '--connection', 'postgres'));
-		$expected = array('test' => 'value', 'table' => true, 'connection' => 'postgres');
+		$expected = array('test' => 'value', 'table' => true, 'connection' => 'postgres', 'help' => false);
 		$this->assertEquals($expected, $result[0], 'multiple options did not parse');
 	}
 
@@ -186,12 +206,12 @@ function testAddOptions() {
  * @return void
  */
 	function testOptionWithBooleanParam() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('no-commit', array('boolean' => true))
 			->addOption('table', array('short' => 't'));
 
 		$result = $parser->parse(array('--table', 'posts', '--no-commit', 'arg1', 'arg2'));
-		$expected = array(array('table' => 'posts', 'no-commit' => true), array('arg1', 'arg2'));
+		$expected = array(array('table' => 'posts', 'no-commit' => true, 'help' => false), array('arg1', 'arg2'));
 		$this->assertEquals($expected, $result, 'Boolean option did not parse correctly.');
 	}
 
@@ -201,7 +221,7 @@ function testOptionWithBooleanParam() {
  * @expectedException InvalidArgumentException
  */
 	function testOptionThatDoesNotExist() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('no-commit', array('boolean' => true));
 
 		$result = $parser->parse(array('--fail', 'other'));
@@ -214,11 +234,11 @@ function testOptionThatDoesNotExist() {
  * @return void
  */
 	function testOptionWithChoices() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('name', array('choices' => array('mark', 'jose')));
 		
 		$result = $parser->parse(array('--name', 'mark'));
-		$expected = array('name' => 'mark');
+		$expected = array('name' => 'mark', 'help' => false);
 		$this->assertEquals($expected, $result[0], 'Got the correct value.');
 
 		$result = $parser->parse(array('--name', 'jimmy'));
@@ -230,7 +250,7 @@ function testOptionWithChoices() {
  * @return void
  */
 	function testPositionalArgument() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$result = $parser->addArgument('name', array('help' => 'An argument'));
 		$this->assertEquals($parser, $result, 'Should returnn this');
 	}
@@ -241,7 +261,7 @@ function testPositionalArgument() {
  * @return void
  */
 	function testPositionalArgOverwrite() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addArgument('name', array('help' => 'An argument'))
 			->addArgument('other', array('index' => 0));
 
@@ -256,7 +276,7 @@ function testPositionalArgOverwrite() {
  * @return void
  */
 	function testParseArgumentTooMany() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addArgument('name', array('help' => 'An argument'))
 			->addArgument('other');
 
@@ -274,7 +294,7 @@ function testParseArgumentTooMany() {
  * @return void
  */
 	function testPositionalArgNotEnough() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addArgument('name', array('required' => true))
 			->addArgument('other', array('required' => true));
 
@@ -288,7 +308,7 @@ function testPositionalArgNotEnough() {
  * @return void
  */
 	function testPositionalArgWithChoices() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addArgument('name', array('choices' => array('mark', 'jose')))
 			->addArgument('alias', array('choices' => array('cowboy', 'samurai')))
 			->addArgument('weapon', array('choices' => array('gun', 'sword')));
@@ -306,7 +326,7 @@ function testPositionalArgWithChoices() {
  * @return void
  */
 	function testAddArguments() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$result = $parser->addArguments(array(
 			'name' => array('help' => 'The name'),
 			'other' => array('help' => 'The other arg')
@@ -323,7 +343,7 @@ function testAddArguments() {
  * @return void
  */
 	function testSubcommand() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$result = $parser->addSubcommand('initdb', array(
 			'help' => 'Initialize the database'
 		));
@@ -336,7 +356,7 @@ function testSubcommand() {
  * @return void
  */
 	function testAddSubcommands() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$result = $parser->addSubcommands(array(
 			'initdb' => array('help' => 'Initialize the database'),
 			'create' => array('help' => 'Create something')
@@ -589,7 +609,7 @@ function testBuildFromArray() {
  * @return void
  */
 	function testParsingWithSubParser() {
-		$parser = new ConsoleOptionParser();
+		$parser = new ConsoleOptionParser('test', false);
 		$parser->addOption('primary')
 			->addArgument('one', array('required' => true, 'choices' => array('a', 'b')))
 			->addArgument('two', array('required' => true))
@@ -606,7 +626,12 @@ function testParsingWithSubParser() {
 			));
 		
 		$result = $parser->parse(array('--secondary', '--fourth', '4', 'c'), 'sub');
-		$expected = array(array('secondary' => true, 'fourth' => '4'), array('c'));
+		$expected = array(array(
+			'secondary' => true,
+			'fourth' => '4',
+			'help' => false,
+			'verbose' => false,
+			'quiet' => false), array('c'));
 		$this->assertEquals($expected, $result, 'Sub parser did not parse request.');
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -445,8 +445,15 @@ public function parse($argv, $command = null) {`
`445`	`445`	`}`
`446`	`446`	`}`
`447`	`447`	`foreach ($this->_options as $option) {`
`448`		`- if ($option->defaultValue() !== null && !isset($params[$option->name()]) && !$option->isBoolean()) {`
`449`		`- $params[$option->name()] = $option->defaultValue();`
	`448`	`+ $name = $option->name();`
	`449`	`+ $isBoolean = $option->isBoolean();`
	`450`	`+ $default = $option->defaultValue();`
	`451`	`+`
	`452`	`+ if ($default !== null && !isset($params[$name]) && !$isBoolean) {`
	`453`	`+ $params[$name] = $default;`
	`454`	`+ }`
	`455`	`+ if ($isBoolean && !isset($params[$name])) {`
	`456`	`+ $params[$name] = false;`
`450`	`457`	`}`
`451`	`458`	`}`
`452`	`459`	`return array($params, $args);`
Original file line number	Diff line number	Diff line change
`@@ -331,7 +331,7 @@ public function runCommand($command, $argv) {`
`331`	`331`	`if (($isTask \|\| $isMethod \|\| $isMain) && $command !== 'execute' ) {`
`332`	`332`	`$this->startup();`
`333`	`333`	`}`
`334`		`- if (isset($this->params['help'])) {`
	`334`	`+ if (!empty($this->params['help'])) {`
`335`	`335`	`return $this->out($this->OptionParser->help($command));`
`336`	`336`	`}`
`337`	`337`