From 766c8aa1c83887ccf68f55cfff558cc9e89653fa Mon Sep 17 00:00:00 2001 From: Cauhx Milloy Date: Sat, 16 Mar 2024 20:27:43 -0700 Subject: [PATCH 1/3] Add `buildifier` Json File Configuration This PR adds the `buildifierConfigJsonPath` setting, which allows a custom path to be set to pull the json configuration file for `buildifier` (this is often `.buildifier.json`). If the `buildifierConfigJsonPath` path is set, when `buildifier` is run within this extension, the `--config ` CLI arguments will be passed to `buildifier`. While it could be said that this PR is now adding support for the `.buildifier.json` file -- it's only adding support for the customization of that file path. https://github.com/bazelbuild/vscode-bazel/pull/350 actually pulled out yet another drive-by fix (of https://github.com/bazelbuild/vscode-bazel/issues/208) by adding the working directory. In that PR, `cwd: vscode.workspace.workspaceFolders?.[0]?.uri?.fsPath` is setting the working directory for the `buildifier` execution to be the local workspace. This means any local `.buildifier.json` file will automatically get pulled in and used. This PR is allowing further customization on top of that. The logic of this PR was tested by manually editing the js of my local extension installation with the equivalent changes (that's how I found that `cwd: ...` ended up working with a local `.buildifier.json`). --- package.json | 8 +++++++- src/buildifier/buildifier.ts | 16 ++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index af8e3618..2854ac64 100644 --- a/package.json +++ b/package.json @@ -121,6 +121,12 @@ "markdownDescription": "The name of the Buildifier executable. This may be an absolute path, or a simple name that will be searched for on the system path. Paths starting with `@` are interpreted as Bazel targets and are run via `bazel run`. If empty, \"buildifier\" on the system path will be used.\n\nBuildifier can be downloaded from https://github.com/bazelbuild/buildtools/releases.", "scope": "machine-overridable" }, + "bazel.buildifierConfigJsonPath": { + "type": "string", + "default": "", + "markdownDescription": "The path of the Buildifier config json file, ex: `.buildifier.json`. This may be an absolute path, or a relative path within the workspace.", + "scope": "machine-overridable" + }, "bazel.buildifierFixOnFormat": { "type": "boolean", "default": false, @@ -461,4 +467,4 @@ "vscode-uri": "^3.0.2", "which": "^4.0.0" } -} +} \ No newline at end of file diff --git a/src/buildifier/buildifier.ts b/src/buildifier/buildifier.ts index 99746a8c..f394c8a6 100644 --- a/src/buildifier/buildifier.ts +++ b/src/buildifier/buildifier.ts @@ -174,6 +174,18 @@ export function getDefaultBuildifierExecutablePath(): string { return buildifierExecutable; } +/** + * Gets the path to the buildifier json configuration file specified by the + * workspace configuration, if present. + * + * @returns The path to the buildifier json configuration file specified in the + * workspace configuration, or an empty string if not present. + */ +export function getDefaultBuildifierJsonConfigPath(): string { + const bazelConfig = vscode.workspace.getConfiguration("bazel"); + return bazelConfig.get("buildifierConfigJsonPath"); +} + /** * Executes buildifier with the given file content and arguments. * @@ -191,6 +203,10 @@ export function executeBuildifier( return new Promise((resolve, reject) => { // Determine the executable let executable = getDefaultBuildifierExecutablePath(); + const buildifierConfigJsonPath = getDefaultBuildifierJsonConfigPath(); + if (buildifierConfigJsonPath.length !== 0) { + args = ["--config", buildifierConfigJsonPath, ...args]; + } // Paths starting with an `@` are referring to Bazel targets if (executable.startsWith("@")) { const targetName = executable; From fbaed7006c33ee29f6f2490030ba9efcf2869e70 Mon Sep 17 00:00:00 2001 From: Cauhx Milloy Date: Sat, 16 Mar 2024 20:58:19 -0700 Subject: [PATCH 2/3] * Updating doc string in configuration json. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2854ac64..546bf4c8 100644 --- a/package.json +++ b/package.json @@ -124,7 +124,7 @@ "bazel.buildifierConfigJsonPath": { "type": "string", "default": "", - "markdownDescription": "The path of the Buildifier config json file, ex: `.buildifier.json`. This may be an absolute path, or a relative path within the workspace.", + "markdownDescription": "The path of the Buildifier config json file, ex: `.buildifier.json`. This may be an absolute path, or a relative path within the workspace. If this option is unset, `buildifier` will automatically look for a `.buildifier.json` file in the workspace.", "scope": "machine-overridable" }, "bazel.buildifierFixOnFormat": { From 2b0294d7bb6e2eabfb46cb5a94e3bea386d8f2dd Mon Sep 17 00:00:00 2001 From: Cauhx Milloy Date: Sun, 17 Mar 2024 17:13:31 -0700 Subject: [PATCH 3/3] * Changing to use default value overload of `get()` to return an empty string by default. --- src/buildifier/buildifier.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/buildifier/buildifier.ts b/src/buildifier/buildifier.ts index f394c8a6..006a8070 100644 --- a/src/buildifier/buildifier.ts +++ b/src/buildifier/buildifier.ts @@ -183,7 +183,7 @@ export function getDefaultBuildifierExecutablePath(): string { */ export function getDefaultBuildifierJsonConfigPath(): string { const bazelConfig = vscode.workspace.getConfiguration("bazel"); - return bazelConfig.get("buildifierConfigJsonPath"); + return bazelConfig.get("buildifierConfigJsonPath", ""); } /**