Notebook fixes: Fix #4129, fix #4116, Fix #3913, fix empty results error (#4150)

- Fixes #4129 Overlapping command help windows in notebook
  - Do not show parameter hints for inactive cells, to avoid them hanging around when no longer selected
- Fixes #4116 Notebooks: Intellisense Doesn't Work using Add New Connection
  - Move connect/disconnect logic to 1 place (code component) instead of 2
  - Handle the case where you connect after choosing active cell. We now hook to the event and update connection
- Fix issues in sql session manager where result outputs 0 rows. This was failing to show the empty resultset contents, which is a regression vs. query editor. It also put unhandled error on the debug console
- Fix #3913 Notebook: words selected in other cells should be unselected on cell change

Note: after fix, now looks as follows. Need to do follow up to get correct grid min height

![image](https://user-images.githubusercontent.com/10819925/53280226-9e629580-36cc-11e9-8f40-59cd913caeee.png)

src/sql/parts/modelComponents/queryTextEditor.ts

@@ -81,6 +81,8 @@ export class QueryTextEditor extends BaseTextEditor {
 			options.hideCursorInOverviewRuler = true;
 			if (!this._selected) {
 				options.renderLineHighlight = 'none';
+				options.parameterHints = { enabled: false };
+				options.matchBrackets = false;
 			}
 			if (this._hideLineNumbers) {
 				options.lineNumbers = 'off';

src/sql/parts/notebook/cellViews/code.component.ts

@@ -4,7 +4,7 @@
 *--------------------------------------------------------------------------------------------*/
 import 'vs/css!./code';
 
-import { OnInit, Component, Input, Inject, forwardRef, ElementRef, ChangeDetectorRef, ViewChild, Output, EventEmitter, OnChanges, SimpleChange } from '@angular/core';
+import { OnInit, Component, Input, Inject, ElementRef, ViewChild, Output, EventEmitter, OnChanges, SimpleChange } from '@angular/core';
 
 import { AngularDisposable } from 'sql/base/node/lifecycle';
 import { QueryTextEditor } from 'sql/parts/modelComponents/queryTextEditor';
@@ -32,6 +32,7 @@ import { CellTypes } from 'sql/parts/notebook/models/contracts';
 import { OVERRIDE_EDITOR_THEMING_SETTING } from 'sql/workbench/services/notebook/common/notebookService';
 import * as notebookUtils from 'sql/parts/notebook/notebookUtils';
 import { UntitledEditorModel } from 'vs/workbench/common/editor/untitledEditorModel';
+import { IConnectionManagementService } from 'sql/platform/connection/common/connectionManagement';
 
 export const CODE_SELECTOR: string = 'code-component';
 const MARKDOWN_CLASS = 'markdown';
@@ -67,6 +68,9 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 			this.checkForLanguageMagics();
 			this.updateLanguageMode();
 		}));
+		this._register(value.onValidConnectionSelected(() => {
+			this.updateConnectionState(this.isActive());
+		}));
 	}
 
 	@Input() set activeCellId(value: string) {
@@ -105,6 +109,8 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 		this._cellToggleMoreActions = this._instantiationService.createInstance(CellToggleMoreActions);
 		this._register(debounceEvent(this._layoutEmitter.event, (l, e) => e, 250, /*leading=*/false)
 		(() => this.layout()));
+		// Handle disconnect on removal of the cell, if it was the active cell
+		this._register({ dispose: () => this.updateConnectionState(false) });
 
 	}
 
@@ -121,11 +127,7 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 			if (propName === 'activeCellId') {
 				let changedProp = changes[propName];
 				let isActive = this.cellModel.id === changedProp.currentValue;
-				if (isActive && this._model.defaultKernel.display_name === notebookConstants.SQL
+				this.updateConnectionState(isActive);
-					&& this.cellModel.cellType === CellTypes.Code
-					&& this.cellModel.cellUri) {
-					this._model.notebookOptions.connectionService.connect(this._model.activeConnection, this.cellModel.cellUri.toString()).catch(e => console.log(e));
-				}
 				this.toggleMoreActionsButton(isActive);
 				if (this._editor) {
 					this._editor.toggleEditorSelected(isActive);
@@ -135,6 +137,28 @@ export class CodeComponent extends AngularDisposable implements OnInit, OnChange
 		}
 	}
 
