Building on postcss-log-warnings

[davidtheclark]

@postcss/owners I've made some minor preliminary alterations to adapt postcss-log-warnings to this new name. Also, I changed the keepWarnings option to its reverse, clearWarnings, since people seemed to want that.

(For reference: the discussion for moving this was at postcss/postcss-browser-reporter#17)

I could try to rewrite names and docs so that they're discussing "messages" more generically, rather than "warnings", specifically; but that might be kind of confusing since there are no other types of messages to give examples of, right? (Except errors, which this plugin is ignoring currently --- should it log them?)

Thoughts?

[MoOx]

but that might be kind of confusing since there are no other types of messages

Any plugin might use some "debug" or "info" type (or even "error" - for recoverable error (eg: postcss-custom-properties will use "error" when a undefined variable is referenced)

[ai]

postcss-console-log-messages is soooo long name :(. Wy not postcss-console-messages? Especially if you want to support not only log type.

[MoOx]

why not. "log" was used to be clear that this plugin will display messages.

[MoOx]

postcss-console-messages lgtm

[ben-eb]

Why not postcss-log or postcss-logger? Shorter still. 😄

[ai]

@ben-eb +1, I think that "log" word already means output to console

[MoOx]

because you can show messages into terminal (console) or in the css (via postcss-messages).
Related: postcss/postcss-browser-reporter#18

[MoOx]

I think we should have some coherence between postcss-messages & postcss-log-warnings.
So something like

• postcss-log-css + postcss-log-console
• postcss-css-log + postcss-console-log
• postcss-messages-css + postcss-messages-log
• postcss-css-messages + postcss-console-messages

[ben-eb]

I'm not sure if there is any benefit to separating this out into two modules. postcss-log could build in support for both browser and console, or even custom functions. For example you could inject some function that would send out a message to AnyBar to change colour on warnings, errors, messages, etc.

If they must be two modules then I think I would like to see browser in the name of the module rather than messages which is too vague.

[ai]

@ben-eb console and browsers output are very different in my opinion. There are many options only for browsers output. Also terminal and browsers output is about different user cases.

[MoOx]

Those two modules does not share the same code I think (one append some css & the other output via console). That said, in the usage section of this module, we can see that the plugin that output into css might use it https://github.com/postcss/postcss-console-log-messages#usage

I have some stuff to push in postcss-messages so I think I will make a PR with the use of this module.

So what about

• postcss-log-console & postcss-log-browser
• postcss-console-logger & postcss-browser-logger

We can imagine to make a more abstract postcss-logger that doesn't do anything special without a function as an argument (runners might want to use this to display with their own methods (eg: Prepros app might want to use this))

[ai]

@MoOx nice plan about common postcss-log. I like postcss-log-console & postcss-log-browser more, because it is shorter :).

[ben-eb]

@ai That is true, but I am just thinking about how Sass works by default; it will both emit an error in the console and write some warning in the CSS file itself. This way it is absolutely clear to the user that something went wrong.

@MoOx I like your second two ideas, postcss-console-logger and postcss-browser-logger. Perhaps we can write postcss-logger to consume both of those to mimic this Sass option. 😄

[ai]

@ben-eb do I remember correct, that Sass output only errors to CSS, not a warnings.

[ben-eb]

@ai Yep, I think just errors, not warnings.

[MoOx]

-logger make more sense to me than log-

Also for the we should have a postcss-logger that can consume reporter and return a string, that will be used by postcss-*-logger.

postcss-log-* might make sense but postcss-log is not at all. So I prefer the logger thing

[ai]

Good point, I am OK with logger.

[ben-eb]

@MoOx 👍

[gazay]

I'm 👍 for postcss-browser-logger and postcss-console-logger

[davidtheclark]

These shorter names sound good to me.

@lydell added the section to the readme about using lib/processResult as a library because he wanted to then use that library in postcss-messages (davidtheclark/postcss-log-warnings#10). If we wanted instead to have a new module that included the possibly shared code, I think it might postcss-stringify-messages or something; and it would have an options to add console colors via chalk and to clearWarnings when finished. Would we prefer to have this abstraction used by both -console- and -browser- loggers?

Another Q: If there are different types of messages being displayed --- if the console-logger shows errors, warnings, and any other type of message from any other plugin --- then should we change the format in some way? Should we log them all together but include the message's type in the readout? Or log separate sections: all warnings together, all errors together, all other messages together, etc.?

[MoOx]

Maybe we should support reporter like lots of tool does ?

[ben-eb]

@MoOx This would be pretty sweet.

• postcss-console-reporter
• postcss-browser-reporter

[davidtheclark]

I like the -reporter idea, too. How about combining that with postcss-stringify-messages, which both console and browser reporters would use?

[ben-eb]

Let's keep the naming simple & clear; the main module can provide the user with customisation of what log levels he/she wants, and can simply be called postcss-logger. It should then emit events which other modules can consume, such as postcss-console-reporter, postcss-browser-reporter, postcss-tap-reporter or postcss-json-reporter. That way output is easily configurable for usage in everything from browser environments to continuous integration testing.

Perhaps it should bundle postcss-console-reporter and then link to the other reporters on its readme.

[MoOx]

Console & browser should be able to consume the reporters (which are just message formatters). Maybe formatter is a more accurate word.

[ben-eb]

I'm happy with formatter also. 👍

[davidtheclark]

Ok, so what I was calling postcss-stringify-messages you're calling a formatter, right? Which is: a module that takes the messages object from a postcss Result and outputs that information in some other format (whether string for console, XML for CLI, or whatever). Is that right?

[ben-eb]

Yes; essentially we are abstracting the logic of what to show in one module, and how that is shown in a separate reporter/formatter module. 😄

[davidtheclark]

Hm. I'm still a little confused. Let's outline the proposed modules. Correct me if I'm wrong please:

• postcss-console-formatter: formats messages to be read in the console, returns a string
• postcss-browser-formatter: formats messages to be read in the browser, returns a string
• postcss-logger: (this is where I feel unsure) consumes the formatters; end-users tell it how they want to log their messages, then it uses the proper formatter, makes that formatted info visible however the user specified (e.g. in console or in browser), optionally clears the messages so they won't be logged again.

Close?

[ben-eb]

@davidtheclark You can see an example of this architecture in my css-minifier tests repo. test.js contains the actual testing logic, then reporter.js just prints the result to console. You could swap out that reporter.js for something that went to the browser instead, for example.

https://github.com/ben-eb/css-minifier-tests

[MoOx]

It's more - reporters (where): cli, browser, file... - formatters: default, stylish (see jshint/eslint), tap...

[davidtheclark]

I was confused by thinking that you two were using reporter/formatter interchangeably, or deciding between those two terms. I'm still confused, though: are you two in agreement or not? Because @ben-eb's reporter.js both formats and reports. And that makes some sense to me, because if we format a string for colorful console output it's really not useful to "report" that formatted string anywhere except the console, right? It would really help if you would outline the modules that you imagine creating and how they would work together.

[MoOx]

I will do that when I will be on my computer.
For a string that has been colored for console, we can strip color easily (webpack dev server do that when showing messages from terminal in the browser)

[ben-eb]

Yeah I think I may have got those mixed up. Here is another use case: https://www.npmjs.com/package/mocha-osx-reporter

So we need some main module to let the user decide which message/warnings/errors to display, then reporter modules to consume those messages and output them in a specific environment, and then optionally formatters to customise that display. Although I feel that those formatters are most likely only relevant to postcss-console-reporter. 😄

[MoOx]

So I am thinking about:

• postcss-cli-reporter (or maybe just postcss-reporter)
• postcss-style-reporter (because -browser- is to vague and a browser have a console - for example, webpack-dev-server reports console.log() in both cli & browser console) or maybe postcss-css-reporter (I admit this one sounds weird, even if it's accurate)
• ... postcss-notifier-reporter (we can imagine a crazy cross OS notifier using simple lib like https://www.npmjs.com/package/node-notifier, yes :D)
• postcss-gnu-formatter (simple file:l:c: message)
• postcss-stylish-formatter (like https://www.npmjs.com/package/jshint-stylish) (probably current postcss-log-warnings output)
• postcss-*-formatter for people that would like their own formatter.

I think all formatter might add cli color, because most people will use the cli reporter. Then other reporter will be able to strip colors (using chalk or just https://www.npmjs.com/package/strip-ansi) to use the ouput.

Note that I would like to get verbose name like postcss-*-messages-(reporter|formatter) to be clear about what it is reporting, but I guess @ai will yell because modules names are too long... :)

[lydell]

👍 To the ideas in this issue. Good thing I never started my plans from davidtheclark/postcss-log-warnings#10 :)

[ben-eb]

👍 to @MoOx's plan. But I think browser is better than style. If people want to consume postcss-cli-reporter in the browser, that is fine; style doesn't make sense from just looking at the module name. I think even if browser is vague, it is better than style.

[MoOx]

What about postcss-browser-style-messages-reporter ? Long but clear.
And for console reporter, maybe we should stick to postcss-console-messages-reporter ?

[ben-eb]

I think it is clear enough that postcss-cli-reporter is for the command line and postcss-browser-reporter is for the browser. If it is too long then people will not remember it. 😄

[davidtheclark]

Thanks for writing out that list. Sorry but I'm still unclear about the relationship between "reporter" and "formatter" -- because in all of the examples given so far the module combines both parts that it seems you're proposing we split up: the module takes an object in, turns it into a legible string, then "reports" that string in one way or another.

What, for example, would the output look like for postcss-cli-reporter if the user did not also include postcss-stylish-formatter? Or would postcsss-cli-reporter use postcss-stylish-formatter in the background all the time and the user won't need to know it's a separate module?

[MoOx]

turns it into a legible string

That the formatter job.

All reporters will need to choose & use a default formatter.

See http://eslint.org/docs/user-guide/command-line-interface#f-format

[ben-eb]

The reporter should not turn something into a legible string - it simply consumes the PostCSS messages and emits events to its formatter, which returns a formatted string (in the case of postcss-cli-reporter), or some CSS styles (in the case of postcss-browser-reporter) which is then output accordingly.

[davidtheclark]

Ok. So reporters all have a default formatter, but they can use different formatters if told to. I got it now. Thanks for bearing with me.

I will not have a lot of time for a week or two but would like to take on the cli-reporter and stylish-formatter, if that's alright with everyone.

[ben-eb]

Ok. So reporters all have a default formatter, but they can use different formatters if told to.

Exactly. 😃

[MoOx]

So here is what I think we should do to go forward without making 10 packages:

• rename this plugin to postcss-[messages-]reporter (no need for console/cli since it will be the most used/main package
• rename postcss-messages to postcss-browser-[messages-]reporter
• refactor postcss-browser-[messages-]reporter to use what it can from postcss-[messages-]reporter

Then after that we might want to :

• directly include some formatters in postcss-[messages-]reporter
• add a formatter option to both plugins to allow custom formatter

And maybe we can also do:

• postcss-os-notifier-[messages-]reporter

I would like to see *-messages-reporter instead of just "reporter" because some people might want to do linter or other plugins that might do some reporting using messages api (eg: postcss-oldcss-reporter that might want to add warnings when crazy old css has been detected - for example old ms filters); so I think "messages-reporter" is more clear on the purpose of the plugin.

If you find "messages-reporter" too long, maybe using "logger" seems a better idea, but I don't really like it anymore :)

I have some wip on that for cssnext I really want to push to communicate with cssnext users so I can probably handle some changes if @davidtheclark and @gazay are open too. Please let me know :)

[ben-eb]

I would like to see *-messages-reporter instead of just "reporter" because some people might want to do linter or other plugins that might do some reporting using messages api (eg: postcss-oldcss-reporter that might want to add warnings when crazy old css has been detected - for example old ms filters); so I think "messages-reporter" is more clear on the purpose of the plugin.

@MoOx IMO, the purpose of this module is to be a singular source of displaying errors/warnings/messages from other PostCSS modules. So in this case, someone would write postcss-oldcss and then use PostCSS messages API to send the messages downstream to the reporter module. So I think it is not necessary to add messages to the module name.

The rest looks great. 👍

[MoOx]

Ok so I guess others will say the same.
I am ok for postcss-reporter and postcss-browser-reporter (the last one will have the first one as dep)

[davidtheclark]

Sounds good.

[lydell]

Sounds like davidtheclark/postcss-log-warnings#10.

[davidtheclark]

It does. I think the main changes from the log-warnings codebase would be:

• processResult should be renamed defaultFormatter (or something) and should be replaceable with another formatter function (then another reporter like browser would use defaultFormatter as a lib)
• non-warning messages need to be logged, as well (errors, and whatever else) --- so the formatter needs to distinguish message type in some way

I also would like to improve the organization of the tests and do some integration tests in addition to the unit tests that mock postcss results.

[davidtheclark]

@lydell I've had a couple of minutes to work on this and I thought of two possible changes that would affect that plan of using this as an library. I was thinking that maybe the division-of-labor would make more sense if the reporter were responsible for determining which messages get logged (reporter has the plugins option) and for clearing messages after they are logged (the clearWarnings option). That means the formatter would receive an already-filtered array of messages rather than receiving the original PostCSS result. It also means that if other modules used that formatter, they would not also get the plugins and clearWarnings functionality built in. Again, the reason I think this division makes sense is because in my mind it seems that the formatter should only be responsible for transforming data to a string, not for filtering the data or mutating it.

What do you think? Any other @postcss/owners have opinions?

[MoOx]

the reason I think this division makes sense is because in my mind it seems that the formatter should only be responsible for transforming data to a string, not for filtering the data or mutating it.

I totally agree

[ben-eb]

@MoOx @davidtheclark Yep, agreed. This is how tape works; it always outputs an unformatted standard output. It is up to formatters to make that look pretty (or do something else with it). 😄

[davidtheclark]

I just published v0.1.0. Feedback welcome!