From cda3fbe7f83bd45859ac7e902c2dce2d9b961b0f Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Fri, 11 Sep 2020 16:22:08 -0700 Subject: [PATCH] debug: use only js-debug auto attach, collapse settings This PR removes the hook in node-debug's auto attach, and uses only js-debug auto attach. As referenced in the linked issues, this involves removing `debug.javascript.usePreviewAutoAttach` and collapsing `debug.node.autoAttach` into `debug.javascript.autoAttachFilter`. The latter option gains a new state: `disabled`. Since there's no runtime cost to having auto attach around, there is now no distinct off versus disabled state. The status bar item and the `Debug: Toggle Auto Attach` command now open a quickpick, which looks like this: ![](https://memes.peet.io/img/20-09-9d2b6c0a-8b3f-4481-b2df-0753c54ee02b.png) The current setting value is selected in the quickpick. If there is a workspace setting for auto attach, the quickpick toggle the setting there by default. Otherwise (as in the image) it will target the user settings. The targeting is more explicit and defaults to the user instead of the workspace, which should help reduce confusion (#97087). Selecting the "scope change" item will reopen the quickpick in that location. Aside from the extra options for the `disabled` state in js-debug's contributions, there's no changes required to it or its interaction with debug-auto-launch. Side note: I really wanted a separator between the states and the scope change item, but this is not possible from an extension #74967. Fixes https://github.com/microsoft/vscode/issues/105883 Fixes https://github.com/microsoft/vscode-js-debug/issues/732 (the rest of it) Fixes https://github.com/microsoft/vscode/issues/105963 Fixes https://github.com/microsoft/vscode/issues/97087 --- extensions/debug-auto-launch/package.json | 33 +- extensions/debug-auto-launch/package.nls.json | 7 - extensions/debug-auto-launch/src/extension.ts | 420 +++++++++--------- 3 files changed, 221 insertions(+), 239 deletions(-) diff --git a/extensions/debug-auto-launch/package.json b/extensions/debug-auto-launch/package.json index 3cb11ef1844..f0dc778263e 100644 --- a/extensions/debug-auto-launch/package.json +++ b/extensions/debug-auto-launch/package.json @@ -17,33 +17,6 @@ "watch": "gulp watch-extension:debug-auto-launch" }, "contributes": { - "configuration": { - "title": "Node debug", - "properties": { - "debug.node.autoAttach": { - "scope": "window", - "type": "string", - "enum": [ - "disabled", - "on", - "off" - ], - "enumDescriptions": [ - "%debug.node.autoAttach.disabled.description%", - "%debug.node.autoAttach.on.description%", - "%debug.node.autoAttach.off.description%" - ], - "description": "%debug.node.autoAttach.description%", - "default": "disabled" - }, - "debug.javascript.usePreviewAutoAttach": { - "scope": "window", - "type": "boolean", - "default": true, - "description": "%debug.javascript.usePreviewAutoAttach%" - } - } - }, "commands": [ { "command": "extension.node-debug.toggleAutoAttach", @@ -57,5 +30,11 @@ }, "devDependencies": { "@types/node": "^12.11.7" + }, + "prettier": { + "printWidth": 100, + "trailingComma": "all", + "singleQuote": true, + "arrowParens": "avoid" } } diff --git a/extensions/debug-auto-launch/package.nls.json b/extensions/debug-auto-launch/package.nls.json index 1179563a6c5..ba9f80dfe8b 100644 --- a/extensions/debug-auto-launch/package.nls.json +++ b/extensions/debug-auto-launch/package.nls.json @@ -1,12 +1,5 @@ { "displayName": "Node Debug Auto-attach", "description": "Helper for auto-attach feature when node-debug extensions are not active.", - - "debug.node.autoAttach.description": "Automatically attach node debugger when node.js was launched in debug mode from integrated terminal.", - "debug.javascript.usePreviewAutoAttach": "Whether to use the preview debugger's version of auto attach.", - "debug.node.autoAttach.disabled.description": "Auto attach is disabled and not shown in status bar.", - "debug.node.autoAttach.on.description": "Auto attach is active.", - "debug.node.autoAttach.off.description": "Auto attach is inactive.", - "toggle.auto.attach": "Toggle Auto Attach" } diff --git a/extensions/debug-auto-launch/src/extension.ts b/extensions/debug-auto-launch/src/extension.ts index 8bd96450c6f..cd00d9eed94 100644 --- a/extensions/debug-auto-launch/src/extension.ts +++ b/extensions/debug-auto-launch/src/extension.ts @@ -8,121 +8,152 @@ import * as vscode from 'vscode'; import * as nls from 'vscode-nls'; const localize = nls.loadMessageBundle(); -const ON_TEXT = localize('status.text.auto.attach.on', 'Auto Attach: On'); -const OFF_TEXT = localize('status.text.auto.attach.off', 'Auto Attach: Off'); +const TEXT_ALWAYS = localize('status.text.auto.attach.always', 'Auto Attach: Always'); +const TEXT_SMART = localize('status.text.auto.attach.smart', 'Auto Attach: Smart'); +const TEXT_WITH_FLAG = localize('status.text.auto.attach.withFlag', 'Auto Attach: With Flag'); +const TEXT_STATE_DESCRIPTION = { + [State.Disabled]: localize( + 'debug.javascript.autoAttach.disabled.description', + 'Auto attach is disabled and not shown in status bar', + ), + [State.Always]: localize( + 'debug.javascript.autoAttach.always.description', + 'Auto attach to every Node.js process launched in the terminal', + ), + [State.Smart]: localize( + 'debug.javascript.autoAttach.smart.description', + "Auto attach when running scripts that aren't in a node_modules folder", + ), + [State.OnlyWithFlag]: localize( + 'debug.javascript.autoAttach.onlyWithFlag.description', + 'Only auto attach when the `--inspect` flag is given', + ), +}; const TOGGLE_COMMAND = 'extension.node-debug.toggleAutoAttach'; -const JS_DEBUG_SETTINGS = 'debug.javascript'; -const JS_DEBUG_USEPREVIEWAA = 'usePreviewAutoAttach'; -const JS_DEBUG_IPC_KEY = 'jsDebugIpcState'; -const JS_DEBUG_REFRESH_SETTINGS = ['autoAttachSmartPattern', 'autoAttachFilter']; // settings that, when changed, should cause us to refresh js-debug vars -const NODE_DEBUG_SETTINGS = 'debug.node'; -const AUTO_ATTACH_SETTING = 'autoAttach'; -const LAST_STATE_STORAGE_KEY = 'lastState'; +const STORAGE_IPC = 'jsDebugIpcState'; +const SETTING_SECTION = 'debug.javascript'; +const SETTING_STATE = 'autoAttachFilter'; -type AUTO_ATTACH_VALUES = 'disabled' | 'on' | 'off'; +/** + * settings that, when changed, should cause us to refresh the state vars + */ +const SETTINGS_CAUSE_REFRESH = new Set( + ['autoAttachSmartPattern', SETTING_STATE].map(s => `${SETTING_SECTION}.${s}`), +); const enum State { - Disabled, - Off, - OnWithJsDebug, - OnWithNodeDebug, + Disabled = 'disabled', + OnlyWithFlag = 'onlyWithFlag', + Smart = 'smart', + Always = 'always', } -// on activation this feature is always disabled... -let currentState: Promise<{ context: vscode.ExtensionContext, state: State; transitionData: unknown }>; +let currentState: Promise<{ context: vscode.ExtensionContext; state: State | null }>; let statusItem: vscode.StatusBarItem | undefined; // and there is no status bar item +let server: Promise | undefined; // auto attach server export function activate(context: vscode.ExtensionContext): void { - const previousState = context.workspaceState.get(LAST_STATE_STORAGE_KEY, State.Disabled); - currentState = Promise.resolve(transitions[previousState].onActivate?.(context, readCurrentState())) - .then(() => ({ context, state: State.Disabled, transitionData: null })); - - context.subscriptions.push(vscode.commands.registerCommand(TOGGLE_COMMAND, toggleAutoAttachSetting)); - - // settings that can result in the "state" being changed--on/off/disable or useV3 toggles - const effectualConfigurationSettings = [ - `${NODE_DEBUG_SETTINGS}.${AUTO_ATTACH_SETTING}`, - `${JS_DEBUG_SETTINGS}.${JS_DEBUG_USEPREVIEWAA}`, - ]; - - const refreshConfigurationSettings = JS_DEBUG_REFRESH_SETTINGS.map(s => `${JS_DEBUG_SETTINGS}.${s}`); + currentState = Promise.resolve({ context, state: null }); context.subscriptions.push( - vscode.workspace.onDidChangeConfiguration((e) => { - if (effectualConfigurationSettings.some(setting => e.affectsConfiguration(setting))) { - updateAutoAttach(); - } else if (refreshConfigurationSettings.some(setting => e.affectsConfiguration(setting))) { - currentState = currentState.then(async s => { - if (s.state !== State.OnWithJsDebug) { - return s; - } - - await transitions[State.OnWithJsDebug].exit?.(context, s.transitionData); - await clearJsDebugAttachState(context); - const transitionData = await transitions[State.OnWithJsDebug].enter?.(context); - return { context, state: State.OnWithJsDebug, transitionData }; - }); - } - }) + vscode.commands.registerCommand(TOGGLE_COMMAND, toggleAutoAttachSetting), ); - updateAutoAttach(); + context.subscriptions.push( + vscode.workspace.onDidChangeConfiguration(e => { + // Whenever a setting is changed, disable auto attach, and re-enable + // it (if necessary) to refresh variables. + if ( + e.affectsConfiguration(`${SETTING_SECTION}.${SETTING_STATE}`) || + [...SETTINGS_CAUSE_REFRESH].some(setting => e.affectsConfiguration(setting)) + ) { + updateAutoAttach(State.Disabled); + updateAutoAttach(readCurrentState()); + } + }), + ); + + updateAutoAttach(readCurrentState()); } export async function deactivate(): Promise { - const { context, state, transitionData } = await currentState; - await transitions[state].exit?.(context, transitionData); + await destroyAttachServer(); } -function toggleAutoAttachSetting() { - const conf = vscode.workspace.getConfiguration(NODE_DEBUG_SETTINGS); - if (conf) { - let value = conf.get(AUTO_ATTACH_SETTING); - if (value === 'on') { - value = 'off'; - } else { - value = 'on'; - } +type StatePickItem = + | (vscode.QuickPickItem & { state: State }) + | (vscode.QuickPickItem & { scope: vscode.ConfigurationTarget }) + | (vscode.QuickPickItem & { type: 'separator' }); - const info = conf.inspect(AUTO_ATTACH_SETTING); - let target: vscode.ConfigurationTarget = vscode.ConfigurationTarget.Global; - if (info) { - if (info.workspaceFolderValue) { - target = vscode.ConfigurationTarget.WorkspaceFolder; - } else if (info.workspaceValue) { - target = vscode.ConfigurationTarget.Workspace; - } else if (info.globalValue) { - target = vscode.ConfigurationTarget.Global; - } else if (info.defaultValue) { - // setting not yet used: store setting in workspace - if (vscode.workspace.workspaceFolders) { - target = vscode.ConfigurationTarget.Workspace; - } - } - } - conf.update(AUTO_ATTACH_SETTING, value, target); +function getDefaultScope(info: ReturnType) { + if (!info) { + return vscode.ConfigurationTarget.Global; + } else if (info.workspaceFolderValue) { + return vscode.ConfigurationTarget.WorkspaceFolder; + } else if (info.workspaceValue) { + return vscode.ConfigurationTarget.Workspace; + } else if (info.globalValue) { + return vscode.ConfigurationTarget.Global; } + + return vscode.ConfigurationTarget.Global; } -function autoAttachWithJsDebug() { - const jsDebugConfig = vscode.workspace.getConfiguration(JS_DEBUG_SETTINGS); - return jsDebugConfig.get(JS_DEBUG_USEPREVIEWAA, true); +async function toggleAutoAttachSetting(scope?: vscode.ConfigurationTarget): Promise { + const section = vscode.workspace.getConfiguration(SETTING_SECTION); + scope = scope || getDefaultScope(section.inspect(SETTING_STATE)); + + const stateItems = [State.Always, State.Smart, State.OnlyWithFlag, State.Disabled].map(state => ({ + state, + label: state.slice(0, 1).toUpperCase() + state.slice(1), + description: TEXT_STATE_DESCRIPTION[state], + alwaysShow: true, + })); + + const scopeItem = + scope === vscode.ConfigurationTarget.Global + ? { + label: localize('scope.workspace', 'Toggle in this workspace $(arrow-right)'), + scope: vscode.ConfigurationTarget.Workspace, + } + : { + label: localize('scope.global', 'Toggle for this machine $(arrow-right)'), + scope: vscode.ConfigurationTarget.Global, + }; + + const quickPick = vscode.window.createQuickPick(); + // todo: have a separator here, see https://github.com/microsoft/vscode/issues/74967 + quickPick.items = [...stateItems, scopeItem]; + + quickPick.show(); + const current = readCurrentState(); + quickPick.activeItems = stateItems.filter(i => i.state === current); + + const result = await new Promise(resolve => { + quickPick.onDidAccept(() => resolve(quickPick.selectedItems[0])); + quickPick.onDidHide(() => resolve()); + }); + + quickPick.dispose(); + + if (!result) { + return; + } + + if ('scope' in result) { + return await toggleAutoAttachSetting(result.scope); + } + + if ('state' in result) { + section.update(SETTING_STATE, result.state, scope); + } } function readCurrentState(): State { - const nodeConfig = vscode.workspace.getConfiguration(NODE_DEBUG_SETTINGS); - const autoAttachState = nodeConfig.get(AUTO_ATTACH_SETTING); - switch (autoAttachState) { - case 'off': - return State.Off; - case 'on': - return autoAttachWithJsDebug() ? State.OnWithJsDebug : State.OnWithNodeDebug; - case 'disabled': - default: - return State.Disabled; - } + const section = vscode.workspace.getConfiguration(SETTING_SECTION); + return section.get(SETTING_STATE) ?? State.Disabled; } /** @@ -134,7 +165,7 @@ function ensureStatusBarExists(context: vscode.ExtensionContext) { statusItem.command = TOGGLE_COMMAND; statusItem.tooltip = localize( 'status.tooltip.auto.attach', - 'Automatically attach to node.js processes in debug mode' + 'Automatically attach to node.js processes in debug mode', ); statusItem.show(); context.subscriptions.push(statusItem); @@ -146,8 +177,63 @@ function ensureStatusBarExists(context: vscode.ExtensionContext) { } async function clearJsDebugAttachState(context: vscode.ExtensionContext) { - await context.workspaceState.update(JS_DEBUG_IPC_KEY, undefined); + await context.workspaceState.update(STORAGE_IPC, undefined); await vscode.commands.executeCommand('extension.js-debug.clearAutoAttachVariables'); + await destroyAttachServer(); +} + +/** + * Turns auto attach on, and returns the server auto attach is listening on + * if it's successful. + */ +async function createAttachServer(context: vscode.ExtensionContext) { + const ipcAddress = await getIpcAddress(context); + if (!ipcAddress) { + return undefined; + } + + server = new Promise((resolve, reject) => { + const s = createServer(socket => { + let data: Buffer[] = []; + socket.on('data', async chunk => { + if (chunk[chunk.length - 1] !== 0) { + // terminated with NUL byte + data.push(chunk); + return; + } + + data.push(chunk.slice(0, -1)); + + try { + await vscode.commands.executeCommand( + 'extension.js-debug.autoAttachToProcess', + JSON.parse(Buffer.concat(data).toString()), + ); + socket.write(Buffer.from([0])); + } catch (err) { + socket.write(Buffer.from([1])); + console.error(err); + } + }); + }) + .on('error', reject) + .listen(ipcAddress, () => resolve(s)); + }).catch(err => { + console.error(err); + return undefined; + }); + + return await server; +} + +/** + * Destroys the auto-attach server, if it's running. + */ +async function destroyAttachServer() { + const instance = await server; + if (instance) { + await new Promise(r => instance.close(r)); + } } interface CachedIpcState { @@ -156,124 +242,46 @@ interface CachedIpcState { settingsValue: string; } -interface StateTransition { - onActivate?(context: vscode.ExtensionContext, currentState: State): Promise; - exit?(context: vscode.ExtensionContext, stateData: StateData): Promise | void; - enter?(context: vscode.ExtensionContext): Promise | StateData; -} - -const makeTransition = (tsn: StateTransition) => tsn; // helper to apply generic type - /** * Map of logic that happens when auto attach states are entered and exited. * All state transitions are queued and run in order; promises are awaited. */ -const transitions: { [S in State]: StateTransition } = { - [State.Disabled]: makeTransition({ - async enter(context) { - statusItem?.hide(); - await clearJsDebugAttachState(context); - }, - }), +const transitions: { [S in State]: (context: vscode.ExtensionContext) => Promise } = { + async [State.Disabled](context) { + await clearJsDebugAttachState(context); + statusItem?.hide(); + }, - [State.Off]: makeTransition({ - enter(context) { - const statusItem = ensureStatusBarExists(context); - statusItem.text = OFF_TEXT; - }, - }), + async [State.OnlyWithFlag](context) { + await createAttachServer(context); + const statusItem = ensureStatusBarExists(context); + statusItem.text = TEXT_WITH_FLAG; + }, - [State.OnWithNodeDebug]: makeTransition({ - async enter(context) { - const statusItem = ensureStatusBarExists(context); - const vscode_pid = process.env['VSCODE_PID']; - const rootPid = vscode_pid ? parseInt(vscode_pid) : 0; - await vscode.commands.executeCommand('extension.node-debug.startAutoAttach', rootPid); - statusItem.text = ON_TEXT; - }, + async [State.Smart](context) { + await createAttachServer(context); + const statusItem = ensureStatusBarExists(context); + statusItem.text = TEXT_SMART; + }, - async exit() { - await vscode.commands.executeCommand('extension.node-debug.stopAutoAttach'); - }, - }), - - [State.OnWithJsDebug]: makeTransition({ - async enter(context) { - const ipcAddress = await getIpcAddress(context); - if (!ipcAddress) { - return null; - } - - const server = await new Promise((resolve, reject) => { - const s = createServer((socket) => { - let data: Buffer[] = []; - socket.on('data', async (chunk) => { - if (chunk[chunk.length - 1] !== 0) { // terminated with NUL byte - data.push(chunk); - return; - } - - data.push(chunk.slice(0, -1)); - - try { - await vscode.commands.executeCommand( - 'extension.js-debug.autoAttachToProcess', - JSON.parse(Buffer.concat(data).toString()) - ); - socket.write(Buffer.from([0])); - } catch (err) { - socket.write(Buffer.from([1])); - console.error(err); - } - }); - }) - .on('error', reject) - .listen(ipcAddress, () => resolve(s)); - }).catch(console.error); - - const statusItem = ensureStatusBarExists(context); - statusItem.text = ON_TEXT; - return server || null; - }, - - async exit(context, server) { - // we don't need to clear the environment variables--the bootloader will - // no-op if the debug server is closed. This prevents having to reload - // terminals if users want to turn it back on. - if (server) { - await new Promise((resolve) => server.close(resolve)); - } - - // but if they toggled auto attach use js-debug off, go ahead and do so - if (!autoAttachWithJsDebug()) { - await clearJsDebugAttachState(context); - } - }, - - async onActivate(context, currentState) { - if (currentState === State.OnWithNodeDebug || currentState === State.Disabled) { - await clearJsDebugAttachState(context); - } - } - }), + async [State.Always](context) { + await createAttachServer(context); + const statusItem = ensureStatusBarExists(context); + statusItem.text = TEXT_ALWAYS; + }, }; /** * Updates the auto attach feature based on the user or workspace setting */ -function updateAutoAttach() { - const newState = readCurrentState(); - - currentState = currentState.then(async ({ context, state: oldState, transitionData }) => { +function updateAutoAttach(newState: State) { + currentState = currentState.then(async ({ context, state: oldState }) => { if (newState === oldState) { - return { context, state: oldState, transitionData }; + return { context, state: oldState }; } - await transitions[oldState].exit?.(context, transitionData); - const newData = await transitions[newState].enter?.(context); - await context.workspaceState.update(LAST_STATE_STORAGE_KEY, newState); - - return { context, state: newState, transitionData: newData }; + await transitions[newState](context); + return { context, state: newState }; }); } @@ -285,41 +293,43 @@ async function getIpcAddress(context: vscode.ExtensionContext) { // Iff the `cachedData` is present, the js-debug registered environment // variables for this workspace--cachedData is set after successfully // invoking the attachment command. - const cachedIpc = context.workspaceState.get(JS_DEBUG_IPC_KEY); + const cachedIpc = context.workspaceState.get(STORAGE_IPC); // We invalidate the IPC data if the js-debug path changes, since that // indicates the extension was updated or reinstalled and the // environment variables will have been lost. // todo: make a way in the API to read environment data directly without activating js-debug? - const jsDebugPath = vscode.extensions.getExtension('ms-vscode.js-debug-nightly')?.extensionPath - || vscode.extensions.getExtension('ms-vscode.js-debug')?.extensionPath; + const jsDebugPath = + vscode.extensions.getExtension('ms-vscode.js-debug-nightly')?.extensionPath || + vscode.extensions.getExtension('ms-vscode.js-debug')?.extensionPath; const settingsValue = getJsDebugSettingKey(); - if (cachedIpc && cachedIpc.jsDebugPath === jsDebugPath && cachedIpc.settingsValue === settingsValue) { + if (cachedIpc?.jsDebugPath === jsDebugPath && cachedIpc?.settingsValue === settingsValue) { return cachedIpc.ipcAddress; } - const result = await vscode.commands.executeCommand<{ ipcAddress: string; }>( + const result = await vscode.commands.executeCommand<{ ipcAddress: string }>( 'extension.js-debug.setAutoAttachVariables', - cachedIpc?.ipcAddress + cachedIpc?.ipcAddress, ); if (!result) { return; } const ipcAddress = result.ipcAddress; - await context.workspaceState.update( - JS_DEBUG_IPC_KEY, - { ipcAddress, jsDebugPath, settingsValue } as CachedIpcState, - ); + await context.workspaceState.update(STORAGE_IPC, { + ipcAddress, + jsDebugPath, + settingsValue, + } as CachedIpcState); return ipcAddress; } function getJsDebugSettingKey() { let o: { [key: string]: unknown } = {}; - const config = vscode.workspace.getConfiguration(JS_DEBUG_SETTINGS); - for (const setting of JS_DEBUG_REFRESH_SETTINGS) { + const config = vscode.workspace.getConfiguration(SETTING_SECTION); + for (const setting of SETTINGS_CAUSE_REFRESH) { o[setting] = config.get(setting); }