(GH-1437) Fix detecting contiguous comment blocks and regions (#1438)

* (maint) Refactor region folding detection

Previously the region comment detection used a little convoluted method to
detect regions in a document.  This commit simplifies the detection by
extracting the line from the document and using regex's similar to that used by
the PowerShell language configuration.  This also removes the need for the
emptyline and subsequentText method calls.  While the performance of the folder
is pretty quick, this should in theory make it faster on larger documents by
doing less calls to the VSCode Document API.

* (GH-1437) Fix detecting contiguous comment blocks and regions

Previously the syntax folding feature would not correctly identify comment
blocks and comment regions if they appeared all together.  This commit changes
the comment block detection to ignore line comments that start with region and
endregion, i.e. region block start/end directives.  This commit also adds test
for this scenario.

Author: glennsarti

Diff for: src/features/Folding.ts

@@ -173,6 +173,22 @@ interface ILineNumberRangeList extends Array<LineNumberRange> { }
 export class FoldingProvider implements vscode.FoldingRangeProvider {
     private powershellGrammar: IGrammar;
 
+    /**
+     * These regular expressions are used to match lines which mark the start and end of region comment in a PowerShell
+     * script. They are based on the defaults in the VS Code Language Configuration at;
+     * https://github.com/Microsoft/vscode/blob/64186b0a26/extensions/powershell/language-configuration.json#L26-L31
+     */
+    private readonly startRegionText = /^\s*#region\b/i;
+    private readonly endRegionText = /^\s*#endregion\b/i;
+    /**
+     * This regular expressions is used to detect a line comment (as opposed to an inline comment), that is not a region
+     * block directive i.e.
+     * - No text between the beginning of the line and `#`
+     * - Comment does start with region
+     * - Comment does start with endregion
+     */
+    private readonly lineCommentText = /\s*#(?!region\b|endregion\b)/i;
+
     constructor(
         powershellGrammar: IGrammar,
     ) {
@@ -310,34 +326,16 @@ export class FoldingProvider implements vscode.FoldingRangeProvider {
     }
 
     /**
-     * Given a zero based offset, find the line text preceeding it in the document
+     * Given a zero based offset, find the line in the document
      * @param offset   Zero based offset in the document
      * @param document The source text document
-     * @returns        The line text preceeding the offset, not including the preceeding Line Feed
+     * @returns        The line at the offset
      */
-    private preceedingText(
+    private lineAtOffset(
         offset: number,
         document: vscode.TextDocument,
-    ): string {
-        const endPos = document.positionAt(offset);
-        const startPos = endPos.translate(0, -endPos.character);
-
-        return document.getText(new vscode.Range(startPos, endPos));
-    }
-
-    /**
-     * Given a zero based offset, find the line text after it in the document
-     * @param offset   Zero based offset in the document
-     * @param document The source text document
-     * @returns        The line text after the offset, not including the subsequent Line Feed
-     */
-    private subsequentText(
-        offset: number,
-        document: vscode.TextDocument,
-    ): string {
-        const startPos: vscode.Position = document.positionAt(offset);
-        const endPos: vscode.Position = document.lineAt(document.positionAt(offset)).range.end;
-        return document.getText(new vscode.Range(startPos, endPos));
+    ): vscode.TextLine {
+        return document.lineAt(document.positionAt(offset));
     }
 
     /**
@@ -353,19 +351,16 @@ export class FoldingProvider implements vscode.FoldingRangeProvider {
         document: vscode.TextDocument,
     ): ILineNumberRangeList {
         const result = [];
-
-        const emptyLine = /^\s*$/;
-
         let startLine: number = -1;
         let nextLine: number = -1;
 
         tokens.forEach((token) => {
             if (token.scopes.indexOf("punctuation.definition.comment.powershell") !== -1) {
+                const line: vscode.TextLine = this.lineAtOffset(token.startIndex, document);
                 // The punctuation.definition.comment.powershell token matches new-line comments
-                // and inline comments e.g. `$x = 'foo' # inline comment`.  We are only interested
-                // in comments which begin the line i.e. no preceeding text
-                if (emptyLine.test(this.preceedingText(token.startIndex, document))) {
-                    const lineNum = document.positionAt(token.startIndex).line;
+                // and inline comments e.g. `$x = 'foo' # inline comment`.
+                if (this.lineCommentText.test(line.text)) {
+                    const lineNum = line.lineNumber;
                     // A simple pattern for keeping track of contiguous numbers in a known sorted array
                     if (startLine === -1) {
                         startLine = lineNum;
@@ -420,21 +415,14 @@ export class FoldingProvider implements vscode.FoldingRangeProvider {
         document: vscode.TextDocument,
     ): ITokenList {
         const result = [];
-
-        const emptyLine = /^\s*$/;
-        const startRegionText = /^#region\b/i;
-        const endRegionText = /^#endregion\b/i;
-
         tokens.forEach((token) => {
             if (token.scopes.indexOf("punctuation.definition.comment.powershell") !== -1) {
-                if (emptyLine.test(this.preceedingText(token.startIndex, document))) {
-                    const commentText = this.subsequentText(token.startIndex, document);
-                    if (startRegionText.test(commentText)) {
-                        result.push(this.addTokenScope(token, "custom.start.region"));
-                    }
-                    if (endRegionText.test(commentText)) {
-                        result.push(this.addTokenScope(token, "custom.end.region"));
-                    }
+                const line: string = this.lineAtOffset(token.startIndex, document).text;
+                if (this.startRegionText.test(line)) {
+                    result.push(this.addTokenScope(token, "custom.start.region"));
+                }
+                if (this.endRegionText.test(line)) {
+                    result.push(this.addTokenScope(token, "custom.end.region"));
                 }
             }
         });

Diff for: test/features/folding.test.ts

@@ -50,6 +50,9 @@ suite("Features", () => {
                 { start: 41, end: 45, kind: 3 },
                 { start: 51, end: 53, kind: 3 },
                 { start: 56, end: 59, kind: 3 },
+                { start: 64, end: 66, kind: 1 },
+                { start: 67, end: 72, kind: 3 },
+                { start: 68, end: 70, kind: 1 },
             ];
 
             test("Can detect all of the foldable regions in a document with CRLF line endings", async () => {

Diff for: test/fixtures/folding-crlf.ps1

@@ -59,3 +59,15 @@ double quoted herestrings should also fold
   'should fold2'
   )
 }
+
+# Make sure contiguous comment blocks can be folded properly
+
+# Comment Block 1
+# Comment Block 1
+# Comment Block 1
+#region Comment Block 3
+# Comment Block 2
+# Comment Block 2
+# Comment Block 2
+$something = $true
+#endregion Comment Block 3

Diff for: test/fixtures/folding-lf.ps1

@@ -59,3 +59,15 @@ double quoted herestrings should also fold
   'should fold2'
   )
 }
+
+# Make sure contiguous comment blocks can be folded properly
+
+# Comment Block 1
+# Comment Block 1
+# Comment Block 1
+#region Comment Block 3
+# Comment Block 2
+# Comment Block 2
+# Comment Block 2
+$something = $true
+#endregion Comment Block 3