-
Notifications
You must be signed in to change notification settings - Fork 92
fix: correct www autofix in no-bare-urls
#720
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
base: main
Are you sure you want to change the base?
Changes from all commits
02691dd
a874ed8
01f6976
548fe33
ef0c2f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -168,7 +168,25 @@ export default /** @satisfies {NoBareUrlsRuleDefinition} */ ({ | |
| node: linkNode, | ||
| messageId: "bareUrl", | ||
| fix(fixer) { | ||
| return fixer.replaceText(linkNode, `<${text}>`); | ||
| let replacementText = `<${text}>`; | ||
| // GFM parses `www` autolinks with an `http://` URL. | ||
| if (url === `http://${text}`) { | ||
| const escapedLinkText = text.replace( | ||
| /[[\]\\*_~`]/gu, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the http://www.example.com/$a$b$c |
||
| "\\$&", | ||
| ); | ||
| const escapedLinkDestination = url.replace( | ||
| /[\\()]/gu, | ||
| "\\$&", | ||
| ); | ||
|
Comment on lines
+174
to
+181
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current escaping breaks For example: | URL |
| --- |
| www.example.com/a\|b |The current fixer produces: | [www.example.com/a\\|b](http://www.example.com/a\\|b) |Because the pipe is now preceded by an even number of backslashes, it is interpreted as a table-cell delimiter. This changes the table structure and can cause Could we escape
Comment on lines
+174
to
+181
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For example, the GFM autolink: www.example.com/a&bcurrently becomes: [www.example.com/a&b](http://www.example.com/a&b)When the fixed output is parsed again, CommonMark interprets Escaping the ampersand as |
||
|
|
||
| replacementText = `[${escapedLinkText}](${escapedLinkDestination})`; | ||
| } | ||
|
|
||
| return fixer.replaceText( | ||
| linkNode, | ||
| replacementText, | ||
| ); | ||
| }, | ||
| }); | ||
| } | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. GFM allows any character other than whitespace and www.google.com/search?q=(business))+okThe current autofix produces: [www.google.com/search?q=(business))+ok](http://www.google.com/search?q=(business))+ok)When parsed again, the actual link destination is truncated to: This happens because CommonMark requires parentheses in a link destination without angle brackets to be escaped or balanced. Similarly, after autofixing
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another incorrect autofix behavior occurs when autofixing the following valid GFM input: www.example.com/a]bproduces invalid link text: [www.example.com/a]b](http://www.example.com/a]b)I reproduced an issue where Additionally, after autofixing |
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.
Could we move the two regular expressions to top-level helpers? That way, we wouldn’t have to create
RegExpobjects every time the fix is performed.