-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Allow multiple options for --colors flag #1526
Conversation
I'm sorry, I haven't see this pull request before I sent mine (#1527). |
if (is_null($option[1]) || in_array($option[1], array('always', 'force', 'yes'))) { | ||
$this->arguments['colors'] = true; | ||
} elseif (in_array($option[1], array('auto', 'tty', 'if-tty'))) { | ||
if (function_exists('posix_isatty') && !posix_isatty(STDOUT)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to use $console->hasColorSupport() here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no $console in scope, which from the Environment project only wraps the same check?
@@ -280,8 +280,18 @@ protected function handleArguments(array $argv) | |||
foreach ($this->options[0] as $option) { | |||
switch ($option[0]) { | |||
case '--colors': { | |||
$this->arguments['colors'] = true; | |||
if (is_null($option[1]) || in_array($option[1], array('always', 'force', 'yes'))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO its too much code to put here, you may create another method for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, as @remicollet already said in the other pul request you must add true
here.
@jbladen, besides the help message, may you create a PHPT test to ensure the |
@@ -280,8 +280,18 @@ protected function handleArguments(array $argv) | |||
foreach ($this->options[0] as $option) { | |||
switch ($option[0]) { | |||
case '--colors': { | |||
$this->arguments['colors'] = true; | |||
if (is_null($option[1]) || in_array($option[1], array('always', 'force', 'yes'))) { | |||
$this->arguments['colors'] = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not going to work as expected because of the checking in PHPUnit_TextUI_ResultPrinter.
At first glance I think that I prefer #1527 over this one. |
Retargetted from #1525