Skip to content
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

fix(pacmak): xmldom error when generating packages #2713

Merged
merged 2 commits into from
Mar 16, 2021

Conversation

RomainMuller
Copy link
Contributor

In some cases, an xmldom error would occur when trying to render C#
XML-style documentation blocks, where inline html within a Markdown
document is not valid as far as the xmldom parser is considered. This
introduces a safer fall-back strategy.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

In some cases, an `xmldom` error would occur when trying to render C#
XML-style documentation blocks, where inline html within a Markdown
document is not valid as far as the `xmldom` parser is considered. This
introduces a safer fall-back strategy.
@RomainMuller RomainMuller requested a review from iliapolo March 16, 2021 15:40
@RomainMuller RomainMuller self-assigned this Mar 16, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Mar 16, 2021
return new XMLSerializer().serializeToString(doc);
} catch {
// Could not parse - we'll escape unsafe XML entities here...
return html.replace(/[<>&]/g, (char: string) => {
Copy link
Contributor

@iliapolo iliapolo Mar 16, 2021

Choose a reason for hiding this comment

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

So in case of success this method returns the result of serializeToString, but in case of failure it just replaces some chars in the html? Shouldn't this go back through the serializer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no XML/HTML to parse anymore once you've stripped out all the <, and >, is there?

I've tried it out of curiosity, and it results in double-XML-escaping the entities (which is weird, but heh).

Copy link
Contributor

@iliapolo iliapolo Mar 16, 2021

Choose a reason for hiding this comment

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

It just dawned on me that the thing we are parsing is an XML comment 👍

@RomainMuller RomainMuller merged commit 6b2bbe8 into main Mar 16, 2021
@RomainMuller RomainMuller deleted the rmuller/fix-integ branch March 16, 2021 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants