Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: don't attach debugger to terminal until a process starts #221

Merged
merged 3 commits into from
Jan 15, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix: use terminal attach as delegate mode instead of different type
  • Loading branch information
connor4312 committed Jan 15, 2020
commit 0a659d07061bd869925f2e12bbda442fa6e9babb
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,7 @@
"onCommand:extension.NAMESPACE(chrome-debug).addCustomBreakpoints",
"onCommand:extension.NAMESPACE(chrome-debug).removeAllCustomBreakpoints",
"onCommand:extension.NAMESPACE(chrome-debug).removeCustomBreakpoint",
"onDebugResolve:NAMESPACE(extensionHost)",
"onDebugResolve:NAMESPACE(js-debug-delegate)"
"onDebugResolve:NAMESPACE(extensionHost)"
],
"contributes": {
"views": {
Expand Down
12 changes: 2 additions & 10 deletions src/build/generate-contributions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
IChromeBaseConfiguration,
IChromeLaunchConfiguration,
IChromeAttachConfiguration,
INodeTerminalConfiguration,
ITerminalLaunchConfiguration,
baseDefaults,
} from '../configuration';
import { JSONSchema6 } from 'json-schema';
Expand Down Expand Up @@ -467,7 +467,7 @@ const nodeLaunchConfig: IDebugger<INodeLaunchConfiguration> = {
},
};

const nodeTerminalConfiguration: IDebugger<INodeTerminalConfiguration> = {
const nodeTerminalConfiguration: IDebugger<ITerminalLaunchConfiguration> = {
type: Contributions.TerminalDebugType,
request: 'launch',
label: refString('debug.terminal.label'),
Expand Down Expand Up @@ -715,14 +715,6 @@ function buildDebuggers() {
};
}

// Add fake delegate type:
output.push({
type: Contributions.DelegateDebugType,
label: 'a',
languages: [],
configurationAttributes: { launch: {} },
});

return walkObject(output, sortKeys);
}

Expand Down
5 changes: 2 additions & 3 deletions src/common/contributionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*--------------------------------------------------------*/

import { Command, WorkspaceConfiguration, WorkspaceFolder, commands } from 'vscode';
import { INodeTerminalConfiguration } from '../configuration';
import { ITerminalLaunchConfiguration } from '../configuration';

