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

feat(formatter): Add support for multiple formatters and class formatters #357

Merged
merged 9 commits into from Feb 11, 2020

Conversation

olivernybroe
Copy link
Sponsor Collaborator

@olivernybroe olivernybroe commented Feb 7, 2020

Q A
Bug fix? no
New feature? yes

This PR adds support for passing multiple formatters by using the following syntax

php phpinsights analyse --format=json --format=github-action

It also allows to reference a formatter by class, as long as it is loaded.

php phpinsights analyse -n  -format=\\NunoMaduro\\PhpInsights\\Application\\Console\\Formatters\\Json

This means you can now create a custom formatter to use, to integrate with your own custom system if needed.

Copy link
Collaborator

@Jibbarth Jibbarth left a comment

Nice addition @olivernybroe 👍

How can we manage the output by format ?
Imagine I want github-formatter in stdout, but checkstyle formatter in a file.xml ?

src/Application/Console/Formatters/FormatResolver.php Outdated Show resolved Hide resolved
src/Application/Console/Formatters/FormatResolver.php Outdated Show resolved Hide resolved
Copy link
Collaborator

@Jibbarth Jibbarth left a comment

@olivernybroe Look good to me 👍

One last question: What's happen when we call phpinsights like this

vendor/bin/phpinsights --format=test --format=test2 --format=console

As test and test2 are not formatters, the fallback will use console.

So the output will be displayed 3 times ?

Maybe we should do something in Multiple formatter to remove duplicated formatter ?

@olivernybroe
Copy link
Sponsor Collaborator Author

@olivernybroe olivernybroe commented Feb 10, 2020

@Jibbarth Forgot to answer your previous question also 👍

So output by format is simply something I haven't added in. I think it is something worth considering, however for us to do that, we need to add proper support for exporting to file instead of piping the results. I think that should be in another PR.

Regarding multiple invalid formatters, then yes, it will actually result in the console formatter being used multiple times. I'll look into that one, as that is just plain stupid 👍

@olivernybroe olivernybroe requested a review from Jibbarth Feb 11, 2020
Copy link
Collaborator

@Jibbarth Jibbarth left a comment

👌

@olivernybroe olivernybroe merged commit baaadad into nunomaduro:master Feb 11, 2020
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants