Run and Add Cell keybinding support (#3896)

- As part of this, fixed bug in the insertCell API where it didn't add to the end / failed if no cells existed

extensions/notebook/package.json

@@ -45,6 +45,18 @@
 					"dark": "resources/dark/open_notebook_inverse.svg",
 					"light": "resources/light/open_notebook.svg"
 				}
+			},
+			{
+				"command": "notebook.command.runactivecell",
+				"title": "%notebook.command.runactivecell%"
+			},
+			{
+				"command": "notebook.command.addcode",
+				"title": "%notebook.command.addcode%"
+			},
+			{
+				"command": "notebook.command.addtext",
+				"title": "%notebook.command.addtext%"
 			}
 		],
 		"menus": {
@@ -54,6 +66,18 @@
 				},
 				{
 					"command": "notebook.command.open"
+				},
+				{
+					"command": "notebook.command.runactivecell",
+					"when": "notebookEditorVisible"
+				},
+				{
+					"command": "notebook.command.addcode",
+					"when": "notebookEditorVisible"
+				},
+				{
+					"command": "notebook.command.addtext",
+					"when": "notebookEditorVisible"
 				}
 			],
 			"objectExplorer/item/context": [
@@ -68,6 +92,21 @@
 			{
 				"command": "notebook.command.new",
 				"key": "Ctrl+Shift+N"
+			},
+			{
+				"command": "notebook.command.runactivecell",
+				"key": "F5",
+				"when": "notebookEditorVisible"
+			},
+			{
+				"command": "notebook.command.addcode",
+				"key": "Ctrl+Shift+C",
+				"when": "notebookEditorVisible"
+			},
+			{
+				"command": "notebook.command.addtext",
+				"key": "Ctrl+Shift+T",
+				"when": "notebookEditorVisible"
 			}
 		]
 	},

extensions/notebook/package.nls.json

@@ -5,5 +5,8 @@
 	"notebook.pythonPath.description": "Local path to python installation used by Notebooks.",
 	"notebook.sqlKernelEnabled.description": "Enable SQL kernel in notebook editor (Preview). Requires reloading this window to take effect",
 	"notebook.command.new": "New Notebook",
-	"notebook.command.open": "Open Notebook"
+	"notebook.command.open": "Open Notebook",
+	"notebook.command.runactivecell": "Run Cell",
+	"notebook.command.addcode": "Add Code Cell",
+	"notebook.command.addtext": "Add Text Cell"
 }

extensions/notebook/src/extension.ts

@@ -11,31 +11,43 @@ import * as nls from 'vscode-nls';
 const localize = nls.loadMessageBundle();
 
 let counter = 0;
-
+const noNotebookVisible = localize('noNotebookVisible', 'No notebook editor is active');
 export function activate(extensionContext: vscode.ExtensionContext) {
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.new', ( connectionId? : string) => {
-		let title = `Untitled-${counter++}`;
-		let untitledUri = vscode.Uri.parse(`untitled:${title}`);
-        let options: sqlops.nb.NotebookShowOptions =  connectionId? {
-			viewColumn : null,
-			preserveFocus : true,
-			preview: null,
-            providerId : null,
-			connectionId : connectionId,
-			defaultKernel : null
-		} : null;
-		sqlops.nb.showNotebookDocument(untitledUri, options).then(success => {
-
-		}, (err: Error) => {
-			vscode.window.showErrorMessage(err.message);
-		});
+		newNotebook(connectionId);
 	}));
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.open', () => {
 		openNotebook();
 	}));
+	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.runactivecell', () => {
+		runActiveCell();
+	}));
+	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.addcode', () => {
+		addCell('code');
+	}));
+	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.addtext', () => {
+		addCell('markdown');
+	}));
 
 }
 
+function newNotebook(connectionId: string) {
+	let title = `Untitled-${counter++}`;
+	let untitledUri = vscode.Uri.parse(`untitled:${title}`);
+	let options: sqlops.nb.NotebookShowOptions = connectionId ? {
+		viewColumn: null,
+		preserveFocus: true,
+		preview: null,
+		providerId: null,
+		connectionId: connectionId,
+		defaultKernel: null
+	} : null;
+	sqlops.nb.showNotebookDocument(untitledUri, options).then(success => {
+	}, (err: Error) => {
+		vscode.window.showErrorMessage(err.message);
+	});
+}
+
 async function openNotebook(): Promise<void> {
 	try {
 		let filter = {};
@@ -53,6 +65,38 @@ async function openNotebook(): Promise<void> {
 	}
 }
 
+async function runActiveCell(): Promise<void> {
+	try {
+		let notebook = sqlops.nb.activeNotebookEditor;
+		if (notebook) {
+			await notebook.runCell();
+		} else {
+			throw new Error(noNotebookVisible);
+		}
+	} catch (err) {
+		vscode.window.showErrorMessage(err);
+	}
+}
+
+async function addCell(cellType: sqlops.nb.CellType): Promise<void> {
+	try {
+		let notebook = sqlops.nb.activeNotebookEditor;
+		if (notebook) {
+			await notebook.edit((editBuilder: sqlops.nb.NotebookEditorEdit) => {
+				// TODO should prompt and handle cell placement
+				editBuilder.insertCell({
+					cell_type: cellType,
+					source: ''
+				});
+			});
+		} else {
+			throw new Error(noNotebookVisible);
+		}
+	} catch (err) {
+		vscode.window.showErrorMessage(err);
+	}
+}
+
 // this method is called when your extension is deactivated
 export function deactivate() {
 }

src/sql/parts/notebook/models/modelInterfaces.ts

@@ -256,6 +256,12 @@ export interface INotebookModel {
 	 * Cell List for this model
 	 */
 	readonly cells: ReadonlyArray<ICellModel>;
+
+	/**
+	 * The active cell for this model. May be undefined
+	 */
+	readonly activeCell: ICellModel;
+
 	/**
 	 * Client Session in the notebook, used for sending requests to the notebook service
 	 */

src/sql/parts/notebook/models/notebookModel.ts

@@ -286,11 +286,7 @@ export class NotebookModel extends Disposable implements INotebookModel {
 			index = undefined;
 		}
 		// Set newly created cell as active cell
-		if (this._activeCell) {
-			this._activeCell.active = false;
-		}
-		this._activeCell = cell;
-		this._activeCell.active = true;
+		this.updateActiveCell(cell);
 
 		this._contentChangedEmitter.fire({
 			changeType: NotebookChangeType.CellsAdded,
@@ -301,6 +297,14 @@ export class NotebookModel extends Disposable implements INotebookModel {
 		return cell;
 	}
 
+	private updateActiveCell(cell: ICellModel) {
+		if (this._activeCell) {
+			this._activeCell.active = false;
+		}
+		this._activeCell = cell;
+		this._activeCell.active = true;
+	}
+
 	private createCell(cellType: CellType): ICellModel {
 		let singleCell: nb.ICellContents = {
 			cell_type: cellType,
@@ -341,6 +345,9 @@ export class NotebookModel extends Disposable implements INotebookModel {
 				newCells.push(this.notebookOptions.factory.createCell(contents, { notebook: this, isTrusted: this._trustedMode }));
 			}
 			this._cells.splice(edit.range.start, edit.range.end - edit.range.start, ...newCells);
+			if (newCells.length > 0) {
+				this.updateActiveCell(newCells[0]);
+			}
 			this._contentChangedEmitter.fire({
 				changeType: NotebookChangeType.CellsAdded
 			});

src/sql/parts/notebook/notebookStyles.ts

@@ -31,7 +31,7 @@ registerThemingParticipant((theme: ITheme, collector: ICssStyleCollector) => {
 				border-width: 1px;
 			}
 		`);
-		// toolbar
+		// toolbar color set only when active
 		collector.addRule(`
 			code-component .toolbar {
 				background-color: ${inactiveBorder};

src/sql/sqlops.proposed.d.ts

@@ -1571,10 +1571,10 @@ declare module 'sqlops' {
 			 * Kicks off execution of a cell. Thenable will resolve only once the full execution is completed.
 			 *
 			 *
-			 * @param cell A cell in this notebook which should be executed
+			 * @param cell An optional cell in this notebook which should be executed. If no cell is defined, it will run the active cell instead
 			 * @return A promise that resolves with a value indicating if the cell was run or not.
 			 */
-			runCell(cell: NotebookCell): Thenable<boolean>;
+			runCell(cell?: NotebookCell): Thenable<boolean>;
 		}
 
 		export interface NotebookCell {

src/sql/workbench/api/node/extHostNotebookEditor.ts

@@ -85,7 +85,7 @@ export class NotebookEditorEdit {
 	insertCell(value:  Partial<sqlops.nb.ICellContents>, location?: number): void {
 		if (location === null || location === undefined) {
 			// If not specified, assume adding to end of list
-			location = this._document.cells.length - 1;
+			location = this._document.cells.length;
 		}
 		this._pushEdit(new CellRange(location, location), value, true);
 	}
@@ -153,7 +153,8 @@ export class ExtHostNotebookEditor implements sqlops.nb.NotebookEditor, IDisposa
 	}
 
 	public runCell(cell: sqlops.nb.NotebookCell): Thenable<boolean> {
-		return this._proxy.$runCell(this._id, cell.uri);
+		let uri = cell ? cell.uri : undefined;
+		return this._proxy.$runCell(this._id, uri);
 	}
 
 	public edit(callback: (editBuilder: sqlops.nb.NotebookEditorEdit) => void, options?: { undoStopBefore: boolean; undoStopAfter: boolean; }): Thenable<boolean> {

src/sql/workbench/api/node/mainThreadNotebookDocumentsAndEditors.ts

@@ -5,7 +5,6 @@
 'use strict';
 
 import * as sqlops from 'sqlops';
-import * as util from 'util';
 import { extHostNamedCustomer } from 'vs/workbench/api/electron-browser/extHostCustomers';
 import { Disposable, IDisposable } from 'vs/base/common/lifecycle';
 import URI, { UriComponents } from 'vs/base/common/uri';
@@ -29,7 +28,7 @@ import { getProvidersForFileName, getStandardKernelsForProvider } from 'sql/part
 import { ISingleNotebookEditOperation } from 'sql/workbench/api/common/sqlExtHostTypes';
 import { disposed } from 'vs/base/common/errors';
 import { ICellModel, NotebookContentChange, INotebookModel } from 'sql/parts/notebook/models/modelInterfaces';
-import { NotebookChangeType } from 'sql/parts/notebook/models/contracts';
+import { NotebookChangeType, CellTypes } from 'sql/parts/notebook/models/contracts';
 
 class MainThreadNotebookEditor extends Disposable {
 	private _contentChangedEmitter = new Emitter<NotebookContentChange>();
@@ -333,10 +332,20 @@ export class MainThreadNotebookDocumentsAndEditors extends Disposable implements
 		if (!editor) {
 			return TPromise.wrapError<boolean>(disposed(`TextEditor(${id})`));
 		}
-		let uriString = URI.revive(cellUri).toString();
-		let cell = editor.cells.find(c => c.cellUri.toString() === uriString);
+		let cell: ICellModel;
+		if (cellUri) {
+			let uriString = URI.revive(cellUri).toString();
+			cell = editor.cells.find(c => c.cellUri.toString() === uriString);
+			// If it's markdown what should we do? Show notification??
+		} else {
+			// Use the active cell in this case, or 1st cell if there's none active
+			cell = editor.model.activeCell;
+			if (!cell) {
+				cell = editor.cells.find(c => c.cellType === CellTypes.Code);
+			}
+		}
 		if (!cell) {
-			return TPromise.wrapError<boolean>(disposed(`TextEditorCell(${uriString})`));
+			return TPromise.wrapError<boolean>(disposed(`Could not find cell for this Notebook`));
 		}
 
 		return TPromise.wrap(editor.runCell(cell));

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

@@ -0,0 +1,15 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the Source EULA. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+'use strict';
+
+import { RawContextKey } from 'vs/platform/contextkey/common/contextkey';
+
+/**
+ * Context Keys to use with keybindings for the notebook editor
+ */
+export const notebookEditorVisibleId = 'notebookEditorVisible';
+
+export const NotebookEditorVisibleContext = new RawContextKey<boolean>(notebookEditorVisibleId, false);

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

@@ -30,7 +30,11 @@ import { Deferred } from 'sql/base/common/promise';
 import { SqlSessionManager } from 'sql/workbench/services/notebook/common/sqlSessionManager';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { sqlNotebooksEnabled } from 'sql/parts/notebook/notebookUtils';
-import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
+import { IContextKeyService, IContextKey } from 'vs/platform/contextkey/common/contextkey';
+import { NotebookEditorVisibleContext } from 'sql/workbench/services/notebook/common/notebookContext';
+import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
+import { NotebookEditor } from 'sql/parts/notebook/notebookEditor';
+import { IEditorGroupsService } from 'vs/workbench/services/group/common/editorGroupsService';
 
 export interface NotebookProviderProperties {
 	provider: string;
@@ -84,13 +88,16 @@ export class NotebookService extends Disposable implements INotebookService {
 	private _providerToStandardKernels = new Map<string, nb.IStandardKernel[]>();
 	private _registrationComplete = new Deferred<void>();
 	private _isRegistrationComplete = false;
+	private notebookEditorVisible: IContextKey<boolean>;
 
 	constructor(
 		@IStorageService private _storageService: IStorageService,
 		@IExtensionService extensionService: IExtensionService,
 		@IExtensionManagementService extensionManagementService: IExtensionManagementService,
 		@IInstantiationService private _instantiationService: IInstantiationService,
-		@IContextKeyService private _contextKeyService: IContextKeyService
+		@IContextKeyService private _contextKeyService: IContextKeyService,
+		@IEditorService private readonly _editorService: IEditorService,
+		@IEditorGroupsService private readonly _editorGroupsService: IEditorGroupsService
 	) {
 		super();
 		this._register(notebookRegistry.onNewRegistration(this.updateRegisteredProviders, this));
@@ -106,6 +113,23 @@ export class NotebookService extends Disposable implements INotebookService {
 		if (extensionManagementService) {
 			this._register(extensionManagementService.onDidUninstallExtension(({ identifier }) => this.removeContributedProvidersFromCache(identifier, extensionService)));
 		}
+		this.hookContextKeyListeners();
+	}
+
+	private hookContextKeyListeners() {
+		const updateEditorContextKeys = () => {
+			const visibleEditors = this._editorService.visibleControls;
+			this.notebookEditorVisible.set(visibleEditors.some(control => control.getId() === NotebookEditor.ID));
+		};
+		if (this._contextKeyService) {
+			this.notebookEditorVisible = NotebookEditorVisibleContext.bindTo(this._contextKeyService);
+		}
+		if (this._editorService) {
+			this._register(this._editorService.onDidActiveEditorChange(() => updateEditorContextKeys()));
+			this._register(this._editorService.onDidVisibleEditorsChange(() => updateEditorContextKeys()));
+			this._register(this._editorGroupsService.onDidAddGroup(() => updateEditorContextKeys()));
+			this._register(this._editorGroupsService.onDidRemoveGroup(() => updateEditorContextKeys()));
+		}
 	}
 
 	private updateRegisteredProviders(p: { id: string; registration: NotebookProviderRegistration; }) {

src/sqltest/parts/notebook/common.ts

@@ -27,6 +27,9 @@ export class NotebookModelStub implements INotebookModel {
 	get cells(): ReadonlyArray<ICellModel> {
 		throw new Error('method not implemented.');
 	}
+	get activeCell(): ICellModel {
+		throw new Error('method not implemented.');
+	}
 	get clientSession(): IClientSession {
 		throw new Error('method not implemented.');
 	}