Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: ability to set severity similar to ESLint #65

Closed
kgryte opened this issue May 21, 2016 · 6 comments
Closed

Feature request: ability to set severity similar to ESLint #65

kgryte opened this issue May 21, 2016 · 6 comments
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have

Comments

@kgryte
Copy link

kgryte commented May 21, 2016

Currently, all rules are warnings. By default, even if rules are violated, the process exits with status code 0. To indicate failure, one can set the --frail command-line option to exit with status code 1.

The issue here is that this is an all-or-nothing approach, with violations of any rule being treated equally. And further, setting a rule is binary: either on (true) or off (false).

An extension would be to allow setting rule severity similar to ESLint, which allows off (0), warning (1), or error (2). This would allow users to indicate that violations of certain rules are more prohibited than others, with others being more akin to suggestions.

@wooorm
Copy link
Member

wooorm commented May 21, 2016

Yup, that’s true!

In essence, there are three kinds of messages, a “normal” message, a warning, and an error. And, the difference between the three is whether it’s fatal property is nully (null or undefined), for messages; false, for warnings; and true, for errors.

Fatal errors have the added semantics that they stop the process for the file: no further transformers are applied. They signal that a process (which evolves around said file) is hopelessly corrupt. Think of it as ESLint receiving illegal JavaScript.

Other messages, warning or not, have no influence on the process (other than being included in the report). However, I like to think of them as warnings being something an author can fix, whereas “normal” messages are purely informational.

Now, I personally think ESLint’s system is more extensive than in needs to be. What’s the point in showing messages if I’m not going to fix them, in my opinion. And all messages in remark-lint are written to signal something is wrong and should be fixed!

I’d love to know a) what use case shows that the all-or-nothing approach is not sufficient, and b) what the API for this would be, if added to remark-lint itself (possibly also re message control markers)?

P.S. All warnings triggered by remark-lint have ruleIds, plus as stated above the difference is the value of fatal, so it would be feasible to create a new plug-in for this too!

@wooorm wooorm added 🙉 open/needs-info This needs some more info 🦋 type/enhancement This is great to have labels May 21, 2016
@kgryte
Copy link
Author

kgryte commented Jun 12, 2016

@wooorm One instance where an all-or-nothing approach can cause issues is when linting as part of a build step. For certain classes of errors, while possibly requiring fixes, one may not want to break a build. For instance, because of how remark-lint handles HTML comments, the following results in an error:

// Config...
{
  "final-definition": true
}

Markdown:

<!-- <links> => last section of the file -->

[beep]: http://boop.com

<!-- </links> -->

as the links are not considered as being at the end of the file.

I may still want this rule turned on, for as a developer, I can quickly glance at the console output and see that links are clustered and toward the end of the file, but linting doesn't like the closing comment. And I can also see if a rogue link does exist which is not at the end of the file. Nevertheless, even if a rogue link is not in the closing section, I may not want the build to break, so long as the link works!

And that comes to the bigger point that some errors, while possibly bad, should not prevent a build from happening. As long as the Markdown is functional and not "broken", then an artifact can, and in many cases should be, created.

With the current approach, this is not possible without managing multiple configurations / turning on or off settings based on the environment which, from my POV, is onerous and error prone.

Sound reasonable?

@wooorm
Copy link
Member

wooorm commented Jun 14, 2016

Yes, very! I think it actually may be possible to do this by just overloading the options. We’ll have to see about remark-message-control though, as it will have to patch fatal to true or false.

The only other interference comes from fenced-code-flag, which accepts an array as options, but as it’s an array of strings we could work around it.

@localjo
Copy link

localjo commented Jul 29, 2016

In quality-docs I'm modifying message.fatal inside of the process function based on the ruleId. I pass in an array of "fatal" ruleIds that I want to be shown as errors instead of warnings. https://github.com/SparkartGroupInc/quality-docs/blob/v1.6.1/index.js#L71-L72

I haven't found that to stop the linting at all. The output looks like this;
screen shot 2016-07-29 at 12 22 08 pm

@wooorm
Copy link
Member

wooorm commented Jul 29, 2016

I haven't found that to stop the linting at all.

That’s because in your case, outside of unified’s scope, everything has stopped already!

But I like the idea of having a something, e.g., vfile-severity, which maps sources and ruleIds to messages, warnings, and errors!

@wooorm
Copy link
Member

wooorm commented Aug 16, 2016

Depending on how it goes with remarkjs/remark#183, I’ll do a big update with this feature somewhere this/next week, or I’ll just do a minor bump somewhere sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

No branches or pull requests

3 participants