Add `url` secondary option to any rules
What is the problem you're trying to solve?
Some generic rules (e.g. rule-selector-property-disallowed-list) can be used multiple times, for different reasons, but can link to only one URL. As it is, rule authors cannot point to the relevant documentation for each different use of the rule.
What solution would you like to see?
Enabling the use of custom URLs in the config file, like with messages.
- [ ]
rule-selector-property-disallowed-list - [ ]
declaration-property-value-allowed-list
@emmacharp Thanks for the proposal. Sounds interesting to me.
Like the message secondary option, a new option (e.g. url) seems to be as follows:
const config = {
rules: {
'custom-property-pattern': [/^[a-z]+$/, {
url: 'https://your.custom.rule/resource', // as a string
}],
'color-no-hex': [true, {
url: (rule) => `https://your.custom.rule/${rule}`, // as a function
}]
}
};
Does the example above satisfy your requirements?
Yes, it seems like it does!
Thanks a bunch for the quick response. Looking forward to put that into action!
Got it. Let's wait for other member's feedback.
There seem no objections, so let's transition to "ready to implement".
Rule URLs are used only by a few formatters (for reporting). We can change the logic. E.g.
https://github.com/stylelint/stylelint/blob/5e2eff74a474af5fb0bdfd4c678f3e17bb758935/lib/formatters/verboseFormatter.mjs#L142-L148
if (typeof ruleConfig.secondaryOptions.url === 'function') {
// `ruleConfig` can be retrieved from `returnValue.results[0]?._postcssResult?.stylelint.config?.rules`
const url = ruleConfig.secondaryOptions.url({ name: rule, config: ruleConfig, message: reportedRuleMessage, ... });
// ...
}
This issue is older than one month. Please ask before opening a pull request, as it may no longer be relevant.
@ybiquitous, I'm having a try at implementing this and can't seem to get anything in the results array you specified above...
Maybe I'm being dense but if a try and log it in the verbose formatter, it always return undefined.
Am I using it wrong?
Thanks for the help!
@emmacharp Can you share your branch?
You are fast, @ybiquitous! Hehe. Sorry for the delay. I didn't commit anything since I was only experimenting.
I tried this:
function ruleLink(rule, metadata, returnValue) {
console.log(returnValue.results[0]);
if (metadata && metadata.url) {
return terminalLink(rule, metadata.url);
}
return rule;
}
and then added returnValue in verboseFormatter function:
output += dim(` ${ruleLink(rule, meta, returnValue)}: ${list.length}${additional}\n`);
I also tried logging directly in the verboseFormatter function with the same undefined result.
If this message is not clear I can push this code to my fork and share it...
Ah, I understand my explanation is not enough. 😅
Probably, we may have to save url properties in secondary options like message or severity properties.
https://github.com/stylelint/stylelint/blob/a5965a09b557fd4a5420a6b48f1c1353ecf83aa2/lib/lintPostcssResult.mjs#L102-L105
Users cannot change a rule metadata.
I see. I'll have a look there. But maybe it's above my pay grade. hehe.
But then, about my previous question, shouldn't I be able to access returnValue.results[0]in the verboseFormatter function?
For instance, ant the bottom of the function like this:
...
console.log(returnValue.results[0]);
return `${output}\n`;
}
Sorry for the inconvenience. If I can't get it to work, I'll leave this to professionals and leave you be!
@emmacharp Ah, I understand what you mean in the questions above. When a formatter function is called, lint results are not yet set to returnValue.results. See the code:
https://github.com/stylelint/stylelint/blob/a5965a09b557fd4a5420a6b48f1c1353ecf83aa2/lib/prepareReturnValue.mjs#L67-L69
I forgot why, but I guess we cannot easily change the formatter function signature for backward compatibility. Formatter authors can use the first argument of a formatter function to access lint problems reported.
Thanks for heads up, @ybiquitous! I'll see if I can make progress and I'll report back. Any pointers as to where/how I should try and implement this (if you think of anything) will be welcome.
Reporting back, @ybiquitous !
Here are the changes (in progress) made: 301bec8. I have two questions as of now:
- I can access the
urlthrough therulesproperty mentioned above (not sure I did right though) but I wonder ifurlshould be treated as the message in theWarningobject. Any thoughts? - To make it work, I had to add
urlin the possible options in a rule... but if a custom url is possible on any rule, I guess there would be a better way to go about it?
Not sure it's done right but hope it is. Hehe. Thanks!
@emmacharp The commit looks good. Can you open a PR if you're ready?
Rather than the message property, adding a new url property to the Warning object is better. E.g.
{
"line": 1,
"column": 1,
"endLine": 10,
"endColumn": 2,
"rule": "unit-disallowed-list"
"severity": "error",
"text": "Unexpected unit \"px\" (unit-disallowed-list)"
+ "url": "https://stylelint.io/user-guide/rules/declaration-property-unit-disallowed-list"
}
This addition will benefit for the string and json formatters, too.
@ybiquitous, yes that's what I thought about.
But I'm not sure how/where to set the value of a url property in the Warning object from the config . Can you give me a pointer?
And do we need to add the url secondary option in every rule or is there a more efficient way to do it (like the custom messages, for instance) ?
I'll gladly open a PR afterwards!
@emmacharp You can set url to warningProperties as customMessages:
https://github.com/stylelint/stylelint/blob/a5965a09b557fd4a5420a6b48f1c1353ecf83aa2/lib/utils/report.mjs#L119-L125
Super, I'll have a look at this. Thanks!
Hi @ybiquitous !
It's been a while but I've come back to this and I think I understand what's going on here. Still, I wonder: customMessagesand the result of warn() are strings. If I want to include an url there, I'll have to change the type to an array, for instance, changing the definition of warn(). It seems to be quite risky, is it not?
I don't see how to do it another way but maybe I'm mistaken.
Would be glad to have your thoughts on this!
@emmacharp It'd be great if you could come back! My explanation might be wrong, so let me explain again. My rough idea on this feature is as follows:
- Add
customUrls(likecustomMessages) toStylelintPostcssResult. This will storeurlsecondary option values in users' configuration.-
type StylelintPostcssResult = { customUrls: { [ruleName: string]: string }; // to keep it simple, only string is given. }; - https://github.com/stylelint/stylelint/blob/e3615618321833fe4b0b57c065416da977c87def/types/stylelint/index.d.ts#L156-L159
- https://github.com/stylelint/stylelint/blob/e3615618321833fe4b0b57c065416da977c87def/lib/lintPostcssResult.mjs#L100-L103
-
- Add
url?toWarningOptions.-
type WarningOptions = { url?: string; }; - https://github.com/stylelint/stylelint/blob/e3615618321833fe4b0b57c065416da977c87def/types/stylelint/index.d.ts#L171-L175
-
- Add
url?toWarning.-
type Warning = { url?: string; }; - https://github.com/stylelint/stylelint/blob/e3615618321833fe4b0b57c065416da977c87def/types/stylelint/index.d.ts#L480-L501
-
- In
report(), set a custom URL towarn()as a warning option.-
const { customUrls } = result.stylelint; if (customUrls.[ruleName]) { warningProperties.url = customUrls[ruleName]; } result.warn(warningMessage, warningProperties); - https://github.com/stylelint/stylelint/blob/e3615618321833fe4b0b57c065416da977c87def/lib/utils/report.mjs#L115-L121
-
- In
createPartialStylelintResult(), copy a URL fromwarningstomessages.- https://github.com/stylelint/stylelint/blob/e3615618321833fe4b0b57c065416da977c87def/lib/createPartialStylelintResult.mjs#L64
Please feel free to comment if you still have any questions. 😃
Oh wow. You pretty much did the work for me... I'm kind of sorry if you felt you had to. Heh.
I'll look into this later in the day. Thank you for the support, as always!
It works!
I must admit to being confused by "messages" which seems to mean two different things if I'm not mistaken (first the warning message as a whole and then the text message inside of it)?
Now, I think I may have to add the url option to every rule, is that right? Or is there a more efficient way?
I'll make a pull request afterwards. Thanks, you're a boss!
Myimplementation idea would add the support for the url option, but there seem to be no problem. 👍
Ok, just to make sure: your implementation does not "implicitly" support an url option, right?
I have to add url to validOptions in every rule, like this:
{
actual: secondaryOptions,
possible: {
ignore: ['comments'],
url: [isString],
},
optional: true,
}
It does not seem to work without it.
No, we don't have to check url in every rule. 🤔
If you are ready for PR, I recommend opening it. We can discuss code on that PR in details.
Done! #7743