-
Notifications
You must be signed in to change notification settings - Fork 9.1k
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
Preserve rfc section links #3956
Conversation
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.
Looks good. 👍
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.
Minor comments, but I'm fine with this going in.
//TODO: unconventional references to RFCs in 3.0.4 and 3.1.1, for example | ||
// [RFC3986 §5.1.2 – 5.1.4](https://tools.ietf.org/html/rfc3986#section-5.1.2) | ||
// RFC6570 [mentions](https://www.rfc-editor.org/rfc/rfc6570.html#section-2.4.2) | ||
// [are not](https://datatracker.ietf.org/doc/html/rfc3986#appendix-A) | ||
// [special behavior](https://www.rfc-editor.org/rfc/rfc1866#section-8.2.1) | ||
// [RFC6570 considers to be _undefined_](https://datatracker.ietf.org/doc/html/rfc6570#section-2.3) |
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 entirely fine with making these more normal rather than working around them in the build script.
//TODO: non-link mentions of RFCs in 3.0.4 and 3.1.1, for example | ||
// RFC3986's definition of [reserved](https://datatracker.ietf.org/doc/html/rfc3986#section-2.2) |
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.
Also fine with these being made more typical.
if (Math.abs(delta)>1) console.warn(delta,line); | ||
// if (Math.abs(delta)>1) console.warn(delta,line); |
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.
Any particular reason to keep this line as a comment instead of delete?
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.
Reverence for @MikeRalphson 😄
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.
Will remove them in a future PR
Fixes #3863
See for example