-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Configure severity with unified-lint-rule #67
base: main
Are you sure you want to change the base?
Conversation
Hi! It seems some of the things asked in the template are missing? Please edit your post to fill out everything.
You won’t get any more notifications from me, but I’ll keep on updating this comment, and remove it when done! If you need it, here’s the original template<!--
Please check the needed checkboxes ([ ] -> [x]). Leave the
comments as they are, they won’t show on GitHub.
We are excited about pull requests, but please try to limit the scope, provide
a general description of the changes, and remember, it’s up to you to convince
us to land it.
-->
### Initial checklist
* [ ] I read the support docs <!-- https://github.com/remarkjs/.github/blob/main/support.md -->
* [ ] I read the contributing guide <!-- https://github.com/remarkjs/.github/blob/main/contributing.md -->
* [ ] I agree to follow the code of conduct <!-- https://github.com/remarkjs/.github/blob/main/code-of-conduct.md -->
* [ ] I searched issues and couldn’t find anything (or linked relevant results below) <!-- https://github.com/search?q=user%3Aremarkjs&type=Issues -->
* [ ] If applicable, I’ve added docs and tests
### Description of changes
TODO
<!--do not edit: pr-->
Thanks, |
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m open to the idea but am not sure about this approach (see inline comments).
Note that also docs and tests are missing
* @type {import('unified').Plugin<[(Options|[Severity, Options?])?, FileSet?], Root>} | ||
* https://github.com/microsoft/TypeScript/pull/48132 | ||
* @param [rawOptions] | ||
* @param [fileSet] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The
typedef
should not be in this block: that makes the description belong to it. The description belongs to the plugin. You can move the typedef up. - Severity and label are different things as types, severity does not encompass both, so I think it’s better to not import both as one name, but import them separately under their own names
- Add type for rules' callback parameter remark-lint#283 (comment)
checkAll([file], next) | ||
return lintRule( | ||
{ | ||
origin: 'validate-links', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This most likely prevents ruleId
s from being set. We currently use different ruleId
s, and this overwrites them:
remark-validate-links/lib/check/validate.js
Lines 73 to 88 in 78871b0
if (hash) { | |
reason = 'Link to unknown heading' | |
ruleId = constants.headingRuleId | |
if (base && path.join(base, filePath) !== absolute) { | |
reason += ' in `' + filePath + '`' | |
ruleId = constants.headingInFileRuleId | |
} | |
reason += ': `' + hash + '`' | |
} else { | |
reason = 'Link to unknown file: `' + filePath + '`' | |
ruleId = constants.fileRuleId | |
} | |
const origin = [constants.sourceId, ruleId].join(':') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It’s likely not correct to use use lintRule
. That’s meant for one rule (one reason for emitting things). This package supports different reasons.
Perhaps coerce
should be used, and exposed from unified-lint-rule
?
Initial checklist
Description of changes
What do you think about configuring the message severity in the standard way, using unified-lint-rule? e.g.
I wrapped the
remarkValidateLinks
transformer inlintRule()
and pass it the raw options, maybe with a severity.