Skip to content

Use plaintext part of the MDN template - #2078

Closed
Kagami Sascha Rosylight (saschanaz) wants to merge 3 commits into
microsoft:mainfrom
saschanaz:fix-comment-plaintext
Closed

Kagami Sascha Rosylight (saschanaz) wants to merge 3 commits into
microsoft:mainfrom
saschanaz:fix-comment-plaintext

Conversation

@saschanaz

Copy link
Copy Markdown
Contributor

Depends on #2077.

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the PR!

This section of the codebase is owned by Kagami Sascha Rosylight (@saschanaz) - if they write a comment saying "LGTM" then it will be merged.

Comment thread src/build/mdn-comments.ts Fixed
Comment thread src/build/mdn-comments.ts
// For all other cases, the last argument is the plaintext
return trimEnds(args.at(-1)!);
})
// Keep link text but remove URLs

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically you can add links in the comments. So why not include links?
Just a suggestion

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part is from your PR 😛

We can do it, but not here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That part is from your PR 😛

We can do it, but not here.

Correct 😃
I will open another PR for it

The existing regex was expecting `{{domxref(text)}}` form and did not dela with `{{domxref(link, text}}` form. This patch fixes that.

Many more lines added but regex was reduced. Despite that I think it's now more readable.
@saschanaz

Copy link
Copy Markdown
Contributor Author

Looking it again, it should probably be wrapped as a code than plaintext.

@saschanaz

Copy link
Copy Markdown
Contributor Author

Superceded by #2171

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants