Should also report errors by default

[onigoetz]

Hi, it seems that at some point ( 56a8e5a ) there was a change that hid .type === "error" reports.

This is a surprising behaviour and definitely looks like a bug to me

[ai]

Right now, we recommend to throwing errors by Node#error().

Do you have use case for messages for errors?

[onigoetz]

oh interesting, I did not know of this method.

My use case is to make a series of checks on a bundle (so Stylelint is not applicable here).
Then report them all in a nice way and stop the build if there was at least an error.

I'm not interested in just reporting a single error at a time.

[ai]

Hm. In this case, we should throw some error from postcss-reporter to have non-zero exit code for the app.

Maybe it is better to create your own custom plugin because we will need think a lot to create a good design for that for everyone.

[onigoetz]

This is already handled here : https://github.com/postcss/postcss-reporter/blob/main/lib/reporter.js#L83-L89

although I am a bit puzzled that it would also throw on a warning, but I guess this is a matter of taste.
Some other tools like gulp-stylelint or gulp-eslint-new have a failAfterError option that fails only if an error was reported.

[ai]

This is already handled here : https://github.com/postcss/postcss-reporter/blob/main/lib/reporter.js#L83-L89

In this case we should have thrown always on error and update docs to highlight that that option is about warnings.

And change error message.

[onigoetz]

I can add this to this PR (or a separate PR if you prefer)

[ai]

Let's have one feature PR where feature will be done to check the final design before merging

[onigoetz]

sure!

In the meantime is the current PR ok for you?

[ai]

It is hard to say, because in PR I look for overall changes.

Let's finish all changes first.

[ai]

BTW, how we will format errors?

[onigoetz]

ah okay, so I misunderstood what you said earlier, I'll add the changes to errors here as well.

Currently, errors use the same behaviour as a warning, it's an entry with text, there isn't any stacktrace

[ai]

We don't need a stacktrace, but we need a different color and icon to separate them from warnings

[onigoetz]

Indeed, I'll include that and make a proposal for that as well

[onigoetz]

Hi, I made the following changes

• Added a cross icon for errors when formatting
• Always throw an error when an error is present in the messages
• The throwError option still exists but will make it also throw when a warning is reported (kept the name for backwards compatibility)
• Added a summary at the end of the report

Here is how the summary looks like :

One thing I'm unclear on at this stage is :

• Should the option to throw on warnings be renamed?
• Should there be an option not to throw on error?

[ai]

Looks amazing. I am going to release it today.

[ai]

This feature was released in 7.1.

[onigoetz]

Awesome, thanks :)