Merge branch 'master' into issue-206

.github/workflows/release.yml

@@ -0,0 +1,27 @@
+name: "Release to Marketplace"
+
+on:
+  push:
+    tags:
+      - "v*"
+
+jobs:
+  release:
+    runs-on: "ubuntu-latest"
+    steps:
+    - name: "Checkout"
+      uses: "actions/checkout@v2"
+    
+    - name: "Setup NodeJS"
+      uses: "actions/setup-node@v2.1.0"
+      
+    - name: "Install Dependencies"
+      run: "npm install"
+
+    - name: "Run Tests"
+      run: "npm run test"
+
+    - name: "Publish to Marketplace"
+      uses: "sigma/vsce-publish-action@v0.0.2"
+      with:
+        vsce_token: ${{ secrets.VSCE_TOKEN }}

.github/workflows/test.yml

@@ -0,0 +1,22 @@
+name: "Run Tests"
+
+on:
+  pull_request:
+    branches:
+      - "master"
+
+jobs:
+  test:
+    runs-on: "ubuntu-latest"
+    steps:
+    - name: "Checkout"
+      uses: "actions/checkout@v2"
+    
+    - name: "Setup NodeJS"
+      uses: "actions/setup-node@v2.1.0"
+      
+    - name: "Install Dependencies"
+      run: "npm install"
+
+    - name: "Run Tests"
+      run: "npm run test"

package.json

@@ -261,7 +261,8 @@
         "compile": "npm run lint && tsc -p ./",
         "watch": "tsc -watch -p ./",
         "postinstall": "node ./node_modules/vscode/bin/install",
-        "test": "npm run compile && mocha ./out/test/**/*.js",
+        "test": "npm run compile && mocha './out/test/**/*.js'",
+        "test-windows": "npm run compile && mocha ./out/test/**/*.js",
         "lint": "tslint -p tslint.json --fix"
     },
     "devDependencies": {

src/formatting/commands/textToXml.ts

@@ -19,7 +19,14 @@ export function textToXml(textEditor: TextEditor): void {
                 );
             }
             const txt = textEditor.document.getText(new Range(selection.start, selection.end));
-            const transformed = txt.replace(/&lt;/g, "<").replace(/&gt;/g, ">");
+            const transformed = txt
+                .replace(/&lt;/g, "<")
+                .replace(/&gt;/g, ">")
+                .replace(/&amp;/g, "&")
+                // tslint:disable-next-line
+                .replace(/&quot;/g, '"')
+                .replace(/&apos;/g, "'");
+
             textEdit.replace(selection, transformed);
         });
     });

src/formatting/commands/xmlToText.ts

@@ -19,7 +19,13 @@ export function xmlToText(textEditor: TextEditor): void {
                 );
             }
             const txt = textEditor.document.getText(new Range(selection.start, selection.end));
-            const transformed = txt.replace(/</g, "&lt;").replace(/>/g, "&gt;");
+            const transformed = txt
+                .replace(/</g, "&lt;")
+                .replace(/>/g, "&gt;")
+                .replace(/&/g, "&amp;")
+                .replace(/"/g, "&quot;")
+                .replace(/'/g, "&apos;");
+
             textEdit.replace(selection, transformed);
         });
     });

src/formatting/formatters/classic-xml-formatter.ts

