feature #21228 [Console] Explicitly passed options without value (or empty) should remain empty (chalasr)
This PR was merged into the 3.3-dev branch.
Discussion
----------
[Console] Explicitly passed options without value (or empty) should remain empty
| Q | A
| ------------- | ---
| Branch? | master
| Bug fix? | yes
| New feature? | no
| BC breaks? | no
| Deprecations? | no
| Tests pass? | yes
| Fixed tickets | #21215 https://github.com/symfony/symfony/issues/11572 https://github.com/symfony/symfony/pull/12773
| License | MIT
| Doc PR | n/a (maybe look at updating the existing one)
This conserves empty values for options instead of returning their default values.
Code:
```php
// cli.php
$application = new Application();
$application
->register('echo')
->addOption('prefix', null, InputOption::VALUE_OPTIONAL, null, 'my-default')
->addArgument('value', InputArgument::REQUIRED)
->setCode(function ($input, $output) {
var_dump($input->getOption('prefix'));
});
$application->run();
```
Before:
![before](http://image.prntscr.com/image/157d9c6c054240da8b0dce54c9ce24d6.png)
After:
![after](http://image.prntscr.com/image/4aeded77f8084d3c985687fc8cc7b54e.png)
Commits
-------
80867422ac
[Console] Explicitly passed options without value (or empty) should remain empty
This commit is contained in:
commit
4aa9508ae3
@ -6,6 +6,62 @@ ClassLoader
|
|||||||
|
|
||||||
* The component is deprecated and will be removed in 4.0. Use Composer instead.
|
* The component is deprecated and will be removed in 4.0. Use Composer instead.
|
||||||
|
|
||||||
|
Console
|
||||||
|
-------
|
||||||
|
|
||||||
|
* `Input::getOption()` no longer returns the default value for options
|
||||||
|
with value optional explicitly passed empty.
|
||||||
|
|
||||||
|
For:
|
||||||
|
|
||||||
|
```php
|
||||||
|
protected function configure()
|
||||||
|
{
|
||||||
|
$this
|
||||||
|
// ...
|
||||||
|
->setName('command')
|
||||||
|
->addOption('foo', null, InputOption::VALUE_OPTIONAL, '', 'default')
|
||||||
|
;
|
||||||
|
}
|
||||||
|
|
||||||
|
protected function execute(InputInterface $input, OutputInterface $output)
|
||||||
|
{
|
||||||
|
var_dump($input->getOption('foo'));
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Before:
|
||||||
|
|
||||||
|
```
|
||||||
|
$ php console.php command
|
||||||
|
"default"
|
||||||
|
|
||||||
|
$ php console.php command --foo
|
||||||
|
"default"
|
||||||
|
|
||||||
|
$ php console.php command --foo ""
|
||||||
|
"default"
|
||||||
|
|
||||||
|
$ php console.php command --foo=
|
||||||
|
"default"
|
||||||
|
```
|
||||||
|
|
||||||
|
After:
|
||||||
|
|
||||||
|
```
|
||||||
|
$ php console.php command
|
||||||
|
"default"
|
||||||
|
|
||||||
|
$ php console.php command --foo
|
||||||
|
NULL
|
||||||
|
|
||||||
|
$ php console.php command --foo ""
|
||||||
|
""
|
||||||
|
|
||||||
|
$ php console.php command --foo=
|
||||||
|
""
|
||||||
|
```
|
||||||
|
|
||||||
Debug
|
Debug
|
||||||
-----
|
-----
|
||||||
|
|
||||||
|
@ -6,6 +6,8 @@ CHANGELOG
|
|||||||
|
|
||||||
* added `ExceptionListener`
|
* added `ExceptionListener`
|
||||||
* added `AddConsoleCommandPass` (originally in FrameworkBundle)
|
* added `AddConsoleCommandPass` (originally in FrameworkBundle)
|
||||||
|
* [BC BREAK] `Input::getOption()` no longer returns the default value for options
|
||||||
|
with value optional explicitly passed empty
|
||||||
|
|
||||||
3.2.0
|
3.2.0
|
||||||
------
|
------
|
||||||
|
@ -148,7 +148,12 @@ class ArgvInput extends Input
|
|||||||
|
|
||||||
if (false !== $pos = strpos($name, '=')) {
|
if (false !== $pos = strpos($name, '=')) {
|
||||||
if (0 === strlen($value = substr($name, $pos + 1))) {
|
if (0 === strlen($value = substr($name, $pos + 1))) {
|
||||||
array_unshift($this->parsed, null);
|
// if no value after "=" then substr() returns "" since php7 only, false before
|
||||||
|
// see http://php.net/manual/fr/migration70.incompatible.php#119151
|
||||||
|
if (PHP_VERSION_ID < 70000 && false === $value) {
|
||||||
|
$value = '';
|
||||||
|
}
|
||||||
|
array_unshift($this->parsed, $value);
|
||||||
}
|
}
|
||||||
$this->addLongOption(substr($name, 0, $pos), $value);
|
$this->addLongOption(substr($name, 0, $pos), $value);
|
||||||
} else {
|
} else {
|
||||||
@ -221,23 +226,16 @@ class ArgvInput extends Input
|
|||||||
|
|
||||||
$option = $this->definition->getOption($name);
|
$option = $this->definition->getOption($name);
|
||||||
|
|
||||||
// Convert empty values to null
|
|
||||||
if (!isset($value[0])) {
|
|
||||||
$value = null;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (null !== $value && !$option->acceptValue()) {
|
if (null !== $value && !$option->acceptValue()) {
|
||||||
throw new RuntimeException(sprintf('The "--%s" option does not accept a value.', $name));
|
throw new RuntimeException(sprintf('The "--%s" option does not accept a value.', $name));
|
||||||
}
|
}
|
||||||
|
|
||||||
if (null === $value && $option->acceptValue() && count($this->parsed)) {
|
if (in_array($value, array('', null), true) && $option->acceptValue() && count($this->parsed)) {
|
||||||
// if option accepts an optional or mandatory argument
|
// if option accepts an optional or mandatory argument
|
||||||
// let's see if there is one provided
|
// let's see if there is one provided
|
||||||
$next = array_shift($this->parsed);
|
$next = array_shift($this->parsed);
|
||||||
if (isset($next[0]) && '-' !== $next[0]) {
|
if ((isset($next[0]) && '-' !== $next[0]) || in_array($next, array('', null), true)) {
|
||||||
$value = $next;
|
$value = $next;
|
||||||
} elseif (empty($next)) {
|
|
||||||
$value = null;
|
|
||||||
} else {
|
} else {
|
||||||
array_unshift($this->parsed, $next);
|
array_unshift($this->parsed, $next);
|
||||||
}
|
}
|
||||||
@ -248,8 +246,8 @@ class ArgvInput extends Input
|
|||||||
throw new RuntimeException(sprintf('The "--%s" option requires a value.', $name));
|
throw new RuntimeException(sprintf('The "--%s" option requires a value.', $name));
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!$option->isArray()) {
|
if (!$option->isArray() && !$option->isValueOptional()) {
|
||||||
$value = $option->isValueOptional() ? $option->getDefault() : true;
|
$value = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -179,7 +179,9 @@ class ArrayInput extends Input
|
|||||||
throw new InvalidOptionException(sprintf('The "--%s" option requires a value.', $name));
|
throw new InvalidOptionException(sprintf('The "--%s" option requires a value.', $name));
|
||||||
}
|
}
|
||||||
|
|
||||||
$value = $option->isValueOptional() ? $option->getDefault() : true;
|
if (!$option->isValueOptional()) {
|
||||||
|
$value = true;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
$this->options[$name] = $value;
|
$this->options[$name] = $value;
|
||||||
|
@ -158,7 +158,7 @@ abstract class Input implements InputInterface, StreamableInputInterface
|
|||||||
throw new InvalidArgumentException(sprintf('The "%s" option does not exist.', $name));
|
throw new InvalidArgumentException(sprintf('The "%s" option does not exist.', $name));
|
||||||
}
|
}
|
||||||
|
|
||||||
return isset($this->options[$name]) ? $this->options[$name] : $this->definition->getOption($name)->getDefault();
|
return array_key_exists($name, $this->options) ? $this->options[$name] : $this->definition->getOption($name)->getDefault();
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -48,7 +48,7 @@ class ArgvInputTest extends TestCase
|
|||||||
$input = new ArgvInput($input);
|
$input = new ArgvInput($input);
|
||||||
$input->bind(new InputDefinition($options));
|
$input->bind(new InputDefinition($options));
|
||||||
|
|
||||||
$this->assertEquals($expectedOptions, $input->getOptions(), $message);
|
$this->assertSame($expectedOptions, $input->getOptions(), $message);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function provideOptions()
|
public function provideOptions()
|
||||||
@ -75,14 +75,32 @@ class ArgvInputTest extends TestCase
|
|||||||
array(
|
array(
|
||||||
array('cli.php', '--foo='),
|
array('cli.php', '--foo='),
|
||||||
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)),
|
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)),
|
||||||
array('foo' => null),
|
array('foo' => ''),
|
||||||
'->parse() parses long options with optional value which is empty (with a = separator) as null',
|
'->parse() parses long options with optional value which is empty (with a = separator) as empty string',
|
||||||
),
|
),
|
||||||
array(
|
array(
|
||||||
array('cli.php', '--foo=', 'bar'),
|
array('cli.php', '--foo=', 'bar'),
|
||||||
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
|
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
|
||||||
|
array('foo' => ''),
|
||||||
|
'->parse() parses long options with optional value without value specified or an empty string (with a = separator) followed by an argument as empty string',
|
||||||
|
),
|
||||||
|
array(
|
||||||
|
array('cli.php', 'bar', '--foo'),
|
||||||
|
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
|
||||||
array('foo' => null),
|
array('foo' => null),
|
||||||
'->parse() parses long options with optional value which is empty (with a = separator) followed by an argument',
|
'->parse() parses long options with optional value which is empty (with a = separator) preceded by an argument',
|
||||||
|
),
|
||||||
|
array(
|
||||||
|
array('cli.php', '--foo', '', 'bar'),
|
||||||
|
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL), new InputArgument('name', InputArgument::REQUIRED)),
|
||||||
|
array('foo' => ''),
|
||||||
|
'->parse() parses long options with optional value which is empty as empty string even followed by an argument',
|
||||||
|
),
|
||||||
|
array(
|
||||||
|
array('cli.php', '--foo'),
|
||||||
|
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL)),
|
||||||
|
array('foo' => null),
|
||||||
|
'->parse() parses long options with optional value specified with no separator and no value as null',
|
||||||
),
|
),
|
||||||
array(
|
array(
|
||||||
array('cli.php', '-f'),
|
array('cli.php', '-f'),
|
||||||
@ -252,14 +270,14 @@ class ArgvInputTest extends TestCase
|
|||||||
|
|
||||||
$input = new ArgvInput(array('cli.php', '--name=foo', '--name=bar', '--name='));
|
$input = new ArgvInput(array('cli.php', '--name=foo', '--name=bar', '--name='));
|
||||||
$input->bind(new InputDefinition(array(new InputOption('name', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY))));
|
$input->bind(new InputDefinition(array(new InputOption('name', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY))));
|
||||||
$this->assertSame(array('name' => array('foo', 'bar', null)), $input->getOptions(), '->parse() parses empty array options as null ("--option=value" syntax)');
|
$this->assertSame(array('name' => array('foo', 'bar', '')), $input->getOptions(), '->parse() parses empty array options as null ("--option=value" syntax)');
|
||||||
|
|
||||||
$input = new ArgvInput(array('cli.php', '--name', 'foo', '--name', 'bar', '--name', '--anotherOption'));
|
$input = new ArgvInput(array('cli.php', '--name', 'foo', '--name', 'bar', '--name', '--anotherOption'));
|
||||||
$input->bind(new InputDefinition(array(
|
$input->bind(new InputDefinition(array(
|
||||||
new InputOption('name', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY),
|
new InputOption('name', null, InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY),
|
||||||
new InputOption('anotherOption', null, InputOption::VALUE_NONE),
|
new InputOption('anotherOption', null, InputOption::VALUE_NONE),
|
||||||
)));
|
)));
|
||||||
$this->assertSame(array('name' => array('foo', 'bar', null), 'anotherOption' => true), $input->getOptions(), '->parse() parses empty array options as null ("--option value" syntax)');
|
$this->assertSame(array('name' => array('foo', 'bar', null), 'anotherOption' => true), $input->getOptions(), '->parse() parses empty array options ("--option value" syntax)');
|
||||||
}
|
}
|
||||||
|
|
||||||
public function testParseNegativeNumberAfterDoubleDash()
|
public function testParseNegativeNumberAfterDoubleDash()
|
||||||
|
@ -90,9 +90,15 @@ class ArrayInputTest extends TestCase
|
|||||||
'->parse() parses long options with a default value',
|
'->parse() parses long options with a default value',
|
||||||
),
|
),
|
||||||
array(
|
array(
|
||||||
array('--foo' => null),
|
array(),
|
||||||
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL, '', 'default')),
|
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL, '', 'default')),
|
||||||
array('foo' => 'default'),
|
array('foo' => 'default'),
|
||||||
|
'->parse() uses the default value for long options with value optional which are not passed',
|
||||||
|
),
|
||||||
|
array(
|
||||||
|
array('--foo' => null),
|
||||||
|
array(new InputOption('foo', 'f', InputOption::VALUE_OPTIONAL, '', 'default')),
|
||||||
|
array('foo' => null),
|
||||||
'->parse() parses long options with a default value',
|
'->parse() parses long options with a default value',
|
||||||
),
|
),
|
||||||
array(
|
array(
|
||||||
|
@ -37,6 +37,14 @@ class InputTest extends TestCase
|
|||||||
$input = new ArrayInput(array('--name' => 'foo'), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
|
$input = new ArrayInput(array('--name' => 'foo'), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
|
||||||
$this->assertEquals('default', $input->getOption('bar'), '->getOption() returns the default value for optional options');
|
$this->assertEquals('default', $input->getOption('bar'), '->getOption() returns the default value for optional options');
|
||||||
$this->assertEquals(array('name' => 'foo', 'bar' => 'default'), $input->getOptions(), '->getOptions() returns all option values, even optional ones');
|
$this->assertEquals(array('name' => 'foo', 'bar' => 'default'), $input->getOptions(), '->getOptions() returns all option values, even optional ones');
|
||||||
|
|
||||||
|
$input = new ArrayInput(array('--name' => 'foo', '--bar' => ''), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
|
||||||
|
$this->assertEquals('', $input->getOption('bar'), '->getOption() returns null for options explicitly passed without value (or an empty value)');
|
||||||
|
$this->assertEquals(array('name' => 'foo', 'bar' => ''), $input->getOptions(), '->getOptions() returns all option values.');
|
||||||
|
|
||||||
|
$input = new ArrayInput(array('--name' => 'foo', '--bar' => null), new InputDefinition(array(new InputOption('name'), new InputOption('bar', '', InputOption::VALUE_OPTIONAL, '', 'default'))));
|
||||||
|
$this->assertNull($input->getOption('bar'), '->getOption() returns null for options explicitly passed without value (or an empty value)');
|
||||||
|
$this->assertEquals(array('name' => 'foo', 'bar' => null), $input->getOptions(), '->getOptions() returns all option values');
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
Reference in New Issue
Block a user