search

postcss / postcss-cli

CLI for postcss
Other
840 stars 93 forks source link

Only log warnings #244

Closed stefanfisk closed 5 years ago

stefanfisk commented 6 years ago

Currently messages are only logged if there are warnings, but if there are then all messages are logged.

stefanfisk commented 6 years ago

The failing test is caused by https://github.com/postcss/postcss-cli/issues/123.

RyanZim commented 6 years ago

From https://github.com/postcss/postcss-reporter#purpose:

By default, only warnings are logged. If you would like to see more messages, you can change the filter function.

Isn't this actually working this way?

stefanfisk commented 6 years ago

Not for me at least. postcss-reporter is, as far as I can tell, only used for formatting the messages: https://github.com/postcss/postcss-cli/blob/af81c55300c7745751b5f44d682f29b7029d60f9/index.js#L15

The reason I submitted the PR was that I am seeing all of the dependency messages printed as undefined [undefined] if there was also a warning message when using postcss-import. Merely importing an empty file is enough to recreate the issue.

I was kinda hyper focused yesterday, so I realise now that opening an issue and actually describing the situation would have been clearer, sorry about that :/

RyanZim commented 6 years ago

Tests are failing because you didn't run npm run format to prettify your code, not because of #123

stefanfisk commented 5 years ago

Sorry for the super late reply to this, and for my overall sloppiness with the PR 😕

When I run npm run format it does not fix index.js, and the only thing that fails for npm run ci is "error › invalid --config".

However, if I change the prettier script to prettier --single-quote --no-semi *.js **/*.{js,md} I get the same output that Travis is generating above. After investigating it seems like maybe the glob should be quoted? https://prettier.io/docs/en/cli.html

I have updated the PR with a properly formatted patch, so now everything should be OK, but you might want to be aware of the prettier issue I encountered.

RyanZim commented 5 years ago

@stefanfisk What's your OS?

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.5%) to 75.524% when pulling f28937352e0c128620c4f83964473e6c0853237a on stefanfisk:patch-1 into 54cdf3a932a23ff85b67b10b9fb7b014bbfb4230 on postcss:master.

stefanfisk commented 5 years ago

@RyanZim macOS Mojave. I am running zsh, but as far as I can remember the issue was reproducible when I switched to Bash. Quoting the glob also worked if I remember correctly, but I have not investigated how exactly npm executes the scripts and what the implications of not quoting are.

RyanZim commented 5 years ago

Ah, makes sense. Globstar (**) support isn't turned on by default in most shells. If we quote it, prettier expands the glob itself, if we don't, the shell tries to expand it, but messes up.

Will fix.

nodaguti commented 5 years ago

Coinsidentally I tried to fix the same issue from the postcss-reporter side, and am happy with it being resolved anyway 😄 https://github.com/postcss/postcss-reporter/pull/46

Thanks to both of you!

XhmikosR commented 5 years ago

@RyanZim this causes empty lines printed. https://travis-ci.org/twbs/bootstrap/jobs/475278409#L666-L695

stefanfisk commented 5 years ago

@XhmikosR what have done to verify that this change is the cause of the blank lines?

XhmikosR commented 5 years ago

Obviously I tested it.