Skip to content

WIP attempt to fix code folding provider in VSCode 1.25 #1410

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

Closed
wants to merge 2 commits into from
Closed
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
73 changes: 49 additions & 24 deletions src/features/Folding.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
/*---------------------------------------------------------
* Copyright (C) Microsoft Corporation. All rights reserved.
*--------------------------------------------------------*/

import fs = require("fs");
import * as path from "path";
import * as vscode from "vscode";
import {
DocumentSelector,
LanguageClient,
} from "vscode-languageclient";
import { IFeature } from "../feature";
import { Logger } from "../logging";
import { ILogger } from "../logging";
import * as Settings from "../settings";

/**
Expand Down Expand Up @@ -497,23 +497,26 @@ export class FoldingFeature implements IFeature {
* @param logger The logging object to send messages to
* @param documentSelector documentSelector object for this Folding Provider
*/
constructor(private logger: Logger, documentSelector: DocumentSelector) {
const grammar: IGrammar = this.grammar(logger);

constructor(private logger: ILogger, documentSelector: DocumentSelector) {
const settings = Settings.load();
if (!(settings.codeFolding && settings.codeFolding.enable)) { return; }

// If the PowerShell grammar is not available for some reason, don't register a folding provider,
// which reverts VSCode to the default indentation style folding
if (grammar == null) {
logger.writeWarning("Unable to load the PowerShell grammar file");
return;
}
this.loadGrammar(logger)
.then((grammar) => {
// If the PowerShell grammar is not available for some reason, don't register a folding provider,
// which reverts VSCode to the default indentation style folding
if (!grammar) {
logger.writeWarning("Unable to load the PowerShell grammar file");
return;
}

this.foldingProvider = new FoldingProvider(grammar);
vscode.languages.registerFoldingRangeProvider(documentSelector, this.foldingProvider);
this.foldingProvider = new FoldingProvider(grammar);
vscode.languages.registerFoldingRangeProvider(documentSelector, this.foldingProvider);

logger.write("Syntax Folding Provider registered");
logger.write("Syntax Folding Provider registered");
}, (err) => {
this.logger.writeError(`Failed to load grammar file - error: ${err}`);
});
}

/* dispose() is required by the IFeature interface, but is not required by this feature */
Expand All @@ -527,21 +530,43 @@ export class FoldingFeature implements IFeature {
* @param logger The logging object to send messages to
* @returns A grammar parser for the PowerShell language is succesful or undefined if an error occured
*/
public grammar(logger: Logger): IGrammar {
public loadGrammar(logger: ILogger): Thenable<IGrammar> {
const tm = this.getCoreNodeModule("vscode-textmate", logger);
if (tm == null) { return undefined; }
logger.writeDiagnostic(`Loaded the vscode-textmate module`);
const registry = new tm.Registry();
if (registry == null) { return undefined; }
logger.writeDiagnostic(`Created the textmate Registry`);

const grammarPath = this.powerShellGrammarPath();
if (grammarPath == null) { return undefined; }
logger.writeDiagnostic(`PowerShell grammar file specified as ${grammarPath}`);
try {
return registry.loadGrammarFromPathSync(grammarPath);
} catch (err) {
logger.writeError(`Error while loading the PowerShell grammar file at ${grammarPath}`, err);
}

const grammarPaths = {
powershell: grammarPath,
};

const registry = new tm.Registry({
loadGrammar(scopeName) {
const gpath = grammarPaths[scopeName];
if (gpath) {
return new Promise((c, e) => {
fs.readFile(gpath, (error, content) => {
if (error) {
e(error);
} else {
const rawGrammar = tm.parseRawGrammar(content.toString(), gpath);
c(rawGrammar);
}
});
});
}
return null;
},
});

if (registry == null) { return undefined; }
logger.writeDiagnostic(`Created the textmate Registry`);

const grammar = registry.loadGrammar("powershell");
return grammar;
}

/**
Expand All @@ -552,7 +577,7 @@ export class FoldingFeature implements IFeature {
* @param logger The logging object to send messages to
* @returns The required module, or null if the module cannot be required
*/
private getCoreNodeModule(moduleName: string, logger: Logger) {
private getCoreNodeModule(moduleName: string, logger: ILogger) {
// Attempt to load the module from known locations
const loadLocations: string[] = [
`${vscode.env.appRoot}/node_modules.asar/${moduleName}`,
Expand Down
14 changes: 13 additions & 1 deletion src/logging.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,19 @@ export enum LogLevel {
Error,
}

export class Logger {
/** Interface for logging operations. New features should use this interface for the "type" of logger.
* This will allow for easy mocking of the logger during unit tests.
*/
export interface ILogger {
write(message: string, ...additionalMessages: string[]);
writeDiagnostic(message: string, ...additionalMessages: string[]);
writeVerbose(message: string, ...additionalMessages: string[]);
writeWarning(message: string, ...additionalMessages: string[]);
writeAndShowWarning(message: string, ...additionalMessages: string[]);
writeError(message: string, ...additionalMessages: string[]);
}

export class Logger implements ILogger {

public logBasePath: string;
public logSessionPath: string;
Expand Down
4 changes: 2 additions & 2 deletions test/features/folding.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ function assertFoldingRegions(result, expected): void {

suite("Features", () => {

suite("Folding Provider", () => {
suite("Folding Provider", async () => {
const logger: MockLogger = new MockLogger();
const mockSelector: DocumentSelector = [
{ language: "powershell", scheme: "file" },
];
const psGrammar = (new folding.FoldingFeature(logger, mockSelector)).grammar(logger);
const psGrammar = await (new folding.FoldingFeature(logger, mockSelector)).loadGrammar(logger);
const provider = (new folding.FoldingProvider(psGrammar));

test("Can detect the PowerShell Grammar", () => {
Expand Down
16 changes: 2 additions & 14 deletions test/test_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,9 @@
* Copyright (C) Microsoft Corporation. All rights reserved.
*--------------------------------------------------------*/

import { Logger, LogLevel } from "../src/logging";

export class MockLogger extends Logger {
// Note - This is not a true mock as the constructor is inherited and causes errors due to trying load
// the "PowerShell Extension Logs" multiple times. Ideally logging should be via an interface and then
// we can mock correctly.

public dispose() { return undefined; }

public getLogFilePath(baseName: string): string { return "mock"; }

public writeAtLevel(logLevel: LogLevel, message: string, ...additionalMessages: string[]) { return undefined; }
import { ILogger } from "../src/logging";

export class MockLogger implements ILogger {
public write(message: string, ...additionalMessages: string[]) { return undefined; }

public writeDiagnostic(message: string, ...additionalMessages: string[]) { return undefined; }
Expand All @@ -28,6 +18,4 @@ export class MockLogger extends Logger {
public writeError(message: string, ...additionalMessages: string[]) { return undefined; }

public writeAndShowError(message: string, ...additionalMessages: string[]) { return undefined; }

public startNewLog(minimumLogLevel: string = "Normal") { return undefined; }
}