Skip to content

Prevent CDATA Formatting #320

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

Merged
merged 2 commits into from
Jul 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions src/formatting/formatters/v2-xml-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ const MagicalStringOfWonders = "~::~MAAAGIC~::~";
/* tslint:disable no-use-before-declare */
export class V2XmlFormatter implements XmlFormatter {
formatXml(xml: string, options: XmlFormattingOptions): string {
// this replaces all "<" brackets inside of comments to a magical string
// so the following minification steps don't mess with comment formatting
xml = this._sanitizeComments(xml);
// this replaces all "<" brackets inside of comments and CDATA to a magical string
// so the following minification steps don't mess with comment and CDATA formatting
xml = this._sanitizeCommentsAndCDATA(xml);

// remove whitespace from between tags, except for line breaks
xml = xml.replace(/>\s{0,}</g, (match: string) => {
Expand All @@ -25,7 +25,7 @@ export class V2XmlFormatter implements XmlFormatter {
});

// the coast is clear - we can drop those "<" brackets back in
xml = this._unsanitizeComments(xml);
xml = this._unsanitizeCommentsAndCDATA(xml);

let output = "";

Expand Down Expand Up @@ -298,30 +298,30 @@ export class V2XmlFormatter implements XmlFormatter {
return text.replace(/[^\r\n\S]+$/, "");
}

private _sanitizeComments(xml: string): string {
private _sanitizeCommentsAndCDATA(xml: string): string {
let output = "";
let inComment = false;
let inCommentOrCDATA = false;

for (let i = 0; i < xml.length; i++) {
const cc = xml[i];
const nc = xml.charAt(i + 1);
const nnc = xml.charAt(i + 2);
const pc = xml.charAt(i - 1);

if (!inComment && cc === "<" && nc === "!" && nnc === "-") {
inComment = true;
output += "<!--";
if (!inCommentOrCDATA && cc === "<" && nc === "!" && (nnc === "-" || nnc === "[")) {
inCommentOrCDATA = true;
output += (nnc === "-") ? "<!--" : "<![CDATA[";

i += 3;
i += (nnc === "-") ? 3 : 8;
}

else if (inComment && cc === "<") {
else if (inCommentOrCDATA && cc === "<") {
output += MagicalStringOfWonders;
}

else if (inComment && cc === "-" && nc === "-" && nnc === ">") {
inComment = false;
output += "-->";
else if (inCommentOrCDATA && (cc === "-" && nc === "-" && nnc === ">") || (cc === "]" && nc === "]" && nnc === ">")) {
inCommentOrCDATA = false;
output += (cc === "-") ? "-->" : "]]>";

i += 2;
}
Expand All @@ -334,7 +334,7 @@ export class V2XmlFormatter implements XmlFormatter {
return output;
}

private _unsanitizeComments(xml: string): string {
private _unsanitizeCommentsAndCDATA(xml: string): string {
return xml.replace(new RegExp(MagicalStringOfWonders, "g"), "<");
}
}
Expand Down
8 changes: 6 additions & 2 deletions src/test/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ describe("V2XmlFormatter", () => {
it("should handle mixed content as a child of another element", () => {
testFormatter(xmlFormatter, options, "issue-257");
});

it("should not touch CDATA content", () => {
testFormatter(xmlFormatter, options, "issue-293");
});
});

describe("#minifyXml(xml, options)", () => {
Expand Down Expand Up @@ -130,7 +134,7 @@ function testFormatter(xmlFormatter: XmlFormatter, options: XmlFormattingOptions
const actualFormattedXml = xmlFormatter.formatXml(unformattedXml, options).replace(/\r/g, "");

// tslint:disable-next-line
assert.ok((actualFormattedXml === expectedFormattedXml), `Actual formatted XML does not match expected formatted XML.\n\nACTUAL\n${actualFormattedXml.replace(/\s/, "~ws~")}\n\nEXPECTED\n${expectedFormattedXml.replace(/\s/, "~ws~")}`);
assert.ok((actualFormattedXml === expectedFormattedXml), `Actual formatted XML does not match expected formatted XML.\n\nACTUAL\n${actualFormattedXml.replace(/\s/g, "~ws~")}\n\nEXPECTED\n${expectedFormattedXml.replace(/\s/g, "~ws~")}`);
}

function testMinifier(xmlFormatter: XmlFormatter, options: XmlFormattingOptions, fileLabel: string): void {
Expand All @@ -140,5 +144,5 @@ function testMinifier(xmlFormatter: XmlFormatter, options: XmlFormattingOptions,
const actualMinifiedXml = xmlFormatter.minifyXml(unminifiedXml, options).replace(/\r/g, "");

// tslint:disable-next-line
assert.ok((actualMinifiedXml === expectedMinifiedXml), `Actual minified XML does not match expected minified XML.\n\nACTUAL\n${actualMinifiedXml.replace(/\s/, "~ws~")}\n\nEXPECTED\n${expectedMinifiedXml.replace(/\s/, "~ws~")}`);
assert.ok((actualMinifiedXml === expectedMinifiedXml), `Actual minified XML does not match expected minified XML.\n\nACTUAL\n${actualMinifiedXml.replace(/\s/g, "~ws~")}\n\nEXPECTED\n${expectedMinifiedXml.replace(/\s/g, "~ws~")}`);
}
6 changes: 6 additions & 0 deletions src/test/test-data/issue-293.formatted.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<xml>
<element><![CDATA[asdf]]></element>
<element><![CDATA[<secondXml>
<formattedNode>val</formattedNode>
</secondXml>]]></element>
</xml>
6 changes: 6 additions & 0 deletions src/test/test-data/issue-293.unformatted.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<xml>
<element><![CDATA[asdf]]></element>
<element><![CDATA[<secondXml>
<formattedNode>val</formattedNode>
</secondXml>]]></element>
</xml>