@@ -66,7 +66,7 @@ export class ClassicXmlFormatter implements XmlFormatter {
     }
 
     minifyXml(xml: string, options: XmlFormattingOptions): string {
-        xml = this._stripLineBreaks(options, xml); // all line breaks outside of CDATA elements
+        xml = this._stripLineBreaks(options, xml); // all line breaks outside of CDATA elements and comments
         xml = (options.removeCommentsOnMinify) ? xml.replace(/\<![ \r\n\t]*(--([^\-]|[\r\n]|-[^\-])*--[ \r\n\t]*)\>/g, "") : xml;
         xml = xml.replace(/>\s{0,}</g, "><"); // insignificant whitespace between tags
         xml = xml.replace(/"\s+(?=[^\s]+=)/g, "\" "); // spaces between attributes
@@ -87,28 +87,62 @@ export class ClassicXmlFormatter implements XmlFormatter {
         return `${options.newLine}${indentPattern.repeat(level)}${trailingValue}`;
     }
 
+    /**
+     * Removes line breaks outside of CDATA, comment, and xml:space="preserve" blocks.
+     */
     private _stripLineBreaks(options: XmlFormattingOptions, xml: string): string {
         let output = "";
         const inTag = false;
         const inTagName = false;
-        let inCdata = false;
+        let inCdataOrComment = false;
         const inAttribute = false;
 
+        let preserveSpace = false;
+        let level = 0;
+        let levelpreserveSpaceActivated = 0;
+
         for (let i = 0; i < xml.length; i++) {
             const char: string = xml.charAt(i);
             const prev: string = xml.charAt(i - 1);
             const next: string = xml.charAt(i + 1);
 
+            // CDATA and comments
             if (char === "!" && (xml.substr(i, 8) === "![CDATA[" || xml.substr(i, 3) === "!--")) {
-                inCdata = true;
-            } else if (char === "]" && (xml.substr(i, 3) === "]]>")) {
-                inCdata = false;
-            } else if (char === "-" && (xml.substr(i, 3) === "-->")) {
-                inCdata = false;
-            } else if (char.search(/[\r\n]/g) > -1 && !inCdata) {
+                inCdataOrComment = true;
+            }
+
+            else if (char === "]" && (xml.substr(i, 3) === "]]>")) {
+                inCdataOrComment = false;
+            }
+
+            else if (char === "-" && (xml.substr(i, 3) === "-->")) {
+                inCdataOrComment = false;
+            }
+
+            // xml:space="preserve"
+            if (char === ">" && prev !== "/") {
+                level++;
+            }
+
+            else if (!inCdataOrComment && char === "/" && (prev === "<" || next === ">")) {
+                level--;
+            }
+
+            if (char === "x" && (xml.substr(i, 20).toLowerCase() === `xml:space="preserve"`)) {
+                preserveSpace = true;
+                levelpreserveSpaceActivated = level;
+            }
+
+            else if (!inCdataOrComment && preserveSpace && (char === "/" && (prev === "<" || next === ">")) && (level === levelpreserveSpaceActivated)) {
+                preserveSpace = false;
+            }
+
+            if (char.search(/[\r\n]/g) > -1 && !inCdataOrComment && !preserveSpace) {
                 if (/\r/.test(char) && /\S|\r|\n/.test(prev) && /\S|\r|\n/.test(xml.charAt(i + options.newLine.length))) {
                     output += char;
-                } else if (/\n/.test(char) && /\S|\r|\n/.test(xml.charAt(i - options.newLine.length)) && /\S|\r|\n/.test(next)) {
+                }
+
+                else if (/\n/.test(char) && /\S|\r|\n/.test(xml.charAt(i - options.newLine.length)) && /\S|\r|\n/.test(next)) {
                     output += char;
                 }
 

src/formatting/formatters/v2-xml-formatter.ts

@@ -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) => {
@@ -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 = "";
 
@@ -291,16 +291,16 @@ export class V2XmlFormatter implements XmlFormatter {
     }
 
     private _getIndent(options: XmlFormattingOptions, indentLevel: number): string {
-        return ((options.editorOptions.insertSpaces) ? " ".repeat(options.editorOptions.tabSize) : "\t").repeat(indentLevel);
+        return ((options.editorOptions.insertSpaces) ? " ".repeat(options.editorOptions.tabSize) : "\t").repeat(Math.max(indentLevel, 0));
     }
 
     private _removeTrailingNonBreakingWhitespace(text: string): string {
         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];
@@ -308,20 +308,20 @@ export class V2XmlFormatter implements XmlFormatter {
             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;
             }
@@ -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"), "<");
     }
 }

src/test/extension.test.ts

@@ -99,15 +99,50 @@ 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)", () => {
+
+        const options = {
+            editorOptions: {
+                insertSpaces: true,
+                tabSize: 4
+            },
+            enforcePrettySelfClosingTagOnFormat: false,
+            newLine: "\r\n",
+            removeCommentsOnMinify: false,
+            splitAttributesOnFormat: false,
+            splitXmlnsOnFormat: true
+        };
+
+        it("should preserve whitespace on minify if xml:space is set to 'preserve-whitespace'", () => {
+            testMinifier(xmlFormatter, options, "issue-262");
+        });
+
     });
 
 });
 
 function testFormatter(xmlFormatter: XmlFormatter, options: XmlFormattingOptions, fileLabel: string): void {
-    const expectedFormattedXml = TestDataLoader.load(`${fileLabel}.formatted.xml`);
+    const expectedFormattedXml = TestDataLoader.load(`${fileLabel}.formatted.xml`).replace(/\r/g, "");
     const unformattedXml = TestDataLoader.load(`${fileLabel}.unformatted.xml`);
 
-    const actualFormattedXml = xmlFormatter.formatXml(unformattedXml, options);
+    const actualFormattedXml = xmlFormatter.formatXml(unformattedXml, options).replace(/\r/g, "");
 
-    assert.equal(actualFormattedXml, expectedFormattedXml, "Actual formatted XML does not match expected formatted XML.");
+    // tslint:disable-next-line
+    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 {
+    const expectedMinifiedXml = TestDataLoader.load(`${fileLabel}.minified.xml`).replace(/\r/g, "");
+    const unminifiedXml = TestDataLoader.load(`${fileLabel}.unminified.xml`);
+
+    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/g, "~ws~")}\n\nEXPECTED\n${expectedMinifiedXml.replace(/\s/g, "~ws~")}`);
 }

src/test/test-data/issue-262.minified.xml

@@ -0,0 +1,10 @@
+<tests><test>1.
+2.
+3.
+4.</test><test xml:space="preserve">1.
+2.
+3.
+4.</test><test>1. 2. 3. 4.</test><test xml:space="preserve">1.
+ 2.
+ 3.
+ 4.</test></tests>

src/test/test-data/issue-262.unminified.xml

@@ -0,0 +1,18 @@
+<tests>
+<test>1.
+2.
+3.
+4.</test>
+<test xml:space="preserve">1.
+2.
+3.
+4.</test>
+<test>1.
+ 2.
+ 3.
+ 4.</test>
+<test xml:space="preserve">1.
+ 2.
+ 3.
+ 4.</test>
+</tests>

src/test/test-data/issue-293.formatted.xml

@@ -0,0 +1,6 @@
+<xml>
+    <element><![CDATA[asdf]]></element>
+    <element><![CDATA[<secondXml>
+    <formattedNode>val</formattedNode>
+</secondXml>]]></element>
+</xml>

src/test/test-data/issue-293.unformatted.xml

@@ -0,0 +1,6 @@
+<xml>
+    <element><![CDATA[asdf]]></element>
+    <element><![CDATA[<secondXml>
+    <formattedNode>val</formattedNode>
+</secondXml>]]></element>
+</xml>