Skip to content

Commit b3bd64a

Browse files
DanTupDanTup
committed
Add an option for disabling the built-in formatter
Fixes #1543.
1 parent 33c8aed commit b3bd64a

7 files changed

Lines changed: 98 additions & 17 deletions

File tree

‎package.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,12 @@
678678
"type": "object",
679679
"title": "Dart & Flutter",
680680
"properties": {
681+
"dart.enableSdkFormatter": {
682+
"type": "boolean",
683+
"default": true,
684+
"description": "Whether to enable the dart_style formatter included with the Dart SDK.",
685+
"scope": "window"
686+
},
681687
"dart.env": {
682688
"type": "object",
683689
"description": "Additional environment variables to add to all Dart/Flutter processes spawned by the Dart and Flutter extensions.",

‎src/config.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ class Config {
3737
get checkForSdkUpdates() { return this.getConfig<boolean>("checkForSdkUpdates"); }
3838
public setCheckForSdkUpdates(value: boolean): Thenable<void> { return this.setConfig("checkForSdkUpdates", value, ConfigurationTarget.Global); }
3939
get closingLabels() { return this.getConfig<boolean>("closingLabels"); }
40+
get devToolsTheme() { return this.getConfig<string>("devToolsTheme"); }
4041
get extensionLogFile() { return createFolderForFile(resolvePaths(this.getConfig<string>("extensionLogFile"))); }
4142
get env() { return this.getConfig<object>("env"); }
4243
get flutterCreateAndroidLanguage() { return this.getConfig<string>("flutterCreateAndroidLanguage"); }
@@ -62,8 +63,8 @@ class Config {
6263
get showTodos() { return this.getConfig<boolean>("showTodos"); }
6364
get showIgnoreQuickFixes() { return this.getConfig<boolean>("showIgnoreQuickFixes"); }
6465
get theme() { return this.getConfig<string>("theme"); }
65-
get devToolsTheme() { return this.getConfig<string>("devToolsTheme"); }
6666
get triggerSignatureHelpAutomatically() { return this.getConfig<boolean>("triggerSignatureHelpAutomatically"); }
67+
get enableSdkFormatter() { return this.getConfig<boolean>("enableSdkFormatter"); }
6768
get warnWhenEditingFilesOutsideWorkspace() { return this.getConfig<boolean>("warnWhenEditingFilesOutsideWorkspace"); }
6869
public setWarnWhenEditingFilesOutsideWorkspace(value: boolean): Thenable<void> { return this.setConfig("warnWhenEditingFilesOutsideWorkspace", value, ConfigurationTarget.Global); }
6970

‎src/debug/utils.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export class LogMessage {
3131
}
3232

3333
export interface IAmDisposable {
34-
dispose(): void;
34+
dispose(): void | Promise<void>;
3535
}
3636

3737
// TODO: Remove this, or document why we need it as well as fsPath().

‎src/extension.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ export function activate(context: vs.ExtensionContext, isRestart: boolean = fals
189189
// TODO: Do we need to push all these to subscriptions?!
190190
const hoverProvider = new DartHoverProvider(analyzer);
191191
const formattingEditProvider = new DartFormattingEditProvider(analyzer, extContext);
192+
context.subscriptions.push(formattingEditProvider);
192193
const completionItemProvider = new DartCompletionItemProvider(analyzer);
193194
const referenceProvider = new DartReferenceProvider(analyzer);
194195
const documentHighlightProvider = new DartDocumentHighlightProvider(analyzer);
@@ -209,7 +210,7 @@ export function activate(context: vs.ExtensionContext, isRestart: boolean = fals
209210
const triggerCharacters = ".(${'\"/\\".split("");
210211
activeFileFilters.forEach((filter) => {
211212
context.subscriptions.push(vs.languages.registerHoverProvider(filter, hoverProvider));
212-
context.subscriptions.push(vs.languages.registerDocumentFormattingEditProvider(filter, formattingEditProvider));
213+
formattingEditProvider.registerDocumentFormatter(filter);
213214
context.subscriptions.push(vs.languages.registerCompletionItemProvider(filter, completionItemProvider, ...triggerCharacters));
214215
context.subscriptions.push(vs.languages.registerDefinitionProvider(filter, referenceProvider));
215216
context.subscriptions.push(vs.languages.registerReferenceProvider(filter, referenceProvider));
@@ -221,7 +222,7 @@ export function activate(context: vs.ExtensionContext, isRestart: boolean = fals
221222
});
222223

223224
// Some actions only apply to Dart.
224-
context.subscriptions.push(vs.languages.registerOnTypeFormattingEditProvider(DART_MODE, formattingEditProvider, "}", ";"));
225+
formattingEditProvider.registerTypingFormatter(DART_MODE, "}", ";");
225226
context.subscriptions.push(vs.languages.registerCodeActionsProvider(DART_MODE, sourceCodeActionProvider, sourceCodeActionProvider.metadata));
226227
context.subscriptions.push(vs.languages.registerCodeActionsProvider(DART_MODE, ignoreLintCodeActionProvider, ignoreLintCodeActionProvider.metadata));
227228
context.subscriptions.push(vs.languages.registerImplementationProvider(DART_MODE, implementationProvider));

‎src/providers/dart_formatting_edit_provider.ts‎

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,56 @@
11
import * as minimatch from "minimatch";
2-
import { CancellationToken, DocumentFormattingEditProvider, FormattingOptions, OnTypeFormattingEditProvider, Position, Range, TextDocument, TextEdit, window } from "vscode";
2+
import { CancellationToken, DocumentFilter, DocumentFormattingEditProvider, FormattingOptions, languages, OnTypeFormattingEditProvider, Position, Range, TextDocument, TextEdit, window, workspace } from "vscode";
33
import * as as from "../analysis/analysis_server_types";
44
import { Analyzer } from "../analysis/analyzer";
55
import { config } from "../config";
66
import { Context } from "../context";
7+
import { IAmDisposable } from "../debug/utils";
78
import { fsPath } from "../utils";
89
import { logError } from "../utils/log";
910

10-
export class DartFormattingEditProvider implements DocumentFormattingEditProvider, OnTypeFormattingEditProvider {
11-
constructor(private readonly analyzer: Analyzer, private readonly context: Context) { }
11+
export class DartFormattingEditProvider implements DocumentFormattingEditProvider, OnTypeFormattingEditProvider, IAmDisposable {
12+
constructor(private readonly analyzer: Analyzer, private readonly context: Context) {
13+
workspace.onDidChangeConfiguration((e) => {
14+
if (e.affectsConfiguration("dart.enableSdkFormatter")) {
15+
if (config.enableSdkFormatter)
16+
this.registerAllFormatters();
17+
else
18+
this.unregisterAllFormatters();
19+
}
20+
});
21+
}
22+
23+
private readonly registeredFormatters: IAmDisposable[] = [];
24+
private readonly formatterRegisterFuncs: Array<() => void> = [];
25+
26+
public registerDocumentFormatter(filter: DocumentFilter[]): void {
27+
this.registerFormatter(() => languages.registerDocumentFormattingEditProvider(filter, this));
28+
}
29+
30+
public registerTypingFormatter(filter: DocumentFilter[], firstTriggerCharacter: string, ...moreTriggerCharacters: string[]): void {
31+
this.registerFormatter(() => languages.registerOnTypeFormattingEditProvider(filter, this, firstTriggerCharacter, ...moreTriggerCharacters));
32+
}
33+
34+
private registerFormatter(reg: () => IAmDisposable) {
35+
const registerAndTrack = () => this.registeredFormatters.push(reg());
36+
// Register the formatter immediately if enabled.
37+
if (config.enableSdkFormatter)
38+
registerAndTrack();
39+
40+
// Add it to our list so we can re-register later..
41+
this.formatterRegisterFuncs.push(registerAndTrack);
42+
}
43+
44+
private registerAllFormatters() {
45+
for (const formatterReg of this.formatterRegisterFuncs) {
46+
formatterReg();
47+
}
48+
}
49+
50+
private unregisterAllFormatters() {
51+
this.registeredFormatters.forEach((s) => s.dispose());
52+
this.registeredFormatters.length = 0;
53+
}
1254

1355
public async provideDocumentFormattingEdits(document: TextDocument, options: FormattingOptions, token: CancellationToken): Promise<TextEdit[]> {
1456
try {
@@ -64,4 +106,8 @@ export class DartFormattingEditProvider implements DocumentFormattingEditProvide
64106
edit.replacement,
65107
);
66108
}
109+
110+
public dispose() {
111+
this.unregisterAllFormatters();
112+
}
67113
}

‎test/dart_only/providers/dart_formatting_edit_provider.test.ts‎

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
11
import * as assert from "assert";
22
import * as vs from "vscode";
3-
import { activate, currentDoc, currentEditor, documentEol, emptyExcludedFile, emptyFileInExcludedFolder, openFile, positionOf, setTestContent } from "../../helpers";
3+
import { platformEol } from "../../../src/debug/utils";
4+
import { activate, currentDoc, currentEditor, emptyExcludedFile, emptyFileInExcludedFolder, openFile, positionOf, setConfigForTest, setTestContent } from "../../helpers";
45

56
const formattingOptions: vs.FormattingOptions = { tabSize: 2, insertSpaces: true };
67

78
describe("dart_formatting_edit_provider", () => {
89

910
beforeEach("activate", () => activate());
1011

12+
const unformattedContent = ` main ( ) { }${platformEol}`;
13+
const formattedContent = `main() {}${platformEol}`;
14+
1115
async function formatDocument(expectResult = true): Promise<void> {
1216
const formatResult = await (vs.commands.executeCommand("vscode.executeFormatDocumentProvider", currentDoc().uri, formattingOptions) as Thenable<vs.TextEdit[]>);
1317
if (expectResult) {
@@ -28,29 +32,51 @@ describe("dart_formatting_edit_provider", () => {
2832
}
2933

3034
it("formats the document", async () => {
31-
await setTestContent(" main ( ) { }");
35+
await setTestContent(unformattedContent);
3236
await formatDocument();
33-
assert.equal(currentDoc().getText(), `main() {}${documentEol}`);
37+
assert.equal(currentDoc().getText(), formattedContent);
38+
});
39+
40+
it("formats the document on save", async () => {
41+
await setTestContent(unformattedContent);
42+
await currentDoc().save();
43+
assert.equal(currentDoc().getText(), formattedContent);
44+
});
45+
46+
it("does not format the document if disabled", async () => {
47+
await setConfigForTest("dart", "enableSdkFormatter", false);
48+
await setTestContent(unformattedContent);
49+
await currentDoc().save();
50+
assert.equal(currentDoc().getText(), unformattedContent);
51+
});
52+
53+
54+
it("formats the document if re-enabled", async () => {
55+
await setConfigForTest("dart", "enableSdkFormatter", false);
56+
await setConfigForTest("dart", "enableSdkFormatter", true);
57+
await setTestContent(unformattedContent);
58+
await currentDoc().save();
59+
assert.equal(currentDoc().getText(), formattedContent);
3460
});
3561

3662
it("formats the code when typing a }", async () => {
3763
// Currently we just format the whole doc on format as out formatter doesn't support ranges.
38-
await setTestContent(" main ( ) { }");
64+
await setTestContent(unformattedContent);
3965
await formatOnType("{ ^", "}");
40-
assert.equal(currentDoc().getText(), `main() {}${documentEol}`);
66+
assert.equal(currentDoc().getText(), formattedContent);
4167
});
4268

4369
it("does not format an excluded file", async () => {
4470
await openFile(emptyExcludedFile);
45-
await setTestContent(" main ( ) { }");
71+
await setTestContent(unformattedContent);
4672
await formatDocument(false);
47-
assert.equal(currentDoc().getText(), " main ( ) { }");
73+
assert.equal(currentDoc().getText(), unformattedContent);
4874
});
4975

5076
it("does not format a file in an excluded folder", async () => {
5177
await openFile(emptyFileInExcludedFolder);
52-
await setTestContent(" main ( ) { }");
78+
await setTestContent(unformattedContent);
5379
await formatDocument(false);
54-
assert.equal(currentDoc().getText(), " main ( ) { }");
80+
assert.equal(currentDoc().getText(), unformattedContent);
5581
});
5682
});

‎test/helpers.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ beforeEach("set logger", async function () {
229229
export let sb: sinon.SinonSandbox;
230230
beforeEach("create sinon sandbox", function () { sb = sinon.createSandbox(); }); // tslint:disable-line:only-arrow-functions
231231
afterEach("destroy sinon sandbox", () => sb.restore());
232+
afterEach("make empty file empty", () => fs.writeFileSync(fsPath(emptyFile), ""));
232233

233234
before("throw if DART_CODE_IS_TEST_RUN is not set", () => {
234235
if (!process.env.DART_CODE_IS_TEST_RUN)
@@ -240,7 +241,7 @@ const deferredToLastItems: Array<(result?: "failed" | "passed") => Promise<any>
240241
// tslint:disable-next-line:only-arrow-functions
241242
afterEach("run deferred functions", async function () {
242243
let firstError: any;
243-
for (const d of [...deferredItems, ...deferredToLastItems]) {
244+
for (const d of [...deferredItems.reverse(), ...deferredToLastItems.reverse()]) {
244245
try {
245246
await watchPromise(`afterEach->deferred->${d.toString()}`, d(this.currentTest ? this.currentTest.state : undefined));
246247
} catch (e) {

0 commit comments

Comments
 (0)