From 74172d0df08a805031d2a4cb834b45a62f38cf0e Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 1 Oct 2019 09:30:24 -0700 Subject: [PATCH 01/36] Expand variable explorer tests to work for native editor too. (#7699) * Expand variable explorer tests to work for native editor too. * Add comment about async fix for editor open * Review feedback --- news/3 Code Health/7369.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/3 Code Health/7369.md diff --git a/news/3 Code Health/7369.md b/news/3 Code Health/7369.md new file mode 100644 index 000000000000..58e65e615c03 --- /dev/null +++ b/news/3 Code Health/7369.md @@ -0,0 +1 @@ +Add functional tests for notebook editor's use of the variable list. From e8347e7ea9e33ef755e888dbc68041d7bbb69bc2 Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Tue, 1 Oct 2019 15:19:28 -0700 Subject: [PATCH 02/36] Allow connection to servers with no token and no password (#7708) --- news/2 Fixes/7137.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/7137.md diff --git a/news/2 Fixes/7137.md b/news/2 Fixes/7137.md new file mode 100644 index 000000000000..952440c7f342 --- /dev/null +++ b/news/2 Fixes/7137.md @@ -0,0 +1 @@ +Fix regression to allow connection to servers with no token and no password and add functional test for this scenario \ No newline at end of file From 456167b5e76e5e25592dbdb3862fa5718c380365 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 2 Oct 2019 12:32:08 -0700 Subject: [PATCH 03/36] Add capability to support current file path for notebook root (#7724) * Add capability to support current file path for notebook root * Put launch.json back * Use system variables instead of creating separate resolver * Fix mistake in config settings with passing in a uri instead of a string * Make arguments more explicit * Fix IS_WINDOWS to work on unit tests --- news/1 Enhancements/4441.md | 1 + news/2 Fixes/7688.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 news/1 Enhancements/4441.md create mode 100644 news/2 Fixes/7688.md diff --git a/news/1 Enhancements/4441.md b/news/1 Enhancements/4441.md new file mode 100644 index 000000000000..b3671783354c --- /dev/null +++ b/news/1 Enhancements/4441.md @@ -0,0 +1 @@ +Support other variables for notebookFileRoot besides ${workspaceRoot}. Specifically allow things like ${fileDirName} so that the dir of the first file run in the interactive window is used for the current directory. diff --git a/news/2 Fixes/7688.md b/news/2 Fixes/7688.md new file mode 100644 index 000000000000..9c7cea856132 --- /dev/null +++ b/news/2 Fixes/7688.md @@ -0,0 +1 @@ +When there's no workspace open, use the directory of the opened file as the root directory for a jupyter session. From b4b4d2d87bd1a77fa8fb886d7948722b280ed3a1 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 2 Oct 2019 15:09:54 -0700 Subject: [PATCH 04/36] Better cancellations (#7714) --- src/client/common/cancellation.ts | 43 +++++++++++++++---- .../jupyter/jupyterCommandFinder.ts | 39 +++++++++++++++-- 2 files changed, 69 insertions(+), 13 deletions(-) diff --git a/src/client/common/cancellation.ts b/src/client/common/cancellation.ts index b325c2ff106a..fc30619df3fc 100644 --- a/src/client/common/cancellation.ts +++ b/src/client/common/cancellation.ts @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; -import { CancellationToken } from 'vscode-jsonrpc'; +import { CancellationToken } from 'vscode'; import { createDeferred } from './utils/async'; import * as localize from './utils/localize'; @@ -10,20 +10,46 @@ import * as localize from './utils/localize'; * Error type thrown when canceling. */ export class CancellationError extends Error { - constructor() { super(localize.Common.canceled()); } } +/** + * Create a promise that will either resolve with a default value or reject when the token is cancelled. + * + * @export + * @template T + * @param {({ defaultValue: T; token: CancellationToken; cancelAction: 'reject' | 'resolve' })} options + * @returns {Promise} + */ +export function createPromiseFromCancellation(options: { defaultValue: T; token?: CancellationToken; cancelAction: 'reject' | 'resolve' }): Promise { + return new Promise((resolve, reject) => { + // Never resolve. + if (!options.token) { + return; + } + const complete = () => { + if (options.token!.isCancellationRequested) { + if (options.cancelAction === 'resolve') { + return resolve(options.defaultValue); + } + if (options.cancelAction === 'reject') { + return reject(new CancellationError()); + } + } + }; -export namespace Cancellation { + options.token.onCancellationRequested(complete); + }); +} +export namespace Cancellation { /** * Races a promise and cancellation. Promise can take a cancellation token too in order to listen to cancellation. * @param work function returning a promise to race * @param token token used for cancellation */ - export function race(work : (token?: CancellationToken) => Promise, token?: CancellationToken) : Promise { + export function race(work: (token?: CancellationToken) => Promise, token?: CancellationToken): Promise { if (token) { // Use a deferred promise. Resolves when the work finishes const deferred = createDeferred(); @@ -43,12 +69,12 @@ export namespace Cancellation { // Not canceled yet. When the work finishes // either resolve our promise or cancel. work(token) - .then((v) => { + .then(v => { if (!deferred.completed) { deferred.resolve(v); } }) - .catch((e) => { + .catch(e => { if (!deferred.completed) { deferred.reject(e); } @@ -66,7 +92,7 @@ export namespace Cancellation { * isCanceled returns a boolean indicating if the cancel token has been canceled. * @param cancelToken */ - export function isCanceled(cancelToken?: CancellationToken) : boolean { + export function isCanceled(cancelToken?: CancellationToken): boolean { return cancelToken ? cancelToken.isCancellationRequested : false; } @@ -74,10 +100,9 @@ export namespace Cancellation { * throws a CancellationError if the token is canceled. * @param cancelToken */ - export function throwIfCanceled(cancelToken?: CancellationToken) : void { + export function throwIfCanceled(cancelToken?: CancellationToken): void { if (isCanceled(cancelToken)) { throw new CancellationError(); } } - } diff --git a/src/client/datascience/jupyter/jupyterCommandFinder.ts b/src/client/datascience/jupyter/jupyterCommandFinder.ts index 7ae172b21d4c..28175ef4616f 100644 --- a/src/client/datascience/jupyter/jupyterCommandFinder.ts +++ b/src/client/datascience/jupyter/jupyterCommandFinder.ts @@ -6,7 +6,7 @@ import * as path from 'path'; import { CancellationToken } from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; -import { Cancellation } from '../../common/cancellation'; +import { Cancellation, createPromiseFromCancellation } from '../../common/cancellation'; import { traceInfo, traceWarning } from '../../common/logger'; import { IFileSystem } from '../../common/platform/types'; import { IProcessService, IProcessServiceFactory, IPythonExecutionFactory, SpawnOptions } from '../../common/process/types'; @@ -33,6 +33,18 @@ export interface IFindCommandResult extends IModuleExistsResult { command?: IJupyterCommand; } +const cancelledResult: IFindCommandResult = { + status: ModuleExistsStatus.NotFound, + error: localize.DataScience.noInterpreter() +}; + +function isCommandFinderCancelled(command: JupyterCommands, token?: CancellationToken) { + if (Cancellation.isCanceled(token)) { + traceInfo(`Command finder cancelled for ${command}.`); + return true; + } + return false; +} export class JupyterCommandFinder { private readonly processServicePromise: Promise; private jupyterPath?: string; @@ -152,7 +164,7 @@ export class JupyterCommandFinder { // - Look for module in current interpreter, if found create something with python path and -m module // - Look in other interpreters, if found create something with python path and -m module // - Look on path for jupyter, if found create something with jupyter path and args - // tslint:disable:cyclomatic-complexity + // tslint:disable:cyclomatic-complexity max-func-body-length private async findBestCommandImpl(command: JupyterCommands, cancelToken?: CancellationToken): Promise { let found: IFindCommandResult = { status: ModuleExistsStatus.NotFound @@ -165,6 +177,11 @@ export class JupyterCommandFinder { // First we look in the current interpreter const current = await this.interpreterService.getActiveInterpreter(); + + if (isCommandFinderCancelled(command, cancelToken)) { + return cancelledResult; + } + found = current ? await this.findInterpreterCommand(command, current, cancelToken) : found; if (found.status === ModuleExistsStatus.NotFound) { traceInfo(`Active interpreter does not support ${command} because of error ${found.error}. Interpreter is ${current ? current.displayName : 'undefined'}.`); @@ -174,14 +191,24 @@ export class JupyterCommandFinder { } if (found.status === ModuleExistsStatus.NotFound && this.supportsSearchingForCommands()) { // Look through all of our interpreters (minus the active one at the same time) - const all = await this.interpreterService.getInterpreters(); + const cancelGetInterpreters = createPromiseFromCancellation({ defaultValue: [], cancelAction: 'resolve', token: cancelToken }); + const all = await Promise.race([this.interpreterService.getInterpreters(), cancelGetInterpreters]); + + if (isCommandFinderCancelled(command, cancelToken)) { + return cancelledResult; + } if (!all || all.length === 0) { traceWarning('No interpreters found. Jupyter cannot run.'); } + const cancelFind = createPromiseFromCancellation({ defaultValue: [], cancelAction: 'resolve', token: cancelToken }); const promises = all.filter(i => i !== current).map(i => this.findInterpreterCommand(command, i, cancelToken)); - const foundList = await Promise.all(promises); + const foundList = await Promise.race([Promise.all(promises), cancelFind]); + + if (isCommandFinderCancelled(command, cancelToken)) { + return cancelledResult; + } // Then go through all of the found ones and pick the closest python match if (current && current.version) { @@ -192,6 +219,10 @@ export class JupyterCommandFinder { continue; } const interpreter = await entry.command.interpreter(); + if (isCommandFinderCancelled(command, cancelToken)) { + return cancelledResult; + } + const version = interpreter ? interpreter.version : undefined; if (version) { if (version.major === current.version.major) { From 50f8461ab831e7d5dea4a40dac9720716ff33612 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Thu, 3 Oct 2019 08:16:54 -0700 Subject: [PATCH 05/36] Variable explorer and mime test fixes for tests (#7740) * Change variable explorer tests to wait for variable explorer complete * Fix native mime test * Review feedback * Try to make 2.7 tests pass --- .../datascience/getJupyterVariableValue.py | 6 +-- .../interactiveWindowTypes.ts | 2 + .../interactive-common/mainStateController.ts | 7 +++ .../nativeEditor.functional.test.tsx | 2 +- .../variableexplorer.functional.test.tsx | 50 +++++++++---------- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/pythonFiles/datascience/getJupyterVariableValue.py b/pythonFiles/datascience/getJupyterVariableValue.py index d483eb5cba89..a302cf6df673 100644 --- a/pythonFiles/datascience/getJupyterVariableValue.py +++ b/pythonFiles/datascience/getJupyterVariableValue.py @@ -84,9 +84,9 @@ def __call__(self, obj): return ''.join((x.encode('utf-8') if isinstance(x, unicode) else x) for x in self._repr(obj, 0)) else: return ''.join(self._repr(obj, 0)) - except Exception: + except Exception as e: try: - return 'An exception was raised: %r' % sys.exc_info()[1] + return 'An exception was raised: ' + str(e) except Exception: return 'An exception was raised' @@ -373,7 +373,7 @@ def _bytes_as_unicode_if_possible(self, obj_repr): # locale.getpreferredencoding() and 'utf-8). If no encoding can decode # the input, we return the original bytes. try_encodings = [] - encoding = self.sys_stdout_encoding or getattr(sys.stdout, 'encoding', '') + encoding = self.sys_stdout_encoding or getattr(VC_sys.stdout, 'encoding', '') if encoding: try_encodings.append(encoding.lower()) diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts index 59b3be5b9fae..446563bcb4de 100644 --- a/src/client/datascience/interactive-common/interactiveWindowTypes.ts +++ b/src/client/datascience/interactive-common/interactiveWindowTypes.ts @@ -69,6 +69,7 @@ export namespace InteractiveWindowMessages { export const NotebookClean = 'clean'; export const SaveAll = 'save_all'; export const NativeCommand = 'native_command'; + export const VariablesComplete = 'variables_complete'; } @@ -301,4 +302,5 @@ export class IInteractiveWindowMapping { public [InteractiveWindowMessages.NotebookClean]: never | undefined; public [InteractiveWindowMessages.SaveAll]: ISaveAll; public [InteractiveWindowMessages.NativeCommand]: INativeCommand; + public [InteractiveWindowMessages.VariablesComplete]: never | undefined; } diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index 361ca744b3ae..a0b7cab178f7 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -640,6 +640,8 @@ export class MainStateController implements IMessageHandler { } public renderUpdate(newState: {}) { + const oldCount = this.state.pendingVariableCount; + // This method should be called during the render stage of anything // using this state Controller. That's because after shouldComponentUpdate // render is next and at this point the state has been set. @@ -652,6 +654,11 @@ export class MainStateController implements IMessageHandler { if ('cellVMs' in newState) { this.sendInfo(); } + + // If the new state includes pendingVariableCount and it's gone to zero, send a message + if (this.state.pendingVariableCount === 0 && oldCount !== 0) { + setTimeout(() => this.sendMessage(InteractiveWindowMessages.VariablesComplete), 1); + } } public getState(): IMainState { diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index a57af32ccc32..b2eb2bb71053 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -127,7 +127,7 @@ for _ in range(50): await addCell(wrapper, matPlotLib, true, 5); verifyHtmlOnCell(wrapper, 'NativeCell', matPlotLibResults, CellPosition.Last); - await addCell(wrapper, spinningCursor, true, 3 + (ioc.mockJupyter ? (cursors.length * 3) : 0)); + await addCell(wrapper, spinningCursor, true, 3 + (ioc.mockJupyter ? (cursors.length * 3) : 50)); verifyHtmlOnCell(wrapper, 'NativeCell', '
', CellPosition.Last); }, () => { return ioc; }); diff --git a/src/test/datascience/variableexplorer.functional.test.tsx b/src/test/datascience/variableexplorer.functional.test.tsx index 55c230c0f265..421a6a7effc8 100644 --- a/src/test/datascience/variableexplorer.functional.test.tsx +++ b/src/test/datascience/variableexplorer.functional.test.tsx @@ -8,6 +8,7 @@ import { parse } from 'node-html-parser'; import * as React from 'react'; import { Disposable } from 'vscode'; +import { InteractiveWindowMessages } from '../../client/datascience/interactive-common/interactiveWindowTypes'; import { IJupyterVariable } from '../../client/datascience/types'; import { InteractivePanel } from '../../datascience-ui/history-react/interactivePanel'; import { VariableExplorer } from '../../datascience-ui/interactive-common/variableExplorer'; @@ -16,7 +17,7 @@ import { DataScienceIocContainer } from './dataScienceIocContainer'; import { addCode } from './interactiveWindowTestHelpers'; import { addCell, createNewEditor } from './nativeEditorTestHelpers'; import { waitForUpdate } from './reactHelpers'; -import { runDoubleTest } from './testHelpers'; +import { runDoubleTest, waitForMessage } from './testHelpers'; // tslint:disable:max-func-body-length trailing-comma no-any no-multiline-string suite('DataScience Interactive Window variable explorer tests', () => { @@ -60,10 +61,17 @@ suite('DataScience Interactive Window variable explorer tests', () => { // asyncDump(); //}); - async function addCodeImpartial(wrapper: ReactWrapper, React.Component>, code: string, expectedRenderCount: number = 4, expectError: boolean = false): Promise, React.Component>> { + async function waitForVariablesUpdated(): Promise { + return waitForMessage(ioc, InteractiveWindowMessages.VariablesComplete); + } + + async function addCodeImpartial(wrapper: ReactWrapper, React.Component>, code: string, waitForVariables: boolean = true, expectedRenderCount: number = 4, expectError: boolean = false): Promise, React.Component>> { + const variablesUpdated = waitForVariables ? waitForVariablesUpdated() : Promise.resolve(); const nodes = wrapper.find('InteractivePanel'); if (nodes.length > 0) { - return addCode(ioc, wrapper, code, expectedRenderCount, expectError); + const result = await addCode(ioc, wrapper, code, expectedRenderCount, expectError); + await variablesUpdated; + return result; } else { // For the native editor case, we need to create an editor before hand. if (!createdNotebook) { @@ -72,6 +80,7 @@ suite('DataScience Interactive Window variable explorer tests', () => { expectedRenderCount += 1; } await addCell(wrapper, code, true, expectedRenderCount); + await variablesUpdated; return wrapper; } } @@ -85,8 +94,7 @@ value = 'hello world'`; openVariableExplorer(wrapper); await addCodeImpartial(wrapper, 'a=1\na'); - await addCodeImpartial(wrapper, basicCode, 4); - await waitForUpdate(wrapper, VariableExplorer, 3); + await addCodeImpartial(wrapper, basicCode, true, 4); // We should show a string and show an int, the modules should be hidden let targetVariables: IJupyterVariable[] = [ @@ -100,8 +108,7 @@ value = 'hello world'`; ioc.getSettings().datascience.variableExplorerExclude = `${ioc.getSettings().datascience.variableExplorerExclude};str`; // Add another string and check our vars, strings should be hidden - await addCodeImpartial(wrapper, basicCode2, 4); - await waitForUpdate(wrapper, VariableExplorer, 2); + await addCodeImpartial(wrapper, basicCode2, true, 4); targetVariables = [ {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false} @@ -116,7 +123,6 @@ value = 'hello world'`; openVariableExplorer(wrapper); await addCodeImpartial(wrapper, 'a=1\na'); - await waitForUpdate(wrapper, VariableExplorer, 2); // Check that we have just the 'a' variable let targetVariables: IJupyterVariable[] = [ @@ -125,8 +131,7 @@ value = 'hello world'`; verifyVariables(wrapper, targetVariables); // Add another variable and check it - await addCodeImpartial(wrapper, basicCode, 4); - await waitForUpdate(wrapper, VariableExplorer, 3); + await addCodeImpartial(wrapper, basicCode, true, 4); targetVariables = [ {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false}, @@ -136,8 +141,7 @@ value = 'hello world'`; verifyVariables(wrapper, targetVariables); // Add a second variable and check it - await addCodeImpartial(wrapper, basicCode2, 4); - await waitForUpdate(wrapper, VariableExplorer, 4); + await addCodeImpartial(wrapper, basicCode2, true, 4); targetVariables = [ {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false}, @@ -155,7 +159,7 @@ value = 'hello world'`; openVariableExplorer(wrapper); await addCodeImpartial(wrapper, 'a=1\na'); - await addCodeImpartial(wrapper, basicCode, 4); + await addCodeImpartial(wrapper, basicCode, false, 4); // Here we are only going to wait for two renders instead of the needed three // a should have the value updated, but value should still be loading @@ -187,11 +191,7 @@ myDict = {'a': 1}`; openVariableExplorer(wrapper); await addCodeImpartial(wrapper, 'a=1\na'); - await addCodeImpartial(wrapper, basicCode, 4); - - // Verify that we actually update the variable explorer - // Count here is our main render + a render for each variable row as they come in - await waitForUpdate(wrapper, VariableExplorer, 5); + await addCodeImpartial(wrapper, basicCode, true, 4); const targetVariables: IJupyterVariable[] = [ {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false}, @@ -219,11 +219,7 @@ myTuple = 1,2,3,4,5,6,7,8,9 openVariableExplorer(wrapper); await addCodeImpartial(wrapper, 'a=1\na'); - await addCodeImpartial(wrapper, basicCode, 4); - - // Verify that we actually update the variable explorer - // Count here is our main render + a render for each variable row as they come in - await waitForUpdate(wrapper, VariableExplorer, 9); + await addCodeImpartial(wrapper, basicCode, true, 4); const targetVariables: IJupyterVariable[] = [ {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false}, @@ -255,9 +251,7 @@ strc = 'c'`; openVariableExplorer(wrapper); await addCodeImpartial(wrapper, 'a=1\na'); - await addCodeImpartial(wrapper, basicCode, 4); - - await waitForUpdate(wrapper, VariableExplorer, 7); + await addCodeImpartial(wrapper, basicCode, true, 4); let targetVariables: IJupyterVariable[] = [ {name: 'a', value: '1', supportsDataExplorer: false, type: 'int', size: 54, shape: '', count: 0, truncated: false}, @@ -318,6 +312,10 @@ function sortVariableExplorer(wrapper: ReactWrapper, React.Com // Verify a set of rows versus a set of expected variables function verifyVariables(wrapper: ReactWrapper, React.Component>, targetVariables: IJupyterVariable[]) { + // Force an update so we render whatever the current state is + wrapper.update(); + + // Then search for results. const foundRows = wrapper.find('div.react-grid-Row'); expect(foundRows.length).to.be.equal(targetVariables.length, 'Different number of variable explorer rows and target variables'); From fd547753e0aaa3d6094e23a25a502c1d7c1d4e44 Mon Sep 17 00:00:00 2001 From: Ian Huff Date: Thu, 3 Oct 2019 18:38:05 -0700 Subject: [PATCH 06/36] Fix liveshare guest hang issue (#7764) * initial changes * update notebooks on server session change and add test * await correctly --- news/2 Fixes/7638.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/2 Fixes/7638.md diff --git a/news/2 Fixes/7638.md b/news/2 Fixes/7638.md new file mode 100644 index 000000000000..dea499b65438 --- /dev/null +++ b/news/2 Fixes/7638.md @@ -0,0 +1 @@ +Fix a hang in the Interactive window when connecting guest to host after the host has already started the interactive window. \ No newline at end of file From 899add9d1c035e52a42a562703ca233462992f2c Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 4 Oct 2019 14:54:54 -0700 Subject: [PATCH 07/36] Fix jupyter server startup hang when xeus-cling is installed (#7773) --- news/2 Fixes/7569.md | 1 + src/client/common/process/pythonProcess.ts | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 news/2 Fixes/7569.md diff --git a/news/2 Fixes/7569.md b/news/2 Fixes/7569.md new file mode 100644 index 000000000000..20e5e8a45853 --- /dev/null +++ b/news/2 Fixes/7569.md @@ -0,0 +1 @@ +Fix jupyter server startup hang when xeus-cling kernel is installed. diff --git a/src/client/common/process/pythonProcess.ts b/src/client/common/process/pythonProcess.ts index 6030df4a6e11..9d7d15333768 100644 --- a/src/client/common/process/pythonProcess.ts +++ b/src/client/common/process/pythonProcess.ts @@ -1,17 +1,26 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. - import { injectable } from 'inversify'; import * as path from 'path'; + import { IServiceContainer } from '../../ioc/types'; import { EXTENSION_ROOT_DIR } from '../constants'; import { ErrorUtils } from '../errors/errorUtils'; import { ModuleNotInstalledError } from '../errors/moduleNotInstalledError'; import { traceError } from '../logger'; import { IFileSystem } from '../platform/types'; +import { waitForPromise } from '../utils/async'; import { Architecture } from '../utils/platform'; import { parsePythonVersion } from '../utils/version'; -import { ExecutionResult, InterpreterInfomation, IProcessService, IPythonExecutionService, ObservableExecutionResult, PythonVersionInfo, SpawnOptions } from './types'; +import { + ExecutionResult, + InterpreterInfomation, + IProcessService, + IPythonExecutionService, + ObservableExecutionResult, + PythonVersionInfo, + SpawnOptions +} from './types'; @injectable() export class PythonExecutionService implements IPythonExecutionService { @@ -28,8 +37,12 @@ export class PythonExecutionService implements IPythonExecutionService { public async getInterpreterInformation(): Promise { const file = path.join(EXTENSION_ROOT_DIR, 'pythonFiles', 'interpreterInfo.py'); try { - const jsonValue = await this.procService.exec(this.pythonPath, [file], { mergeStdOutErr: true }) - .then(output => output.stdout.trim()); + // Sometimes the python path isn't valid, timeout if that's the case. + // See these two bugs: + // https://github.com/microsoft/vscode-python/issues/7569 + // https://github.com/microsoft/vscode-python/issues/7760 + const jsonValue = await waitForPromise(this.procService.exec(this.pythonPath, [file], { mergeStdOutErr: true }), 5000) + .then(output => output ? output.stdout.trim() : '--timed out--'); // --timed out-- should cause an exception let json: { versionInfo: PythonVersionInfo; sysPrefix: string; sysVersion: string; is64Bit: boolean }; try { From 14cd95f7cd9234b97a8f3138899bcb920cab0cc9 Mon Sep 17 00:00:00 2001 From: David Kutugata Date: Fri, 4 Oct 2019 18:11:44 -0700 Subject: [PATCH 08/36] Make interactive window and native editor take their fontSize and fontFamily from the settings in VS Code. (#7746) * Make interactive window and native editor take their fontSize and fontFamily from the settings in VS Code. We get them in nativeEditor.tsx and in interactivePanel.tsx to apply them as styles, but we also need to send them as props to eventually assign them in the monaco editor. * Moved style from react code to common.css We now get the VS Code styles from the state of the component, instead of computing a style from an element. * Removed getting the font from nativeEditor and interactivePanel, and put it in the mainStateController * Made the interactiveWindow and the nativeEditor react and change their fonts when the user changes them in VS Code, instead of having to reload. * Changed the font props to be an object instead of two separate props to make it easier to maintain. React Context was not used to avoid making the reuse of components more difficult. * Removed some unnecesary passing around of the font prop * removed unnecesary comments * fixed a failing test --- news/2 Fixes/7624.md | 1 + package-lock.json | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 news/2 Fixes/7624.md diff --git a/news/2 Fixes/7624.md b/news/2 Fixes/7624.md new file mode 100644 index 000000000000..1875f4ca313b --- /dev/null +++ b/news/2 Fixes/7624.md @@ -0,0 +1 @@ +Make interactive window and native take their fontSize and fontFamily from the settings in VS Code. diff --git a/package-lock.json b/package-lock.json index 6a0b36e78cdb..d42ce97da8ad 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13770,7 +13770,7 @@ "dependencies": { "buffer": { "version": "4.9.1", - "resolved": "https://registry.npmjs.org/buffer/-/buffer-4.9.1.tgz", + "resolved": "http://registry.npmjs.org/buffer/-/buffer-4.9.1.tgz", "integrity": "sha1-bRu2AbB6TvztlwlBMgkwJ8lbwpg=", "dev": true, "requires": { @@ -13793,7 +13793,7 @@ }, "readable-stream": { "version": "2.3.6", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-2.3.6.tgz", "integrity": "sha512-tQtKA9WIAhBF3+VLAseyMqZeBjW0AHJoxOtYqSUZNJxauErmLbVm2FW1y+J/YA9dUrAC39ITejlZWhVIwawkKw==", "dev": true, "requires": { @@ -18754,7 +18754,7 @@ "dependencies": { "json5": { "version": "1.0.1", - "resolved": "https://registry.npmjs.org/json5/-/json5-1.0.1.tgz", + "resolved": "http://registry.npmjs.org/json5/-/json5-1.0.1.tgz", "integrity": "sha512-aKS4WQjPenRxiQsC93MNfjx+nbF4PAdYzmd/1JIj8HYzqfbu86beTuNgXDzPknWk0n0uARlyewZo4s++ES36Ow==", "dev": true, "requires": { From 17ee45a5a4880c4e5c574fafce607914ed00ba91 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Mon, 7 Oct 2019 11:45:29 -0700 Subject: [PATCH 09/36] Fix a number of perf issues with large notebooks (#7778) * First idea - load cells all at once. Change updates to not update full cell Change global properties to be per cell Change cells to check for updates before rendering * Fix delete to not select at the same time * Make a pending state and a rendered state to use for rendering Some perf fixes for monaco editor. Skip laying out parent etc until necessary * Refactor intellisense to not take so much time on load * More delete and move fixes for intellisense * Fix markdown not switching Fix markdown editor background * Fix some of the time on first render * Add some more unit tests and fix intellisense issue * Add news entry and fix variable explorer open * Remove unnecessary state change * Remove unnecessary merge * Comment change * Postpone widget parent changes * Add new dependency * Update some comments and names to be clearer --- news/2 Fixes/7483.md | 1 + package-lock.json | 4 +- package.datascience-ui.dependencies.json | 1 + .../intellisense/baseIntellisenseProvider.ts | 89 +++-- .../intellisense/intellisenseDocument.ts | 372 ++++++++++-------- .../interactiveWindowTypes.ts | 15 + .../history-react/interactiveCell.tsx | 17 +- .../history-react/interactivePanel.tsx | 2 - .../interactive-common/mainState.ts | 22 +- .../interactive-common/mainStateController.ts | 355 ++++++++++------- .../interactive-common/markdown.tsx | 4 + .../native-editor/nativeCell.tsx | 81 ++-- .../native-editor/nativeEditor.less | 10 + .../native-editor/nativeEditor.tsx | 19 +- .../nativeEditorStateController.ts | 13 +- .../react-common/monacoEditor.tsx | 36 +- .../datascience/intellisense.unit.test.ts | 133 ++++++- .../nativeEditor.functional.test.tsx | 8 +- 18 files changed, 741 insertions(+), 441 deletions(-) create mode 100644 news/2 Fixes/7483.md diff --git a/news/2 Fixes/7483.md b/news/2 Fixes/7483.md new file mode 100644 index 000000000000..c8b22aada88f --- /dev/null +++ b/news/2 Fixes/7483.md @@ -0,0 +1 @@ +Perf improvements for opening notebooks with more than 100 cells. diff --git a/package-lock.json b/package-lock.json index d42ce97da8ad..2c4fd706f33d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14145,7 +14145,7 @@ }, "os-locale": { "version": "3.1.0", - "resolved": "https://registry.npmjs.org/os-locale/-/os-locale-3.1.0.tgz", + "resolved": false, "integrity": "sha512-Z8l3R4wYWM40/52Z+S265okfFj8Kt2cC2MKY+xNi3kFs+XGI7WXu/I309QQQYbRW4ijiZ+yxs9pqEhJh0DqW3Q==", "dev": true, "requires": { @@ -14162,7 +14162,7 @@ }, "resolve-from": { "version": "4.0.0", - "resolved": "https://registry.npmjs.org/resolve-from/-/resolve-from-4.0.0.tgz", + "resolved": false, "integrity": "sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==", "dev": true }, diff --git a/package.datascience-ui.dependencies.json b/package.datascience-ui.dependencies.json index 41f0d9ff43e2..1916cd4904a5 100644 --- a/package.datascience-ui.dependencies.json +++ b/package.datascience-ui.dependencies.json @@ -85,6 +85,7 @@ "escape-carriage", "extend", "fast-plist", + "immutable", "inherits", "is-alphabetical", "is-alphanumerical", diff --git a/src/client/datascience/interactive-common/intellisense/baseIntellisenseProvider.ts b/src/client/datascience/interactive-common/intellisense/baseIntellisenseProvider.ts index cd79d8b090bd..eed2dc888ff4 100644 --- a/src/client/datascience/interactive-common/intellisense/baseIntellisenseProvider.ts +++ b/src/client/datascience/interactive-common/intellisense/baseIntellisenseProvider.ts @@ -24,17 +24,12 @@ import { IFileSystem, TemporaryFile } from '../../../common/platform/types'; import { createDeferred, Deferred, waitForPromise } from '../../../common/utils/async'; import { concatMultilineString } from '../../common'; import { Identifiers, Settings } from '../../constants'; -import { - IInteractiveWindowInfo, - IInteractiveWindowListener, - IInteractiveWindowProvider, - IJupyterExecution, - INotebook -} from '../../types'; +import { IInteractiveWindowListener, IInteractiveWindowProvider, IJupyterExecution, INotebook } from '../../types'; import { IAddCell, ICancelIntellisenseRequest, IEditCell, + IInsertCell, IInteractiveWindowMapping, ILoadAllCells, INotebookIdentity, @@ -42,7 +37,8 @@ import { IProvideCompletionItemsRequest, IProvideHoverRequest, IProvideSignatureHelpRequest, - IRemoveCell + IRemoveCell, + ISwapCells } from '../interactiveWindowTypes'; import { convertStringsToSuggestions } from './conversion'; import { IntellisenseDocument } from './intellisenseDocument'; @@ -110,10 +106,18 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList this.dispatchMessage(message, payload, this.addCell); break; + case InteractiveWindowMessages.InsertCell: + this.dispatchMessage(message, payload, this.insertCell); + break; + case InteractiveWindowMessages.RemoveCell: this.dispatchMessage(message, payload, this.removeCell); break; + case InteractiveWindowMessages.SwapCells: + this.dispatchMessage(message, payload, this.swapCells); + break; + case InteractiveWindowMessages.DeleteAllCells: this.dispatchMessage(message, payload, this.removeAllCells); break; @@ -130,10 +134,6 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList this.dispatchMessage(message, payload, this.loadAllCells); break; - case InteractiveWindowMessages.SendInfo: - this.dispatchMessage(message, payload, this.handleNativeEditorChanges); - break; - default: break; } @@ -337,6 +337,15 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList } } + private async insertCell(request: IInsertCell): Promise { + // Get the document and then pass onto the sub class + const document = await this.getDocument(); + if (document) { + const changes = document.insertCell(request.id, request.code, request.codeCellAbove); + return this.handleChanges(undefined, document, changes); + } + } + private async editCell(request: IEditCell): Promise { // First get the document const document = await this.getDocument(); @@ -346,47 +355,45 @@ export abstract class BaseIntellisenseProvider implements IInteractiveWindowList } } - private removeCell(_request: IRemoveCell): Promise { - // Skip this request. The logic here being that - // a user can remove a cell from the UI, but it's still loaded into the Jupyter kernel. - return Promise.resolve(); + private async removeCell(request: IRemoveCell): Promise { + // First get the document + const document = await this.getDocument(); + if (document) { + const changes = document.remove(request.id); + return this.handleChanges(undefined, document, changes); + } } - private removeAllCells(): Promise { - // Skip this request. The logic here being that - // a user can remove a cell from the UI, but it's still loaded into the Jupyter kernel. - return Promise.resolve(); + private async swapCells(request: ISwapCells): Promise { + // First get the document + const document = await this.getDocument(); + if (document) { + const changes = document.swap(request.firstCellId, request.secondCellId); + return this.handleChanges(undefined, document, changes); + } } - private async loadAllCells(payload: ILoadAllCells) { + private async removeAllCells(): Promise { + // First get the document const document = await this.getDocument(); if (document) { - document.switchToEditMode(); - await Promise.all(payload.cells.map(async cell => { - if (cell.data.cell_type === 'code') { - const text = concatMultilineString(cell.data.source); - const addCell: IAddCell = { - fullText: text, - currentText: text, - file: cell.file, - id: cell.id - }; - await this.addCell(addCell); - } - })); + const changes = document.removeAll(); + return this.handleChanges(undefined, document, changes); } } - private async handleNativeEditorChanges(payload: IInteractiveWindowInfo) { + private async loadAllCells(payload: ILoadAllCells) { const document = await this.getDocument(); - let changes: TextDocumentContentChangeEvent[][] = []; - const file = payload.visibleCells[0] ? payload.visibleCells[0].file : undefined; - if (document) { - changes = document.handleNativeEditorCellChanges(payload.visibleCells); - } + const changes = document.loadAllCells(payload.cells.filter(c => c.data.cell_type === 'code').map(cell => { + return { + code: concatMultilineString(cell.data.source), + id: cell.id + }; + })); - await Promise.all(changes.map(c => this.handleChanges(file, document, c))); + await this.handleChanges(Identifiers.EmptyFileName, document, changes); + } } private async restartKernel(): Promise { diff --git a/src/client/datascience/interactive-common/intellisense/intellisenseDocument.ts b/src/client/datascience/interactive-common/intellisense/intellisenseDocument.ts index 14ac6495a19a..fc07f5a37cff 100644 --- a/src/client/datascience/interactive-common/intellisense/intellisenseDocument.ts +++ b/src/client/datascience/interactive-common/intellisense/intellisenseDocument.ts @@ -8,9 +8,7 @@ import { EndOfLine, Position, Range, TextDocument, TextDocumentContentChangeEven import * as vscodeLanguageClient from 'vscode-languageclient'; import { PYTHON_LANGUAGE } from '../../../common/constants'; -import { concatMultilineString } from '../../common'; import { Identifiers } from '../../constants'; -import { ICell } from '../../types'; import { DefaultWordPattern, ensureValidWordDefinition, getWordAtText, regExpLeadsToEndlessLoop } from './wordHelper'; class IntellisenseLine implements TextLine { @@ -118,10 +116,6 @@ export class IntellisenseDocument implements TextDocument { return this._lines.length; } - public switchToEditMode() { - this.inEditMode = true; - } - public lineAt(position: Position | number): TextLine { if (typeof position === 'number') { return this._lines[position as number]; @@ -198,46 +192,56 @@ export class IntellisenseDocument implements TextDocument { }; } - public handleNativeEditorCellChanges(cells: ICell[]): TextDocumentContentChangeEvent[][] { - const changes: TextDocumentContentChangeEvent[][] = []; - - if (this.inEditMode) { - const incomingCells = cells.filter(c => c.data.cell_type === 'code'); - const currentCellCount = this._cellRanges.length - 1; - - if (currentCellCount < incomingCells.length) { // Cell was added - incomingCells.forEach((cell, i) => { - if (!this.hasCell(cell.id)) { - const text = concatMultilineString(cell.data.source); - - // addCell to the end of the document, or if adding in the middle, - // send the id of the next cell to get its offset in the document - if (i + 1 > incomingCells.length - 1) { - changes.push(this.addCell(text, text, cell.id)); - } else { - changes.push(this.addCell(text, text, cell.id, incomingCells[i + 1].id)); - } - } - }); - } else if (currentCellCount > incomingCells.length) { // Cell was deleted - const change = this.lookForCellToDelete(incomingCells); - - if (change.length > 0) { - changes.push(change); - } - } else { // Cell might have moved - const change = this.lookForCellMovement(incomingCells); + public loadAllCells(cells: { code: string; id: string }[]): TextDocumentContentChangeEvent[] { + let changes: TextDocumentContentChangeEvent[] = []; + if (!this.inEditMode) { + this.inEditMode = true; + this._version += 1; - if (change.length > 0) { - changes.push(change); + // Normalize all of the cells, removing \r and separating each + // with a newline + const normalized = cells.map(c => { + return { + id: c.id, + code: `${c.code.replace(/\r/g, '')}\n` + }; + }); + + // Contents are easy, just load all of the code in a row + this._contents = normalized.map(c => c.code).reduce((p, c) => { + return `${p}${c}`; + }); + + // Cell ranges are slightly more complicated + let prev: number = 0; + this._cellRanges = normalized.map(c => { + const result = { + id: c.id, + start: prev, + fullEnd: prev + c.code.length, + currentEnd: prev + c.code.length + }; + prev += c.code.length; + return result; + }); + + // Then create the lines. + this._lines = this.createLines(); + + // Return our changes + changes = [ + { + range: this.createSerializableRange(new Position(0, 0), new Position(0, 0)), + rangeOffset: 0, + rangeLength: 0, // Adds are always zero + text: this._contents } - } + ]; } - return changes; } - public addCell(fullCode: string, currentCode: string, id: string, cellId?: string): TextDocumentContentChangeEvent[] { + public addCell(fullCode: string, currentCode: string, id: string): TextDocumentContentChangeEvent[] { // This should only happen once for each cell. this._version += 1; @@ -251,51 +255,66 @@ export class IntellisenseDocument implements TextDocument { const newCode = `${normalized}\n`; const newCurrentCode = `${normalizedCurrent}\n`; - // We should start just before the last cell for the interactive window - // But return the start of the next cell for the native editor, - // in case we add a cell at the end in the native editor, - // just don't send a cellId to get an offset at the end of the document - const fromOffset = this.getEditCellOffset(cellId); + // We should start just before the last cell. + const fromOffset = this.getEditCellOffset(); // Split our text between the edit text and the cells above const before = this._contents.substr(0, fromOffset); const after = this._contents.substr(fromOffset); const fromPosition = this.positionAt(fromOffset); - // for the interactive window or if the cell was added last, - // add cell to the end - let splicePosition = this._cellRanges.length - 1; + // Save the range for this cell () + this._cellRanges.splice(this._cellRanges.length - 1, 0, + { id, start: fromOffset, fullEnd: fromOffset + newCode.length, currentEnd: fromOffset + newCurrentCode.length }); - // for the native editor, find the index to add the cell to - if (cellId) { - const index = this._cellRanges.findIndex(c => c.id === cellId); + // Update our entire contents and recompute our lines + this._contents = `${before}${newCode}${after}`; + this._lines = this.createLines(); + this._cellRanges[this._cellRanges.length - 1].start += newCode.length; + this._cellRanges[this._cellRanges.length - 1].fullEnd += newCode.length; + this._cellRanges[this._cellRanges.length - 1].currentEnd += newCode.length; - if (index > -1) { - splicePosition = index; + return [ + { + range: this.createSerializableRange(fromPosition, fromPosition), + rangeOffset: fromOffset, + rangeLength: 0, // Adds are always zero + text: newCode } - } + ]; + } - // Save the range for this cell () - this._cellRanges.splice(splicePosition, 0, - { id, start: fromOffset, fullEnd: fromOffset + newCode.length, currentEnd: fromOffset + newCurrentCode.length }); + public insertCell(id: string, code: string, codeCellAbove: string | undefined): TextDocumentContentChangeEvent[] { + // This should only happen once for each cell. + this._version += 1; + + // Make sure to put a newline between this code and the next code + const newCode = `${code.replace(/\r/g, '')}\n`; + + // Figure where this goes + const aboveIndex = this._cellRanges.findIndex(r => r.id === codeCellAbove); + const insertIndex = aboveIndex + 1; + + // Compute where we start from. + const fromOffset = insertIndex < this._cellRanges.length ? this._cellRanges[insertIndex].start : this._contents.length; + + // Split our text between the text and the cells above + const before = this._contents.substr(0, fromOffset); + const after = this._contents.substr(fromOffset); + const fromPosition = this.positionAt(fromOffset); // Update our entire contents and recompute our lines this._contents = `${before}${newCode}${after}`; this._lines = this.createLines(); - if (cellId) { - // With the native editor, we fix all the positions that changed after adding - for (let i = splicePosition + 1; i < this._cellRanges.length; i += 1) { - this._cellRanges[i].start += newCode.length; - this._cellRanges[i].fullEnd += newCode.length; - this._cellRanges[i].currentEnd += newCode.length; - } - } else { - // with the interactive window, we just fix the positon of the last cell - this._cellRanges[this._cellRanges.length - 1].start += newCode.length; - this._cellRanges[this._cellRanges.length - 1].fullEnd += newCode.length; - this._cellRanges[this._cellRanges.length - 1].currentEnd += newCode.length; + // Move all the other cell ranges down + for (let i = insertIndex; i <= this._cellRanges.length - 1; i += 1) { + this._cellRanges[i].start += newCode.length; + this._cellRanges[i].fullEnd += newCode.length; + this._cellRanges[i].currentEnd += newCode.length; } + this._cellRanges.splice(insertIndex, 0, + { id, start: fromOffset, fullEnd: fromOffset + newCode.length, currentEnd: fromOffset + newCode.length }); return [ { @@ -308,12 +327,12 @@ export class IntellisenseDocument implements TextDocument { } public removeAllCells(): TextDocumentContentChangeEvent[] { - // Remove everything up to the edit cell - if (this._cellRanges.length > 1) { + // Remove everything + if (this.inEditMode) { this._version += 1; // Compute the offset for the edit cell - const toOffset = this._cellRanges[this._cellRanges.length - 1].start; + const toOffset = this._cellRanges[this._cellRanges.length - 1].fullEnd; const from = this.positionAt(0); const to = this.positionAt(toOffset); @@ -321,12 +340,7 @@ export class IntellisenseDocument implements TextDocument { const result = this.removeRange('', from, to, 0); // Update our cell range - this._cellRanges = [{ - id: Identifiers.EditCellId, - start: 0, - fullEnd: this._cellRanges[this._cellRanges.length - 1].fullEnd - toOffset, - currentEnd: this._cellRanges[this._cellRanges.length - 1].fullEnd - toOffset - }]; + this._cellRanges = []; return result; } @@ -362,6 +376,123 @@ export class IntellisenseDocument implements TextDocument { return []; } + public remove(id: string): TextDocumentContentChangeEvent[] { + let change: TextDocumentContentChangeEvent[] = []; + + const index = this._cellRanges.findIndex(c => c.id === id); + // Ignore unless in edit mode. For non edit mode, cells are still there. + if (index >= 0 && this.inEditMode) { + this._version += 1; + + const found = this._cellRanges[index]; + const foundLength = found.currentEnd - found.start; + const from = new Position(this.getLineFromOffset(found.start), 0); + const to = this.positionAt(found.currentEnd); + + // Remove from the cell ranges. + for (let i = index + 1; i <= this._cellRanges.length - 1; i += 1) { + this._cellRanges[i].start -= foundLength; + this._cellRanges[i].fullEnd -= foundLength; + this._cellRanges[i].currentEnd -= foundLength; + } + this._cellRanges.splice(index, 1); + + // Recreate the contents + const before = this._contents.substr(0, found.start); + const after = this._contents.substr(found.currentEnd); + this._contents = `${before}${after}`; + this._lines = this.createLines(); + + change = [ + { + range: this.createSerializableRange(from, to), + rangeOffset: found.start, + rangeLength: foundLength, + text: '' + } + ]; + } + + return change; + } + + public swap(first: string, second: string): TextDocumentContentChangeEvent[] { + let change: TextDocumentContentChangeEvent[] = []; + + const firstIndex = this._cellRanges.findIndex(c => c.id === first); + const secondIndex = this._cellRanges.findIndex(c => c.id === second); + if (firstIndex >= 0 && secondIndex >= 0 && firstIndex !== secondIndex && this.inEditMode) { + this._version += 1; + + const topIndex = firstIndex < secondIndex ? firstIndex : secondIndex; + const bottomIndex = firstIndex > secondIndex ? firstIndex : secondIndex; + const top = { ...this._cellRanges[topIndex] }; + const bottom = { ...this._cellRanges[bottomIndex] }; + + const from = new Position(this.getLineFromOffset(top.start), 0); + const to = this.positionAt(bottom.currentEnd); + + // Swap everything + this._cellRanges[topIndex].id = bottom.id; + this._cellRanges[topIndex].fullEnd = top.start + (bottom.fullEnd - bottom.start); + this._cellRanges[topIndex].currentEnd = top.start + (bottom.currentEnd - bottom.start); + this._cellRanges[bottomIndex].id = top.id; + this._cellRanges[bottomIndex].start = this._cellRanges[topIndex].fullEnd; + this._cellRanges[bottomIndex].fullEnd = this._cellRanges[topIndex].fullEnd + (top.fullEnd - top.start); + this._cellRanges[bottomIndex].currentEnd = this._cellRanges[topIndex].fullEnd + (top.currentEnd - top.start); + + const fromOffset = this.convertToOffset(from); + const toOffset = this.convertToOffset(to); + + // Recreate our contents, and then recompute all of our lines + const before = this._contents.substr(0, fromOffset); + const topText = this._contents.substr(top.start, top.fullEnd - top.start); + const bottomText = this._contents.substr(bottom.start, bottom.fullEnd - bottom.start); + const after = this._contents.substr(toOffset); + const replacement = `${bottomText}${topText}`; + this._contents = `${before}${replacement}${after}`; + this._lines = this.createLines(); + + // Change is a full replacement + change = [ + { + range: this.createSerializableRange(from, to), + rangeOffset: fromOffset, + rangeLength: toOffset - fromOffset, + text: replacement + } + ]; + } + + return change; + } + + public removeAll(): TextDocumentContentChangeEvent[] { + let change: TextDocumentContentChangeEvent[] = []; + // Ignore unless in edit mode. + if (this._lines.length > 0 && this.inEditMode) { + this._version += 1; + + const from = this._lines[0].range.start; + const to = this._lines[this._lines.length - 1].rangeIncludingLineBreak.end; + const length = this._contents.length; + this._cellRanges = []; + this._contents = ''; + this._lines = []; + + change = [ + { + range: this.createSerializableRange(from, to), + rangeOffset: 0, + rangeLength: length, + text: '' + } + ]; + } + + return change; + } + public convertToDocumentPosition(id: string, line: number, ch: number): Position { // Monaco is 1 based, and we need to add in our cell offset. const cellIndex = this._cellRanges.findIndex(c => c.id === id); @@ -405,11 +536,6 @@ export class IntellisenseDocument implements TextDocument { return this._cellRanges[this._cellRanges.length - 1].start; } - private hasCell(cellId: string) { - const foundIt = this._cellRanges.find(c => c.id === cellId); - return foundIt ? true : false; - } - private getLineFromOffset(offset: number) { let lineCounter = 0; @@ -422,83 +548,6 @@ export class IntellisenseDocument implements TextDocument { return lineCounter; } - private lookForCellToDelete(incomingCells: ICell[]): TextDocumentContentChangeEvent[] { - let change: TextDocumentContentChangeEvent[] = []; - - this._cellRanges.forEach((cell, i) => { - const foundIt = incomingCells.find(c => c.id === cell.id); - - // if cell is not found in the document and its not the last edit cell, we remove it - if (!foundIt && i !== this._cellRanges.length - 1) { - const from = new Position(this.getLineFromOffset(cell.start), 0); - const to = this.positionAt(cell.currentEnd); - - // for some reason, start for the next cell isn't updated on removeRange, - // so we update it here - this._cellRanges[i + 1].start = cell.start; - this._cellRanges.splice(i, 1); - change = this.removeRange('', from, to, i); - } - }); - - return change; - } - - private lookForCellMovement(incomingCells: ICell[]): TextDocumentContentChangeEvent[] { - for (let i = 0; i < incomingCells.length && this._cellRanges.length > 1; i += 1) { - - if (incomingCells[i].id !== this._cellRanges[i].id) { - const lineBreak = '\n'; - const text = this._contents.substr(this._cellRanges[i].start, this._cellRanges[i].currentEnd - this._cellRanges[i].start - 1); - const newText = concatMultilineString(incomingCells[i].data.source) + lineBreak + text + lineBreak; - - // swap contents - this._contents = this._contents.substring(0, this._cellRanges[i].start) - + this._contents.substring(this._cellRanges[i + 1].start, this._cellRanges[i + 1].fullEnd) - + this._contents.substring(this._cellRanges[i].start, this._cellRanges[i].fullEnd) - + this._contents.substring(this._cellRanges[i + 1].fullEnd); - - // create lines - this._lines = this.createLines(); - - // swap cell ranges - const temp1Id = this._cellRanges[i].id; - const temp1Start = this._cellRanges[i].start; - const temp1End = this._cellRanges[i].fullEnd; - const temp1Length = temp1End - temp1Start; - - const temp2Id = this._cellRanges[i + 1].id; - const temp2Start = this._cellRanges[i + 1].start; - const temp2End = this._cellRanges[i + 1].fullEnd; - const temp2Length = temp2End - temp2Start; - - this._cellRanges[i].id = temp2Id; - this._cellRanges[i].start = temp1Start; - this._cellRanges[i].currentEnd = temp1Start + temp2Length; - this._cellRanges[i].fullEnd = temp1Start + temp2Length; - - this._cellRanges[i + 1].id = temp1Id; - this._cellRanges[i + 1].start = temp1Start + temp2Length; - this._cellRanges[i + 1].currentEnd = temp1Start + temp2Length + temp1Length; - this._cellRanges[i + 1].fullEnd = temp1Start + temp2Length + temp1Length; - - const from = new Position(this.getLineFromOffset(temp1Start), 0); - const to = new Position(this.getLineFromOffset(temp2End - 1), temp2End - temp2Start); - const fromOffset = temp1Start; - const toOffset = temp2End; - - return [{ - range: this.createSerializableRange(from, to), - rangeOffset: fromOffset, - rangeLength: toOffset - fromOffset, - text: newText - }]; - } - } - - return []; - } - private removeRange(newText: string, from: Position, to: Position, cellIndex: number): TextDocumentContentChangeEvent[] { const fromOffset = this.convertToOffset(from); const toOffset = this.convertToOffset(to); @@ -552,6 +601,9 @@ export class IntellisenseDocument implements TextDocument { } private createSerializableRange(start: Position, end: Position): Range { + // This funciton is necessary so that the Range can be passed back + // over a remote connection without including all of the extra fields that + // VS code puts into a Range object. const result = { start: { line: start.line, diff --git a/src/client/datascience/interactive-common/interactiveWindowTypes.ts b/src/client/datascience/interactive-common/interactiveWindowTypes.ts index 446563bcb4de..c46a6bad5dd3 100644 --- a/src/client/datascience/interactive-common/interactiveWindowTypes.ts +++ b/src/client/datascience/interactive-common/interactiveWindowTypes.ts @@ -51,6 +51,8 @@ export namespace InteractiveWindowMessages { export const AddCell = 'add_cell'; export const EditCell = 'edit_cell'; export const RemoveCell = 'remove_cell'; + export const SwapCells = 'swap_cells'; + export const InsertCell = 'insert_cell'; export const LoadOnigasmAssemblyRequest = 'load_onigasm_assembly_request'; export const LoadOnigasmAssemblyResponse = 'load_onigasm_assembly_response'; export const LoadTmLanguageRequest = 'load_tmlanguage_request'; @@ -210,6 +212,17 @@ export interface IRemoveCell { id: string; } +export interface ISwapCells { + firstCellId: string; + secondCellId: string; +} + +export interface IInsertCell { + id: string; + code: string; + codeCellAbove: string | undefined; +} + export interface IShowDataViewer { variableName: string; columnSize: number; @@ -284,6 +297,8 @@ export class IInteractiveWindowMapping { public [InteractiveWindowMessages.AddCell]: IAddCell; public [InteractiveWindowMessages.EditCell]: IEditCell; public [InteractiveWindowMessages.RemoveCell]: IRemoveCell; + public [InteractiveWindowMessages.SwapCells]: ISwapCells; + public [InteractiveWindowMessages.InsertCell]: IInsertCell; public [InteractiveWindowMessages.LoadOnigasmAssemblyRequest]: never | undefined; public [InteractiveWindowMessages.LoadOnigasmAssemblyResponse]: Buffer; public [InteractiveWindowMessages.LoadTmLanguageRequest]: never | undefined; diff --git a/src/datascience-ui/history-react/interactiveCell.tsx b/src/datascience-ui/history-react/interactiveCell.tsx index e5ba31e99266..1e1284a41319 100644 --- a/src/datascience-ui/history-react/interactiveCell.tsx +++ b/src/datascience-ui/history-react/interactiveCell.tsx @@ -4,6 +4,7 @@ import '../../client/common/extensions'; import { nbformat } from '@jupyterlab/coreutils'; +import * as fastDeepEqual from 'fast-deep-equal'; import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api'; import * as React from 'react'; @@ -35,10 +36,6 @@ interface IInteractiveCellProps { editorOptions?: monacoEditor.editor.IEditorOptions; editExecutionCount?: string; editorMeasureClassName?: string; - selectedCell?: string; - focusedCell?: string; - hideOutput?: boolean; - showLineNumbers?: boolean; font: IFont; onCodeChange(changes: monacoEditor.editor.IModelContentChange[], cellId: string, modelId: string): void; onCodeCreated(code: string, file: string, cellId: string, modelId: string): void; @@ -72,11 +69,15 @@ export class InteractiveCell extends React.Component { } public componentDidUpdate(prevProps: IInteractiveCellProps) { - if (this.props.selectedCell === this.props.cellVM.cell.id && prevProps.selectedCell !== this.props.selectedCell) { - this.giveFocus(this.props.focusedCell === this.props.cellVM.cell.id); + if (this.props.cellVM.selected && !prevProps.cellVM.selected) { + this.giveFocus(this.props.cellVM.focused); } } + public shouldComponentUpdate(nextProps: IInteractiveCellProps): boolean { + return !fastDeepEqual(this.props, nextProps); + } + public scrollAndFlash() { if (this.wrapperRef && this.wrapperRef.current) { this.wrapperRef.current.scrollIntoView({ behavior: 'auto', block: 'nearest', inline: 'nearest' }); @@ -211,7 +212,7 @@ export class InteractiveCell extends React.Component { focused={this.onCodeFocused} unfocused={this.onCodeUnfocused} keyDown={this.props.keyDown} - showLineNumbers={this.props.showLineNumbers} + showLineNumbers={this.props.cellVM.showLineNumbers} font={this.props.font} /> ); @@ -240,7 +241,7 @@ export class InteractiveCell extends React.Component { } private shouldRenderResults(): boolean { - return this.isCodeCell() && this.hasOutput() && this.getCodeCell().outputs && this.getCodeCell().outputs.length > 0 && !this.props.hideOutput; + return this.isCodeCell() && this.hasOutput() && this.getCodeCell().outputs && this.getCodeCell().outputs.length > 0 && !this.props.cellVM.hideOutput; } private onCellKeyDown = (event: React.KeyboardEvent) => { diff --git a/src/datascience-ui/history-react/interactivePanel.tsx b/src/datascience-ui/history-react/interactivePanel.tsx index 5038823a0730..46a0adf7c7d5 100644 --- a/src/datascience-ui/history-react/interactivePanel.tsx +++ b/src/datascience-ui/history-react/interactivePanel.tsx @@ -350,8 +350,6 @@ export class InteractivePanel extends React.Component diff --git a/src/datascience-ui/interactive-common/mainState.ts b/src/datascience-ui/interactive-common/mainState.ts index 67bd7ed57ac4..49ca98e6a586 100644 --- a/src/datascience-ui/interactive-common/mainState.ts +++ b/src/datascience-ui/interactive-common/mainState.ts @@ -26,6 +26,8 @@ export interface ICellViewModel { showLineNumbers?: boolean; hideOutput?: boolean; useQuickEdit?: boolean; + selected: boolean; + focused: boolean; inputBlockToggled(id: string): void; } @@ -53,11 +55,11 @@ export interface IMainState { pendingVariableCount: number; debugging: boolean; dirty?: boolean; - selectedCell?: string; - focusedCell?: string; + selectedCellId?: string; + focusedCellId?: string; enableGather: boolean; isAtBottom: boolean; - newCell?: string; + newCellId?: string; loadTotal?: number; } @@ -145,7 +147,9 @@ export function createEditableCellVM(executionCount: number): ICellViewModel { inputBlockShow: true, inputBlockText: '', inputBlockCollapseNeeded: false, - inputBlockToggled: noop + inputBlockToggled: noop, + selected: false, + focused: false }; } @@ -186,12 +190,14 @@ export function createCellVM(inputCell: ICell, settings: IDataScienceSettings | inputBlockShow: true, inputBlockText: inputText, inputBlockCollapseNeeded: (inputLinesCount > 1), - inputBlockToggled: inputBlockToggled + inputBlockToggled: inputBlockToggled, + selected: false, + focused: false }; } function generateVMs(inputBlockToggled: (id: string) => void, filePath: string, editable: boolean): ICellViewModel[] { - const cells = generateCells(filePath); + const cells = generateCells(filePath, 10); return cells.map((cell: ICell) => { const vm = createCellVM(cell, undefined, inputBlockToggled, editable); vm.useQuickEdit = false; @@ -199,10 +205,10 @@ function generateVMs(inputBlockToggled: (id: string) => void, filePath: string, }); } -function generateCells(filePath: string): ICell[] { +export function generateCells(filePath: string, repetitions: number): ICell[] { // Dupe a bunch times for perf reasons let cellData: (nbformat.ICodeCell | nbformat.IMarkdownCell | nbformat.IRawCell | IMessageCell)[] = []; - for (let i = 0; i < 10; i += 1) { + for (let i = 0; i < repetitions; i += 1) { cellData = [...cellData, ...generateCellData()]; } return cellData.map((data: nbformat.ICodeCell | nbformat.IMarkdownCell | nbformat.IRawCell | IMessageCell, key: number) => { diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index a0b7cab178f7..328d0cf00a34 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -2,6 +2,7 @@ // Licensed under the MIT License. 'use strict'; import * as fastDeepEqual from 'fast-deep-equal'; +import * as immutable from 'immutable'; import { min } from 'lodash'; // tslint:disable-next-line: no-require-imports import cloneDeep = require('lodash/cloneDeep'); @@ -31,7 +32,14 @@ import { getSettings, updateSettings } from '../react-common/settingsReactSide'; import { detectBaseTheme } from '../react-common/themeDetector'; import { InputHistory } from './inputHistory'; import { IntellisenseProvider } from './intellisenseProvider'; -import { createCellVM, createEditableCellVM, extractInputText, generateTestState, ICellViewModel, IMainState } from './mainState'; +import { + createCellVM, + createEditableCellVM, + extractInputText, + generateTestState, + ICellViewModel, + IMainState +} from './mainState'; import { initializeTokenizer, registerMonacoLanguage } from './tokenizer'; export interface IMainStateControllerProps { @@ -49,7 +57,8 @@ export interface IMainStateControllerProps { // tslint:disable-next-line: max-func-body-length export class MainStateController implements IMessageHandler { private stackLimit = 10; - private state: IMainState; + private pendingState: IMainState; + private renderedState: IMainState; private postOffice: PostOffice = new PostOffice(); private intellisenseProvider: IntellisenseProvider; private onigasmPromise: Deferred | undefined; @@ -60,7 +69,7 @@ export class MainStateController implements IMessageHandler { // tslint:disable-next-line:max-func-body-length constructor(private props: IMainStateControllerProps) { - this.state = { + this.renderedState = { editorOptions: this.computeEditorOptions(), cellVMs: [], busy: true, @@ -85,7 +94,7 @@ export class MainStateController implements IMessageHandler { // Add test state if necessary if (!this.props.skipDefault) { - this.state = generateTestState(this.inputBlockToggled, '', this.props.defaultEditable); + this.renderedState = generateTestState(this.inputBlockToggled, '', this.props.defaultEditable); } // Setup the completion provider for monaco. We only need one @@ -95,7 +104,7 @@ export class MainStateController implements IMessageHandler { if (this.props.skipDefault) { if (this.props.testMode) { // Running a test, skip the tokenizer. We want the UI to display synchronously - this.state = { tokenizerLoaded: true, ...this.state }; + this.renderedState = { tokenizerLoaded: true, ...this.renderedState }; // However we still need to register python as a language registerMonacoLanguage(); @@ -104,6 +113,9 @@ export class MainStateController implements IMessageHandler { } } + // Copy the rendered state + this.pendingState = { ...this.renderedState }; + // Add ourselves as a handler for the post office this.postOffice.addHandler(this); @@ -247,16 +259,16 @@ export class MainStateController implements IMessageHandler { } public stopBusy = () => { - if (this.state.busy) { + if (this.pendingState.busy) { this.setState({ busy: false }); } } public redo = () => { // Pop one off of our redo stack and update our undo - const cells = this.state.redoStack[this.state.redoStack.length - 1]; - const redoStack = this.state.redoStack.slice(0, this.state.redoStack.length - 1); - const undoStack = this.pushStack(this.state.undoStack, this.state.cellVMs); + const cells = this.pendingState.redoStack[this.pendingState.redoStack.length - 1]; + const redoStack = this.pendingState.redoStack.slice(0, this.pendingState.redoStack.length - 1); + const undoStack = this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs); this.sendMessage(InteractiveWindowMessages.Redo); this.setState({ cellVMs: cells, @@ -268,9 +280,9 @@ export class MainStateController implements IMessageHandler { public undo = () => { // Pop one off of our undo stack and update our redo - const cells = this.state.undoStack[this.state.undoStack.length - 1]; - const undoStack = this.state.undoStack.slice(0, this.state.undoStack.length - 1); - const redoStack = this.pushStack(this.state.redoStack, this.state.cellVMs); + const cells = this.pendingState.undoStack[this.pendingState.undoStack.length - 1]; + const undoStack = this.pendingState.undoStack.slice(0, this.pendingState.undoStack.length - 1); + const redoStack = this.pushStack(this.pendingState.redoStack, this.pendingState.cellVMs); this.sendMessage(InteractiveWindowMessages.Undo); this.setState({ cellVMs: cells, @@ -281,15 +293,30 @@ export class MainStateController implements IMessageHandler { } public deleteCell = (cellId: string) => { - const cellVM = this.state.cellVMs.find(c => c.cell.id === cellId); - if (cellVM) { + const index = this.findCellIndex(cellId); + if (index >= 0) { this.sendMessage(InteractiveWindowMessages.DeleteCell); - this.sendMessage(InteractiveWindowMessages.RemoveCell, { id: cellVM.cell.id }); + this.sendMessage(InteractiveWindowMessages.RemoveCell, { id: cellId }); + + // Recompute select/focus if this item has either + let newSelection = this.pendingState.selectedCellId; + let newFocused = this.pendingState.focusedCellId; + const newVMs = [...this.pendingState.cellVMs.filter(c => c.cell.id !== cellId)]; + const nextOrPrev = index === this.pendingState.cellVMs.length - 1 ? index - 1 : index; + if (this.pendingState.selectedCellId === cellId || this.pendingState.focusedCellId === cellId) { + if (nextOrPrev >= 0) { + newVMs[nextOrPrev] = { ...newVMs[nextOrPrev], selected: true, focused: this.pendingState.focusedCellId === cellId }; + newSelection = newVMs[nextOrPrev].cell.id; + newFocused = newVMs[nextOrPrev].focused ? newVMs[nextOrPrev].cell.id : undefined; + } + } // Update our state this.setState({ - cellVMs: this.state.cellVMs.filter(c => c.cell.id !== cellId), - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), + cellVMs: newVMs, + selectedCell: newSelection, + focusedCell: newFocused, + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), skipNextScroll: true }); } @@ -326,7 +353,7 @@ export class MainStateController implements IMessageHandler { public save = () => { // We have to take the current value of each cell to make sure we have the correct text. - this.state.cellVMs.forEach(c => this.updateCellSource(c.cell.id)); + this.pendingState.cellVMs.forEach(c => this.updateCellSource(c.cell.id)); // Then send the save with the new state. this.sendMessage(InteractiveWindowMessages.SaveAll, { cells: this.getNonEditCellVMs().map(cvm => cvm.cell) }); @@ -357,11 +384,11 @@ export class MainStateController implements IMessageHandler { } public canRedo = () => { - return this.state.redoStack.length > 0; + return this.pendingState.redoStack.length > 0; } public canUndo = () => { - return this.state.undoStack.length > 0; + return this.pendingState.undoStack.length > 0; } public canClearAllOutputs = () => { @@ -369,10 +396,8 @@ export class MainStateController implements IMessageHandler { } public clearAllOutputs = () => { - const newList = this.state.cellVMs.map(cellVM => { - const newVM = cloneDeep(cellVM); - newVM.cell.data.outputs = []; - return newVM; + const newList = this.pendingState.cellVMs.map(cellVM => { + return immutable.updateIn(cellVM, ['cell', 'data', 'outputs'], () => []); }); this.setState({ cellVMs: newList @@ -381,7 +406,7 @@ export class MainStateController implements IMessageHandler { public gotoCellCode = (cellId: string) => { // Find our cell - const cellVM = this.state.cellVMs.find(c => c.cell.id === cellId); + const cellVM = this.pendingState.cellVMs.find(c => c.cell.id === cellId); // Send a message to the other side to jump to a particular cell if (cellVM) { @@ -391,9 +416,9 @@ export class MainStateController implements IMessageHandler { public copyCellCode = (cellId: string) => { // Find our cell. This is also supported on the edit cell - let cellVM = this.state.cellVMs.find(c => c.cell.id === cellId); - if (!cellVM && this.state.editCellVM && cellId === this.state.editCellVM.cell.id) { - cellVM = this.state.editCellVM; + let cellVM = this.pendingState.cellVMs.find(c => c.cell.id === cellId); + if (!cellVM && this.pendingState.editCellVM && cellId === this.pendingState.editCellVM.cell.id) { + cellVM = this.pendingState.editCellVM; } // Send a message to the other side to jump to a particular cell @@ -420,19 +445,19 @@ export class MainStateController implements IMessageHandler { public export = () => { // Send a message to the other side to export our current list - const cellContents: ICell[] = this.state.cellVMs.map((cellVM: ICellViewModel, _index: number) => { return cellVM.cell; }); + const cellContents: ICell[] = this.pendingState.cellVMs.map((cellVM: ICellViewModel, _index: number) => { return cellVM.cell; }); this.sendMessage(InteractiveWindowMessages.Export, cellContents); } // When the variable explorer wants to refresh state (say if it was expanded) public refreshVariables = (newExecutionCount?: number) => { - this.sendMessage(InteractiveWindowMessages.GetVariablesRequest, newExecutionCount === undefined ? this.state.currentExecutionCount : newExecutionCount); + this.sendMessage(InteractiveWindowMessages.GetVariablesRequest, newExecutionCount === undefined ? this.pendingState.currentExecutionCount : newExecutionCount); } public toggleVariableExplorer = () => { - this.sendMessage(InteractiveWindowMessages.VariableExplorerToggle, !this.state.variablesVisible); - this.setState({ variablesVisible: !this.state.variablesVisible }); - if (!this.state.variablesVisible) { + this.sendMessage(InteractiveWindowMessages.VariableExplorerToggle, !this.pendingState.variablesVisible); + this.setState({ variablesVisible: !this.pendingState.variablesVisible }); + if (this.pendingState.variablesVisible) { this.refreshVariables(); } } @@ -452,7 +477,7 @@ export class MainStateController implements IMessageHandler { } public readOnlyCodeCreated = (_text: string, file: string, id: string, monacoId: string) => { - const cell = this.state.cellVMs.find(c => c.cell.id === id); + const cell = this.pendingState.cellVMs.find(c => c.cell.id === id); if (cell) { // Pass this onto the completion provider running in the extension this.sendMessage(InteractiveWindowMessages.AddCell, { @@ -476,28 +501,67 @@ export class MainStateController implements IMessageHandler { public codeLostFocus = (cellId: string) => { this.onCodeLostFocus(cellId); - if (this.state.focusedCell === cellId) { + if (this.pendingState.focusedCellId === cellId) { + const newVMs = [...this.pendingState.cellVMs]; + // Switch the old vm + const oldSelect = this.findCellIndex(cellId); + if (oldSelect >= 0) { + newVMs[oldSelect] = { ...newVMs[oldSelect], focused: false }; + } // Only unfocus if we haven't switched somewhere else yet - this.setState({ focusedCell: undefined }); + this.setState({ focusedCell: undefined, cellVMs: newVMs }); } } public codeGotFocus = (cellId: string | undefined) => { - this.setState({ selectedCell: cellId, focusedCell: cellId }); + // Skip if already has focus + if (cellId !== this.pendingState.focusedCellId) { + const newVMs = [...this.pendingState.cellVMs]; + // Switch the old vm + const oldSelect = this.findCellIndex(this.pendingState.selectedCellId); + if (oldSelect >= 0) { + newVMs[oldSelect] = { ...newVMs[oldSelect], selected: false, focused: false }; + } + const newSelect = this.findCellIndex(cellId); + if (newSelect >= 0) { + newVMs[newSelect] = { ...newVMs[newSelect], selected: true, focused: true }; + } + + // Save the whole thing in our state. + this.setState({ selectedCell: cellId, focusedCell: cellId, cellVMs: newVMs }); + } } public selectCell = (cellId: string, focusedCell?: string) => { - this.setState({ selectedCell: cellId, focusedCell }); + // Skip if already the same cell + if (this.pendingState.selectedCellId !== cellId || this.pendingState.focusedCellId !== focusedCell) { + const newVMs = [...this.pendingState.cellVMs]; + // Switch the old vm + const oldSelect = this.findCellIndex(this.pendingState.selectedCellId); + if (oldSelect >= 0) { + newVMs[oldSelect] = { ...newVMs[oldSelect], selected: false, focused: false }; + } + const newSelect = this.findCellIndex(cellId); + if (newSelect >= 0) { + newVMs[newSelect] = { ...newVMs[newSelect], selected: true, focused: focusedCell === newVMs[newSelect].cell.id }; + } + + // Save the whole thing in our state. + this.setState({ selectedCell: cellId, focusedCell, cellVMs: newVMs }); + } } public changeCellType = (cellId: string, newType: 'code' | 'markdown') => { - const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); - if (index >= 0 && this.state.cellVMs[index].cell.data.cell_type !== newType) { - const newVM = cloneDeep(this.state.cellVMs[index]); - newVM.cell.data.cell_type = newType; - const cellVMs = [...this.state.cellVMs]; - cellVMs.splice(index, 1, newVM); + const index = this.pendingState.cellVMs.findIndex(c => c.cell.id === cellId); + if (index >= 0 && this.pendingState.cellVMs[index].cell.data.cell_type !== newType) { + const cellVMs = [...this.pendingState.cellVMs]; + cellVMs[index] = immutable.updateIn(this.pendingState.cellVMs[index], ['cell', 'data', 'cell_type'], () => newType); this.setState({ cellVMs }); + if (newType === 'code') { + this.sendMessage(InteractiveWindowMessages.InsertCell, { id: cellId, code: concatMultilineString(cellVMs[index].cell.data.source), codeCellAbove: this.firstCodeCellAbove(cellId) }); + } else { + this.sendMessage(InteractiveWindowMessages.RemoveCell, { id: cellId }); + } } } @@ -552,9 +616,9 @@ export class MainStateController implements IMessageHandler { // Stick in a new cell at the bottom that's editable and update our state // so that the last cell becomes busy this.setState({ - cellVMs: [...this.state.cellVMs, newCell], - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), - redoStack: this.state.redoStack, + cellVMs: [...this.pendingState.cellVMs, newCell], + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), + redoStack: this.pendingState.redoStack, skipNextScroll: false, submittedText: true }); @@ -564,74 +628,79 @@ export class MainStateController implements IMessageHandler { this.sendMessage(InteractiveWindowMessages.SubmitNewCell, { code, id: newCell.cell.id }); } } else if (inputCell.cell.data.cell_type === 'code') { - // Update our input cell to be in progress again - inputCell.cell.state = CellState.executing; - - // Clear our outputs - inputCell.cell.data.outputs = []; - - // Update our state to display the new status - this.setState({ - cellVMs: [...this.state.cellVMs] - }); + const index = this.findCellIndex(inputCell.cell.id); + if (index >= 0) { + // Update our input cell to be in progress again and clear outputs + const newVMs = [...this.pendingState.cellVMs]; + newVMs[index] = { ...inputCell, cell: { ...inputCell.cell, state: CellState.executing, data: { ...inputCell.cell.data, outputs: [] } } }; + this.setState({ + cellVMs: newVMs + }); + } // Send a message to rexecute this code this.sendMessage(InteractiveWindowMessages.ReExecuteCell, { code, id: inputCell.cell.id }); } else if (inputCell.cell.data.cell_type === 'markdown') { - // Change the input on the cell - inputCell.cell.data.source = code; - inputCell.inputBlockText = code; - - // Update our state to display the new status - this.setState({ - cellVMs: [...this.state.cellVMs] - }); + const index = this.findCellIndex(inputCell.cell.id); + if (index >= 0) { + // Change the input on the cell + const newVMs = [...this.pendingState.cellVMs]; + newVMs[index] = { ...inputCell, inputBlockText: code, cell: { ...inputCell.cell, data: { ...inputCell.cell.data, source: code } } }; + this.setState({ + cellVMs: newVMs + }); + } } } public findCell(cellId?: string): ICellViewModel | undefined { - const nonEdit = this.state.cellVMs.find(cvm => cvm.cell.id === cellId); + const nonEdit = this.pendingState.cellVMs.find(cvm => cvm.cell.id === cellId); if (!nonEdit && cellId === Identifiers.EditCellId) { - return this.state.editCellVM; + return this.pendingState.editCellVM; } return nonEdit; } + public findCellIndex(cellId?: string): number { + return this.pendingState.cellVMs.findIndex(cvm => cvm.cell.id === cellId); + } + public getMonacoId(cellId: string): string | undefined { return this.cellIdToMonacoId.get(cellId); } public toggleLineNumbers = (cellId: string) => { - const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); + const index = this.pendingState.cellVMs.findIndex(c => c.cell.id === cellId); if (index >= 0) { - const newVMs = [...this.state.cellVMs]; - newVMs[index] = cloneDeep(newVMs[index]); - newVMs[index].showLineNumbers = !newVMs[index].showLineNumbers; + const newVMs = [...this.pendingState.cellVMs]; + newVMs[index] = immutable.merge(newVMs[index], { showLineNumbers: !newVMs[index].showLineNumbers }); this.setState({ cellVMs: newVMs }); } } public toggleOutput = (cellId: string) => { - const index = this.state.cellVMs.findIndex(c => c.cell.id === cellId); + const index = this.pendingState.cellVMs.findIndex(c => c.cell.id === cellId); if (index >= 0) { - const newVMs = [...this.state.cellVMs]; - newVMs[index] = cloneDeep(newVMs[index]); - newVMs[index].hideOutput = !newVMs[index].hideOutput; + const newVMs = [...this.pendingState.cellVMs]; + newVMs[index] = immutable.merge(newVMs[index], { hideOutput: !newVMs[index].hideOutput }); this.setState({ cellVMs: newVMs }); } } public setState(newState: {}, callback?: () => void) { + // Add to writable state (it should always reflect the current conditions) + this.pendingState = { ...this.pendingState, ...newState }; + if (this.suspendUpdateCount > 0) { // Just save our new state - this.state = { ...this.state, ...newState }; + this.renderedState = { ...this.renderedState, ...newState }; if (callback) { callback(); } } else { // Send a UI update this.props.setState(newState, () => { - this.state = { ...this.state, ...newState }; + this.renderedState = { ...this.renderedState, ...newState }; if (callback) { callback(); } @@ -640,7 +709,7 @@ export class MainStateController implements IMessageHandler { } public renderUpdate(newState: {}) { - const oldCount = this.state.pendingVariableCount; + const oldCount = this.renderedState.pendingVariableCount; // This method should be called during the render stage of anything // using this state Controller. That's because after shouldComponentUpdate @@ -648,7 +717,7 @@ export class MainStateController implements IMessageHandler { // See https://reactjs.org/docs/react-component.html // Otherwise we set the state in the callback during setState and this can be // too late for any render code to use the stateController. - this.state = { ...this.state, ...newState }; + this.renderedState = { ...this.renderedState, ...newState }; // If the new state includes any cellVM changes, send an update to the other side if ('cellVMs' in newState) { @@ -656,13 +725,13 @@ export class MainStateController implements IMessageHandler { } // If the new state includes pendingVariableCount and it's gone to zero, send a message - if (this.state.pendingVariableCount === 0 && oldCount !== 0) { + if (this.renderedState.pendingVariableCount === 0 && oldCount !== 0) { setTimeout(() => this.sendMessage(InteractiveWindowMessages.VariablesComplete), 1); } } public getState(): IMainState { - return this.state; + return this.pendingState; } // Adjust the visibility or collapsed state of a cell @@ -730,37 +799,38 @@ export class MainStateController implements IMessageHandler { this.insertCell(cell); } - protected insertCell(cell: ICell, position?: number, isMonaco?: boolean): ICellViewModel | undefined { - if (cell) { - const showInputs = getSettings().showCellInputCode; - const collapseInputs = getSettings().collapseCellInputCodeByDefault; - let cellVM: ICellViewModel = createCellVM(cell, getSettings(), this.inputBlockToggled, this.props.defaultEditable); + protected prepareCellVM(cell: ICell, isMonaco?: boolean): ICellViewModel { + const showInputs = getSettings().showCellInputCode; + const collapseInputs = getSettings().collapseCellInputCodeByDefault; + let cellVM: ICellViewModel = createCellVM(cell, getSettings(), this.inputBlockToggled, this.props.defaultEditable); - // Set initial cell visibility and collapse - cellVM = this.alterCellVM(cellVM, showInputs, !collapseInputs); + // Set initial cell visibility and collapse + cellVM = this.alterCellVM(cellVM, showInputs, !collapseInputs); - if (cellVM) { - if (isMonaco) { - cellVM.useQuickEdit = false; - } + if (isMonaco) { + cellVM.useQuickEdit = false; + } - const newList = [...this.state.cellVMs]; - // Make sure to use the same array so our entire state doesn't update - if (position !== undefined && position >= 0) { - newList.splice(position, 0, cellVM); - } else { - newList.push(cellVM); - } - this.setState({ - cellVMs: newList, - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), - redoStack: this.state.redoStack, - skipNextScroll: false - }); + return cellVM; + } - return cellVM; - } + protected insertCell(cell: ICell, position?: number, isMonaco?: boolean): ICellViewModel { + const cellVM = this.prepareCellVM(cell, isMonaco); + const newList = [...this.pendingState.cellVMs]; + // Make sure to use the same array so our entire state doesn't update + if (position !== undefined && position >= 0) { + newList.splice(position, 0, cellVM); + } else { + newList.push(cellVM); } + this.setState({ + cellVMs: newList, + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), + redoStack: this.pendingState.redoStack, + skipNextScroll: false + }); + + return cellVM; } protected suspendUpdates() { @@ -771,7 +841,7 @@ export class MainStateController implements IMessageHandler { if (this.suspendUpdateCount > 0) { this.suspendUpdateCount -= 1; if (this.suspendUpdateCount === 0) { - this.setState(this.state); // This should cause an update + this.setState(this.pendingState); // This should cause an update } } @@ -790,6 +860,15 @@ export class MainStateController implements IMessageHandler { return [...slicedUndo, copy]; } + protected firstCodeCellAbove(cellId: string): string | undefined { + const codeCells = this.pendingState.cellVMs.filter(c => c.cell.data.cell_type === 'code'); + const index = codeCells.findIndex(c => c.cell.id === cellId); + if (index > 0) { + return codeCells[index - 1].cell.id; + } + return undefined; + } + private computeEditorOptions(): monacoEditor.editor.IEditorOptions { const intellisenseOptions = getSettings().intellisenseOptions; const extraSettings = getSettings().extraSettings; @@ -826,12 +905,12 @@ export class MainStateController implements IMessageHandler { // Turn off updates so we generate all of the cell vms without rendering. this.suspendUpdates(); - // Update all of the vms + // Generate all of the VMs const cells = payload.cells as ICell[]; - cells.forEach(c => this.finishCell(c)); + const vms = cells.map(c => this.prepareCellVM(c, true)); // Set our state to not being busy anymore. Clear undo stack as this can't be undone. - this.setState({ busy: false, loadTotal: payload.cells.length, undoStack: [] }); + this.setState({ busy: false, loadTotal: payload.cells.length, undoStack: [], cellVMs: vms }); // Turn updates back on and resend the state. this.resumeUpdates(); @@ -842,15 +921,14 @@ export class MainStateController implements IMessageHandler { this.suspendUpdates(); // When we restart, make sure to turn off all executing cells. They aren't executing anymore - const executingCells = this.state.cellVMs + const executingCells = this.pendingState.cellVMs .map((cvm, i) => { return { cvm, i }; }) .filter(s => s.cvm.cell.state !== CellState.error && s.cvm.cell.state !== CellState.finished); if (executingCells && executingCells.length) { - const newVMs = [...this.state.cellVMs]; + const newVMs = [...this.pendingState.cellVMs]; executingCells.forEach(s => { - newVMs[s.i] = cloneDeep(s.cvm); - newVMs[s.i].cell.state = CellState.finished; + newVMs[s.i] = immutable.updateIn(s.cvm, ['cell', 'state'], () => CellState.finished); }); this.setState({ cellVMs: newVMs }); } @@ -902,7 +980,7 @@ export class MainStateController implements IMessageHandler { // Update theme if necessary const newSettings = JSON.parse(payload as string); const dsSettings = newSettings as IDataScienceExtraSettings; - if (dsSettings && dsSettings.extraSettings && dsSettings.extraSettings.theme !== this.state.theme) { + if (dsSettings && dsSettings.extraSettings && dsSettings.extraSettings.theme !== this.pendingState.theme) { // User changed the current theme. Rerender this.postOffice.sendUnsafeMessage(CssMessages.GetCssRequest, { isDark: this.computeKnownDark() }); this.postOffice.sendUnsafeMessage(CssMessages.GetMonacoThemeRequest, { isDark: this.computeKnownDark() }); @@ -928,7 +1006,7 @@ export class MainStateController implements IMessageHandler { private getAllCells = () => { // Send all of our cells back to the other side - const cells = this.state.cellVMs.map((cellVM: ICellViewModel) => { + const cells = this.pendingState.cellVMs.map((cellVM: ICellViewModel) => { return cellVM.cell; }); @@ -936,14 +1014,14 @@ export class MainStateController implements IMessageHandler { } private getNonEditCellVMs(): ICellViewModel[] { - return this.state.cellVMs; + return this.pendingState.cellVMs; } private clearAllSilent = () => { // Update our state this.setState({ cellVMs: [], - undoStack: this.pushStack(this.state.undoStack, this.state.cellVMs), + undoStack: this.pushStack(this.pendingState.undoStack, this.pendingState.cellVMs), skipNextScroll: true, busy: false // No more progress on delete all }); @@ -951,7 +1029,7 @@ export class MainStateController implements IMessageHandler { private inputBlockToggled = (id: string) => { // Create a shallow copy of the array, let not const as this is the shallow array copy that we will be changing - const cellVMArray: ICellViewModel[] = [...this.state.cellVMs]; + const cellVMArray: ICellViewModel[] = [...this.pendingState.cellVMs]; const cellVMIndex = cellVMArray.findIndex((value: ICellViewModel) => { return value.cell.id === id; }); @@ -987,7 +1065,7 @@ export class MainStateController implements IMessageHandler { } private alterAllCellVMs = (visible: boolean, expanded: boolean) => { - const newCells = this.state.cellVMs.map((value: ICellViewModel) => { + const newCells = this.pendingState.cellVMs.map((value: ICellViewModel) => { return this.alterCellVM(value, visible, expanded); }); @@ -1001,14 +1079,14 @@ export class MainStateController implements IMessageHandler { const info: IInteractiveWindowInfo = { visibleCells: this.getNonEditCellVMs().map(cvm => cvm.cell), cellCount: this.getNonEditCellVMs().length, - undoCount: this.state.undoStack.length, - redoCount: this.state.redoStack.length + undoCount: this.pendingState.undoStack.length, + redoCount: this.pendingState.redoStack.length }; this.sendMessage(InteractiveWindowMessages.SendInfo, info); } private updateOrAdd = (cell: ICell, allowAdd?: boolean) => { - const index = this.state.cellVMs.findIndex((c: ICellViewModel) => { + const index = this.pendingState.cellVMs.findIndex((c: ICellViewModel) => { return c.cell.id === cell.id && c.cell.line === cell.line && c.cell.file === cell.file; @@ -1017,9 +1095,9 @@ export class MainStateController implements IMessageHandler { // This means the cell existed already so it was actual executed code. // Use its execution count to update our execution count. const newExecutionCount = cell.data.execution_count ? - Math.max(this.state.currentExecutionCount, parseInt(cell.data.execution_count.toString(), 10)) : - this.state.currentExecutionCount; - if (newExecutionCount !== this.state.currentExecutionCount && this.state.variablesVisible) { + Math.max(this.pendingState.currentExecutionCount, parseInt(cell.data.execution_count.toString(), 10)) : + this.pendingState.currentExecutionCount; + if (newExecutionCount !== this.pendingState.currentExecutionCount && this.pendingState.variablesVisible) { // We also need to update our variable explorer when the execution count changes // Use the ref here to maintain var explorer independence this.refreshVariables(newExecutionCount); @@ -1027,17 +1105,16 @@ export class MainStateController implements IMessageHandler { // Have to make a copy of the cell VM array or // we won't actually update. - const newVMs = [...this.state.cellVMs]; - newVMs[index] = cloneDeep(newVMs[index]); + const newVMs = [...this.pendingState.cellVMs]; // Check to see if our code still matches for the cell (in liveshare it might be updated from the other side) - if (concatMultilineString(newVMs[index].cell.data.source) !== concatMultilineString(cell.data.source)) { + if (concatMultilineString(this.pendingState.cellVMs[index].cell.data.source) !== concatMultilineString(cell.data.source)) { const newText = extractInputText(cell, getSettings()); - newVMs[index].inputBlockText = newText; + newVMs[index] = { ...newVMs[index], cell: cell, inputBlockText: newText }; + } else { + newVMs[index] = { ...newVMs[index], cell: cell }; } - newVMs[index].cell = cell; - this.setState({ cellVMs: newVMs, currentExecutionCount: newExecutionCount @@ -1095,14 +1172,14 @@ export class MainStateController implements IMessageHandler { const variable = payload as IJupyterVariable; // Only send the updated variable data if we are on the same execution count as when we requested it - if (variable && variable.executionCount !== undefined && variable.executionCount === this.state.currentExecutionCount) { - const stateVariable = this.state.variables.findIndex(v => v.name === variable.name); + if (variable && variable.executionCount !== undefined && variable.executionCount === this.pendingState.currentExecutionCount) { + const stateVariable = this.pendingState.variables.findIndex(v => v.name === variable.name); if (stateVariable >= 0) { - const newState = [...this.state.variables]; + const newState = [...this.pendingState.variables]; newState.splice(stateVariable, 1, variable); this.setState({ variables: newState, - pendingVariableCount: Math.max(0, this.state.pendingVariableCount - 1) + pendingVariableCount: Math.max(0, this.pendingState.pendingVariableCount - 1) }); } } @@ -1116,7 +1193,7 @@ export class MainStateController implements IMessageHandler { const variablesResponse = payload as IJupyterVariablesResponse; // Check to see if we have moved to a new execution count only send our update if we are on the same count as the request - if (variablesResponse.executionCount === this.state.currentExecutionCount) { + if (variablesResponse.executionCount === this.pendingState.currentExecutionCount) { this.setState({ variables: variablesResponse.variables, pendingVariableCount: variablesResponse.variables.length @@ -1182,7 +1259,7 @@ export class MainStateController implements IMessageHandler { // We also get this in our response, but computing is more reliable // than searching for it. - if (this.state.knownDark !== computedKnownDark) { + if (this.pendingState.knownDark !== computedKnownDark) { this.darkChanged(computedKnownDark); } diff --git a/src/datascience-ui/interactive-common/markdown.tsx b/src/datascience-ui/interactive-common/markdown.tsx index 708a0c920ff7..7e7f5daf617f 100644 --- a/src/datascience-ui/interactive-common/markdown.tsx +++ b/src/datascience-ui/interactive-common/markdown.tsx @@ -36,7 +36,10 @@ export class Markdown extends React.Component { } public render() { + const classes = 'markdown-editor-area'; + return ( +
{ useQuickEdit={this.props.useQuickEdit} font={this.props.font} /> +
); } diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index eed7222c1cc4..75c6cc58ad7b 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -4,6 +4,7 @@ import '../../client/common/extensions'; import { nbformat } from '@jupyterlab/coreutils'; +import * as fastDeepEqual from 'fast-deep-equal'; import * as React from 'react'; import { concatMultilineString } from '../../client/datascience/common'; @@ -32,20 +33,14 @@ interface INativeCellProps { maxTextSize?: number; stateController: NativeEditorStateController; monacoTheme: string | undefined; - hideOutput?: boolean; - showLineNumbers?: boolean; - selectedCell?: string; - focusedCell?: string; + lastCell: boolean; font: IFont; focusCell(cellId: string, focusCode: boolean): void; selectCell(cellId: string): void; } -interface INativeCellState { - showingMarkdownEditor: boolean; -} // tslint:disable: react-this-binding-issue -export class NativeCell extends React.Component { +export class NativeCell extends React.Component { private inputRef: React.RefObject = React.createRef(); private wrapperRef: React.RefObject = React.createRef(); private lastKeyPressed: string | undefined; @@ -53,7 +48,6 @@ export class NativeCell extends React.Component { - return this.props.selectedCell === this.cellId; + return this.props.cellVM.selected; } private isFocused = () => { - return this.props.focusedCell === this.cellId; + return this.props.cellVM.focused; } private renderNormalCell() { @@ -149,7 +144,7 @@ export class NativeCell extends React.Component
{this.renderCollapseBar(false)} @@ -185,11 +180,16 @@ export class NativeCell extends React.Component) => { - // When we receive a click, propagate upwards. Might change our state - ev.stopPropagation(); - this.lastKeyPressed = undefined; - const focusedCell = this.isFocused() ? this.cellId : undefined; - this.props.stateController.selectCell(this.cellId, focusedCell); + if (ev.nativeEvent.target) { + const elem = ev.nativeEvent.target as HTMLElement; + if (!elem.className.includes('image-button')) { + // Not a click on an button in a toolbar, select the cell. + ev.stopPropagation(); + this.lastKeyPressed = undefined; + const focusedCell = this.isFocused() ? this.cellId : undefined; + this.props.stateController.selectCell(this.cellId, focusedCell); + } + } } private onMouseDoubleClick = (ev: React.MouseEvent) => { @@ -203,7 +203,11 @@ export class NativeCell extends React.Component { - return (this.isMarkdownCell() && (this.state.showingMarkdownEditor || this.props.cellVM.cell.id === Identifiers.EditCellId)); + return (this.isMarkdownCell() && (this.isShowingMarkdownEditor() || this.props.cellVM.cell.id === Identifiers.EditCellId)); + } + + private isShowingMarkdownEditor = (): boolean => { + return (this.isMarkdownCell() && this.props.cellVM.focused); } private shouldRenderInput(): boolean { @@ -221,9 +225,9 @@ export class NativeCell extends React.Component @@ -545,14 +545,8 @@ export class NativeCell extends React.Component { const cellId = this.props.cellVM.cell.id; const deleteCell = () => { - const cellToSelect = this.getNextCellId() || this.getPrevCellId(); this.props.stateController.possiblyDeleteCell(cellId); this.props.stateController.sendCommand(NativeCommandType.DeleteCell, 'mouse'); - setTimeout(() => { - if (cellToSelect) { - this.moveSelection(cellToSelect); - } - }, 10); }; const runAbove = () => { this.props.stateController.runAbove(cellId); @@ -582,7 +576,7 @@ export class NativeCell extends React.Component ); @@ -678,8 +672,6 @@ export class NativeCell extends React.Component { - this.props.stateController.codeLostFocus(this.cellId); - // There might be a pending focus loss handler. if (this.pendingFocusLoss) { const func = this.pendingFocusLoss; @@ -687,10 +679,7 @@ export class NativeCell extends React.Component { @@ -728,10 +717,10 @@ export class NativeCell extends React.Component { let classes = 'collapse-bar'; - if (this.props.selectedCell === this.props.cellVM.cell.id && this.props.focusedCell !== this.props.cellVM.cell.id) { + if (this.isSelected() && !this.isFocused()) { classes += ' collapse-bar-selected'; } - if (this.props.focusedCell === this.props.cellVM.cell.id) { + if (this.isFocused()) { classes += ' collapse-bar-focused'; } diff --git a/src/datascience-ui/native-editor/nativeEditor.less b/src/datascience-ui/native-editor/nativeEditor.less index a5a3f8d8e96c..80c7bbb55ea3 100644 --- a/src/datascience-ui/native-editor/nativeEditor.less +++ b/src/datascience-ui/native-editor/nativeEditor.less @@ -115,6 +115,16 @@ background: var(--override-widget-background, var(--vscode-notifications-background)); } +.markdown-editor-area { + position: relative; + width:100%; + padding-right: 10px; + margin-bottom: 0px; + padding-left: 2px; + padding-top: 2px; + background: var(--override-widget-background, var(--vscode-notifications-background)); +} + .code-watermark { top: 5px; /* Account for extra padding and border in native editor */ } diff --git a/src/datascience-ui/native-editor/nativeEditor.tsx b/src/datascience-ui/native-editor/nativeEditor.tsx index 204046809856..21c99415bd3d 100644 --- a/src/datascience-ui/native-editor/nativeEditor.tsx +++ b/src/datascience-ui/native-editor/nativeEditor.tsx @@ -24,8 +24,6 @@ import { NativeEditorStateController } from './nativeEditorStateController'; // tslint:disable: react-this-binding-issue // tslint:disable-next-line:no-require-imports no-var-requires const debounce = require('lodash/debounce') as typeof import('lodash/debounce'); -// tslint:disable-next-line: no-require-imports -import cloneDeep = require('lodash/cloneDeep'); interface INativeEditorProps { skipDefault: boolean; @@ -149,7 +147,7 @@ export class NativeEditor extends React.Component { // Cell should already exist in the UI if (this.contentPanelRef) { - const wasFocused = this.state.focusedCell !== undefined; + const wasFocused = this.state.focusedCellId !== undefined; this.stateController.selectCell(cellId, wasFocused ? cellId : undefined); this.focusCell(cellId, wasFocused ? true : false); } @@ -161,7 +159,7 @@ export class NativeEditor extends React.Component c.id === id)) { // Force selection change right now as we don't need the cell to exist // to make it selected (otherwise we'll get a flash) - const wasFocused = this.state.focusedCell !== undefined; + const wasFocused = this.state.focusedCellId !== undefined; this.stateController.selectCell(id, wasFocused ? id : undefined); // Then wait to give it actual input focus @@ -297,8 +295,7 @@ export class NativeEditor extends React.Component @@ -414,6 +408,7 @@ export class NativeEditor extends React.Component { + this.stateController.selectCell(cellId, focusCode ? cellId : undefined); const ref = this.cellRefs.get(cellId); if (ref && ref.current) { ref.current.giveFocus(focusCode); @@ -421,8 +416,8 @@ export class NativeEditor extends React.Component { - if (this.state.newCell) { - const newCell = this.state.newCell; + if (this.state.newCellId) { + const newCell = this.state.newCellId; this.stateController.setState({newCell: undefined}); // Bounce this so state has time to update. setTimeout(() => { diff --git a/src/datascience-ui/native-editor/nativeEditorStateController.ts b/src/datascience-ui/native-editor/nativeEditorStateController.ts index 94c685b9d182..80433cf6dfee 100644 --- a/src/datascience-ui/native-editor/nativeEditorStateController.ts +++ b/src/datascience-ui/native-editor/nativeEditorStateController.ts @@ -87,12 +87,13 @@ export class NativeEditorStateController extends MainStateController { public addNewCell = (): ICellViewModel | undefined => { const cells = this.getState().cellVMs; - const selectedCell = this.getState().selectedCell; + const selectedCell = this.getState().selectedCellId; this.suspendUpdates(); const id = uuid(); - const pos = selectedCell ? cells.findIndex(cvm => cvm.cell.id === this.getState().selectedCell) + 1 : cells.length; + const pos = selectedCell ? cells.findIndex(cvm => cvm.cell.id === this.getState().selectedCellId) + 1 : cells.length; this.setState({ newCell: id }); const vm = this.insertCell(createEmptyCell(id, null), pos); + this.sendMessage(InteractiveWindowMessages.InsertCell, { id, code: '', codeCellAbove: this.firstCodeCellAbove(id) }); if (vm) { // Make sure the new cell is monaco vm.useQuickEdit = false; @@ -112,7 +113,9 @@ export class NativeEditorStateController extends MainStateController { inputBlockShow: true, inputBlockText: '', inputBlockCollapseNeeded: false, - inputBlockToggled: noop + inputBlockToggled: noop, + selected: cells[0].selected, + focused: cells[0].focused }; this.setState({ cellVMs: [newVM], undoStack: this.pushStack(this.getState().undoStack, cells) }); } else { @@ -151,6 +154,7 @@ export class NativeEditorStateController extends MainStateController { const id = uuid(); this.setState({ newCell: id }); this.insertCell(createEmptyCell(id, null), index, isMonaco); + this.sendMessage(InteractiveWindowMessages.InsertCell, { id, code: '', codeCellAbove: this.firstCodeCellAbove(id) }); this.resumeUpdates(); return id; } @@ -164,6 +168,7 @@ export class NativeEditorStateController extends MainStateController { const id = uuid(); this.setState({ newCell: id }); this.insertCell(createEmptyCell(id, null), index + 1, isMonaco); + this.sendMessage(InteractiveWindowMessages.InsertCell, { id, code: '', codeCellAbove: this.firstCodeCellAbove(id) }); this.resumeUpdates(); return id; } @@ -179,6 +184,7 @@ export class NativeEditorStateController extends MainStateController { cellVMs: cellVms, undoStack: this.pushStack(this.getState().undoStack, origVms) }); + this.sendMessage(InteractiveWindowMessages.SwapCells, { firstCellId: cellId!, secondCellId: cellVms[index].cell.id }); } } @@ -192,6 +198,7 @@ export class NativeEditorStateController extends MainStateController { cellVMs: cellVms, undoStack: this.pushStack(this.getState().undoStack, origVms) }); + this.sendMessage(InteractiveWindowMessages.SwapCells, { firstCellId: cellId!, secondCellId: cellVms[index].cell.id }); } } diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 6117fde784d7..28430bc99677 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -37,7 +37,8 @@ interface IMonacoEditorState { editor?: monacoEditor.editor.IStandaloneCodeEditor; model: monacoEditor.editor.ITextModel | null; visibleLineCount: number; - attached: boolean; + attached: boolean; // Keeps track of when we reparent the editor out of the dummy dom node. + widgetsReparented: boolean; // Keeps track of when we reparent the hover widgets so they work inside something with overflow } // Need this to prevent wiping of the current value on a componentUpdate. react-monaco-editor has that problem. @@ -61,7 +62,7 @@ export class MonacoEditor extends React.Component(); this.measureWidthRef = React.createRef(); this.debouncedUpdateEditorSize = debounce(this.updateEditorSize.bind(this), 150); @@ -156,6 +157,12 @@ export class MonacoEditor extends React.Component { + this.throttledUpdateWidgetPosition(); + this.updateWidgetParent(editor); + })); + // Update our margin to include the correct line number style this.updateMargin(editor); @@ -376,19 +383,25 @@ export class MonacoEditor extends React.Component { + if (!this.widgetParent && !this.state.widgetsReparented && this.monacoContainer) { + // Only need to do this once, but update the widget parents and move them. + this.updateWidgetParent(this.state.editor); + this.startUpdateWidgetPosition(); + + // Since only doing this once, remove the listener. + this.monacoContainer.removeEventListener('mousemove', this.onContainerMove); + } + } + private onHoverLeave = () => { // If the hover is active, make sure to hide it. if (this.state.editor && this.widgetParent) { @@ -452,14 +465,15 @@ export class MonacoEditor extends React.Component