Skip to content
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

Add a renderer which writes an ANSI report string #595

Merged
merged 3 commits into from Aug 5, 2019

Conversation

PtrTn
Copy link
Contributor

@PtrTn PtrTn commented Feb 8, 2019

I've implemented an ANSI renderer which should improve the experience for users reading a report straight from the command line.

Using the ANSI renderer a report without violations will look something like this:
php-md-no-mess

A report which does contain either a violation, an error or both will look like:
php-md-mess

@ravage84 ravage84 added this to the 2.7.0 milestone Jun 25, 2019
MarkVaughn
MarkVaughn previously approved these changes Jul 1, 2019
@ravage84 ravage84 modified the milestones: 2.7.0, 2.8.0 Jul 9, 2019
…ort-renderer

# Conflicts:
#	README.rst
#	src/main/php/PHPMD/TextUI/CommandLineOptions.php
#	src/test/php/PHPMD/TextUI/CommandLineOptionsTest.php
kylekatarnls
kylekatarnls previously approved these changes Jul 28, 2019
@kylekatarnls
Copy link
Member

Resolved conflicts with master branch.

@PtrTn
Copy link
Contributor Author

PtrTn commented Jul 28, 2019

Thanks for the effort @kylekatarnls 😄

MarkVaughn
MarkVaughn previously approved these changes Jul 29, 2019
BackEndTea
BackEndTea previously approved these changes Jul 29, 2019
@ravage84 ravage84 self-requested a review July 29, 2019 14:03
@ravage84
Copy link
Member

What if we replace the current text renderer but add a --ansi and a --no-ansi CLI parameter to control the output?
If I recall correctly PHPUnit and/or PHPCS do it that way.

@MarkVaughn
Copy link
Collaborator

There might be reasons to keep the default text renderer without breaking expected behavior?
Maybe v3 can default to ANSI part of the "new, shiny and improved" launch hype

@kylekatarnls kylekatarnls dismissed stale reviews from BackEndTea, MarkVaughn, and themself via 839a182 July 30, 2019 09:43
@kylekatarnls
Copy link
Member

Sorry guys, my commit staled your review but we had "four renderers" when adding ANSI would have make this statement wrong. I removed it so we don't have to maintain this count.

@kylekatarnls kylekatarnls merged commit 2c1d460 into phpmd:master Aug 5, 2019
@tvbeek tvbeek mentioned this pull request Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants