Skip to content

Commit

Permalink
pw_ide: VSC extension refactoring
Browse files Browse the repository at this point in the history 
This is almost entirely a refactor, at least in spirit, if not
completely by the book

Embrace the Visual Studio code event system:
  Remove some redundant event handling in favor of leaning into the VSC
  event system, except for the refresh manager, which needs to remain
  editor-agnostic, but nonetheless needs a bridge to connect it to the
  VSC event system.

Standardize on disposable class instances:
  There was a melange of different ways of initializing components and
  linking them together. Now we standardize on disposable class
  instances and an ad hoc constructor dependency injection system, plus
  event subscriptions in each component's constructor, reducing
  coupling. This should make things a lot clearer.

Standardize import formatting
Fix a bad dependency import
Add pre-release build commands

Change-Id: I75425b587decd64040e9917c6ebe8aeebe90d7a8
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/226059
Lint: Lint 🤖 <[email protected]>
Reviewed-by: Asad Memon <[email protected]>
Commit-Queue: Chad Norvell <[email protected]>
Presubmit-Verified: CQ Bot Account <[email protected]>
  • Loading branch information
@chadnorvell
chadnorvell authored and CQ Bot Account committed Jul 26, 2024
1 parent f73ddb4 commit 84803f3
Show file tree
Hide file tree
Showing 14 changed files with 736 additions and 444 deletions.
5 changes: 3 additions & 2 deletions pw_ide/ts/pigweed-vscode/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,10 @@
"lint": "eslint src --ext ts",
"prePackage": "tsx scripts/prepostPackage.mts --pre",
"doPackage": "vsce package",
"doPackagePrerelease": "vsce package --pre-release",
"postPackage": "tsx scripts/prepostPackage.mts --post",
"package": "npm run clean && npm run build && npm run bundle && npm run prePackage && npm run doPackage && npm run postPackage",
"packagePrerelease": "npm run clean && npm run build && npm run bundle && npm run prePackage && npm run doPackagePrerelease && npm run postPackage",
"test:jest": "jest '.*.test.ts'",
"test:vscode": "vscode-test"
},
Expand All @@ -170,8 +172,7 @@
"glob": "^10.4.5",
"hjson": "^3.2.2",
"js-yaml": "^4.1.0",
"node_modules-path": "^2.0.8",
"strip-ansi": "^7.1.0"
"node_modules-path": "^2.0.8"
},
"devDependencies": {
"@types/glob": "^8.1.0",
Expand Down
32 changes: 21 additions & 11 deletions pw_ide/ts/pigweed-vscode/src/bazel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,25 +12,26 @@
// License for the specific language governing permissions and limitations under
// the License.

/** Set up and manage the Bazel tools integration. */
import * as child_process from 'child_process';
import * as path from 'path';

import * as vscode from 'vscode';

import { execSync } from 'child_process';
import { resolve, basename } from 'path';
import { getNativeBinary as getBazeliskBinary } from '@bazel/bazelisk';
import node_modules from 'node_modules-path';

import logger from './logging';

import {
bazel_executable,
buildifier_executable,
settings,
ConfigAccessor,
bazel_codelens,
} from './settings';
import logger from './logging';

/**
* Is there a path to the given tool configured in VS Code settings
* Is there a path to the given tool configured in VS Code settings?
*
* @param name The name of the tool
* @param configAccessor A config accessor for the setting
Expand All @@ -41,7 +42,9 @@ function hasConfiguredPathTo(
configAccessor: ConfigAccessor<string>,
): boolean {
const exe = configAccessor.get();
return exe ? basename(exe).toLowerCase().includes(name.toLowerCase()) : false;
return exe
? path.basename(exe).toLowerCase().includes(name.toLowerCase())
: false;
}

/**
Expand All @@ -52,7 +55,9 @@ function hasConfiguredPathTo(
function findPathsTo(name: string): string[] {
// TODO: https://pwbug.dev/351883170 - This only works on Unix-ish OSes.
try {
const stdout = execSync(`which -a ${name.toLowerCase()}`).toString();
const stdout = child_process
.execSync(`which -a ${name.toLowerCase()}`)
.toString();
// Parse the output into a list of paths, removing any duplicates/blanks.
return [...new Set(stdout.split('\n'))].filter((item) => item.length > 0);
} catch (err: unknown) {
Expand All @@ -69,7 +74,12 @@ export function vendoredBazeliskPath(): string | undefined {
// have a path.
if (typeof result !== 'string') return undefined;

return resolve(node_modules()!, '@bazel', 'bazelisk', basename(result));
return path.resolve(
node_modules()!,
'@bazel',
'bazelisk',
path.basename(result),
);
}

function vendoredBuildifierPath(): string | undefined {
Expand All @@ -82,9 +92,9 @@ function vendoredBuildifierPath(): string | undefined {

// Unlike the @bazel/bazelisk package, @bazel/buildifer doesn't export any
// code. The logic is exactly the same, but with a different name.
const binaryName = basename(result).replace('bazelisk', 'buildifier');
const binaryName = path.basename(result).replace('bazelisk', 'buildifier');

return resolve(node_modules()!, '@bazel', 'buildifier', binaryName);
return path.resolve(node_modules()!, '@bazel', 'buildifier', binaryName);
}

const VENDORED_LABEL = 'Use the version built in to the Pigweed extension';
Expand Down Expand Up @@ -210,7 +220,7 @@ export async function setBazelRecommendedSettings() {
await bazel_codelens.update(true);
}

export async function configureOtherBazelSettings() {
export async function configureBazelSettings() {
await updateVendoredBazelisk();
await updateVendoredBuildifier();

Expand Down
136 changes: 81 additions & 55 deletions pw_ide/ts/pigweed-vscode/src/bazelWatcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,31 +12,50 @@
// License for the specific language governing permissions and limitations under
// the License.

/**
* A file watcher for Bazel files.
*
* We use this to automatically trigger compile commands refreshes on changes
* to the build graph.
*/
import * as child_process from 'child_process';

import * as vscode from 'vscode';

import { spawn } from 'child_process';
import { Disposable } from './disposables';
import logger from './logging';
import { getPigweedProjectRoot } from './project';

import {
RefreshCallback,
OK,
refreshManager,
RefreshManager,
RefreshCallbackResult,
} from './refreshManager';
import logger from './logging';
import { getPigweedProjectRoot } from './project';

import { bazel_executable, settings, workingDir } from './settings';

/** Regex for finding ANSI escape codes. */
const ANSI_PATTERN = new RegExp(
'[\\u001B\\u009B][[\\]()#;?]*(?:(?:(?:(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]+)' +
'*|[a-zA-Z\\d]+(?:;[-a-zA-Z\\d\\/#&.:=?%@~_]*)*)?\\u0007)' +
'|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PR-TZcf-nq-uy=><~]))',
'g',
);

/** Strip ANSI escape codes from a string. */
const stripAnsi = (input: string): string => input.replace(ANSI_PATTERN, '');

/** Remove ANSI escape codes that aren't supported in the output window. */
const cleanLogLine = (line: Buffer) => {
const stripped = stripAnsi(line.toString());

// Remove superfluous newlines
if (stripped.at(-1) === '\n') {
return stripped.substring(0, stripped.length - 1);
}

return stripped;
};

/**
* Create a container for a process running the refresh compile commands target.
*
* @return Refresh callbacks to do the refresh and to abort
* @return Refresh callbacks to do the refresh and to abort it
*/
function createRefreshProcess(): [RefreshCallback, () => void] {
// This provides us a handle to abort the process, if needed.
Expand All @@ -47,23 +66,6 @@ function createRefreshProcess(): [RefreshCallback, () => void] {
// This callback will be registered with the RefreshManager to be called
// when it's time to do the refresh.
const cb: RefreshCallback = async () => {
// This package is an ES module, but we're still building with CommonJS
// modules. This is the workaround.
// TODO: https://pwbug.dev/354034542 - Change when we're ES modules
const { default: stripAnsi } = await import('strip-ansi');

const cleanLogLine = (line: Buffer) => {
// Remove ANSI escape codes that aren't supported in the output window
const stripped = stripAnsi(line.toString());

// Remove superfluous newlines
if (stripped.at(-1) === '\n') {
return stripped.substring(0, stripped.length - 1);
}

return stripped;
};

logger.info('Refreshing compile commands');
const cwd = (await getPigweedProjectRoot(settings, workingDir)) as string;
const cmd = bazel_executable.get();
Expand All @@ -89,7 +91,7 @@ function createRefreshProcess(): [RefreshCallback, () => void] {
// TODO: https://pwbug.dev/350861417 - This should use the Bazel
// extension commands instead, but doing that through the VS Code
// command API is not simple.
const spawnedProcess = spawn(cmd, args, { cwd, signal });
const spawnedProcess = child_process.spawn(cmd, args, { cwd, signal });

// Wrapping this in a promise that only resolves on exit or error ensures
// that this refresh callback blocks until the spawned process is complete.
Expand Down Expand Up @@ -141,37 +143,61 @@ function createRefreshProcess(): [RefreshCallback, () => void] {
return [cb, abort];
}

/** Trigger a refresh compile commands process. */
export async function refreshCompileCommands() {
const [cb, abort] = createRefreshProcess();
/** A file watcher that automatically runs a refresh on Bazel file changes. */
export class BazelRefreshCompileCommandsWatcher extends Disposable {
private refreshManager: RefreshManager<any>;

const wrappedAbort = () => {
abort();
return OK;
};
constructor(refreshManager: RefreshManager<any>, disable = false) {
super();

refreshManager.onOnce(cb, 'refreshing');
refreshManager.onOnce(wrappedAbort, 'abort');
refreshManager.refresh();
}
this.refreshManager = refreshManager;
if (disable) return;

logger.info('Initializing Bazel refresh compile commands file watcher');

/** Create file watchers to refresh compile commands on Bazel file changes. */
export function initRefreshCompileCommandsWatcher(): { dispose: () => void } {
logger.info('Initializing refresh compile commands file watcher');
const watchers = [
vscode.workspace.createFileSystemWatcher('**/WORKSPACE'),
vscode.workspace.createFileSystemWatcher('**/*.bazel'),
vscode.workspace.createFileSystemWatcher('**/*.bzl'),
];

const watchers = [
vscode.workspace.createFileSystemWatcher('**/BUILD.bazel'),
vscode.workspace.createFileSystemWatcher('**/WORKSPACE'),
vscode.workspace.createFileSystemWatcher('**/*.bzl'),
];
watchers.forEach((watcher) => {
watcher.onDidChange(() => {
logger.info(
'[onDidChange] triggered from refresh compile commands watcher',
);
this.refresh();
});

watchers.forEach((watcher) => {
watcher.onDidChange(refreshCompileCommands);
watcher.onDidCreate(refreshCompileCommands);
watcher.onDidDelete(refreshCompileCommands);
});
watcher.onDidCreate(() => {
logger.info(
'[onDidCreate] triggered from refresh compile commands watcher',
);
this.refresh();
});

watcher.onDidDelete(() => {
logger.info(
'[onDidDelete] triggered from refresh compile commands watcher',
);
this.refresh();
});
});

this.disposables.push(...watchers);
}

/** Trigger a refresh compile commands process. */
refresh = () => {
const [cb, abort] = createRefreshProcess();

const wrappedAbort = () => {
abort();
return OK;
};

return {
dispose: () => watchers.forEach((watcher) => watcher.dispose()),
this.refreshManager.onOnce(cb, 'refreshing');
this.refreshManager.onOnce(wrappedAbort, 'abort');
this.refreshManager.refresh();
};
}
Loading

0 comments on commit 84803f3

Please sign in to comment.