Merge pull request #59617 from Microsoft/isidorn/debugStart

Isidorn/debug start

src/vs/workbench/api/electron-browser/mainThreadDebugService.ts

@@ -183,8 +183,8 @@ export class MainThreadDebugService implements MainThreadDebugServiceShape, IDeb
 	public $startDebugging(_folderUri: uri | undefined, nameOrConfiguration: string | IConfig): Thenable<boolean> {
 		const folderUri = _folderUri ? uri.revive(_folderUri) : undefined;
 		const launch = this.debugService.getConfigurationManager().getLaunch(folderUri);
-		return this.debugService.startDebugging(launch, nameOrConfiguration).then(x => {
-			return true;
+		return this.debugService.startDebugging(launch, nameOrConfiguration).then(success => {
+			return success;
 		}, err => {
 			return TPromise.wrapError(new Error(err && err.message ? err.message : 'cannot start debugging'));
 		});

src/vs/workbench/parts/debug/common/debug.ts

@@ -759,8 +759,11 @@ export interface IDebugService {
 	 * Starts debugging. If the configOrName is not passed uses the selected configuration in the debug dropdown.
 	 * Also saves all files, manages if compounds are present in the configuration
 	 * and resolveds configurations via DebugConfigurationProviders.
+	 *
+	 * Returns true if the start debugging was successfull. For compound launches, all configurations have to start successfuly for it to return success.
+	 * On errors the startDebugging will throw an error, however some error and cancelations are handled and in that case will simply return false.
 	 */
-	startDebugging(launch: ILaunch, configOrName?: IConfig | string, noDebug?: boolean): TPromise<void>;
+	startDebugging(launch: ILaunch, configOrName?: IConfig | string, noDebug?: boolean): TPromise<boolean>;
 
 	/**
 	 * Restarts a session or creates a new one if there is no active session.

src/vs/workbench/parts/debug/electron-browser/debugService.ts

@@ -257,14 +257,17 @@ export class DebugService implements IDebugService {
 
 	/**
 	 * main entry point
+	 * properly manages compounds, checks for errors and handles the initializing state.
 	 */
-	startDebugging(launch: ILaunch, configOrName?: IConfig | string, noDebug = false, unresolvedConfiguration?: IConfig, ): TPromise<void> {
+	startDebugging(launch: ILaunch, configOrName?: IConfig | string, noDebug = false, unresolvedConfig?: IConfig, ): TPromise<boolean> {
 
+		this.startInitializingState();
 		// make sure to save all files and that the configuration is up to date
 		return this.extensionService.activateByEvent('onDebug').then(() =>
 			this.textFileService.saveAll().then(() => this.configurationService.reloadConfiguration(launch ? launch.workspace : undefined).then(() =>
 				this.extensionService.whenInstalledExtensionsRegistered().then(() => {
 
+					// If it is the very first start debugging we need to clear the repl and our sessions map
 					if (this.model.getSessions().length === 0) {
 						this.removeReplExpressions();
 						this.allSessions.clear();
@@ -291,6 +294,7 @@ export class DebugService implements IDebugService {
 					}
 
 					if (compound) {
+						// we are starting a compound debug, first do some error checking and than start each configuration in the compound
 						if (!compound.configurations) {
 							return TPromise.wrapError(new Error(nls.localize({ key: 'compoundMustHaveConfigurations', comment: ['compound indicates a "compounds" configuration item', '"configurations" is an attribute and should not be localized'] },
 								"Compound must have \"configurations\" attribute set in order to start multiple configurations.")));
@@ -299,7 +303,7 @@ export class DebugService implements IDebugService {
 						return TPromise.join(compound.configurations.map(configData => {
 							const name = typeof configData === 'string' ? configData : configData.name;
 							if (name === compound.name) {
-								return TPromise.as(null);
+								return TPromise.as(false);
 							}
 
 							let launchForName: ILaunch;
@@ -323,114 +327,105 @@ export class DebugService implements IDebugService {
 								}
 							}
 
-							return this.startDebugging(launchForName, name, noDebug, unresolvedConfiguration);
-						}));
+							return this.createSession(launchForName, launchForName.getConfiguration(name), unresolvedConfig, noDebug);
+						})).then(values => values.every(success => !!success)); // Compound launch is a success only if each configuration launched successfully
 					}
+
 					if (configOrName && !config) {
 						const message = !!launch ? nls.localize('configMissing', "Configuration '{0}' is missing in 'launch.json'.", typeof configOrName === 'string' ? configOrName : JSON.stringify(configOrName)) :
 							nls.localize('launchJsonDoesNotExist', "'launch.json' does not exist.");
 						return TPromise.wrapError(new Error(message));
 					}
 
-					// We keep the debug type in a separate variable 'type' so that a no-folder config has no attributes.
-					// Storing the type in the config would break extensions that assume that the no-folder case is indicated by an empty config.
-					let type: string;
-					if (config) {
-						type = config.type;
-					} else {
-						// a no-folder workspace has no launch.config
-						config = <IConfig>{};
-					}
-					unresolvedConfiguration = unresolvedConfiguration || deepClone(config);
-
-					if (noDebug) {
-						config.noDebug = true;
-					}
-
-					return (type ? TPromise.as(null) : this.configurationManager.guessDebugger().then(a => type = a && a.type)).then(() =>
-						this.configurationManager.resolveConfigurationByProviders(launch && launch.workspace ? launch.workspace.uri : undefined, type, config).then(config => {
-							// a falsy config indicates an aborted launch
-							if (config && config.type) {
-								return this.createSession(launch, config, unresolvedConfiguration);
-							}
-
-							if (launch && type && config === null) {	// show launch.json only for "config" being "null".
-								return launch.openConfigFile(false, true, type).then(() => undefined);
-							}
-
-							return undefined;
-						})
-					).then(() => undefined);
+					return this.createSession(launch, config, unresolvedConfig, noDebug);
 				})
-			)));
+			))).then(success => {
+				// make sure to get out of initializing state, and propagate the result
+				this.endInitializingState();
+				return success;
+			}, err => {
+				this.endInitializingState();
+				return TPromise.wrapError(err);
+			});
 	}
 
-	private createSession(launch: ILaunch, config: IConfig, unresolvedConfig: IConfig): TPromise<void> {
+	/**
+	 * gets the debugger for the type, resolves configurations by providers, substitutes variables and runs prelaunch tasks
+	 */
+	private createSession(launch: ILaunch, config: IConfig, unresolvedConfig: IConfig, noDebug: boolean): TPromise<boolean> {
+		// We keep the debug type in a separate variable 'type' so that a no-folder config has no attributes.
+		// Storing the type in the config would break extensions that assume that the no-folder case is indicated by an empty config.
+		let type: string;
+		if (config) {
+			type = config.type;
+		} else {
+			// a no-folder workspace has no launch.config
+			config = Object.create(null);
+		}
+		unresolvedConfig = unresolvedConfig || deepClone(config);
+
+		if (noDebug) {
+			config.noDebug = true;
+		}
 
-		this.startInitializingState();
+		return (type ? TPromise.as(null) : this.configurationManager.guessDebugger().then(a => type = a && a.type)).then(() =>
+			this.configurationManager.resolveConfigurationByProviders(launch && launch.workspace ? launch.workspace.uri : undefined, type, config).then(config => {
+				// a falsy config indicates an aborted launch
+				if (config && config.type) {
+					return this.substituteVariables(launch, config).then(resolvedConfig => {
 
-		return this.textFileService.saveAll().then(() =>
-			this.substituteVariables(launch, config).then(resolvedConfig => {
+						if (!resolvedConfig) {
+							// User canceled resolving of interactive variables, silently return
+							return false;
+						}
 
-				if (!resolvedConfig) {
-					// User canceled resolving of interactive variables, silently return
-					return undefined;
-				}
+						if (!this.configurationManager.getDebugger(resolvedConfig.type) || (config.request !== 'attach' && config.request !== 'launch')) {
+							let message: string;
+							if (config.request !== 'attach' && config.request !== 'launch') {
+								message = config.request ? nls.localize('debugRequestNotSupported', "Attribute '{0}' has an unsupported value '{1}' in the chosen debug configuration.", 'request', config.request)
+									: nls.localize('debugRequesMissing', "Attribute '{0}' is missing from the chosen debug configuration.", 'request');
 
-				if (!this.configurationManager.getDebugger(resolvedConfig.type) || (config.request !== 'attach' && config.request !== 'launch')) {
-					let message: string;
-					if (config.request !== 'attach' && config.request !== 'launch') {
-						message = config.request ? nls.localize('debugRequestNotSupported', "Attribute '{0}' has an unsupported value '{1}' in the chosen debug configuration.", 'request', config.request)
-							: nls.localize('debugRequesMissing', "Attribute '{0}' is missing from the chosen debug configuration.", 'request');
+							} else {
+								message = resolvedConfig.type ? nls.localize('debugTypeNotSupported', "Configured debug type '{0}' is not supported.", resolvedConfig.type) :
+									nls.localize('debugTypeMissing', "Missing property 'type' for the chosen launch configuration.");
+							}
 
-					} else {
-						message = resolvedConfig.type ? nls.localize('debugTypeNotSupported', "Configured debug type '{0}' is not supported.", resolvedConfig.type) :
-							nls.localize('debugTypeMissing', "Missing property 'type' for the chosen launch configuration.");
-					}
+							return this.showError(message).then(() => false);
+						}
 
-					return this.showError(message);
-				}
+						const workspace = launch ? launch.workspace : undefined;
+						return this.runTaskAndCheckErrors(workspace, resolvedConfig.preLaunchTask).then(result => {
+							if (result === TaskRunResult.Success) {
+								return this.doCreateSession(workspace, { resolved: resolvedConfig, unresolved: unresolvedConfig });
+							}
+							return false;
+						});
+					}, err => {
+						if (err && err.message) {
+							return this.showError(err.message).then(() => false);
+						}
+						if (this.contextService.getWorkbenchState() === WorkbenchState.EMPTY) {
+							return this.showError(nls.localize('noFolderWorkspaceDebugError', "The active file can not be debugged. Make sure it is saved on disk and that you have a debug extension installed for that file type."))
+								.then(() => false);
+						}
 
-				const workspace = launch ? launch.workspace : undefined;
-				return this.runTaskAndCheckErrors(workspace, resolvedConfig.preLaunchTask).then(result => {
-					if (result === TaskRunResult.Success) {
-						return this.doCreateSession(workspace, { resolved: resolvedConfig, unresolved: unresolvedConfig });
-					}
-					return undefined;
-				});
-			}, err => {
-				if (err && err.message) {
-					return this.showError(err.message);
+						return launch && launch.openConfigFile(false, true).then(() => false);
+					});
 				}
-				if (this.contextService.getWorkbenchState() === WorkbenchState.EMPTY) {
-					return this.showError(nls.localize('noFolderWorkspaceDebugError', "The active file can not be debugged. Make sure it is saved on disk and that you have a debug extension installed for that file type."));
+
+				if (launch && type && config === null) {	// show launch.json only for "config" being "null".
+					return launch.openConfigFile(false, true, type).then(() => false);
 				}
 
-				return launch && launch.openConfigFile(false, true).then(editor => void 0);
+				return false;
 			})
-		).then(() => {
-			this.endInitializingState();
-		}, err => {
-			this.endInitializingState();
-			return TPromise.wrapError(err);
-		});
-	}
-
-	private launchOrAttachToSession(session: IDebugSession, focus = true): TPromise<void> {
-		const dbgr = this.configurationManager.getDebugger(session.configuration.type);
-		return session.initialize(dbgr).then(() => {
-			return session.launchOrAttach(session.configuration).then(() => {
-				if (focus) {
-					this.focusStackFrame(undefined, undefined, session);
-				}
-			});
-		}).then(undefined, err => {
-			session.shutdown();
-			return TPromise.wrapError(err);
-		});
+		);
 	}
 
-	private doCreateSession(root: IWorkspaceFolder, configuration: { resolved: IConfig, unresolved: IConfig }): TPromise<any> {
+	/**
+	 * instantiates the new session, initializes the session, registers session listeners and reports telemetry
+	 */
+	private doCreateSession(root: IWorkspaceFolder, configuration: { resolved: IConfig, unresolved: IConfig }): TPromise<boolean> {
 
 		const session = this.instantiationService.createInstance(DebugSession, configuration, root, this.model);
 		this.allSessions.set(session.getId(), session);
@@ -465,7 +460,7 @@ export class DebugService implements IDebugService {
 			}
 
 			return this.telemetryDebugSessionStart(root, session.configuration.type);
-		}).then(() => session, (error: Error | string) => {
+		}).then(() => true, (error: Error | string) => {
 
 			if (errors.isPromiseCanceledError(error)) {
 				// don't show 'canceled' error messages to the user #7906
@@ -484,7 +479,21 @@ export class DebugService implements IDebugService {
 
 			const errorMessage = error instanceof Error ? error.message : error;
 			this.telemetryDebugMisconfiguration(session.configuration ? session.configuration.type : undefined, errorMessage);
-			return this.showError(errorMessage, errors.isErrorWithActions(error) ? error.actions : []);
+			return this.showError(errorMessage, errors.isErrorWithActions(error) ? error.actions : []).then(() => false);
+		});
+	}
+
+	private launchOrAttachToSession(session: IDebugSession, focus = true): TPromise<void> {
+		const dbgr = this.configurationManager.getDebugger(session.configuration.type);
+		return session.initialize(dbgr).then(() => {
+			return session.launchOrAttach(session.configuration).then(() => {
+				if (focus) {
+					this.focusStackFrame(undefined, undefined, session);
+				}
+			});
+		}).then(undefined, err => {
+			session.shutdown();
+			return TPromise.wrapError(err);
 		});
 	}
 
@@ -627,7 +636,7 @@ export class DebugService implements IDebugService {
 		return TPromise.as(config);
 	}
 
-	private showError(message: string, actions: IAction[] = []): TPromise<boolean> {
+	private showError(message: string, actions: IAction[] = []): TPromise<void> {
 		const configureAction = this.instantiationService.createInstance(debugactions.ConfigureAction, debugactions.ConfigureAction.ID, debugactions.ConfigureAction.LABEL);
 		actions.push(configureAction);
 		return this.dialogService.show(severity.Error, message, actions.map(a => a.label).concat(nls.localize('cancel', "Cancel")), { cancelId: actions.length }).then(choice => {
@@ -635,7 +644,7 @@ export class DebugService implements IDebugService {
 				return actions[choice].run();
 			}
 
-			return TPromise.as(false);
+			return void 0;
 		});
 	}
 

src/vs/workbench/parts/debug/test/common/mockDebug.ts

@@ -97,8 +97,8 @@ export class MockDebugService implements IDebugService {
 
 	public removeWatchExpressions(id?: string): void { }
 
-	public startDebugging(launch: ILaunch, configOrName?: IConfig | string, noDebug?: boolean): TPromise<any> {
-		return TPromise.as(null);
+	public startDebugging(launch: ILaunch, configOrName?: IConfig | string, noDebug?: boolean): TPromise<boolean> {
+		return TPromise.as(true);
 	}
 
 	public restartSession(): TPromise<any> {