+	private updateConnectionState(isConnected: boolean) {
+		if (this.isSqlCodeCell()) {
+			let cellUri = this.cellModel.cellUri.toString();
+			let connectionService = this.connectionService;
+			if (!isConnected && connectionService && connectionService.isConnected(cellUri)) {
+				connectionService.disconnect(cellUri).catch(e => console.log(e));
+			} else if (this._model.activeConnection && this._model.activeConnection.id !== '-1') {
+				connectionService.connect(this._model.activeConnection, cellUri).catch(e => console.log(e));
+			}
+		}
+	}
+
+	private get connectionService(): IConnectionManagementService {
+		return this._model && this._model.notebookOptions && this._model.notebookOptions.connectionService;
+	}
+
+	private isSqlCodeCell() {
+		return this._model.defaultKernel.display_name === notebookConstants.SQL
+			&& this.cellModel.cellType === CellTypes.Code
+			&& this.cellModel.cellUri;
+	}
+
 	ngAfterContentInit(): void {
 		this.createEditor();
 		this._register(DOM.addDisposableListener(window, DOM.EventType.RESIZE, e => {

src/sql/parts/notebook/notebook.component.ts

@@ -134,7 +134,6 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 	}
 
 	ngOnDestroy() {
-		this.disconnect();
 		this.dispose();
 		if (this.notebookService) {
 			this.notebookService.removeNotebookEditor(this);
@@ -168,7 +167,6 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 		}
 		if (cell !== this.model.activeCell) {
 			if (this.model.activeCell) {
-				this.disconnect();
 				this.model.activeCell.active = false;
 			}
 			this._model.activeCell = cell;
@@ -177,14 +175,6 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe
 		}
 	}
 
-	private disconnect() {
-		if (this._model.defaultKernel.display_name === notebookConstants.SQL) {
-			if (this._model.activeCell && this._model.activeCell.cellType === CellTypes.Code && this._model.activeCell.cellUri) {
-				this.connectionManagementService.disconnect(this._model.activeCell.cellUri.toString()).catch(e => console.log(e));
-			}
-		}
-	}
-
 	public unselectActiveCell() {
 		if (this.model && this.model.activeCell) {
 			this.model.activeCell.active = false;

src/sql/workbench/services/notebook/common/sqlSessionManager.ts

@@ -5,7 +5,7 @@
 'use strict';
 
 import * as os from 'os';
-import { nb, QueryExecuteSubsetResult, IDbColumn, BatchSummary, IResultMessage } from 'sqlops';
+import { nb, QueryExecuteSubsetResult, IDbColumn, BatchSummary, IResultMessage, ResultSetSummary } from 'sqlops';
 import { localize } from 'vs/nls';
 import * as strings from 'vs/base/common/strings';
 import { FutureInternal, ILanguageMagic } from 'sql/parts/notebook/models/modelInterfaces';
@@ -398,32 +398,51 @@ export class SQLFuture extends Disposable implements FutureInternal {
 	public handleBatchEnd(batch: BatchSummary): void {
 		if (this.ioHandler) {
 			this.handleMessage(strings.format(elapsedTimeLabel, batch.executionElapsed));
+			this.processResultSets(batch);
+		}
+	}
+
+	private async processResultSets(batch: BatchSummary): Promise<void> {
+		try {
 			for (let resultSet of batch.resultSetSummaries) {
 				let rowCount = resultSet.rowCount > this.configuredMaxRows ? this.configuredMaxRows : resultSet.rowCount;
-				this._queryRunner.getQueryRows(0, rowCount, resultSet.batchId, resultSet.id).then(d => {
+				await this.sendResultSetAsIOPub(rowCount, resultSet);
-
-					let msg: nb.IIOPubMessage = {
-						channel: 'iopub',
-						type: 'iopub',
-						header: <nb.IHeader>{
-							msg_id: undefined,
-							msg_type: 'execute_result'
-						},
-						content: <nb.IExecuteResult>{
-							output_type: 'execute_result',
-							metadata: {},
-							execution_count: this._executionCount,
-							data: { 'application/vnd.dataresource+json': this.convertToDataResource(resultSet.columnInfo, d), 'text/html': this.convertToHtmlTable(resultSet.columnInfo, d) }
-						},
-						metadata: undefined,
-						parent_header: undefined
-					};
-					this.ioHandler.handle(msg);
-				});
 			}
+		} catch (err) {
+			// TODO should we output this somewhere else?
+			console.log(`Error outputting result sets from Notebook query: ${err}`);
 		}
 	}
 
+	private async sendResultSetAsIOPub(rowCount: number, resultSet: ResultSetSummary): Promise<void> {
+		let subsetResult: QueryExecuteSubsetResult;
+		if (rowCount > 0) {
+			subsetResult = await this._queryRunner.getQueryRows(0, rowCount, resultSet.batchId, resultSet.id);
+		} else {
+			subsetResult = { message: '', resultSubset: { rowCount: 0, rows: [] }};
+		}
+		let msg: nb.IIOPubMessage = {
+			channel: 'iopub',
+			type: 'iopub',
+			header: <nb.IHeader>{
+				msg_id: undefined,
+				msg_type: 'execute_result'
+			},
+			content: <nb.IExecuteResult>{
+				output_type: 'execute_result',
+				metadata: {},
+				execution_count: this._executionCount,
+				data: {
+					'application/vnd.dataresource+json': this.convertToDataResource(resultSet.columnInfo, subsetResult),
+					'text/html': this.convertToHtmlTable(resultSet.columnInfo, subsetResult)
+				}
+			},
+			metadata: undefined,
+			parent_header: undefined
+		};
+		this.ioHandler.handle(msg);
+	}
+
 	setIOPubHandler(handler: nb.MessageHandler<nb.IIOPubMessage>): void {
 		this.ioHandler = handler;
 	}