Layout orientation correctly

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

@@ -146,7 +146,7 @@ export interface ITerminalService {
 
 	activeTerminalInstanceIndex: number;
 	configHelper: ITerminalConfigHelper;
-	onActiveInstanceChanged: Event<string>;
+	onActiveTabChanged: Event<void>;
 	onTabDisposed: Event<ITerminalTab>;
 	onInstanceDisposed: Event<ITerminalInstance>;
 	onInstanceProcessIdReady: Event<ITerminalInstance>;
@@ -182,6 +182,7 @@ export interface ITerminalTab {
 	terminalInstances: ITerminalInstance[];
 	title: string;
 
+	attachToElement(element: HTMLElement): void;
 	setVisible(visible: boolean): void;
 	layout(width: number, height: number): void;
 	addDisposable(disposable: IDisposable): void;

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

@@ -27,11 +27,13 @@ export abstract class TerminalService implements ITerminalService {
 	protected _terminalTabs: ITerminalTab[];
 	protected abstract _terminalInstances: ITerminalInstance[];
 
+	private _activeTabIndex: number;
+	// TODO: Remove _activeTerminalInstanceIndex
 	private _activeTerminalInstanceIndex: number;
-	private _onActiveInstanceChanged: Emitter<string>;
+	private _onActiveTabChanged: Emitter<void>;
 
 	public get activeTerminalInstanceIndex(): number { return this._activeTerminalInstanceIndex; }
-	public get onActiveInstanceChanged(): Event<string> { return this._onActiveInstanceChanged.event; }
+	public get onActiveTabChanged(): Event<void> { return this._onActiveTabChanged.event; }
 	public get onTabDisposed(): Event<ITerminalTab> { return this._onTabDisposed.event; }
 	public get onInstanceDisposed(): Event<ITerminalInstance> { return this._onInstanceDisposed.event; }
 	public get onInstanceProcessIdReady(): Event<ITerminalInstance> { return this._onInstanceProcessIdReady.event; }
@@ -48,10 +50,11 @@ export abstract class TerminalService implements ITerminalService {
 		@IPartService private _partService: IPartService,
 		@ILifecycleService lifecycleService: ILifecycleService
 	) {
+		this._activeTabIndex = 0;
 		this._activeTerminalInstanceIndex = 0;
 		this._isShuttingDown = false;
 
-		this._onActiveInstanceChanged = new Emitter<string>();
+		this._onActiveTabChanged = new Emitter<void>();
 		this._onTabDisposed = new Emitter<ITerminalTab>();
 		this._onInstanceDisposed = new Emitter<ITerminalInstance>();
 		this._onInstanceProcessIdReady = new Emitter<ITerminalInstance>();
@@ -102,11 +105,11 @@ export abstract class TerminalService implements ITerminalService {
 
 	private _removeTab(tab: ITerminalTab): void {
 		let index = this._terminalTabs.indexOf(tab);
-		let wasActiveInstance = tab === this._getActiveTab();
+		let wasActiveTab = tab === this._getActiveTab();
 		if (index !== -1) {
 			this._terminalTabs.splice(index, 1);
 		}
-		if (wasActiveInstance && this._terminalTabs.length > 0) {
+		if (wasActiveTab && this._terminalTabs.length > 0) {
 			let newIndex = index < this._terminalTabs.length ? index : this._terminalTabs.length - 1;
 			this.setActiveInstanceByIndex(newIndex);
 			// TODO: Needs to be made to work with multiple instances in a tab
@@ -122,8 +125,8 @@ export abstract class TerminalService implements ITerminalService {
 		}
 		// TODO: This should be onTabsChanged?
 		this._onInstancesChanged.fire();
-		if (wasActiveInstance) {
-			this._onActiveInstanceChanged.fire();
+		if (wasActiveTab) {
+			this._onActiveTabChanged.fire();
 		}
 	}
 
@@ -155,6 +158,21 @@ export abstract class TerminalService implements ITerminalService {
 		this.setActiveInstanceByIndex(this._getIndexFromId(terminalInstance.id));
 	}
 
+	public setActiveTabByIndex(tabIndex: number): void {
+		if (tabIndex >= this._terminalTabs.length) {
+			return;
+		}
+
+		const didTabChange = this._activeTabIndex !== tabIndex;
+		this._activeTabIndex = tabIndex;
+
+		this._terminalTabs.forEach((t, i) => t.setVisible(i === this._activeTabIndex));
+		if (didTabChange) {
+			this._onActiveTabChanged.fire();
+		}
+	}
+
+	// TODO: Remove setActiveInstanceByIndex?
 	public setActiveInstanceByIndex(terminalIndex: number): void {
 		if (terminalIndex >= this._terminalInstances.length) {
 			return;
@@ -173,7 +191,8 @@ export abstract class TerminalService implements ITerminalService {
 		// });
 		// Only fire the event if there was a change
 		if (didInstanceChange) {
-			this._onActiveInstanceChanged.fire();
+			// TODO: If this method is being kept this should only fire when the tab is actually changed
+			this._onActiveTabChanged.fire();
 		}
 	}
 

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

@@ -493,7 +493,7 @@ export class SwitchTerminalInstanceActionItem extends SelectActionItem {
 		super(null, action, terminalService.getTabLabels(), terminalService.activeTerminalInstanceIndex, contextViewService);
 
 		this.toDispose.push(terminalService.onInstancesChanged(this._updateItems, this));
-		this.toDispose.push(terminalService.onActiveInstanceChanged(this._updateItems, this));
+		this.toDispose.push(terminalService.onActiveTabChanged(this._updateItems, this));
 		this.toDispose.push(terminalService.onInstanceTitleChanged(this._updateItems, this));
 		this.toDispose.push(attachSelectBoxStyler(this.selectBox, themeService));
 	}

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

@@ -202,10 +202,16 @@ export class TerminalInstance implements ITerminalInstance {
 	 * @return The terminal's width if it requires a layout.
 	 */
 	private _evaluateColsAndRows(width: number, height: number): number {
+		// Ignore if dimensions are undefined or 0
+		if (!width || !height) {
+			return null;
+		}
+
 		const dimension = this._getDimension(width, height);
 		if (!dimension) {
 			return null;
 		}
+
 		const font = this._configHelper.getFont();
 
 		// Because xterm.js converts from CSS pixels to actual pixels through

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

@@ -247,6 +247,7 @@ export class TerminalService extends AbstractTerminalService implements ITermina
 	public setContainers(panelContainer: HTMLElement, terminalContainer: HTMLElement): void {
 		this._configHelper.panelContainer = panelContainer;
 		this._terminalContainer = terminalContainer;
+		this._terminalTabs.forEach(tab => tab.attachToElement(this._terminalContainer));
 		this._terminalInstances.forEach(terminalInstance => {
 			terminalInstance.attachToElement(this._terminalContainer);
 		});

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

@@ -9,7 +9,7 @@ import { TerminalConfigHelper } from 'vs/workbench/parts/terminal/electron-brows
 import { IContextKey } from 'vs/platform/contextkey/common/contextkey';
 import { TerminalInstance } from 'vs/workbench/parts/terminal/electron-browser/terminalInstance';
 import Event, { Emitter, anyEvent } from 'vs/base/common/event';
-import { IDisposable } from 'vs/base/common/lifecycle';
+import { IDisposable, Disposable } from 'vs/base/common/lifecycle';
 import { SplitView, Orientation, IView } from 'vs/base/browser/ui/splitview/splitview';
 
 class SplitPane implements IView {
@@ -64,6 +64,7 @@ class SplitPane implements IView {
 
 	private addChild(size: number, orthogonalSize: number, instance: ITerminalInstance, index?: number, needsReattach?: boolean): void {
 		const child = new SplitPane(this, orthogonalSize, needsReattach);
+		child.orientation = this.orientation;
 		child.instance = instance;
 		this._splitView.addView(child, size, index);
 
@@ -77,6 +78,9 @@ class SplitPane implements IView {
 	}
 
 	public render(container: HTMLElement): void {
+		if (!container) {
+			return;
+		}
 		this._container = container;
 		console.log('render');
 		// throw new Error("Method not implemented.");
@@ -101,17 +105,18 @@ class SplitPane implements IView {
 		console.log('layout', size, this.orthogonalSize);
 
 		if (this.orientation === Orientation.VERTICAL) {
-			this.instance.layout({ width: this._size, height: this.orthogonalSize });
-		} else {
 			this.instance.layout({ width: this.orthogonalSize, height: this._size });
+		} else {
+			this.instance.layout({ width: this._size, height: this.orthogonalSize });
 		}
 	}
 
 	public orthogonalLayout(size: number): void {
 		this.orthogonalSize = size;
+		console.log('orthogonalLayout', this._size, this.orthogonalSize);
 
 		if (this._splitView) {
-			this._splitView.layout(size);
+			this._splitView.layout(this.orthogonalSize);
 		}
 	}
 }
@@ -146,12 +151,13 @@ class RootSplitPane extends SplitPane {
 	}
 }
 
-export class TerminalTab implements ITerminalTab {
+export class TerminalTab extends Disposable implements ITerminalTab {
 	private _terminalInstances: ITerminalInstance[] = [];
-	private _disposables: IDisposable[] = [];
 	private _rootSplitPane: RootSplitPane;
 	private _splitPanes: SplitPane[] = [];
 
+	// private _activeInstanceIndex: number;
+
 	public get terminalInstances(): ITerminalInstance[] { return this._terminalInstances; }
 
 	private _onDisposed: Emitter<ITerminalTab>;
@@ -164,6 +170,7 @@ export class TerminalTab implements ITerminalTab {
 		shellLaunchConfig: IShellLaunchConfig,
 		@IInstantiationService private _instantiationService: IInstantiationService
 	) {
+		super();
 		this._onDisposed = new Emitter<ITerminalTab>();
 
 		const instance = this._instantiationService.createInstance(TerminalInstance,
@@ -172,6 +179,7 @@ export class TerminalTab implements ITerminalTab {
 			undefined,
 			shellLaunchConfig);
 		this._terminalInstances.push(instance);
+		// this._activeInstanceIndex = 0;
 		instance.addDisposable(instance.onDisposed(instance => this._onInstanceDisposed(instance)));
 
 		this._rootSplitPane = new RootSplitPane();
@@ -182,6 +190,23 @@ export class TerminalTab implements ITerminalTab {
 		this._splitPanes.push(this._rootSplitPane);
 	}
 
+	public dispose(): void {
+		super.dispose();
+		this._terminalInstances = [];
+	}
+
+	private _onInstanceDisposed(instance: ITerminalInstance): void {
+
+		// TODO: Listen for disposed on TerminalService and handle appropriately (remove the tab and its instance from the service)
+
+		this._onDisposed.fire(this);
+		this.dispose();
+	}
+
+	public attachToElement(element: HTMLElement): void {
+		this._container = element;
+	}
+
 	public get title(): string {
 		let title = this.terminalInstances[0].title;
 		for (let i = 1; i < this.terminalInstances.length; i++) {
@@ -191,7 +216,9 @@ export class TerminalTab implements ITerminalTab {
 	}
 
 	public setVisible(visible: boolean): void {
-		this._container.style.display = visible ? 'block' : 'none';
+		if (this._container) {
+			this._container.style.display = visible ? 'block' : 'none';
+		}
 		// TODO: probably don't need to tell terminal instances about visiblility anymore?
 		this.terminalInstances.forEach(i => i.setVisible(visible));
 	}
@@ -211,20 +238,12 @@ export class TerminalTab implements ITerminalTab {
 
 		this._rootSplitPane.orientation = Orientation.HORIZONTAL;
 		this._rootSplitPane.split(instance);
-		const pane2 = new SplitPane();
-		this._splitPanes.push(pane2);
-	}
-
-	private _onInstanceDisposed(instance: ITerminalInstance): void {
-
-		// TODO: Listen for disposed on TerminalService and handle appropriately (remove the tab and its instance from the service)
-
-		this._onDisposed.fire(this);
-		this._terminalInstances = [];
+		// const pane2 = new SplitPane();
+		// this._splitPanes.push(pane2);
 	}
 
 	public addDisposable(disposable: IDisposable): void {
-		this._disposables.push(disposable);
+		this._register(disposable);
 	}
 
 	public layout(width: number, height: number): void {