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

replace settings variables in debugger config #8042 #8784

Merged
merged 2 commits into from
Jul 19, 2016
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
8 changes: 4 additions & 4 deletions src/vs/base/common/parsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export abstract class AbstractSystemVariables implements ISystemVariables {
public resolve(value: IStringDictionary<IStringDictionary<string>>): IStringDictionary<IStringDictionary<string>>;
public resolve(value: any): any {
if (Types.isString(value)) {
return this.__resolveString(value);
return this.resolveString(value);
} else if (Types.isArray(value)) {
return this.__resolveArray(value);
} else if (Types.isObject(value)) {
Expand All @@ -143,7 +143,7 @@ export abstract class AbstractSystemVariables implements ISystemVariables {
resolveAny<T>(value: T): T;
resolveAny<T>(value: any): any {
if (Types.isString(value)) {
return this.__resolveString(value);
return this.resolveString(value);
} else if (Types.isArray(value)) {
return this.__resolveAnyArray(value);
} else if (Types.isObject(value)) {
Expand All @@ -153,7 +153,7 @@ export abstract class AbstractSystemVariables implements ISystemVariables {
return value;
}

private __resolveString(value: string): string {
protected resolveString(value: string): string {
let regexp = /\$\{(.*?)\}/g;
return value.replace(regexp, (match: string, name: string) => {
let newValue = (<any>this)[name];
Expand Down Expand Up @@ -185,7 +185,7 @@ export abstract class AbstractSystemVariables implements ISystemVariables {
}

private __resolveArray(value: string[]): string[] {
return value.map(s => this.__resolveString(s));
return value.map(s => this.resolveString(s));
}

private __resolveAnyArray<T>(value: T[]): T[];
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/parts/debug/node/debugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import objects = require('vs/base/common/objects');
import paths = require('vs/base/common/paths');
import platform = require('vs/base/common/platform');
import debug = require('vs/workbench/parts/debug/common/debug');
import { SystemVariables } from 'vs/workbench/parts/lib/node/systemVariables';
import { ISystemVariables } from 'vs/base/common/parsers';
import { IExtensionDescription } from 'vs/platform/extensions/common/extensions';

export class Adapter {
Expand All @@ -25,7 +25,7 @@ export class Adapter {
public enableBreakpointsFor: { languageIds: string[] };
public aiKey: string;

constructor(rawAdapter: debug.IRawAdapter, systemVariables: SystemVariables, public extensionDescription: IExtensionDescription) {
constructor(rawAdapter: debug.IRawAdapter, systemVariables: ISystemVariables, public extensionDescription: IExtensionDescription) {
if (rawAdapter.windows) {
rawAdapter.win = rawAdapter.windows;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ import { IFileService } from 'vs/platform/files/common/files';
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybindingService';
import debug = require('vs/workbench/parts/debug/common/debug');
import { SystemVariables } from 'vs/workbench/parts/lib/node/systemVariables';
import { Adapter } from 'vs/workbench/parts/debug/node/debugAdapter';
import { IWorkspaceContextService } from 'vs/workbench/services/workspace/common/contextService';
import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
import { IQuickOpenService } from 'vs/workbench/services/quickopen/common/quickOpenService';

import { ConfigVariables } from 'vs/workbench/parts/lib/node/configVariables';
import { ISystemVariables } from 'vs/base/common/parsers';
// debuggers extension point

export var debuggersExtPoint = extensionsRegistry.ExtensionsRegistry.registerExtensionPoint<debug.IRawAdapter[]>('debuggers', {
Expand Down Expand Up @@ -152,9 +152,8 @@ const jsonRegistry = <jsonContributionRegistry.IJSONContributionRegistry>platfor
jsonRegistry.registerSchema(schemaId, schema);

export class ConfigurationManager implements debug.IConfigurationManager {

public configuration: debug.IConfig;
private systemVariables: SystemVariables;
private systemVariables: ISystemVariables;
private adapters: Adapter[];
private allModeIdsForBreakpoints: { [key: string]: boolean };
private _onDidConfigurationChange: Emitter<string>;
Expand All @@ -169,8 +168,8 @@ export class ConfigurationManager implements debug.IConfigurationManager {
@IQuickOpenService private quickOpenService: IQuickOpenService,
@IKeybindingService private keybindingService: IKeybindingService
) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This will probably throw an exception if you open the no-folder workspace.
That is the reason why I originally check for this.contextService.getWorkspace(), I think you need to re-introduce that check.

this.systemVariables = new ConfigVariables(this.configurationService, this.editorService, this.contextService);
this._onDidConfigurationChange = new Emitter<string>();
this.systemVariables = this.contextService.getWorkspace() ? new SystemVariables(this.editorService, this.contextService) : null;
this.setConfiguration(configName);
this.adapters = [];
this.registerListeners();
Expand Down
29 changes: 29 additions & 0 deletions src/vs/workbench/parts/lib/node/configVariables.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
'use strict';

import * as Types from 'vs/base/common/types';
import { SystemVariables } from './systemVariables';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
import { IWorkspaceContextService } from 'vs/workbench/services/workspace/common/contextService';
import URI from 'vs/base/common/uri';

export class ConfigVariables extends SystemVariables {
constructor(private configurationService: IConfigurationService, editorService: IWorkbenchEditorService, contextService: IWorkspaceContextService, workspaceRoot: URI = null, envVariables: { [key: string]: string } = process.env) {
super(editorService, contextService, workspaceRoot, envVariables);
}

protected resolveString(value: string): string {
value = super.resolveString(value);

Copy link
Contributor

Choose a reason for hiding this comment

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

The regexp should have 'config' not 'settings' to allign with other places where we have replacments.

let regexp = /\$\{settings\.(.*?)\}/g;
return value.replace(regexp, (match: string, name: string) => {
let config = this.configurationService.getConfiguration();
let newValue = new Function('_', 'try {return _.' + name + ';} catch (ex) { return "";}')(config);
return Types.isString(newValue) ? newValue : '';
});
}
}
11 changes: 7 additions & 4 deletions src/vs/workbench/parts/lib/node/systemVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,16 @@ export class SystemVariables extends AbstractSystemVariables {
private _execPath: string;

// Optional workspaceRoot there to be used in tests.
constructor(private editorService: IWorkbenchEditorService, contextService: IWorkspaceContextService, workspaceRoot: URI = null) {
constructor(private editorService: IWorkbenchEditorService, contextService: IWorkspaceContextService, workspaceRoot: URI = null, envVariables: { [key: string]: string } = process.env) {
super();
let fsPath = workspaceRoot ? workspaceRoot.fsPath : contextService.getWorkspace().resource.fsPath;
let fsPath = '';
if (workspaceRoot || contextService.getWorkspace()) {
fsPath = workspaceRoot ? workspaceRoot.fsPath : contextService.getWorkspace().resource.fsPath;
}
this._workspaceRoot = Paths.normalize(fsPath, true);
this._execPath = contextService ? contextService.getConfiguration().env.execPath : null;
Object.keys(process.env).forEach(key => {
this[`env.${ key }`] = process.env[key];
Object.keys(envVariables).forEach(key => {
this[`env.${key}`] = envVariables[key];
});
}

Expand Down
101 changes: 101 additions & 0 deletions src/vs/workbench/parts/lib/test/node/configVariables.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/*---------------------------------------------------------------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome work 👍

* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
'use strict';

import * as assert from 'assert';
import URI from 'vs/base/common/uri';
import * as Platform from 'vs/base/common/platform';
import { ConfigVariables } from 'vs/workbench/parts/lib/node/configVariables';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import {TPromise} from 'vs/base/common/winjs.base';

suite('ConfigVariables tests', () => {
test('ConfigVariables: substitute one', () => {
let configurationService: IConfigurationService;
configurationService = new MockConfigurationService({
editor: {
fontFamily: 'foo'
},
terminal: {
integrated: {
fontFamily: 'bar'
}
}
});

let systemVariables: ConfigVariables = new ConfigVariables(configurationService, null, null, URI.parse('file:///VSCode/workspaceLocation'));
assert.strictEqual(systemVariables.resolve('abc ${settings.editor.fontFamily} xyz'), 'abc foo xyz');
});

test('ConfigVariables: substitute many', () => {
let configurationService: IConfigurationService;
configurationService = new MockConfigurationService({
editor: {
fontFamily: 'foo'
},
terminal: {
integrated: {
fontFamily: 'bar'
}
}
});

let systemVariables: ConfigVariables = new ConfigVariables(configurationService, null, null, URI.parse('file:///VSCode/workspaceLocation'));
assert.strictEqual(systemVariables.resolve('abc ${settings.editor.fontFamily} ${settings.terminal.integrated.fontFamily} xyz'), 'abc foo bar xyz');
});
test('SystemVariables: substitute one env variable', () => {
let configurationService: IConfigurationService;
configurationService = new MockConfigurationService({
editor: {
fontFamily: 'foo'
},
terminal: {
integrated: {
fontFamily: 'bar'
}
}
});

let envVariables: { [key: string]: string } = { key1: 'Value for Key1', key2: 'Value for Key2' };
let systemVariables: ConfigVariables = new ConfigVariables(configurationService, null, null, URI.parse('file:///VSCode/workspaceLocation'), envVariables);
if (Platform.isWindows) {
assert.strictEqual(systemVariables.resolve('abc ${settings.editor.fontFamily} ${workspaceRoot} ${env.key1} xyz'), 'abc foo \\VSCode\\workspaceLocation Value for Key1 xyz');
} else {
assert.strictEqual(systemVariables.resolve('abc ${settings.editor.fontFamily} ${workspaceRoot} ${env.key1} xyz'), 'abc foo /VSCode/workspaceLocation Value for Key1 xyz');
}
});

test('SystemVariables: substitute many env variable', () => {
let configurationService: IConfigurationService;
configurationService = new MockConfigurationService({
editor: {
fontFamily: 'foo'
},
terminal: {
integrated: {
fontFamily: 'bar'
}
}
});

let envVariables: { [key: string]: string } = { key1: 'Value for Key1', key2: 'Value for Key2' };
let systemVariables: ConfigVariables = new ConfigVariables(configurationService, null, null, URI.parse('file:///VSCode/workspaceLocation'), envVariables);
if (Platform.isWindows) {
assert.strictEqual(systemVariables.resolve('${settings.editor.fontFamily} ${settings.terminal.integrated.fontFamily} ${workspaceRoot} - ${workspaceRoot} ${env.key1} - ${env.key2}'), 'foo bar \\VSCode\\workspaceLocation - \\VSCode\\workspaceLocation Value for Key1 - Value for Key2');
} else {
assert.strictEqual(systemVariables.resolve('${settings.editor.fontFamily} ${settings.terminal.integrated.fontFamily} ${workspaceRoot} - ${workspaceRoot} ${env.key1} - ${env.key2}'), 'foo bar /VSCode/workspaceLocation - /VSCode/workspaceLocation Value for Key1 - Value for Key2');
}
});
});

class MockConfigurationService implements IConfigurationService {
public serviceId = IConfigurationService;
public constructor(private configuration: any = {}) { }
public loadConfiguration<T>(section?: string): TPromise<T> { return TPromise.as(this.getConfiguration()); }
public getConfiguration(): any { return this.configuration; }
public hasWorkspaceConfiguration(): boolean { return false; }
public onDidUpdateConfiguration() { return { dispose() { } }; }
public setUserConfiguration(key: any, value: any): Thenable<void> { return TPromise.as(null); }
}
19 changes: 19 additions & 0 deletions src/vs/workbench/parts/lib/test/node/systemVariables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,23 @@ suite('SystemVariables tests', () => {
assert.strictEqual(systemVariables.resolve('${workspaceRoot} - ${workspaceRoot}'), '/VSCode/workspaceLocation - /VSCode/workspaceLocation');
}
});
test('SystemVariables: substitute one env variable', () => {
let envVariables: { [key: string]: string } = { key1: 'Value for Key1', key2: 'Value for Key2' };
let systemVariables: SystemVariables = new SystemVariables(null, null, URI.parse('file:///VSCode/workspaceLocation'), envVariables);
if (Platform.isWindows) {
assert.strictEqual(systemVariables.resolve('abc ${workspaceRoot} ${env.key1} xyz'), 'abc \\VSCode\\workspaceLocation Value for Key1 xyz');
} else {
assert.strictEqual(systemVariables.resolve('abc ${workspaceRoot} ${env.key1} xyz'), 'abc /VSCode/workspaceLocation Value for Key1 xyz');
}
});

test('SystemVariables: substitute many env variable', () => {
let envVariables: { [key: string]: string } = { key1: 'Value for Key1', key2: 'Value for Key2' };
let systemVariables: SystemVariables = new SystemVariables(null, null, URI.parse('file:///VSCode/workspaceLocation'), envVariables);
if (Platform.isWindows) {
assert.strictEqual(systemVariables.resolve('${workspaceRoot} - ${workspaceRoot} ${env.key1} - ${env.key2}'), '\\VSCode\\workspaceLocation - \\VSCode\\workspaceLocation Value for Key1 - Value for Key2');
} else {
assert.strictEqual(systemVariables.resolve('${workspaceRoot} - ${workspaceRoot} ${env.key1} - ${env.key2}'), '/VSCode/workspaceLocation - /VSCode/workspaceLocation Value for Key1 - Value for Key2');
}
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import { IPartService } from 'vs/workbench/services/part/common/partService';
import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
import { IWorkspaceContextService } from 'vs/workbench/services/workspace/common/contextService';

import { SystemVariables } from 'vs/workbench/parts/lib/node/systemVariables';
import { ConfigVariables } from 'vs/workbench/parts/lib/node/configVariables';
import { ITextFileService, EventType } from 'vs/workbench/parts/files/common/files';
import { IOutputService, IOutputChannelRegistry, Extensions as OutputExt, IOutputChannel } from 'vs/workbench/parts/output/common/output';

Expand Down Expand Up @@ -215,7 +215,7 @@ class ConfigureTaskRunnerAction extends Action {
const outputChannel = this.outputService.getChannel(TaskService.OutputChannelId);
outputChannel.show();
outputChannel.append(nls.localize('ConfigureTaskRunnerAction.autoDetecting', 'Auto detecting tasks for {0}', selection.id) + '\n');
let detector = new ProcessRunnerDetector(this.fileService, this.contextService, new SystemVariables(this.editorService, this.contextService));
let detector = new ProcessRunnerDetector(this.fileService, this.contextService, new ConfigVariables(this.configurationService, this.editorService, this.contextService));
contentPromise = detector.detect(false, selection.id).then((value) => {
let config = value.config;
if (value.stderr && value.stderr.length > 0) {
Expand Down Expand Up @@ -634,7 +634,7 @@ class TaskService extends EventEmitter implements ITaskService {
this._taskSystem = new NullTaskSystem();
this._taskSystemPromise = TPromise.as(this._taskSystem);
} else {
let variables = new SystemVariables(this.editorService, this.contextService);
let variables = new ConfigVariables(this.configurationService, this.editorService, this.contextService);
let clearOutput = true;
this._taskSystemPromise = TPromise.as(this.configurationService.getConfiguration<TaskConfiguration>('tasks')).then((config: TaskConfiguration) => {
let parseErrors: string[] = config ? (<any>config).$parseErrors : null;
Expand Down
16 changes: 4 additions & 12 deletions src/vs/workbench/parts/tasks/node/processRunnerSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { TerminateResponse, SuccessData, ErrorData } from 'vs/base/common/proces
import { LineProcess, LineData } from 'vs/base/node/processes';

import { IOutputService, IOutputChannel } from 'vs/workbench/parts/output/common/output';
import { SystemVariables } from 'vs/workbench/parts/lib/node/systemVariables';
import { ISystemVariables } from 'vs/base/common/parsers';

import { IMarkerService } from 'vs/platform/markers/common/markers';
import { ValidationStatus } from 'vs/base/common/parsers';
Expand All @@ -37,7 +37,7 @@ export class ProcessRunnerSystem extends EventEmitter implements ITaskSystem {
public static TelemetryEventName: string = 'taskService';

private fileConfig: FileConfig.ExternalTaskRunnerConfiguration;
private variables: SystemVariables;
private variables: ISystemVariables;
private markerService: IMarkerService;
private modelService: IModelService;
private outputService: IOutputService;
Expand All @@ -53,7 +53,7 @@ export class ProcessRunnerSystem extends EventEmitter implements ITaskSystem {
private childProcess: LineProcess;
private activeTaskIdentifier: string;

constructor(fileConfig:FileConfig.ExternalTaskRunnerConfiguration, variables:SystemVariables, markerService:IMarkerService, modelService: IModelService, telemetryService: ITelemetryService, outputService:IOutputService, outputChannelId:string, clearOutput: boolean = true) {
constructor(fileConfig:FileConfig.ExternalTaskRunnerConfiguration, variables:ISystemVariables, markerService:IMarkerService, modelService: IModelService, telemetryService: ITelemetryService, outputService:IOutputService, outputChannelId:string, clearOutput: boolean = true) {
super();
this.fileConfig = fileConfig;
this.variables = variables;
Expand Down Expand Up @@ -388,15 +388,7 @@ export class ProcessRunnerSystem extends EventEmitter implements ITaskSystem {
}

private resolveVariable(value: string): string {
let regexp =/\$\{(.*?)\}/g;
return value.replace(regexp, (match:string, name:string) => {
let value = (<any>this.variables)[name];
if (value) {
return value;
} else {
return match;
}
});
return this.variables.resolve(value);
}

public log(value: string): void {
Expand Down