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

(GH-1514) Show the last line of folding regions as per default VSCode #1557

Merged
merged 1 commit into from
Oct 23, 2018
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
5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,11 @@
"default": true,
"description": "Enables syntax based code folding. When disabled, the default indentation based code folding is used."
},
"powershell.codeFolding.showLastLine": {
"type": "boolean",
"default": true,
"description": "Shows the last line of a folded section similar to the default VSCode folding style. When disabled, the entire folded region is hidden."
},
"powershell.codeFormatting.preset": {
"type": "string",
"enum": [
Expand Down
18 changes: 16 additions & 2 deletions src/features/Folding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,10 @@ class LineNumberRange {
* Creates a vscode.FoldingRange object based on this object
* @returns A Folding Range object for use with the Folding Provider
*/
public toFoldingRange(): vscode.FoldingRange {
public toFoldingRange(settings: Settings.ISettings): vscode.FoldingRange {
if (settings.codeFolding && settings.codeFolding.showLastLine) {
return new vscode.FoldingRange(this.startline, this.endline - 1, this.rangeKind);
}
return new vscode.FoldingRange(this.startline, this.endline, this.rangeKind);
}
}
Expand Down Expand Up @@ -252,15 +255,26 @@ export class FoldingProvider implements vscode.FoldingRangeProvider {
return 0;
});

const settings = this.currentSettings();
return foldableRegions
// It's possible to have duplicate or overlapping ranges, that is, regions which have the same starting
// line number as the previous region. Therefore only emit ranges which have a different starting line
// than the previous range.
.filter((item, index, src) => index === 0 || item.startline !== src[index - 1].startline)
// Convert the internal representation into the VSCode expected type
.map((item) => item.toFoldingRange());
.map((item) => item.toFoldingRange(settings));
}

/**
* A helper to return the current extension settings. This helper is primarily for use by unit testing so
* that extension settings can be mocked.
* - The settings cannot be set in the constructor as the settings should be evalauted on each folding request
* so that setting changes are immediate, i.e. they do not require an extension reload
* - The method signature for provideFoldingRanges can not be changed as it is explicitly set in the VSCode API,
* therefore the settings can not be passed in the method call, which would be preferred
*/
public currentSettings(): Settings.ISettings { return Settings.load(); }
rjmholt marked this conversation as resolved.
Show resolved Hide resolved

/**
* Given a start and end textmate scope name, find matching grammar tokens
* and pair them together. Uses a simple stack to take into account nested regions.
Expand Down
2 changes: 2 additions & 0 deletions src/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export interface IBugReportingSettings {

export interface ICodeFoldingSettings {
enable?: boolean;
showLastLine?: boolean;
}

export interface ICodeFormattingSettings {
Expand Down Expand Up @@ -116,6 +117,7 @@ export function load(): ISettings {

const defaultCodeFoldingSettings: ICodeFoldingSettings = {
enable: true,
showLastLine: false,
};

const defaultCodeFormattingSettings: ICodeFormattingSettings = {
Expand Down
66 changes: 48 additions & 18 deletions test/features/folding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as path from "path";
import * as vscode from "vscode";
import { DocumentSelector } from "vscode-languageclient";
import * as folding from "../../src/features/Folding";
import * as Settings from "../../src/settings";
import { MockLogger } from "../test_utils";

const fixturePath = path.join(__dirname, "..", "..", "..", "test", "fixtures");
Expand All @@ -22,6 +23,13 @@ function assertFoldingRegions(result, expected): void {
assert.equal(result.length, expected.length);
}

// Wrap the FoldingProvider class with our own custom settings for testing
class CustomSettingFoldingProvider extends folding.FoldingProvider {
public customSettings: Settings.ISettings = Settings.load();
rjmholt marked this conversation as resolved.
Show resolved Hide resolved
// Overridde the super currentSettings method with our own custom test settings
public currentSettings(): Settings.ISettings { return this.customSettings; }
}

suite("Features", () => {

suite("Folding Provider", async () => {
Expand All @@ -38,21 +46,21 @@ suite("Features", () => {

suite("For a single document", async () => {
const expectedFoldingRegions = [
{ start: 0, end: 4, kind: 3 },
{ start: 1, end: 3, kind: 1 },
{ start: 10, end: 15, kind: 1 },
{ start: 16, end: 60, kind: null },
{ start: 17, end: 22, kind: 1 },
{ start: 23, end: 26, kind: null },
{ start: 28, end: 31, kind: null },
{ start: 35, end: 37, kind: 1 },
{ start: 39, end: 49, kind: 3 },
{ start: 41, end: 45, kind: 3 },
{ start: 51, end: 53, kind: null },
{ start: 56, end: 59, kind: null },
{ start: 64, end: 66, kind: 1 },
{ start: 67, end: 72, kind: 3 },
{ start: 68, end: 70, kind: 1 },
{ start: 0, end: 3, kind: 3 },
{ start: 1, end: 2, kind: 1 },
{ start: 10, end: 14, kind: 1 },
{ start: 16, end: 59, kind: null },
{ start: 17, end: 21, kind: 1 },
{ start: 23, end: 25, kind: null },
{ start: 28, end: 30, kind: null },
{ start: 35, end: 36, kind: 1 },
{ start: 39, end: 48, kind: 3 },
{ start: 41, end: 44, kind: 3 },
{ start: 51, end: 52, kind: null },
{ start: 56, end: 58, kind: null },
{ start: 64, end: 65, kind: 1 },
{ start: 67, end: 71, kind: 3 },
{ start: 68, end: 69, kind: 1 },
];

test("Can detect all of the foldable regions in a document with CRLF line endings", async () => {
Expand Down Expand Up @@ -83,9 +91,31 @@ suite("Features", () => {
assertFoldingRegions(result, expectedFoldingRegions);
});

suite("Where showLastLine setting is false", async () => {
const customprovider = (new CustomSettingFoldingProvider(psGrammar));
customprovider.customSettings.codeFolding.showLastLine = false;

test("Can detect all foldable regions in a document", async () => {
// Integration test against the test fixture 'folding-lf.ps1' that contains
// all of the different types of folding available
const uri = vscode.Uri.file(path.join(fixturePath, "folding-lf.ps1"));
const document = await vscode.workspace.openTextDocument(uri);
const result = await customprovider.provideFoldingRanges(document, null, null);

// Incrememnt the end line of the expected regions by one as we will
// be hiding the last line
const expectedLastLineRegions = expectedFoldingRegions.map( (item) => {
item.end++;
return item;
});

assertFoldingRegions(result, expectedLastLineRegions);
});
});

test("Can detect all of the foldable regions in a document with mismatched regions", async () => {
const expectedMismatchedFoldingRegions = [
{ start: 2, end: 4, kind: 3 },
{ start: 2, end: 3, kind: 3 },
];

// Integration test against the test fixture 'folding-mismatch.ps1' that contains
Expand All @@ -99,8 +129,8 @@ suite("Features", () => {

test("Does not return duplicate or overlapping regions", async () => {
const expectedMismatchedFoldingRegions = [
{ start: 1, end: 2, kind: null },
{ start: 2, end: 4, kind: null },
{ start: 1, end: 1, kind: null },
{ start: 2, end: 3, kind: null },
];

// Integration test against the test fixture 'folding-mismatch.ps1' that contains
Expand Down