From 15b6dd1edcc158ffd998a6945323325dc226c4d9 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Mon, 16 Jul 2018 11:14:45 +0800 Subject: [PATCH 1/2] (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. --- src/features/Folding.ts | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/src/features/Folding.ts b/src/features/Folding.ts index 501e9df186..9abcfa05c0 100644 --- a/src/features/Folding.ts +++ b/src/features/Folding.ts @@ -173,6 +173,14 @@ interface ILineNumberRangeList extends Array { } 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; + constructor( powershellGrammar: IGrammar, ) { @@ -326,18 +334,16 @@ export class FoldingProvider implements vscode.FoldingRangeProvider { } /** - * Given a zero based offset, find the line text after 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 after the offset, not including the subsequent Line Feed + * @returns The line at the offset */ - private subsequentText( + private lineAtOffset( 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)); } /** @@ -420,21 +426,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")); } } }); From 09ae74345d99dba5a466b2bdb68264070fa797b2 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Mon, 16 Jul 2018 11:26:33 +0800 Subject: [PATCH 2/2] (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. --- src/features/Folding.ts | 35 ++++++++++++---------------------- test/features/folding.test.ts | 3 +++ test/fixtures/folding-crlf.ps1 | 12 ++++++++++++ test/fixtures/folding-lf.ps1 | 12 ++++++++++++ 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/features/Folding.ts b/src/features/Folding.ts index 9abcfa05c0..00e1001765 100644 --- a/src/features/Folding.ts +++ b/src/features/Folding.ts @@ -180,6 +180,14 @@ export class FoldingProvider implements vscode.FoldingRangeProvider { */ 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, @@ -317,22 +325,6 @@ export class FoldingProvider implements vscode.FoldingRangeProvider { return result; } - /** - * Given a zero based offset, find the line text preceeding it 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 - */ - private preceedingText( - 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 in the document * @param offset Zero based offset in the document @@ -359,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; diff --git a/test/features/folding.test.ts b/test/features/folding.test.ts index e8c81cdfc6..404b12e968 100644 --- a/test/features/folding.test.ts +++ b/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 --git a/test/fixtures/folding-crlf.ps1 b/test/fixtures/folding-crlf.ps1 index 1fe9358333..3a5eafc81c 100644 --- a/test/fixtures/folding-crlf.ps1 +++ b/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 --git a/test/fixtures/folding-lf.ps1 b/test/fixtures/folding-lf.ps1 index 1fe9358333..3a5eafc81c 100644 --- a/test/fixtures/folding-lf.ps1 +++ b/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