mfussenegger / nvim-lint

An asynchronous linter plugin for Neovim complementary to the built-in Language Server Protocol support.
GNU General Public License v3.0
1.94k stars 204 forks source link

Use `stderr` for `stylelint` #518

Closed jmpolom closed 4 months ago

jmpolom commented 8 months ago

This seems to and should resolve #500

mfussenegger commented 8 months ago

@cpof-tea can you double check this? You made the last changes to this linter, and I don't use it.

jmpolom commented 8 months ago

Looks like at some point stylelint changed from outputting problems to stdout to stderr: https://github.com/stylelint/stylelint/blob/main/docs/migration-guide/to-16.md#breaking-changes

cpoftea commented 8 months ago

@jmpolom indeed. I have stylelint v14.15.0, and it outputs to stdout

@mfussenegger is there any way we can dynamically change stream property for backward compatibility?

mfussenegger commented 8 months ago

If the linter doesn't output anything in the other stream, setting both could be an option.

Otherwise the linter could be implemented as function() that first calls stylelint --version (if that's supported), parses the version and depending on that returns a different table. But not sure if that's worth it.

jmpolom commented 8 months ago

stylelint -v does return a version string. Would it be possible to define the stream as a function in the linter config as is done for cmd? Seems like that could work if backwards compatibility is necessary.

cpoftea commented 8 months ago

Otherwise the linter could be implemented as function()

I didn't know it is supported, that's why I asked.

first calls stylelint --version (if that's supported), parses the version and depending on that returns a different table. But not sure if that's worth it.

Why not. At least it feels more natural, than just pretending we know if other streams used or not.

I will try to implement

mfussenegger commented 8 months ago

Could you first try setting the stream to both? If that works, it's a much simpler solution.

cpoftea commented 8 months ago

@mfussenegger okay, it works. @jmpolom could you please set both to stream option?

albnavarro commented 6 months ago

I think this pr can be merged as is stream = "sterr" so that it is compatible with the stable version of stylelint. Personally, to have backward compatibility I inserted:

local stylelint = require("lint").linters.stylelint stylelint.stream = "both"

It may be correct that the choice to use both is optional.

jmpolom commented 6 months ago

It seemed that the change by Stylelint to output errors to stderr was permanent. Based on that I agree with @albnavarro and this should be merged as-is. This module should support the current stable upstream linter version which outputs lint messages to stderr.

mfussenegger commented 4 months ago

https://github.com/mfussenegger/nvim-lint/pull/592