Remove a lot of duplication and simplify config helper further

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

@@ -57,7 +57,7 @@ export class TerminalSupport {
 
 		// get the shell configuration for the current platform
 		let shell: string;
-		const shell_config = configurationService.getConfiguration<ITerminalConfiguration>().terminal.integrated.shell;
+		const shell_config = configurationService.getConfiguration<ITerminalConfiguration>().shell;
 		if (platform.isWindows) {
 			shell = shell_config.windows;
 		} else if (platform.isLinux) {

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

@@ -61,15 +61,9 @@ export interface ITerminalConfiguration {
 }
 
 export interface ITerminalConfigHelper {
+	config: ITerminalConfiguration;
 	getTheme(baseThemeId: string): string[];
 	getFont(): ITerminalFont;
-	getFontLigaturesEnabled(): boolean;
-	getCursorBlink(): boolean;
-	getRightClickCopyPaste(): boolean;
-	getCommandsToSkipShell(): string[];
-	getScrollback(): number;
-	getCwd(): string;
-	getConfirmOnExit(): boolean;
 }
 
 export interface ITerminalFont {

src/vs/workbench/parts/terminal/common/terminalService.ts

@@ -64,7 +64,7 @@ export abstract class TerminalService implements ITerminalService {
 	public abstract setContainers(panelContainer: HTMLElement, terminalContainer: HTMLElement): void;
 
 	private _onWillShutdown(): boolean {
-		if (!this.configHelper.getConfirmOnExit()) {
+		if (!this.configHelper.config.confirmOnExit) {
 			// Don't veto if configured to skip confirmation
 			return false;
 		}

src/vs/workbench/parts/terminal/electron-browser/terminalConfigHelper.ts

@@ -8,6 +8,12 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur
 import { ITerminalConfiguration, ITerminalConfigHelper, ITerminalFont, IShellLaunchConfig } from 'vs/workbench/parts/terminal/common/terminal';
 import { Platform } from 'vs/base/common/platform';
 
+interface IFullTerminalConfiguration {
+	terminal: {
+		integrated: ITerminalConfiguration;
+	};
+}
+
 const DEFAULT_LINE_HEIGHT = 1.2;
 
 const DEFAULT_ANSI_COLORS = {
@@ -81,7 +87,9 @@ export class TerminalConfigHelper implements ITerminalConfigHelper {
 		@IConfigurationService private _configurationService: IConfigurationService) {
 	}
 
-	private get _config(): ITerminalConfiguration { return this._configurationService.getConfiguration<ITerminalConfiguration>('terminal.integrated'); }
+	public get config(): ITerminalConfiguration {
+		return this._configurationService.getConfiguration<IFullTerminalConfiguration>().terminal.integrated;
+	}
 
 	public getTheme(baseThemeId: string): string[] {
 		return DEFAULT_ANSI_COLORS[baseThemeId];
@@ -119,7 +127,7 @@ export class TerminalConfigHelper implements ITerminalConfigHelper {
 	public getFont(): ITerminalFont {
 		const config = this._configurationService.getConfiguration();
 		const editorConfig = (<IEditorConfiguration>config).editor;
-		const terminalConfig = this._config;
+		const terminalConfig = this.config;
 
 		const fontFamily = terminalConfig.fontFamily || editorConfig.fontFamily;
 		let fontSize = this._toInteger(terminalConfig.fontSize, 0);
@@ -134,28 +142,8 @@ export class TerminalConfigHelper implements ITerminalConfigHelper {
 		return this._measureFont(fontFamily, fontSize, lineHeight);
 	}
 
-	public getFontLigaturesEnabled(): boolean {
-		return this._config.fontLigatures;
-	}
-
-	public getCursorBlink(): boolean {
-		return this._config.cursorBlinking;
-	}
-
-	public getCursorStyle(): string {
-		return this._config.cursorStyle;
-	}
-
-	public getRightClickCopyPaste(): boolean {
-		return this._config.rightClickCopyPaste;
-	}
-
-	public getCommandsToSkipShell(): string[] {
-		return this._config.commandsToSkipShell;
-	}
-
 	public mergeDefaultShellPathAndArgs(shell: IShellLaunchConfig): IShellLaunchConfig {
-		const config = this._config;
+		const config = this.config;
 
 		shell.executable = '';
 		shell.args = [];
@@ -174,22 +162,6 @@ export class TerminalConfigHelper implements ITerminalConfigHelper {
 		return shell;
 	}
 
-	public getScrollback(): number {
-		return this._config.scrollback;
-	}
-
-	public isSetLocaleVariables(): boolean {
-		return this._config.setLocaleVariables;
-	}
-
-	public getCwd(): string {
-		return this._config.cwd;
-	}
-
-	public getConfirmOnExit(): boolean {
-		return this._config.confirmOnExit;
-	}
-
 	private _toInteger(source: any, minimum?: number): number {
 		let r = parseInt(source, 10);
 		if (isNaN(r)) {

src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts

@@ -171,7 +171,7 @@ export class TerminalInstance implements ITerminalInstance {
 	 */
 	protected _createXterm(): void {
 		this._xterm = xterm({
-			scrollback: this._configHelper.getScrollback()
+			scrollback: this._configHelper.config.scrollback
 		});
 		this._process.on('message', (message) => this._sendPtyDataToXterm(message));
 		this._xterm.on('data', (data) => {
@@ -414,7 +414,7 @@ export class TerminalInstance implements ITerminalInstance {
 		// TODO: Handle non-existent customCwd
 		if (!shell.ignoreConfigurationCwd) {
 			// Evaluate custom cwd first
-			const customCwd = this._configHelper.getCwd();
+			const customCwd = this._configHelper.config.cwd;
 			if (customCwd) {
 				if (path.isAbsolute(customCwd)) {
 					cwd = customCwd;
@@ -433,7 +433,7 @@ export class TerminalInstance implements ITerminalInstance {
 	}
 
 	protected _createProcess(workspace: IWorkspace, shell: IShellLaunchConfig): void {
-		const locale = this._configHelper.isSetLocaleVariables() ? platform.locale : undefined;
+		const locale = this._configHelper.config.setLocaleVariables ? platform.locale : undefined;
 		if (!shell.executable) {
 			this._configHelper.mergeDefaultShellPathAndArgs(shell);
 		}
@@ -646,10 +646,10 @@ export class TerminalInstance implements ITerminalInstance {
 	}
 
 	public updateConfig(): void {
-		this._setCursorBlink(this._configHelper.getCursorBlink());
-		this._setCursorStyle(this._configHelper.getCursorStyle());
-		this._setCommandsToSkipShell(this._configHelper.getCommandsToSkipShell());
-		this._setScrollback(this._configHelper.getScrollback());
+		this._setCursorBlink(this._configHelper.config.cursorBlinking);
+		this._setCursorStyle(this._configHelper.config.cursorStyle);
+		this._setCommandsToSkipShell(this._configHelper.config.commandsToSkipShell);
+		this._setScrollback(this._configHelper.config.scrollback);
 	}
 
 	private _setCursorBlink(blink: boolean): void {

src/vs/workbench/parts/terminal/electron-browser/terminalPanel.ts

@@ -151,7 +151,7 @@ export class TerminalPanel extends Panel {
 				// occurs on the selection itself.
 				this._terminalService.getActiveInstance().focus();
 			} else if (event.which === 3) {
-				if (this._terminalService.configHelper.getRightClickCopyPaste()) {
+				if (this._terminalService.configHelper.config.rightClickCopyPaste) {
 					let terminal = this._terminalService.getActiveInstance();
 					if (terminal.hasSelection()) {
 						terminal.copySelection();
@@ -238,7 +238,7 @@ export class TerminalPanel extends Panel {
 			return;
 		}
 		let newFont = this._terminalService.configHelper.getFont();
-		DOM.toggleClass(this._parentDomElement, 'enable-ligatures', this._terminalService.configHelper.getFontLigaturesEnabled());
+		DOM.toggleClass(this._parentDomElement, 'enable-ligatures', this._terminalService.configHelper.config.fontLigatures);
 		if (!this._font || this._fontsDiffer(this._font, newFont)) {
 			this._fontStyleElement.innerHTML = '.monaco-workbench .panel.integrated-terminal .xterm {' +
 				`font-family: ${newFont.fontFamily};` +

src/vs/workbench/parts/terminal/test/electron-browser/terminalConfigHelper.test.ts

@@ -281,99 +281,4 @@ suite('Workbench - TerminalConfigHelper', () => {
 			'#e5e5e5'
 		], 'The dark terminal theme should be selected when a vs-dark theme is active');
 	});
-
-	test('TerminalConfigHelper - getFontLigaturesEnabled', function () {
-		let configurationService: IConfigurationService;
-		let configHelper: TerminalConfigHelper;
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { fontLigatures: true } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.equal(configHelper.getFontLigaturesEnabled(), true, 'terminal.integrated.fontLigatures should be true');
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { fontLigatures: false } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.equal(configHelper.getFontLigaturesEnabled(), false, 'terminal.integrated.fontLigatures should be false');
-	});
-
-	test('TerminalConfigHelper - getCursorBlink', function () {
-		let configurationService: IConfigurationService;
-		let configHelper: TerminalConfigHelper;
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { cursorBlinking: true } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.equal(configHelper.getCursorBlink(), true, 'terminal.integrated.cursorBlinking should be true');
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { cursorBlinking: false } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.equal(configHelper.getCursorBlink(), false, 'terminal.integrated.cursorBlinking should be false');
-	});
-
-	test('TerminalConfigHelper - isSetLocaleVariables', function () {
-		let configurationService: IConfigurationService;
-		let configHelper: TerminalConfigHelper;
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { setLocaleVariables: true } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.equal(configHelper.isSetLocaleVariables(), true, 'terminal.integrated.setLocaleVariables should be true');
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { setLocaleVariables: false } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.equal(configHelper.isSetLocaleVariables(), false, 'terminal.integrated.setLocaleVariables should be false');
-	});
-
-	test('TerminalConfigHelper - getCommandsToSkipShell', function () {
-		let configurationService: IConfigurationService;
-		let configHelper: TerminalConfigHelper;
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { commandsToSkipShell: [] } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.deepEqual(configHelper.getCommandsToSkipShell(), [], 'terminal.integrated.commandsToSkipShell should be []');
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { commandsToSkipShell: ['foo'] } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.deepEqual(configHelper.getCommandsToSkipShell(), ['foo'], 'terminal.integrated.commandsToSkipShell should be [\'foo\']');
-	});
-
-	test('TerminalConfigHelper - getScrollback', function () {
-		let configurationService: IConfigurationService;
-		let configHelper: TerminalConfigHelper;
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { scrollback: 10 } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.deepEqual(configHelper.getScrollback(), 10, 'terminal.integrated.scrollback should be 10');
-
-		configurationService = new MockConfigurationService({
-			terminal: { integrated: { scrollback: 20 } }
-		});
-		configHelper = new TerminalConfigHelper(Platform.Linux, configurationService);
-		configHelper.panelContainer = fixture;
-		assert.deepEqual(configHelper.getScrollback(), 20, 'terminal.integrated.scrollback should be 20');
-	});
 });
\ No newline at end of file

src/vs/workbench/parts/terminal/test/electron-browser/terminalInstance.test.ts

@@ -78,7 +78,7 @@ suite('Workbench - TerminalInstance', () => {
 	suite('_getCwd', () => {
 		let instance: TestTerminalInstance;
 		let instantiationService: TestInstantiationService;
-		let configHelper: { getCwd: () => string };
+		let configHelper: { config: { cwd: string } };
 
 		setup(() => {
 			let contextKeyService = new MockKeybindingService();
@@ -90,7 +90,9 @@ suite('Workbench - TerminalInstance', () => {
 			instantiationService.stub(IKeybindingService, keybindingService);
 			instantiationService.stub(IContextKeyService, contextKeyService);
 			configHelper = {
-				getCwd: () => null
+				config: {
+					cwd: null
+				}
 			};
 			instance = instantiationService.createInstance(TestTerminalInstance, terminalFocusContextKey, configHelper, null, null, null);
 		});
@@ -109,30 +111,30 @@ suite('Workbench - TerminalInstance', () => {
 		});
 
 		test('should use an absolute custom cwd as is', () => {
-			configHelper.getCwd = () => '/foo';
+			configHelper.config.cwd = '/foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [] }, null), '/foo');
 		});
 
 		test('should normalize a relative custom cwd against the workspace path', () => {
-			configHelper.getCwd = () => 'foo';
+			configHelper.config.cwd = 'foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [] }, { resource: Uri.file('/bar') }), '/bar/foo');
-			configHelper.getCwd = () => './foo';
+			configHelper.config.cwd = './foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [] }, { resource: Uri.file('/bar') }), '/bar/foo');
-			configHelper.getCwd = () => '../foo';
+			configHelper.config.cwd = '../foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [] }, { resource: Uri.file('/bar') }, ), '/foo');
 		});
 
 		test('should fall back for relative a custom cwd that doesn\'t have a workspace', () => {
-			configHelper.getCwd = () => 'foo';
+			configHelper.config.cwd = 'foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [] }, null), os.homedir());
-			configHelper.getCwd = () => './foo';
+			configHelper.config.cwd = './foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [] }, null), os.homedir());
-			configHelper.getCwd = () => '../foo';
+			configHelper.config.cwd = '../foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [] }, null), os.homedir());
 		});
 
 		test('should ignore custom cwd when told to ignore', () => {
-			configHelper.getCwd = () => '/foo';
+			configHelper.config.cwd = '/foo';
 			assertPathsMatch(instance._getCwd({ executable: null, args: [], ignoreConfigurationCwd: true }, { resource: Uri.file('/bar') }), '/bar');
 		});
 	});