Sfoglia il codice sorgente

More Formatter Fixes (#672)

* Fix nodepath function highlighting/tokenization

* Reverted dangerous line removal behavior change

* Fix detection of match keyword vs .match() function

* Rearrange formatter options

* Fix option default value

* Add biome linter/formatter config file

* Fix linter errors

* Add system to supply custom config values in tests

* Remove unused variable

* Implement tests for both formatter options

* Clean up formatter option handling

* Fix extra space inserted in list of nodepaths

* Add token rules for square and curly braces
David Kincaid 1 anno fa
parent
commit
cca25099c4

+ 15 - 0
biome.json

@@ -0,0 +1,15 @@
+{
+	"formatter": {
+		"enabled": true,
+		"formatWithErrors": false,
+		"indentStyle": "tab",
+		"indentWidth": 4,
+		"lineWidth": 120,
+		"lineEnding": "lf",
+		"include": ["src/**/*.ts"]
+	},
+	"files": {
+		"include": ["src/**/*.ts"],
+		"ignore": ["node_modules"]
+	}
+}

+ 9 - 9
package.json

@@ -271,23 +271,23 @@
 					"default": true,
 					"description": "Whether to reveal the terminal when launching the Godot Editor"
 				},
-				"godotTools.formatter.emptyLinesBeforeFunctions": {
+				"godotTools.formatter.maxEmptyLines": {
 					"type": "string",
 					"enum": [
-						"one",
-						"two"
+						"1",
+						"2"
 					],
 					"enumDescriptions": [
-						"One line before functions. A more compact style.",
-						"Two lines before functions. Conforms to the official GDScript style guide."
+						"1 empty line. A more compact style.",
+						"2 empty lines. Conforms to the official GDScript style guide."
 					],
-					"default": "two",
-					"description": "Number of blank lines to leave before functions."
+					"default": "2",
+					"description": "Number of empty lines allowed anywhere in the file"
 				},
-				"godotTools.formatter.denseFunctionDeclarations": {
+				"godotTools.formatter.denseFunctionParameters": {
 					"type": "boolean",
 					"default": false,
-					"description": "Whether extra space should be removed from function declarations"
+					"description": "Whether extra space should be removed from function parameter lists"
 				},
 				"godotTools.lsp.serverProtocol": {
 					"type": [

+ 23 - 3
src/formatter/formatter.test.ts

@@ -2,7 +2,7 @@ import * as vscode from "vscode";
 import * as path from "node:path";
 import * as fs from "node:fs";
 
-import { format_document } from "./textmate";
+import { format_document, type FormatterOptions } from "./textmate";
 
 import * as chai from "chai";
 const expect = chai.expect;
@@ -10,6 +10,20 @@ const expect = chai.expect;
 const dots = ["..", "..", ".."];
 const basePath = path.join(__filename, ...dots);
 
+function get_options(testFolderPath: string) {
+	const options: FormatterOptions = {
+		maxEmptyLines: 2,
+		denseFunctionParameters: false,
+	};
+	const optionsPath = path.join(testFolderPath, "config.json");
+	if (fs.existsSync(optionsPath)) {
+		const file = fs.readFileSync(optionsPath).toString();
+		const config = JSON.parse(file);
+		return { ...options, ...config } as FormatterOptions;
+	}
+	return options;
+}
+
 suite("GDScript Formatter Tests", () => {
 	// Search for all folders in the snapshots folder and run a test for each
 	// comparing the output of the formatter with the expected output.
@@ -18,15 +32,19 @@ suite("GDScript Formatter Tests", () => {
 	const snapshotsFolderPath = path.join(basePath, "src/formatter/snapshots");
 	const testFolders = fs.readdirSync(snapshotsFolderPath);
 
+	// biome-ignore lint/complexity/noForEach: <explanation>
 	testFolders.forEach((testFolder) => {
 		const testFolderPath = path.join(snapshotsFolderPath, testFolder);
 		if (fs.statSync(testFolderPath).isDirectory()) {
 			test(`Snapshot Test: ${testFolder}`, async () => {
 				const uriIn = vscode.Uri.file(path.join(testFolderPath, "in.gd"));
 				const uriOut = vscode.Uri.file(path.join(testFolderPath, "out.gd"));
+
 				const documentIn = await vscode.workspace.openTextDocument(uriIn);
 				const documentOut = await vscode.workspace.openTextDocument(uriOut);
-				const edits = format_document(documentIn);
+
+				const options = get_options(testFolderPath);
+				const edits = format_document(documentIn, options);
 
 				// Apply the formatting edits
 				const workspaceEdit = new vscode.WorkspaceEdit();
@@ -34,7 +52,9 @@ suite("GDScript Formatter Tests", () => {
 				await vscode.workspace.applyEdit(workspaceEdit);
 
 				// Compare the result with the expected output
-				expect(documentIn.getText().replace("\r\n", "\n")).to.equal(documentOut.getText().replace("\r\n", "\n"));
+				expect(documentIn.getText().replace("\r\n", "\n")).to.equal(
+					documentOut.getText().replace("\r\n", "\n"),
+				);
 			});
 		}
 	});

+ 3 - 0
src/formatter/snapshots/max-empty-lines-1/config.json

@@ -0,0 +1,3 @@
+{
+    "maxEmptyLines": 1
+}

+ 0 - 0
src/formatter/snapshots/consecutive-empty-lines-are-removed/in.gd → src/formatter/snapshots/max-empty-lines-1/in.gd


+ 0 - 3
src/formatter/snapshots/consecutive-empty-lines-are-removed/out.gd → src/formatter/snapshots/max-empty-lines-1/out.gd

@@ -1,16 +1,13 @@
 class Test:
 
-
 	func _ready():
 	
 		pass
 
-
 func test():
 
 	pass
 
-
 # comments
 func with_comments():
 

+ 28 - 0
src/formatter/snapshots/max-empty-lines-2/in.gd

@@ -0,0 +1,28 @@
+
+
+
+class Test:
+
+
+
+
+	func _ready():
+
+
+		pass
+
+
+
+func test():
+
+
+	pass
+
+
+
+
+# comments
+func with_comments():
+
+
+	pass

+ 20 - 0
src/formatter/snapshots/max-empty-lines-2/out.gd

@@ -0,0 +1,20 @@
+class Test:
+
+
+	func _ready():
+
+
+		pass
+
+
+func test():
+
+
+	pass
+
+
+# comments
+func with_comments():
+
+
+	pass

+ 0 - 1
src/formatter/symbols.ts

@@ -20,7 +20,6 @@ export const keywords = [
 	"is",
 	"master",
 	"mastersync",
-	"match",
 	"when",
 	"not",
 	"onready",

+ 28 - 40
src/formatter/textmate.ts

@@ -1,4 +1,5 @@
-import { Range, type TextDocument, TextEdit, TextLine } from "vscode";
+import { TextEdit } from "vscode";
+import type { TextDocument, TextLine } from "vscode";
 import * as fs from "node:fs";
 import * as vsctm from "vscode-textmate";
 import * as oniguruma from "vscode-oniguruma";
@@ -51,14 +52,18 @@ interface Token {
 	skip?: boolean;
 }
 
-class FormatterOptions {
-	emptyLinesBeforeFunctions: "one" | "two";
-	denseFunctionDeclarations: boolean;
+export interface FormatterOptions {
+	maxEmptyLines: 1 | 2;
+	denseFunctionParameters: boolean;
+}
 
-	constructor() {
-		this.emptyLinesBeforeFunctions = get_configuration("formatter.emptyLinesBeforeFunctions");
-		this.denseFunctionDeclarations = get_configuration("formatter.denseFunctionDeclarations");
-	}
+function get_formatter_options() {
+	const options: FormatterOptions = {
+		maxEmptyLines: get_configuration("formatter.maxEmptyLines") === "1" ? 1 : 2,
+		denseFunctionParameters: get_configuration("formatter.denseFunctionParameters"),
+	};
+
+	return options;
 }
 
 function parse_token(token: Token) {
@@ -70,6 +75,12 @@ function parse_token(token: Token) {
 	}
 	if (token.scopes.includes("meta.literal.nodepath.gdscript")) {
 		token.skip = true;
+		token.type = "nodepath";
+		return;
+	}
+	if (token.scopes.includes("keyword.control.flow.gdscript")) {
+		token.type = "keyword";
+		return;
 	}
 	if (keywords.includes(token.value)) {
 		token.type = "keyword";
@@ -114,7 +125,7 @@ function between(tokens: Token[], current: number, options: FormatterOptions) {
 	if (prev === "(") return "";
 
 	if (nextToken.param) {
-		if (options.denseFunctionDeclarations) {
+		if (options.denseFunctionParameters) {
 			if (prev === "-") {
 				if (tokens[current - 2]?.value === "=") return "";
 				if (["keyword", "symbol"].includes(tokens[current - 2].type)) {
@@ -171,6 +182,7 @@ function between(tokens: Token[], current: number, options: FormatterOptions) {
 	if (prev === ")" && nextToken.type === "keyword") return " ";
 
 	if (prev === "[" && nextToken.type === "symbol") return "";
+	if (prev === "[" && nextToken.type === "nodepath") return "";
 	if (prev === ":") return " ";
 	if (prev === ";") return " ";
 	if (prev === "##") return " ";
@@ -205,34 +217,28 @@ function is_comment(line: TextLine): boolean {
 	return line.text[line.firstNonWhitespaceCharacterIndex] === "#";
 }
 
-export function format_document(document: TextDocument): TextEdit[] {
+export function format_document(document: TextDocument, _options?: FormatterOptions): TextEdit[] {
 	// quit early if grammar is not loaded
 	if (!grammar) {
 		return [];
 	}
 	const edits: TextEdit[] = [];
 
-	const options = new FormatterOptions();
-
+	const options = _options ?? get_formatter_options();
+	
 	let lineTokens: vsctm.ITokenizeLineResult = null;
 	let onlyEmptyLinesSoFar = true;
 	let emptyLineCount = 0;
-	let firstEmptyLine = 0;
 	for (let lineNum = 0; lineNum < document.lineCount; lineNum++) {
 		const line = document.lineAt(lineNum);
 
 		// skip empty lines
-		if (line.isEmptyOrWhitespace || is_comment(line)) {
+		if (line.isEmptyOrWhitespace) {
 			// delete empty lines at the beginning of the file
 			if (onlyEmptyLinesSoFar) {
 				edits.push(TextEdit.delete(line.rangeIncludingLineBreak));
 			} else {
-				if (emptyLineCount === 0) {
-					firstEmptyLine = lineNum;
-				}
-				if (!is_comment(line)) {
-					emptyLineCount++;
-				}
+				emptyLineCount++;
 			}
 
 			// delete empty lines at the end of the file
@@ -247,26 +253,8 @@ export function format_document(document: TextDocument): TextEdit[] {
 
 		// delete consecutive empty lines
 		if (emptyLineCount) {
-			let maxEmptyLines = 1;
-
-			const start = line.text.trimStart();
-			if (options.emptyLinesBeforeFunctions === "two") {
-				if (start.startsWith("func") || start.startsWith("static func")) {
-					maxEmptyLines++;
-				}
-			}
-			if (start.startsWith("class")) {
-				maxEmptyLines++;
-			}
-			let i = 0;
-			let deletedLines = 0;
-			const linesToDelete = emptyLineCount - maxEmptyLines;
-			while (i < lineNum && deletedLines < linesToDelete) {
-				const candidate = document.lineAt(firstEmptyLine + i++);
-				if (candidate.isEmptyOrWhitespace) {
-					edits.push(TextEdit.delete(candidate.rangeIncludingLineBreak));
-					deletedLines++;
-				}
+			for (let i = emptyLineCount - options.maxEmptyLines; i > 0; i--) {
+				edits.push(TextEdit.delete(document.lineAt(lineNum - i).rangeIncludingLineBreak));
 			}
 			emptyLineCount = 0;
 		}

+ 32 - 2
syntaxes/GDScript.tmLanguage.json

@@ -69,6 +69,9 @@
 				{ "include": "#assignment_operator" },
 				{ "include": "#in_keyword" },
 				{ "include": "#control_flow" },
+				{ "include": "#match_keyword" },
+				{ "include": "#curly_braces" },
+				{ "include": "#square_braces" },
 				{ "include": "#round_braces" },
 				{ "include": "#function_call" },
 				{ "include": "#comment" },
@@ -146,6 +149,8 @@
 			]
 		},
 		"nodepath_function": {
+			"name": "meta.function.gdscript",
+			"contentName": "meta.function.parameters.gdscript",
 			"begin": "(get_node_or_null|has_node|has_node_and_resource|find_node|get_node)\\s*(\\()",
 			"beginCaptures": {
 				"1": { "name": "entity.name.function.gdscript" },
@@ -164,7 +169,8 @@
 							"name": "keyword.control.flow"
 						}
 					]
-				}
+				},
+				{ "include": "#base_expression" }
 			]
 		},
 		"self": {
@@ -231,9 +237,13 @@
 			"name": "keyword.operator.assignment.gdscript"
 		},
 		"control_flow": {
-			"match": "\\b(?:if|elif|else|while|break|continue|pass|return|match|when|yield|await)\\b",
+			"match": "\\b(?:if|elif|else|while|break|continue|pass|return|when|yield|await)\\b",
 			"name": "keyword.control.gdscript"
 		},
+		"match_keyword": {
+			"match": "^\n\\s*(match)",
+			"captures": { "1": { "name": "keyword.control.gdscript" } }
+		},
 		"keywords": {
 			"match": "\\b(?:class|class_name|is|onready|tool|static|export|as|void|enum|preload|assert|breakpoint|sync|remote|master|puppet|slave|remotesync|mastersync|puppetsync|trait|namespace)\\b",
 			"name": "keyword.language.gdscript"
@@ -544,6 +554,26 @@
 				}
 			]
 		},
+		"curly_braces": {
+			"begin": "\\{",
+			"end": "\\}",
+			"beginCaptures": { "0": { "name": "punctuation.definition.dict.begin.gdscript" } },
+			"endCaptures": { "0": { "name": "punctuation.definition.dict.end.gdscript" } },
+			"patterns": [
+				{ "include": "#base_expression" },
+				{ "include": "#any_variable" }
+			]
+		},
+		"square_braces": {
+			"begin": "\\[",
+			"end": "\\]",
+			"beginCaptures": { "0": { "name": "punctuation.definition.list.begin.gdscript" } },
+			"endCaptures": { "0": { "name": "punctuation.definition.list.end.gdscript" } },
+			"patterns": [
+				{ "include": "#base_expression" },
+				{ "include": "#any_variable" }
+			]
+		},
 		"round_braces": {
 			"begin": "\\(",
 			"end": "\\)",

+ 6 - 0
syntaxes/examples/gdscript2.gd

@@ -45,6 +45,12 @@ func f():
     super()
     super.some_function()
 
+	match param3:
+		3:
+			print("param3 is 3!")
+		_:
+			print("param3 is not 3!")
+
     for i in range(1): # `in` is a control keyword
         print(i in range(1)) # `in` is an operator keyword