export const enum Contributions {
PrettyPrintCommand = 'extension.NAMESPACE(node-debug).prettyPrint',
Expand All @@ -21,7 +21,6 @@ export const enum Contributions {
TerminalDebugType = 'NAMESPACE(node-terminal)',
NodeDebugType = 'NAMESPACE(node)',
ChromeDebugType = 'NAMESPACE(chrome)',
DelegateDebugType = 'NAMESPACE(js-debug-delegate)',

BrowserBreakpointsView = 'jsBrowserBreakpoints',
}
Expand All @@ -38,7 +37,7 @@ export const enum Configuration {
export interface IConfigurationTypes {
[Configuration.NpmScriptLens]: 'all' | 'top' | 'never';
[Configuration.WarnOnLongPrediction]: boolean;
[Configuration.TerminalDebugConfig]: Partial<INodeTerminalConfiguration>;
[Configuration.TerminalDebugConfig]: Partial<ITerminalLaunchConfiguration>;
}

export interface ICommandTypes {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is interesting but what's the point of declaring the types here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When dispatching commands between parts of the extension I wanted a way to make sure my types were done up correctly; refactoring some of the commands made me wish that TS could verify my work, so I added these.

Expand Down
8 changes: 8 additions & 0 deletions src/common/objUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@ export const removeNulls = <V>(obj: { [key: string]: V | null }) =>
export const removeUndefined = <V>(obj: { [key: string]: V | undefined }) =>
filterValues(obj, (v): v is V => v !== undefined);

/**
* Asserts that the value is never. If this function is reached, it throws.
*/
export const assertNever = (value: never, message: string): never => {
debugger;
throw new Error(message.replace('{value}', JSON.stringify(value)));
};

/**
* Filters the object by value.
*/
Expand Down
88 changes: 49 additions & 39 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import Dap from './dap/api';
import { Contributions } from './common/contributionUtils';
import { assertNever } from './common/objUtils';

export interface IMandatedConfiguration extends Dap.LaunchParams {
/**
Expand Down Expand Up @@ -336,13 +337,13 @@ export interface IChromeBaseConfiguration extends IBaseConfiguration {
/**
* Launch options to boot a server.
*/
server: INodeLaunchConfiguration | INodeTerminalConfiguration | null;
server: INodeLaunchConfiguration | ITerminalLaunchConfiguration | null;
}

/**
* Opens a debugger-enabled terminal.
*/
export interface INodeTerminalConfiguration extends INodeBaseConfiguration {
export interface ITerminalLaunchConfiguration extends INodeBaseConfiguration {
type: Contributions.TerminalDebugType;
request: 'launch';

Expand Down Expand Up @@ -429,22 +430,23 @@ export interface IChromeAttachConfiguration extends IChromeBaseConfiguration {
/**
* Attach request used internally to inject a pre-built target into the lifecycle.
*/
export interface IDelegateConfiguration extends IBaseConfiguration {
type: Contributions.DelegateDebugType;
request: 'launch';
export interface ITerminalDelegateConfiguration extends INodeBaseConfiguration {
type: Contributions.TerminalDebugType;
request: 'attach';
delegateId: number;
}

export type AnyNodeConfiguration =
| INodeAttachConfiguration
| INodeLaunchConfiguration
| INodeTerminalConfiguration
| IExtensionHostConfiguration;
| ITerminalLaunchConfiguration
| IExtensionHostConfiguration
| ITerminalDelegateConfiguration;
export type AnyChromeConfiguration = IChromeAttachConfiguration | IChromeLaunchConfiguration;
export type AnyLaunchConfiguration =
| AnyChromeConfiguration
| AnyNodeConfiguration
| IDelegateConfiguration;
export type AnyLaunchConfiguration = AnyChromeConfiguration | AnyNodeConfiguration;
export type AnyTerminalConfiguration =
| ITerminalDelegateConfiguration
| ITerminalLaunchConfiguration;

/**
* Where T subtypes AnyLaunchConfiguration, gets the resolving version of T.
Expand All @@ -456,19 +458,25 @@ export type ResolvingExtensionHostConfiguration = ResolvingConfiguration<
>;
export type ResolvingNodeAttachConfiguration = ResolvingConfiguration<INodeAttachConfiguration>;
export type ResolvingNodeLaunchConfiguration = ResolvingConfiguration<INodeLaunchConfiguration>;
export type ResolvingTerminalConfiguration = ResolvingConfiguration<INodeTerminalConfiguration>;
export type ResolvingTerminalDelegateConfiguration = ResolvingConfiguration<
ITerminalDelegateConfiguration
>;
export type ResolvingTerminalLaunchConfiguration = ResolvingConfiguration<
ITerminalLaunchConfiguration
>;
export type ResolvingTerminalConfiguration =
| ResolvingTerminalDelegateConfiguration
| ResolvingTerminalLaunchConfiguration;
export type ResolvingNodeConfiguration =
| ResolvingNodeAttachConfiguration
| ResolvingNodeLaunchConfiguration;
export type ResolvingChromeConfiguration = ResolvingConfiguration<AnyChromeConfiguration>;
export type ResolvingDelegateConfiguration = ResolvingConfiguration<IDelegateConfiguration>;
export type AnyResolvingConfiguration =
| ResolvingExtensionHostConfiguration
| ResolvingChromeConfiguration
| ResolvingNodeAttachConfiguration
| ResolvingNodeLaunchConfiguration
| ResolvingTerminalConfiguration
| ResolvingDelegateConfiguration;
| ResolvingTerminalConfiguration;

/**
* Where T subtypes AnyResolvingConfiguration, gets the resolved version of T.
Expand All @@ -482,7 +490,7 @@ export type ResolvedConfiguration<T> = T extends ResolvingNodeAttachConfiguratio
: T extends ResolvingChromeConfiguration
? AnyChromeConfiguration
: T extends ResolvingTerminalConfiguration
? INodeTerminalConfiguration
? ITerminalLaunchConfiguration
: never;

export const baseDefaults: IBaseConfiguration = {
Expand Down Expand Up @@ -518,18 +526,18 @@ const nodeBaseDefaults: INodeBaseConfiguration = {
autoAttachChildProcesses: true,
};

export const terminalBaseDefaults: INodeTerminalConfiguration = {
export const terminalBaseDefaults: ITerminalLaunchConfiguration = {
...nodeBaseDefaults,
showAsyncStacks: { onceBreakpointResolved: 16 },
type: Contributions.TerminalDebugType,
request: 'launch',
name: 'Debugger Terminal',
};

export const delegateDefaults: IDelegateConfiguration = {
...baseDefaults,
type: Contributions.DelegateDebugType,
request: 'launch',
export const delegateDefaults: ITerminalDelegateConfiguration = {
...nodeBaseDefaults,
type: Contributions.TerminalDebugType,
request: 'attach',
name: 'Debugger Terminal',
delegateId: -1,
};
Expand Down Expand Up @@ -626,31 +634,33 @@ export function applyExtensionHostDefaults(

export function applyTerminalDefaults(
config: ResolvingTerminalConfiguration,
): INodeTerminalConfiguration {
return { ...extensionHostConfigDefaults, ...config };
}

export function applyDelegateDefaults(
config: ResolvingDelegateConfiguration,
): IDelegateConfiguration {
return { ...delegateDefaults, ...config };
): AnyTerminalConfiguration {
return config.request === 'launch'
? { ...terminalBaseDefaults, ...config }
: { ...delegateDefaults, ...config };
}

export const isConfigurationWithEnv = (config: unknown): config is IConfigurationWithEnv =>
typeof config === 'object' && !!config && 'env' in config && 'envFile' in config;

export function applyDefaults(config: AnyResolvingConfiguration): AnyLaunchConfiguration {
let configWithDefaults: AnyLaunchConfiguration;
if (config.type === Contributions.NodeDebugType) configWithDefaults = applyNodeDefaults(config);
else if (config.type === Contributions.ChromeDebugType)
configWithDefaults = applyChromeDefaults(config);
else if (config.type === Contributions.ExtensionHostDebugType)
configWithDefaults = applyExtensionHostDefaults(config);
else if (config.type === Contributions.TerminalDebugType)
configWithDefaults = applyTerminalDefaults(config);
else if (config.type === Contributions.DelegateDebugType)
configWithDefaults = applyDelegateDefaults(config);
else throw new Error(`Unknown config: ${JSON.stringify(config)}`);
switch (config.type) {
case Contributions.NodeDebugType:
configWithDefaults = applyNodeDefaults(config);
break;
case Contributions.ChromeDebugType:
configWithDefaults = applyChromeDefaults(config);
break;
case Contributions.ExtensionHostDebugType:
configWithDefaults = applyExtensionHostDefaults(config);
break;
case Contributions.TerminalDebugType:
configWithDefaults = applyTerminalDefaults(config);
break;
default:
throw assertNever(config, 'Unknown config: {value}');
}

resolveWorkspaceRoot(configWithDefaults);
return configWithDefaults;
Expand Down
4 changes: 0 additions & 4 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,6 @@ export function activate(context: vscode.ExtensionContext) {
const sessionManager = new SessionManager(context, launcherDelegate);
context.subscriptions.push(
vscode.debug.registerDebugAdapterDescriptorFactory(Contributions.NodeDebugType, sessionManager),
vscode.debug.registerDebugAdapterDescriptorFactory(
Contributions.DelegateDebugType,
sessionManager,
),
vscode.debug.registerDebugAdapterDescriptorFactory(
Contributions.TerminalDebugType,
sessionManager,
Expand Down
2 changes: 1 addition & 1 deletion src/targets/delegate/delegateLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class DelegateLauncher implements ILauncher {
params: AnyLaunchConfiguration,
context: ILaunchContext,
): Promise<ILaunchResult> {
if (params.type !== Contributions.DelegateDebugType) {
if (params.type !== Contributions.TerminalDebugType || params.request !== 'attach') {
return { blockSessionTermination: false };
}

Expand Down
10 changes: 6 additions & 4 deletions src/targets/node/terminalNodeLauncher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* Copyright (C) Microsoft Corporation. All rights reserved.
*--------------------------------------------------------*/

import { AnyLaunchConfiguration, INodeTerminalConfiguration } from '../../configuration';
import { AnyLaunchConfiguration, ITerminalLaunchConfiguration } from '../../configuration';
import * as vscode from 'vscode';
import { Contributions } from '../../common/contributionUtils';
import { NodeLauncherBase, IRunData } from './nodeLauncherBase';
Expand Down Expand Up @@ -38,11 +38,13 @@ class VSCodeTerminalProcess implements IProgram {
* A special launcher which only opens a vscode terminal. Used for the
* "debugger terminal" in the extension.
*/
export class TerminalNodeLauncher extends NodeLauncherBase<INodeTerminalConfiguration> {
export class TerminalNodeLauncher extends NodeLauncherBase<ITerminalLaunchConfiguration> {
/**
* @inheritdoc
*/
protected resolveParams(params: AnyLaunchConfiguration): INodeTerminalConfiguration | undefined {
protected resolveParams(
params: AnyLaunchConfiguration,
): ITerminalLaunchConfiguration | undefined {
if (params.type === Contributions.TerminalDebugType && params.request === 'launch') {
return params;
}
Expand All @@ -61,7 +63,7 @@ export class TerminalNodeLauncher extends NodeLauncherBase<INodeTerminalConfigur
/**
* Launches the program.
*/
protected async launchProgram(runData: IRunData<INodeTerminalConfiguration>): Promise<void> {
protected async launchProgram(runData: IRunData<ITerminalLaunchConfiguration>): Promise<void> {
// Make sure that, if we can _find_ a in their path, it's the right
// version so that we don't mysteriously never connect fail.
try {
Expand Down
8 changes: 4 additions & 4 deletions src/terminalDebugConfigurationProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import * as vscode from 'vscode';
import {
ResolvedConfiguration,
terminalBaseDefaults,
INodeTerminalConfiguration,
ITerminalLaunchConfiguration,
} from './configuration';
import { BaseConfigurationProvider } from './baseConfigurationProvider';
import { guessWorkingDirectory } from './nodeDebugConfigurationProvider';
Expand All @@ -17,12 +17,12 @@ import { guessWorkingDirectory } from './nodeDebugConfigurationProvider';
* node-debug, with support for some legacy options (mern, useWSL) removed.
*/
export class TerminalDebugConfigurationProvider
extends BaseConfigurationProvider<INodeTerminalConfiguration>
extends BaseConfigurationProvider<ITerminalLaunchConfiguration>
implements vscode.DebugConfigurationProvider {
protected async resolveDebugConfigurationAsync(
folder: vscode.WorkspaceFolder | undefined,
config: ResolvedConfiguration<INodeTerminalConfiguration>,
): Promise<INodeTerminalConfiguration | undefined> {
config: ResolvedConfiguration<ITerminalLaunchConfiguration>,
): Promise<ITerminalLaunchConfiguration | undefined> {
if (!config.cwd) {
config.cwd = guessWorkingDirectory(undefined, folder);
}
Expand Down
8 changes: 4 additions & 4 deletions src/ui/debugTerminalUI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { TerminalNodeLauncher } from '../targets/node/terminalNodeLauncher';
import { NodePathProvider } from '../targets/node/nodePathProvider';
import { ITarget } from '../targets/targets';
import { DelegateLauncherFactory } from '../targets/delegate/delegateLauncherFactory';
import { applyDefaults, INodeTerminalConfiguration } from '../configuration';
import { applyDefaults, ITerminalLaunchConfiguration } from '../configuration';
import { NeverCancelled } from '../common/cancellation';
import { createPendingDapApi } from '../dap/pending-api';
import { RawTelemetryReporterToDap } from '../telemetry/telemetryReporter';
Expand All @@ -29,7 +29,7 @@ function launchTerminal(
workspaceFolder?: vscode.WorkspaceFolder,
) {
const launcher = new TerminalNodeLauncher(new NodePathProvider());
const baseDebugOptions: Partial<INodeTerminalConfiguration> = {
const baseDebugOptions: Partial<ITerminalLaunchConfiguration> = {
...readConfig(vscode.workspace.getConfiguration(), Configuration.TerminalDebugConfig),
// Prevent switching over the the Debug Console whenever a process starts
internalConsoleOptions: 'neverOpen',
Expand Down Expand Up @@ -65,9 +65,9 @@ function launchTerminal(
workspaceFolder,
applyDefaults({
...baseDebugOptions,
type: Contributions.DelegateDebugType,
type: Contributions.TerminalDebugType,
name: 'Node.js Process',
request: 'launch',
request: 'attach',
delegateId,
}),
);
Expand Down