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
Process output refactor #1118
Process output refactor #1118
Conversation
SpacePossum
commented
Mar 31, 2015
- Move process output writer to dedicated class
- Write process output to STDERR
- Add colored output to process and legend
- Move event representation out of the event class itself
* @var array | ||
*/ | ||
private static $eventStatusMap = array( | ||
FixerFileProcessedEvent::STATUS_UNKNOWN => array('symbol' => '?', 'format' => '%s', 'description' => 'unknown'), |
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.
These shouldn't be aligned.
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.
Symfony's coding style is to not align arrays.
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.
Will change, I just copied those over from here:
https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/1.7/Symfony/CS/FixerFileProcessedEvent.php#L42
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.
Yeh. It was wrong already.
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.
ok :) I changed it, hope I didn't mess it up
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.
@GrahamCampbell is there a fixer for this?
$this->output->write("\n"); | ||
} | ||
|
||
public function close() |
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.
destructor ?
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.
good idea, do you me to call it explicit in the FixerCommand?
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 see no need for that on the first look. And you?
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 already suggested that. I assume my comments on this method were lost when the code changed last.
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'll change this, sorry @GrahamCampbell I don know how I made your comments disappear, that shouldn't be possible on github (at least for a mortal like me ;) ) right?
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.
probably one of outdated
Please fix project itself ;) |
} | ||
|
||
$symbols[] = $symbol; | ||
$this->output->write(sprintf(' %s-%s,', $symbol, $status['description'])); |
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.
is this a comma after last element?
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.
good catch, will fix
… STDERR, add color output to process output
👍 |
Thank you @SpacePossum. |
This PR was merged into the 2.0-dev branch. Discussion ---------- Process output refactor - Move process output writer to dedicated class - Write process output to STDERR - Add colored output to process and legend - Move event representation out of the event class itself Commits ------- 2bc83ad Move process outputter to logic to dedicated class, output process to STDERR, add color output to process output
Thanks @keradus, nice to see this made it :) |
I am really interesting in that output-classes (xml, json, etc.) @SpacePossum ;) |
$this->fixer->setEventDispatcher($this->eventDispatcher); | ||
$this->eventDispatcher->addListener(FixerFileProcessedEvent::NAME, $fileProcessedEventListener); | ||
$progressOutput = new ProcessOutput($this->eventDispatcher); |
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.
why process in the class name and progress in the variable name ?
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.
fair point, I'll rename this in a new PR (about the output classes)