Skip to content

Commit

Permalink
reolved code review comments #8042
Browse files Browse the repository at this point in the history
  • Loading branch information
DonJayamanne committed Jul 10, 2016
1 parent b8c4a03 commit a81902c
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ export class ConfigurationManager implements debug.IConfigurationManager {
@IQuickOpenService private quickOpenService: IQuickOpenService,
@IKeybindingService private keybindingService: IKeybindingService
) {
this.systemVariables = new ConfigVariables(this.configurationService, this.editorService, this.contextService);
this.systemVariables = this.contextService.getWorkspace() ? new ConfigVariables(this.configurationService, this.editorService, this.contextService) : null;
this._onDidConfigurationChange = new Emitter<string>();
this.setConfiguration(configName);
this.adapters = [];
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/parts/lib/node/configVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export class ConfigVariables extends SystemVariables {
protected resolveString(value: string): string {
value = super.resolveString(value);

let regexp = /\$\{settings\.(.*?)\}/g;
let regexp = /\$\{config\.(.*?)\}/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);
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/parts/lib/node/systemVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class SystemVariables extends AbstractSystemVariables {
constructor(private editorService: IWorkbenchEditorService, contextService: IWorkspaceContextService, workspaceRoot: URI = null, envVariables: { [key: string]: string } = process.env) {
super();
let fsPath = '';
if (workspaceRoot || contextService.getWorkspace()) {
if (workspaceRoot || (contextService && contextService.getWorkspace())) {
fsPath = workspaceRoot ? workspaceRoot.fsPath : contextService.getWorkspace().resource.fsPath;
}
this._workspaceRoot = Paths.normalize(fsPath, true);
Expand Down
12 changes: 6 additions & 6 deletions src/vs/workbench/parts/lib/test/node/configVariables.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ suite('ConfigVariables tests', () => {
});

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');
assert.strictEqual(systemVariables.resolve('abc ${config.editor.fontFamily} xyz'), 'abc foo xyz');
});

test('ConfigVariables: substitute many', () => {
Expand All @@ -43,7 +43,7 @@ suite('ConfigVariables tests', () => {
});

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');
assert.strictEqual(systemVariables.resolve('abc ${config.editor.fontFamily} ${config.terminal.integrated.fontFamily} xyz'), 'abc foo bar xyz');
});
test('SystemVariables: substitute one env variable', () => {
let configurationService: IConfigurationService;
Expand All @@ -61,9 +61,9 @@ suite('ConfigVariables tests', () => {
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');
assert.strictEqual(systemVariables.resolve('abc ${config.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');
assert.strictEqual(systemVariables.resolve('abc ${config.editor.fontFamily} ${workspaceRoot} ${env.key1} xyz'), 'abc foo /VSCode/workspaceLocation Value for Key1 xyz');
}
});

Expand All @@ -83,9 +83,9 @@ suite('ConfigVariables tests', () => {
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');
assert.strictEqual(systemVariables.resolve('${config.editor.fontFamily} ${config.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');
assert.strictEqual(systemVariables.resolve('${config.editor.fontFamily} ${config.terminal.integrated.fontFamily} ${workspaceRoot} - ${workspaceRoot} ${env.key1} - ${env.key2}'), 'foo bar /VSCode/workspaceLocation - /VSCode/workspaceLocation Value for Key1 - Value for Key2');
}
});
});
Expand Down

0 comments on commit a81902c

Please sign in to comment.