nvim-lint icon indicating copy to clipboard operation
nvim-lint copied to clipboard

Use `stderr` for `stylelint`

Open jmpolom opened this issue 1 year ago • 10 comments

This seems to and should resolve #500

jmpolom avatar Jan 22 '24 01:01 jmpolom

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

mfussenegger avatar Jan 22 '24 18:01 mfussenegger

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

jmpolom avatar Jan 22 '24 22:01 jmpolom

@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?

cpoftea avatar Jan 23 '24 21:01 cpoftea

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.

mfussenegger avatar Jan 24 '24 19:01 mfussenegger

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.

jmpolom avatar Jan 24 '24 20:01 jmpolom

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

cpoftea avatar Jan 31 '24 15:01 cpoftea

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

mfussenegger avatar Jan 31 '24 16:01 mfussenegger

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

cpoftea avatar Feb 01 '24 09:02 cpoftea

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.

albnavarro avatar Mar 30 '24 11:03 albnavarro

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.

jmpolom avatar Mar 31 '24 00:03 jmpolom

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

mfussenegger avatar May 31 '24 14:05 mfussenegger