Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix Command options parsing for underscored or dashed args #442

Closed
wants to merge 1 commit into from

2 participants

@mehlah

Li3 command line options wasn't parsed correctly in case of an underscored option like --foo_bar=something.
Dashed options are converted to an underscored form. --foo-bar will be available at $this->foo_bar

@davidpersson

We don't (or at least shouldn't) support long options containing underscores. Support for dashes in long options is already there so no need to change the parsing in the Router class. I've built upon one of the tests you've provided in 23d24d1 to reflect this. This behavior has additionally been documented in 884af54.

Thanks for bringing this to attention and providing the test!

@mehlah

We don't (or at least shouldn't) support long options containing underscores.

Okay with that

Support for dashes in long options is already there so no need to change the parsing in the Router class. I've built upon one of the tests you've provided in 23d24d1 to reflect this. This behavior has additionally been documented in 884af54.

The test you've added doesn't reflect that --foo-bar is parsed and available at $this->foo_bar in the command class.
That was the point of the PR.
I don't see where the parsing of dashes is handled and converted to an underscored form.

I missed something ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 24 additions and 1 deletion.
  1. +2 −1  console/Router.php
  2. +22 −0 tests/cases/console/RouterTest.php
View
3  console/Router.php
@@ -34,7 +34,8 @@ public static function parse($request = null) {
$params[$match['key']] = true;
continue;
}
- if (preg_match('/^--(?P<key>[a-z0-9-]+)(?:=(?P<val>.+))?$/i', $arg, $match)) {
+ if (preg_match('/^--(?P<key>[a-z0-9_\-]+)(?:=(?P<val>.+))?$/i', $arg, $match)) {
+ $match['key'] = str_replace('-', '_', $match['key']);
$params[$match['key']] = !isset($match['val']) ? true : $match['val'];
continue;
}
View
22 tests/cases/console/RouterTest.php
@@ -71,6 +71,28 @@ public function testParseGnuStyleLongOptions() {
$this->assertEqual($expected, $result);
}
+ public function testParseUnderscoredOrDashedOption() {
+ $expected = array(
+ 'command' => 'test', 'action' => 'run', 'args' => array(),
+ 'foo_bar' => 'something'
+ );
+ $result = Router::parse(new Request(array(
+ 'args' => array(
+ 'test', 'run',
+ '--foo_bar=something'
+ )
+ )));
+ $this->assertEqual($expected, $result);
+
+ $result = Router::parse(new Request(array(
+ 'args' => array(
+ 'test', 'run',
+ '--foo-bar=something'
+ )
+ )));
+ $this->assertEqual($expected, $result);
+ }
+
public function testParseShortOption() {
$expected = array(
'command' => 'test', 'action' => 'action', 'args' => array(),
Something went wrong with that request. Please try again.