-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Allow use of Markdown in description and notes #21495
Conversation
I'm all for it, though I think this needs a decision from the maintainers. A question: does this lead to a new major release? Maybe we should do it in two phases: changing the schema, allowing some time for consumers (MDN displays these notes, maybe others) to adapt, and then making the textual changes (If I remember well, HTML tags are valid in MD, so it should be ok). |
19ca8e8
to
91a53cc
Compare
No, this actually does not change the schema! To the end user, they'll just see a few notes change, but they're still formatted as HTML. My apologies, I didn't clarify that the schema proposal is for a future schema change. This PR purely changes internal formatting for maintainers. I updated the PR description to clarify this. |
This pull request has merge conflicts that must be resolved before it can be merged. |
This pull request has merge conflicts that must be resolved before it can be merged. |
This pull request has merge conflicts that must be resolved before it can be merged. |
This mitigates an issue where the text inside of `<code>` tags gets parsed as Markdown.
Fixes Performed: - Ensure code tags are inside of anchors (`<code>(<a href='[\w:;#&/=_().-]+'>)([\w\s.,…()&:;-]+)</a></code>` -> `$1<code>$2</code></a>`) - Removed tags (`<strong>`, `<em>`) inside of inline code blocks - Drive-by fix: tweak Edge note for FileSystem API to state the behavior applies to previous versions of Edge - Use backticks instead of `<code>` where it is needed - Add escapes to characters that need them - Add `<code>` blocks to example URLs that needed them - Set a note to mirror from upstream as needed (for accuracy)
I have just pushed a few commits that resolve issues in the Markdown conversion. Looking at the diff, I noticed that a good chunk of differences could be resolved programmatically, and then the remaining issues were a small handful that were either issues in the markup that need to be resolved in the main branch anyways, or small tweaks that can be done within this PR. I pushed a commit that performs the small handful of fixes, and cherry-picked the ones that are not exclusively for Markdown conversion and opened #24911. The remainder of the diff is now all due to notes that are already Markdown-formatted. |
This pull request has merge conflicts that must be resolved before it can be merged. |
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.
LGTM, except one nit.
Compared this branch's build against main with jd:
jd --color data_ma*.json | sed 's/\\u003c/</g' | sed 's/\\u003e/>/g' | sed 's/\\u0026/\&/g'`)
The diff looks good now, so I'm merging. FWIW Here's the full diff:
|
This PR updates the build/lint scripts to allow the use of Markdown for descriptions and notes internally. To retain compatibility with consumers, the Markdown is then converted to HTML.
This is a purely internal change that will not impact consumers, let alone warrant a semantic versioning bump. This will only positively impact maintainers.
Note: some of our notes already have Markdown formatting.
Follow-Up Work
After this PR is approved, we should go through and convert all of our notes to Markdown format in a bulk update.
Future Schema Change Proposal
I plan to open an issue to formally propose this, however I am unsure if we want to do this change now while converting to Markdown, so I am mentioning it here.
To allow consumers to utilize the Markdown versions if they wish to, I am proposing a future change to the schema to convert the
notes
property into an object containing both formats, like so:Alternatively (though I'm less favorable of this), to keep from making a breaking change, we can add a
notesMD
property alongside the HTML version: