-
-
Notifications
You must be signed in to change notification settings - Fork 743
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
[new Rule] no links in headers #762
Comments
This rule is straightforward and not auto-fixable. If this is a rule fits the default set of markdownlint, i will gladly provide a pullrequest :) // @ts-check
"use strict";
module.exports = {
// eslint-disable-next-line no-warning-comments
// TODO Adapt here
"names": [ "no-links-in-headings" ],
"description": "detects if links are hidden in headings",
"information": new URL(
"https://todo.add.url"
),
"tags": [ "<tags>" ],
"function": (params, onError) => {
for (let i = 0; i < params.tokens.length - 1; i++) {
const token = params.tokens[i];
const nextToken = params.tokens[i + 1];
if (token.type === "heading_open" &&
nextToken.type === "inline") {
for (const tokenChildren of nextToken.children) {
if (tokenChildren.type === "link_open") {
onError({
"lineNumber": nextToken.lineNumber,
"detail": nextToken.line,
"context": "link in header",
"range": [ 1, 1 ]
});
}
}
}
}
}
}; |
This makes sense to me and seems reasonable as a built-in rule. I'll warn you now there's probably more to adding a rule then you expect. :) You can look at some previous PRs for context. Some first thoughts:
|
I am going down the rabbit hole :D and tried to find guidance regarding this rule. As it was hard to find something, I thought of using ChatGPT - which sounded helpful, but sadly all mentioned articles seem to be of interest but are not accessible anymore.
So building a real rational explanation with a strong foundation will be hard. Additionally to the hard to find rational, the existing violations within the test-repos is also interesting. Test Repos Result
I am not sure if we want to continue, with this rule, for markdownlint (as a basic rule), or if we should release this rule on its own. sidenotemy current implementation has following test: # test for links in headings
## Heading <https://with-a-link> {MD054}
## Heading with an [inline link in link](<https://with-a-link>){MD054}
## Heading with a [link](https://with-a-link){MD054}
## Heading to an [anchor](#with-an-anchor){MD054}
## Heading with a [relative-link](./){MD054}
## Heading with a [rel-link][rel-link]{MD054}
## With an anchor
[rel-link]: https://with-a-link |
First off, thank you for doing the research here! I absolutely agree with you that links in headings feels like something that should be avoided. BUT at the same time, the results you share show that it’s pretty common in practice and very much deliberate. As you point out, it’s fully legal to do this in HTML and in Markdown and we don’t know of any cases where doing so breaks things or leads to unpredictable behavior. So without much of a reason to tell people to avoid this, I am leaning toward your idea to release this as a separate rule instead of integrating it into the library. |
Perfect, I will close this issue. As always it was my pleasure :) |
Thanks for your understanding! |
# heading with [link](link]
Although it is possible and it gets rendered correctly, links in headings provide a terrible user experience. Within rendered Markdown it is sometimes hard to grasp if a title is now a link or not.
Therefore, I suggest a rule called
no-links-in-headings
.The text was updated successfully, but these errors were encountered: