From abff373c33ffce777e2a0af8f69285a1f15ec505 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 24 May 2019 13:16:24 -0700 Subject: [PATCH 1/4] Fix hyperlinks (#5766) --- news/2 Fixes/5630.md | 1 + package-lock.json | 60 +++++++------------ .../datascience/history/historyTypes.ts | 2 + .../datascience/history/linkProvider.ts | 40 +++++++++++++ src/client/datascience/serviceRegistry.ts | 2 + .../history-react/MainPanel.tsx | 8 ++- src/datascience-ui/history-react/cell.tsx | 2 + src/datascience-ui/history-react/code.tsx | 2 + .../history-react/contentPanel.tsx | 2 + .../react-common/monacoEditor.tsx | 8 +++ 10 files changed, 87 insertions(+), 40 deletions(-) create mode 100644 news/2 Fixes/5630.md create mode 100644 src/client/datascience/history/linkProvider.ts diff --git a/news/2 Fixes/5630.md b/news/2 Fixes/5630.md new file mode 100644 index 000000000000..7b3a289c9673 --- /dev/null +++ b/news/2 Fixes/5630.md @@ -0,0 +1 @@ +Add support for opening hyperlinks from the interactive window. \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 5b0cad043f6f..d286f426343e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3959,14 +3959,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -3981,20 +3979,17 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "core-util-is": { "version": "1.0.2", @@ -4111,8 +4106,7 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "ini": { "version": "1.3.5", @@ -4124,7 +4118,6 @@ "version": "1.0.0", "bundled": true, "dev": true, - "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4139,7 +4132,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4147,14 +4139,12 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, - "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -4173,7 +4163,6 @@ "version": "0.5.1", "bundled": true, "dev": true, - "optional": true, "requires": { "minimist": "0.0.8" } @@ -4254,8 +4243,7 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "object-assign": { "version": "4.1.1", @@ -4267,7 +4255,6 @@ "version": "1.4.0", "bundled": true, "dev": true, - "optional": true, "requires": { "wrappy": "1" } @@ -4389,7 +4376,6 @@ "version": "1.0.2", "bundled": true, "dev": true, - "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -7481,14 +7467,12 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, - "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -7508,8 +7492,7 @@ "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true, - "optional": true + "dev": true }, "console-control-strings": { "version": "1.1.0", @@ -7657,7 +7640,6 @@ "version": "3.0.4", "bundled": true, "dev": true, - "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -12365,7 +12347,7 @@ "dependencies": { "convert-source-map": { "version": "1.6.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/convert-source-map/-/convert-source-map-1.6.0.tgz", "integrity": "sha512-eFu7XigvxdZ1ETfbgPBohgyQ/Z++C0eEhTor0qRwBw9unw+L0/6V8wkSuGgzdThkiS5lSpdptOQPD8Ak40a+7A==", "dev": true, "requires": { @@ -12374,7 +12356,7 @@ }, "execa": { "version": "1.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/execa/-/execa-1.0.0.tgz", "integrity": "sha512-adbxcyWV46qiHyvSp50TKt05tB4tK3HcmF7/nxfAdhnox83seTDbwnaqKO4sXRy7roHAIFqJP/Rw/AuEbX61LA==", "dev": true, "requires": { @@ -12400,7 +12382,7 @@ }, "find-up": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/find-up/-/find-up-3.0.0.tgz", "integrity": "sha512-1yD6RmLI1XBfxugvORwlck6f75tYL+iR0jqwsOrOxMZyGYqUuDhJ0l4AXdO1iX/FTs9cBAMEk1gWSEx1kSbylg==", "dev": true, "requires": { @@ -12415,7 +12397,7 @@ }, "get-stream": { "version": "4.1.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-4.1.0.tgz", "integrity": "sha512-GMat4EJ5161kIy2HevLlr4luNjBgvmj413KaQA7jt4V8B4RDsfpHk7WQ9GVqfYyyx8OS/L66Kox+rJRNklLK7w==", "dev": true, "requires": { @@ -12424,7 +12406,7 @@ }, "glob": { "version": "7.1.3", - "resolved": false, + "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.3.tgz", "integrity": "sha512-vcfuiIxogLV4DlGBHIUOwI0IbrJ8HWPc4MU7HzviGeNho/UJDfi6B5p3sHeWIQ0KGIU0Jpxi5ZHxemQfLkkAwQ==", "dev": true, "requires": { @@ -12438,7 +12420,7 @@ }, "locate-path": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-3.0.0.tgz", "integrity": "sha512-7AO748wWnIhNqAuaty2ZWHkQHRSNfPVIsPIfwEOWO22AmaoVrWavlOcMR5nzTLNYvp36X220/maaRsrec1G65A==", "dev": true, "requires": { @@ -12458,7 +12440,7 @@ }, "os-locale": { "version": "3.1.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/os-locale/-/os-locale-3.1.0.tgz", "integrity": "sha512-Z8l3R4wYWM40/52Z+S265okfFj8Kt2cC2MKY+xNi3kFs+XGI7WXu/I309QQQYbRW4ijiZ+yxs9pqEhJh0DqW3Q==", "dev": true, "requires": { @@ -12469,7 +12451,7 @@ }, "path-exists": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-3.0.0.tgz", "integrity": "sha1-zg6+ql94yxiSXqfYENe1mwEP1RU=", "dev": true }, @@ -12481,7 +12463,7 @@ }, "pkg-dir": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/pkg-dir/-/pkg-dir-3.0.0.tgz", "integrity": "sha512-/E57AYkoeQ25qkxMj5PBOVgF8Kiu/h7cYS30Z5+R7WaiCCBfLq58ZI/dSeaEKb9WVJV5n/03QwrN3IeWIFllvw==", "dev": true, "requires": { @@ -12490,7 +12472,7 @@ }, "pump": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/pump/-/pump-3.0.0.tgz", "integrity": "sha512-LwZy+p3SFs1Pytd/jYct4wpv49HiYCqd9Rlc5ZVdk0V+8Yzv6jR5Blk3TRmPL1ft69TxP0IMZGJ+WPFU2BFhww==", "dev": true, "requires": { @@ -12506,13 +12488,13 @@ }, "resolve-from": { "version": "4.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/resolve-from/-/resolve-from-4.0.0.tgz", "integrity": "sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==", "dev": true }, "rimraf": { "version": "2.6.3", - "resolved": false, + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.6.3.tgz", "integrity": "sha512-mwqeW5XsA2qAejG46gYdENaxXjx9onRNCfn7L0duuP4hCuTIi/QO7PDK07KJfp1d+izWPrzEJDcSqBa0OZQriA==", "dev": true, "requires": { diff --git a/src/client/datascience/history/historyTypes.ts b/src/client/datascience/history/historyTypes.ts index 56fac6a3c194..2ba2bd69639e 100644 --- a/src/client/datascience/history/historyTypes.ts +++ b/src/client/datascience/history/historyTypes.ts @@ -53,6 +53,7 @@ export namespace HistoryMessages { export const LoadOnigasmAssemblyResponse = 'load_onigasm_assembly_response'; export const LoadTmLanguageRequest = 'load_tmlanguage_request'; export const LoadTmLanguageResponse = 'load_tmlanguage_response'; + export const OpenLink = 'open_link'; } // These are the messages that will mirror'd to guest/hosts in @@ -198,4 +199,5 @@ export class IHistoryMapping { public [HistoryMessages.LoadOnigasmAssemblyResponse]: Buffer; public [HistoryMessages.LoadTmLanguageRequest]: never | undefined; public [HistoryMessages.LoadTmLanguageResponse]: string | undefined; + public [HistoryMessages.OpenLink]: string | undefined; } diff --git a/src/client/datascience/history/linkProvider.ts b/src/client/datascience/history/linkProvider.ts new file mode 100644 index 000000000000..c749fd3c8dad --- /dev/null +++ b/src/client/datascience/history/linkProvider.ts @@ -0,0 +1,40 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; +import '../../common/extensions'; + +import { inject, injectable } from 'inversify'; +import { Event, EventEmitter } from 'vscode'; + +import { IApplicationShell } from '../../common/application/types'; +import { noop } from '../../common/utils/misc'; +import { IHistoryListener } from '../types'; +import { HistoryMessages } from './historyTypes'; + +// tslint:disable: no-any +@injectable() +export class LinkProvider implements IHistoryListener { + private postEmitter: EventEmitter<{message: string; payload: any}> = new EventEmitter<{message: string; payload: any}>(); + constructor(@inject(IApplicationShell) private applicationShell: IApplicationShell) { + noop(); + } + + public get postMessage(): Event<{ message: string; payload: any }> { + return this.postEmitter.event; + } + + public onMessage(message: string, payload?: any): void { + switch (message) { + case HistoryMessages.OpenLink: + if (payload) { + this.applicationShell.openUrl(payload.toString()); + } + break; + default: + break; + } + } + public dispose(): void | undefined { + noop(); + } +} diff --git a/src/client/datascience/serviceRegistry.ts b/src/client/datascience/serviceRegistry.ts index 505d9f8914e5..f5e2759b8795 100644 --- a/src/client/datascience/serviceRegistry.ts +++ b/src/client/datascience/serviceRegistry.ts @@ -15,6 +15,7 @@ import { HistoryCommandListener } from './history/historycommandlistener'; import { HistoryProvider } from './history/historyProvider'; import { DotNetIntellisenseProvider } from './history/intellisense/dotNetIntellisenseProvider'; import { JediIntellisenseProvider } from './history/intellisense/jediIntellisenseProvider'; +import { LinkProvider } from './history/linkProvider'; import { JupyterCommandFactory } from './jupyter/jupyterCommand'; import { JupyterExecutionFactory } from './jupyter/jupyterExecutionFactory'; import { JupyterExporter } from './jupyter/jupyterExporter'; @@ -68,4 +69,5 @@ export function registerTypes(serviceManager: IServiceManager) { serviceManager.addSingleton(IExtensionActivationService, Decorator); serviceManager.add(IHistoryListener, DotNetIntellisenseProvider); serviceManager.add(IHistoryListener, JediIntellisenseProvider); + serviceManager.add(IHistoryListener, LinkProvider); } diff --git a/src/datascience-ui/history-react/MainPanel.tsx b/src/datascience-ui/history-react/MainPanel.tsx index 6f80b1ef448d..3b7badd93054 100644 --- a/src/datascience-ui/history-react/MainPanel.tsx +++ b/src/datascience-ui/history-react/MainPanel.tsx @@ -314,6 +314,7 @@ export class MainPanel extends React.Component onCodeCreated={this.editableCodeCreated} onCodeChange={this.codeChange} monacoTheme={this.state.monacoTheme} + openLink={this.openLink} /> @@ -400,7 +401,8 @@ export class MainPanel extends React.Component skipNextScroll: this.state.skipNextScroll ? true : false, monacoTheme: this.state.monacoTheme, onCodeCreated: this.readOnlyCodeCreated, - onCodeChange: this.codeChange + onCodeChange: this.codeChange, + openLink: this.openLink }; } private getToolbarProps = (baseTheme: string): IToolbarPanelProps => { @@ -481,6 +483,10 @@ export class MainPanel extends React.Component this.postOffice.sendMessage(type, payload); } + private openLink = (uri: monacoEditor.Uri) => { + this.sendMessage(HistoryMessages.OpenLink, uri.toString()); + } + private getAllCells = () => { // Send all of our cells back to the other side const cells = this.state.cellVMs.map((cellVM : ICellViewModel) => { diff --git a/src/datascience-ui/history-react/cell.tsx b/src/datascience-ui/history-react/cell.tsx index c797334c20fe..e3936837b9d8 100644 --- a/src/datascience-ui/history-react/cell.tsx +++ b/src/datascience-ui/history-react/cell.tsx @@ -46,6 +46,7 @@ interface ICellProps { submitNewCode(code: string): void; onCodeChange(changes: monacoEditor.editor.IModelContentChange[], cellId: string, modelId: string): void; onCodeCreated(code: string, file: string, cellId: string, modelId: string): void; + openLink(uri: monacoEditor.Uri): void; } export interface ICellViewModel { @@ -228,6 +229,7 @@ export class Cell extends React.Component { onCreated={this.onCodeCreated} outermostParentClass='cell-wrapper' monacoTheme={this.props.monacoTheme} + openLink={this.props.openLink} /> ); diff --git a/src/datascience-ui/history-react/code.tsx b/src/datascience-ui/history-react/code.tsx index 2efb31245e41..73102d885ee5 100644 --- a/src/datascience-ui/history-react/code.tsx +++ b/src/datascience-ui/history-react/code.tsx @@ -26,6 +26,7 @@ export interface ICodeProps { onSubmit(code: string): void; onCreated(code: string, modelId: string): void; onChange(changes: monacoEditor.editor.IModelContentChange[], modelId: string): void; + openLink(uri: monacoEditor.Uri): void; } interface ICodeState { @@ -100,6 +101,7 @@ export class Code extends React.Component { language='python' editorMounted={this.editorDidMount} options={options} + openLink={this.props.openLink} />
{this.getWatermarkString()}
diff --git a/src/datascience-ui/history-react/contentPanel.tsx b/src/datascience-ui/history-react/contentPanel.tsx index be5bd1b4a0ae..d1744c8eefc8 100644 --- a/src/datascience-ui/history-react/contentPanel.tsx +++ b/src/datascience-ui/history-react/contentPanel.tsx @@ -26,6 +26,7 @@ export interface IContentPanelProps { deleteCell(index: number): void; onCodeChange(changes: monacoEditor.editor.IModelContentChange[], cellId: string, modelId: string): void; onCodeCreated(code: string, file: string, cellId: string, modelId: string): void; + openLink(uri: monacoEditor.Uri): void; } export class ContentPanel extends React.Component { @@ -81,6 +82,7 @@ export class ContentPanel extends React.Component { onCodeChange={this.props.onCodeChange} onCodeCreated={this.props.onCodeCreated} monacoTheme={this.props.monacoTheme} + openLink={this.props.openLink} /> ); diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index 0667066d390d..c7e1fde47c6f 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -19,6 +19,7 @@ export interface IMonacoEditorProps { options: monacoEditor.editor.IEditorConstructionOptions; testMode?: boolean; editorMounted(editor: monacoEditor.editor.IStandaloneCodeEditor): void; + openLink(uri: monacoEditor.Uri): void; } interface IMonacoEditorState { @@ -76,6 +77,13 @@ export class MonacoEditor extends React.Component Date: Fri, 24 May 2019 15:00:21 -0700 Subject: [PATCH 2/4] Escape path names during import/export. Skip redundant chdirs (#5770) * Fix security issues. Change to using less instead of sass (as it has a tar dependency) * Fix import/export paths and remove redundant chdirs. --- news/2 Fixes/5386.md | 1 + src/client/datascience/constants.ts | 3 +- .../datascience/jupyter/jupyterExporter.ts | 34 +++++++++++----- .../datascience/jupyter/jupyterImporter.ts | 40 +++++++++++++------ .../datascience/notebook.functional.test.ts | 7 +++- 5 files changed, 58 insertions(+), 27 deletions(-) create mode 100644 news/2 Fixes/5386.md diff --git a/news/2 Fixes/5386.md b/news/2 Fixes/5386.md new file mode 100644 index 000000000000..ab80b86d964e --- /dev/null +++ b/news/2 Fixes/5386.md @@ -0,0 +1 @@ +Fix import/export paths to be escaped on windows. \ No newline at end of file diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 3eedc81d0659..7c1f0f398881 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -122,7 +122,8 @@ export namespace Settings { } export namespace CodeSnippits { - export const ChangeDirectory = ['{0}', 'import os', 'try:', '\tos.chdir(os.path.join(os.getcwd(), \'{1}\'))', '\tprint(os.getcwd())', 'except:', '\tpass', '']; + export const ChangeDirectory = ['{0}', '{1}', 'import os', 'try:', '\tos.chdir(os.path.join(os.getcwd(), \'{2}\'))', '\tprint(os.getcwd())', 'except:', '\tpass', '']; + export const ChangeDirectoryCommentIdentifier = '# ms-python.python added'; // Not translated so can compare. } export namespace Identifiers { diff --git a/src/client/datascience/jupyter/jupyterExporter.ts b/src/client/datascience/jupyter/jupyterExporter.ts index 72ce08bb7037..94c91a0d50f0 100644 --- a/src/client/datascience/jupyter/jupyterExporter.ts +++ b/src/client/datascience/jupyter/jupyterExporter.ts @@ -8,11 +8,12 @@ import * as path from 'path'; import * as uuid from 'uuid/v4'; import { IWorkspaceService } from '../../common/application/types'; -import { IFileSystem } from '../../common/platform/types'; +import { IFileSystem, IPlatformService } from '../../common/platform/types'; import { IConfigurationService, ILogger } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { CellMatcher } from '../cellMatcher'; +import { concatMultilineString } from '../common'; import { CodeSnippits, Identifiers } from '../constants'; import { CellState, ICell, IJupyterExecution, INotebookExporter, ISysInfo } from '../types'; @@ -24,7 +25,9 @@ export class JupyterExporter implements INotebookExporter { @inject(ILogger) private logger: ILogger, @inject(IWorkspaceService) private workspaceService: IWorkspaceService, @inject(IConfigurationService) private configService: IConfigurationService, - @inject(IFileSystem) private fileSystem: IFileSystem) { + @inject(IFileSystem) private fileSystem: IFileSystem, + @inject(IPlatformService) private readonly platform: IPlatformService + ) { } public dispose() { @@ -75,7 +78,7 @@ export class JupyterExporter implements INotebookExporter { const changeDirectory = await this.calculateDirectoryChange(file, cells); if (changeDirectory) { - const exportChangeDirectory = CodeSnippits.ChangeDirectory.join(os.EOL).format(localize.DataScience.exportChangeDirectoryComment(), changeDirectory); + const exportChangeDirectory = CodeSnippits.ChangeDirectory.join(os.EOL).format(localize.DataScience.exportChangeDirectoryComment(), CodeSnippits.ChangeDirectoryCommentIdentifier, changeDirectory); const cell: ICell = { data: { @@ -117,21 +120,30 @@ export class JupyterExporter implements INotebookExporter { } private calculateDirectoryChange = async (notebookFile: string, cells: ICell[]): Promise => { + // Make sure we don't already have a cell with a ChangeDirectory comment in it. let directoryChange: string | undefined; - const notebookFilePath = path.dirname(notebookFile); - // First see if we have a workspace open, this only works if we have a workspace root to be relative to - if (this.workspaceService.hasWorkspaceFolders) { - const workspacePath = await this.firstWorkspaceFolder(cells); - - // Make sure that we have everything that we need here - if (workspacePath && path.isAbsolute(workspacePath) && notebookFilePath && path.isAbsolute(notebookFilePath)) { - directoryChange = path.relative(notebookFilePath, workspacePath); + const haveChangeAlready = cells.find(c => concatMultilineString(c.data.source).includes(CodeSnippits.ChangeDirectoryCommentIdentifier)); + if (!haveChangeAlready) { + const notebookFilePath = path.dirname(notebookFile); + // First see if we have a workspace open, this only works if we have a workspace root to be relative to + if (this.workspaceService.hasWorkspaceFolders) { + const workspacePath = await this.firstWorkspaceFolder(cells); + + // Make sure that we have everything that we need here + if (workspacePath && path.isAbsolute(workspacePath) && notebookFilePath && path.isAbsolute(notebookFilePath)) { + directoryChange = path.relative(notebookFilePath, workspacePath); + } } } // If path.relative can't calculate a relative path, then it just returns the full second path // so check here, we only want this if we were able to calculate a relative path, no network shares or drives if (directoryChange && !path.isAbsolute(directoryChange)) { + // Escape windows path chars so they end up in the source escaped + if (this.platform.isWindows) { + directoryChange = directoryChange.replace('\\', '\\\\'); + } + return directoryChange; } else { return undefined; diff --git a/src/client/datascience/jupyter/jupyterImporter.ts b/src/client/datascience/jupyter/jupyterImporter.ts index 718c60773fb7..3f2a4015d6c3 100644 --- a/src/client/datascience/jupyter/jupyterImporter.ts +++ b/src/client/datascience/jupyter/jupyterImporter.ts @@ -7,7 +7,7 @@ import * as os from 'os'; import * as path from 'path'; import { IWorkspaceService } from '../../common/application/types'; -import { IFileSystem } from '../../common/platform/types'; +import { IFileSystem, IPlatformService } from '../../common/platform/types'; import { IConfigurationService, IDisposableRegistry } from '../../common/types'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; @@ -38,7 +38,9 @@ export class JupyterImporter implements INotebookImporter { @inject(IDisposableRegistry) private disposableRegistry: IDisposableRegistry, @inject(IConfigurationService) private configuration: IConfigurationService, @inject(IJupyterExecution) private jupyterExecution: IJupyterExecution, - @inject(IWorkspaceService) private workspaceService: IWorkspaceService) { + @inject(IWorkspaceService) private workspaceService: IWorkspaceService, + @inject(IPlatformService) private readonly platform: IPlatformService + ) { this.templatePromise = this.createTemplateFile(); } @@ -49,7 +51,7 @@ export class JupyterImporter implements INotebookImporter { const settings = this.configuration.getSettings(); let directoryChange: string | undefined; if (settings.datascience.changeDirOnImportExport) { - directoryChange = this.calculateDirectoryChange(file); + directoryChange = await this.calculateDirectoryChange(file); } // Use the jupyter nbconvert functionality to turn the notebook into a python file @@ -70,27 +72,39 @@ export class JupyterImporter implements INotebookImporter { } private addDirectoryChange = (pythonOutput: string, directoryChange: string): string => { - const newCode = CodeSnippits.ChangeDirectory.join(os.EOL).format(localize.DataScience.importChangeDirectoryComment(), directoryChange); + const newCode = CodeSnippits.ChangeDirectory.join(os.EOL).format(localize.DataScience.importChangeDirectoryComment(), CodeSnippits.ChangeDirectoryCommentIdentifier, directoryChange); return newCode.concat(pythonOutput); } // When importing a file, calculate if we can create a %cd so that the relative paths work - private calculateDirectoryChange = (notebookFile: string): string | undefined => { + private async calculateDirectoryChange(notebookFile: string): Promise { let directoryChange: string | undefined; - const notebookFilePath = path.dirname(notebookFile); - // First see if we have a workspace open, this only works if we have a workspace root to be relative to - if (this.workspaceService.hasWorkspaceFolders) { - const workspacePath = this.workspaceService.workspaceFolders![0].uri.fsPath; - - // Make sure that we have everything that we need here - if (workspacePath && path.isAbsolute(workspacePath) && notebookFilePath && path.isAbsolute(notebookFilePath)) { - directoryChange = path.relative(workspacePath, notebookFilePath); + // Make sure we don't already have an import/export comment in the file + const contents = await this.fileSystem.readFile(notebookFile); + const haveChangeAlready = contents.includes(CodeSnippits.ChangeDirectoryCommentIdentifier); + + if (!haveChangeAlready) { + const notebookFilePath = path.dirname(notebookFile); + // First see if we have a workspace open, this only works if we have a workspace root to be relative to + if (this.workspaceService.hasWorkspaceFolders) { + const workspacePath = this.workspaceService.workspaceFolders![0].uri.fsPath; + + // Make sure that we have everything that we need here + if (workspacePath && path.isAbsolute(workspacePath) && notebookFilePath && path.isAbsolute(notebookFilePath)) { + directoryChange = path.relative(workspacePath, notebookFilePath); + } } } // If path.relative can't calculate a relative path, then it just returns the full second path // so check here, we only want this if we were able to calculate a relative path, no network shares or drives if (directoryChange && !path.isAbsolute(directoryChange)) { + + // Escape windows path chars so they end up in the source escaped + if (this.platform.isWindows) { + directoryChange = directoryChange.replace('\\', '\\\\'); + } + return directoryChange; } else { return undefined; diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index 97c3710999b2..ccf3daa77664 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -354,8 +354,11 @@ suite('DataScience notebook tests', () => { // Try importing this. This should verify export works and that importing is possible const results = await importer.importFromFile(temp.filePath); - // Make sure we added a chdir into our results - assert.ok(results.includes('os.chdir')); + // Make sure we have a single chdir in our results + const first = results.indexOf('os.chdir'); + assert.ok(first >= 0, 'No os.chdir in import'); + const second = results.indexOf('os.chdir', first + 1); + assert.equal(second, -1, 'More than one chdir in the import. It should be skipped'); // Make sure we have a cell in our results assert.ok(/#\s*%%/.test(results), 'No cells in returned import'); From f7df23dfbd1981a7265880ad99b46e8791a0e5f7 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 24 May 2019 16:01:29 -0700 Subject: [PATCH 3/4] Fix 3 bugs (#5773) Run cell moves to next cell Ctrl+Enter keyboard shortcut Add an empty cell --- news/2 Fixes/5067.md | 1 + news/2 Fixes/5667.md | 1 + news/2 Fixes/5673.md | 1 + package.json | 22 ++++ package.nls.json | 1 + src/client/common/application/commands.ts | 1 + src/client/common/types.ts | 1 + src/client/datascience/constants.ts | 4 +- src/client/datascience/datascience.ts | 11 ++ .../editor-integration/codewatcher.ts | 104 +++++++++++------- src/client/datascience/types.ts | 1 + src/client/telemetry/index.ts | 1 + .../codewatcher.unit.test.ts | 10 +- 13 files changed, 109 insertions(+), 50 deletions(-) create mode 100644 news/2 Fixes/5067.md create mode 100644 news/2 Fixes/5667.md create mode 100644 news/2 Fixes/5673.md diff --git a/news/2 Fixes/5067.md b/news/2 Fixes/5067.md new file mode 100644 index 000000000000..b3b7d8cd0f17 --- /dev/null +++ b/news/2 Fixes/5067.md @@ -0,0 +1 @@ +Advance to the next cell if cursor is in the current cell and user clicks 'Run Cell' \ No newline at end of file diff --git a/news/2 Fixes/5667.md b/news/2 Fixes/5667.md new file mode 100644 index 000000000000..b28890c77907 --- /dev/null +++ b/news/2 Fixes/5667.md @@ -0,0 +1 @@ +Add 'Add empty cell to file' command. Shortcut for having to type '#%%' \ No newline at end of file diff --git a/news/2 Fixes/5673.md b/news/2 Fixes/5673.md new file mode 100644 index 000000000000..7dcc8c92c8dd --- /dev/null +++ b/news/2 Fixes/5673.md @@ -0,0 +1 @@ +Add 'ctrl+enter' as a keyboard shortcut for run current cell (runs without advancing) \ No newline at end of file diff --git a/package.json b/package.json index 8d51ea5c8bfc..5fb3ba867ab6 100644 --- a/package.json +++ b/package.json @@ -99,6 +99,11 @@ "command": "python.datascience.runcurrentcelladvance", "key": "shift+enter", "when": "editorFocus && !editorHasSelection && python.datascience.hascodecells && python.datascience.featureenabled" + }, + { + "command": "python.datascience.runcurrentcell", + "key": "ctrl+enter", + "when": "editorFocus && !editorHasSelection && python.datascience.hascodecells && python.datascience.featureenabled" } ], "commands": [ @@ -428,6 +433,11 @@ "command": "python.datascience.collapseallcells", "title": "%python.command.python.datascience.collapseallcells.title%", "category": "Python" + }, + { + "command": "python.datascience.addcellbelow", + "title": "%python.command.python.datascience.addcellbelow.title%", + "category": "Python" } ], "menus": { @@ -704,6 +714,12 @@ "command": "python.datascience.runallcellsabove", "category": "Python", "when": "config.noExists" + }, + { + "command": "python.datascience.addcellbelow", + "title": "%python.command.python.datascience.addcellbelow.title%", + "category": "Python", + "when": "python.datascience.featureenabled" } ], "view/title": [ @@ -1280,6 +1296,12 @@ "description": "Enables code lens for 'cells' in a python file.", "scope": "resource" }, + "python.dataScience.enableAutoMoveToNextCell": { + "type": "boolean", + "default": true, + "description": "Enables moving to the next cell when clicking on a 'Run Cell' code lens.", + "scope": "resource" + }, "python.disableInstallationCheck": { "type": "boolean", "default": false, diff --git a/package.nls.json b/package.nls.json index 35b153314564..d12827f1becd 100644 --- a/package.nls.json +++ b/package.nls.json @@ -53,6 +53,7 @@ "python.command.python.datascience.restartkernel.title": "Restart IPython Kernel", "python.command.python.datascience.expandallcells.title": "Expand all Python Interactive cells", "python.command.python.datascience.collapseallcells.title": "Collapse all Python Interactive cells", + "python.command.python.datascience.addcellbelow.title": "Add empty cell to file", "python.snippet.launch.standard.label": "Python: Current File", "python.snippet.launch.module.label": "Python: Module", "python.snippet.launch.module.default": "enter-your-module-name", diff --git a/src/client/common/application/commands.ts b/src/client/common/application/commands.ts index 23cd998a123c..c8d7c534a70d 100644 --- a/src/client/common/application/commands.ts +++ b/src/client/common/application/commands.ts @@ -49,6 +49,7 @@ interface ICommandNameWithoutArgumentTypeMapping { [DSCommands.ExpandAllCells]: []; [DSCommands.CollapseAllCells]: []; [DSCommands.ExportOutputAsNotebook]: []; + [DSCommands.AddCellBelow]: []; } /** diff --git a/src/client/common/types.ts b/src/client/common/types.ts index 02cb4b5a7d15..da41abb4b587 100644 --- a/src/client/common/types.ts +++ b/src/client/common/types.ts @@ -311,6 +311,7 @@ export interface IDataScienceSettings { liveShareConnectionTimeout?: number; decorateCells?: boolean; enableCellCodeLens?: boolean; + enableAutoMoveToNextCell?: boolean; } export const IConfigurationService = Symbol('IConfigurationService'); diff --git a/src/client/datascience/constants.ts b/src/client/datascience/constants.ts index 7c1f0f398881..979d40888bbb 100644 --- a/src/client/datascience/constants.ts +++ b/src/client/datascience/constants.ts @@ -33,6 +33,7 @@ export namespace Commands { export const ExportOutputAsNotebook = 'python.datascience.exportoutputasnotebook'; export const ExecSelectionInInteractiveWindow = 'python.datascience.execSelectionInteractive'; export const RunFileInInteractiveWindows = 'python.datascience.runFileInteractive'; + export const AddCellBelow = 'python.datascience.addcellbelow'; } export namespace EditorContexts { @@ -109,7 +110,8 @@ export enum Telemetry { PandasTooOld = 'DATASCIENCE.SHOW_DATA_PANDAS_TOO_OLD', DataScienceSettings = 'DATASCIENCE.SETTINGS', VariableExplorerToggled = 'DATASCIENCE.VARIABLE_EXPLORER_TOGGLE', - VariableExplorerVariableCount = 'DATASCIENCE.VARIABLE_EXPLORER_VARIABLE_COUNT' + VariableExplorerVariableCount = 'DATASCIENCE.VARIABLE_EXPLORER_VARIABLE_COUNT', + AddCellBelow = 'DATASCIENCE.ADD_CELL_BELOW' } export namespace HelpLinks { diff --git a/src/client/datascience/datascience.ts b/src/client/datascience/datascience.ts index 2eb8a06e6cb1..445f6ed6ad15 100644 --- a/src/client/datascience/datascience.ts +++ b/src/client/datascience/datascience.ts @@ -235,6 +235,15 @@ export class DataScience implements IDataScience { } } + @captureTelemetry(Telemetry.AddCellBelow) + private async addCellBelow(): Promise { + const activeEditor = this.documentManager.activeTextEditor; + const activeCodeWatcher = this.getCurrentCodeWatcher(); + if (activeEditor && activeCodeWatcher) { + return activeCodeWatcher.addEmptyCellToBottom(); + } + } + private getCurrentCodeLens() : vscode.CodeLens | undefined { const activeEditor = this.documentManager.activeTextEditor; const activeCodeWatcher = this.getCurrentCodeWatcher(); @@ -349,6 +358,8 @@ export class DataScience implements IDataScience { this.disposableRegistry.push(disposable); disposable = this.commandManager.registerCommand(Commands.RunFileInInteractiveWindows, this.runFileInteractive, this); this.disposableRegistry.push(disposable); + disposable = this.commandManager.registerCommand(Commands.AddCellBelow, this.addCellBelow, this); + this.disposableRegistry.push(disposable); this.commandListeners.forEach((listener: IDataScienceCommandListener) => { listener.register(this.commandManager); }); diff --git a/src/client/datascience/editor-integration/codewatcher.ts b/src/client/datascience/editor-integration/codewatcher.ts index d9e5bc2c0fd6..6b963ed8c7fe 100644 --- a/src/client/datascience/editor-integration/codewatcher.ts +++ b/src/client/datascience/editor-integration/codewatcher.ts @@ -206,33 +206,24 @@ export class CodeWatcher implements ICodeWatcher { } @captureTelemetry(Telemetry.RunCell) - public async runCell(range: Range) { - if (this.document) { - // Use that to get our code. - const code = this.document.getText(range); - - try { - const activeHistory = await this.historyProvider.getOrCreateActive(); - await activeHistory.addCode(code, this.getFileName(), range.start.line, this.documentManager.activeTextEditor); - } catch (err) { - this.handleError(err); - } + public runCell(range: Range) : Promise { + if (!this.documentManager.activeTextEditor || !this.documentManager.activeTextEditor.document) { + return Promise.resolve(); } + + // Run the cell clicked. Advance if the cursor is inside this cell and we're allowed to + const advance = range.contains(this.documentManager.activeTextEditor.selection.start) && this.configService.getSettings().datascience.enableAutoMoveToNextCell; + return this.runMatchingCell(range, advance); } @captureTelemetry(Telemetry.RunCurrentCell) - public async runCurrentCell() { + public runCurrentCell() : Promise { if (!this.documentManager.activeTextEditor || !this.documentManager.activeTextEditor.document) { - return; + return Promise.resolve(); } - for (const lens of this.codeLenses) { - // Check to see which RunCell lens range overlaps the current selection start - if (lens.range.contains(this.documentManager.activeTextEditor.selection.start) && lens.command && lens.command.command === Commands.RunCell) { - await this.runCell(lens.range); - break; - } - } + // Run the cell that matches the current cursor position. + return this.runMatchingCell(this.documentManager.activeTextEditor.selection, false); } @captureTelemetry(Telemetry.RunCurrentCellAndAdvance) @@ -241,38 +232,67 @@ export class CodeWatcher implements ICodeWatcher { return; } - let currentRunCellLens: CodeLens | undefined; - let nextRunCellLens: CodeLens | undefined; + // Run the cell that matches the current cursor position. Always advance + return this.runMatchingCell(this.documentManager.activeTextEditor.selection, true); + } - for (const lens of this.codeLenses) { - // If we have already found the current code lens, then the next run cell code lens will give us the next cell - if (currentRunCellLens && lens.command && lens.command.command === Commands.RunCell) { - nextRunCellLens = lens; - break; - } + public async addEmptyCellToBottom() : Promise { + const editor = this.documentManager.activeTextEditor; + if (editor) { + editor.edit((editBuilder) => { + editBuilder.insert(new Position(editor.document.lineCount, 0), '\n\n#%%\n'); + }); - // Check to see which RunCell lens range overlaps the current selection start - if (lens.range.contains(this.documentManager.activeTextEditor.selection.start) && lens.command && lens.command.command === Commands.RunCell) { - currentRunCellLens = lens; - } + const newPosition = new Position(editor.document.lineCount + 3, 0); // +3 to account for the added spaces and to position after the new mark + return this.advanceToRange(new Range(newPosition, newPosition)); } + } + + private async runMatchingCell(range: Range, advance?: boolean) { + const currentRunCellLens = this.getCurrentCellLens(range.start); + const nextRunCellLens = this.getNextCellLens(range.start); if (currentRunCellLens) { - // Either use the next cell that we found, or add a new one into the document - let nextRange: Range; - if (!nextRunCellLens) { - nextRange = this.createNewCell(currentRunCellLens.range); - } else { - nextRange = nextRunCellLens.range; - } + // Move the next cell if allowed. + if (advance) { + // Either use the next cell that we found, or add a new one into the document + let nextRange: Range; + if (!nextRunCellLens) { + nextRange = this.createNewCell(currentRunCellLens.range); + } else { + nextRange = nextRunCellLens.range; + } - if (nextRange) { - this.advanceToRange(nextRange); + if (nextRange) { + this.advanceToRange(nextRange); + } } // Run the cell after moving the selection - await this.runCell(currentRunCellLens.range); + if (this.document) { + // Use that to get our code. + const code = this.document.getText(currentRunCellLens.range); + + try { + const activeHistory = await this.historyProvider.getOrCreateActive(); + await activeHistory.addCode(code, this.getFileName(), range.start.line, this.documentManager.activeTextEditor); + } catch (err) { + this.handleError(err); + } + } + } + } + + private getCurrentCellLens(pos: Position) : CodeLens | undefined { + return this.codeLenses.find(l => l.range.contains(pos) && l.command !== undefined && l.command.command === Commands.RunCell); + } + + private getNextCellLens(pos: Position) : CodeLens | undefined { + const currentIndex = this.codeLenses.findIndex(l => l.range.contains(pos) && l.command !== undefined && l.command.command === Commands.RunCell); + if (currentIndex >= 0) { + return this.codeLenses.find((l: CodeLens, i: number) => l.command !== undefined && l.command.command === Commands.RunCell && i > currentIndex); } + return undefined; } private async runFileInteractiveInternal() { diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index 9ee4e49574f0..d8c9e3ee4e02 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -221,6 +221,7 @@ export interface ICodeWatcher { runAllCellsAbove(stopLine: number, stopCharacter: number): Promise; runCellAndAllBelow(startLine: number, startCharacter: number): Promise; runFileInteractive(): Promise; + addEmptyCellToBottom(): Promise; } export enum CellState { diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index cb3d24f9a449..325c039d26b1 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -320,6 +320,7 @@ export interface IEventNamePropertyMapping { [EventName.WORKSPACE_SYMBOLS_BUILD]: never | undefined; [EventName.WORKSPACE_SYMBOLS_GO_TO]: never | undefined; // Data Science + [Telemetry.AddCellBelow]: never | undefined; [Telemetry.CollapseAll]: never | undefined; [Telemetry.ConnectFailedJupyter]: never | undefined; [Telemetry.ConnectLocalJupyter]: never | undefined; diff --git a/src/test/datascience/editor-integration/codewatcher.unit.test.ts b/src/test/datascience/editor-integration/codewatcher.unit.test.ts index 1bda9c6545bd..53bdd7e44e42 100644 --- a/src/test/datascience/editor-integration/codewatcher.unit.test.ts +++ b/src/test/datascience/editor-integration/codewatcher.unit.test.ts @@ -274,13 +274,9 @@ fourth line test('Test the RunCell command', async () => { const fileName = 'test.py'; const version = 1; - const inputText = ''; // This test overrides getText, so we don't need to fill this in - const document = createDocument(inputText, fileName, version, TypeMoq.Times.atLeastOnce()); - - // Specify our range and text here - const testRange = new Range(0, 0, 10, 10); - const testString = 'testing'; - document.setup(doc => doc.getText(testRange)).returns(() => testString).verifiable(TypeMoq.Times.once()); + const testString = '#%%\ntesting'; + const document = createDocument(testString, fileName, version, TypeMoq.Times.atLeastOnce(), true); + const testRange = new Range(0, 0, 1, 7); codeWatcher.setDocument(document.object); From 3d4f49dc7cb8103ad4c185ebc546b759a1f5f036 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 24 May 2019 16:28:42 -0700 Subject: [PATCH 4/4] Fix up/down when autocomplete is open (#5776) --- news/2 Fixes/5774.md | 1 + package-lock.json | 52 ++++++++++++------- src/datascience-ui/history-react/code.tsx | 21 ++++++-- .../react-common/monacoEditor.tsx | 9 ++++ 4 files changed, 59 insertions(+), 24 deletions(-) create mode 100644 news/2 Fixes/5774.md diff --git a/news/2 Fixes/5774.md b/news/2 Fixes/5774.md new file mode 100644 index 000000000000..7bcdf5aa2ef5 --- /dev/null +++ b/news/2 Fixes/5774.md @@ -0,0 +1 @@ +Fix problem with using up/down arrows in autocomplete. \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index d286f426343e..da48144bd9ee 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3959,12 +3959,14 @@ "balanced-match": { "version": "1.0.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "brace-expansion": { "version": "1.1.11", "bundled": true, "dev": true, + "optional": true, "requires": { "balanced-match": "^1.0.0", "concat-map": "0.0.1" @@ -3979,17 +3981,20 @@ "code-point-at": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "concat-map": { "version": "0.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "console-control-strings": { "version": "1.1.0", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -4106,7 +4111,8 @@ "inherits": { "version": "2.0.3", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "ini": { "version": "1.3.5", @@ -4118,6 +4124,7 @@ "version": "1.0.0", "bundled": true, "dev": true, + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -4132,6 +4139,7 @@ "version": "3.0.4", "bundled": true, "dev": true, + "optional": true, "requires": { "brace-expansion": "^1.1.7" } @@ -4139,12 +4147,14 @@ "minimist": { "version": "0.0.8", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "minipass": { "version": "2.3.5", "bundled": true, "dev": true, + "optional": true, "requires": { "safe-buffer": "^5.1.2", "yallist": "^3.0.0" @@ -4163,6 +4173,7 @@ "version": "0.5.1", "bundled": true, "dev": true, + "optional": true, "requires": { "minimist": "0.0.8" } @@ -4243,7 +4254,8 @@ "number-is-nan": { "version": "1.0.1", "bundled": true, - "dev": true + "dev": true, + "optional": true }, "object-assign": { "version": "4.1.1", @@ -4255,6 +4267,7 @@ "version": "1.4.0", "bundled": true, "dev": true, + "optional": true, "requires": { "wrappy": "1" } @@ -4376,6 +4389,7 @@ "version": "1.0.2", "bundled": true, "dev": true, + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -12347,7 +12361,7 @@ "dependencies": { "convert-source-map": { "version": "1.6.0", - "resolved": "https://registry.npmjs.org/convert-source-map/-/convert-source-map-1.6.0.tgz", + "resolved": false, "integrity": "sha512-eFu7XigvxdZ1ETfbgPBohgyQ/Z++C0eEhTor0qRwBw9unw+L0/6V8wkSuGgzdThkiS5lSpdptOQPD8Ak40a+7A==", "dev": true, "requires": { @@ -12356,7 +12370,7 @@ }, "execa": { "version": "1.0.0", - "resolved": "https://registry.npmjs.org/execa/-/execa-1.0.0.tgz", + "resolved": false, "integrity": "sha512-adbxcyWV46qiHyvSp50TKt05tB4tK3HcmF7/nxfAdhnox83seTDbwnaqKO4sXRy7roHAIFqJP/Rw/AuEbX61LA==", "dev": true, "requires": { @@ -12382,7 +12396,7 @@ }, "find-up": { "version": "3.0.0", - "resolved": "https://registry.npmjs.org/find-up/-/find-up-3.0.0.tgz", + "resolved": false, "integrity": "sha512-1yD6RmLI1XBfxugvORwlck6f75tYL+iR0jqwsOrOxMZyGYqUuDhJ0l4AXdO1iX/FTs9cBAMEk1gWSEx1kSbylg==", "dev": true, "requires": { @@ -12397,7 +12411,7 @@ }, "get-stream": { "version": "4.1.0", - "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-4.1.0.tgz", + "resolved": false, "integrity": "sha512-GMat4EJ5161kIy2HevLlr4luNjBgvmj413KaQA7jt4V8B4RDsfpHk7WQ9GVqfYyyx8OS/L66Kox+rJRNklLK7w==", "dev": true, "requires": { @@ -12406,7 +12420,7 @@ }, "glob": { "version": "7.1.3", - "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.3.tgz", + "resolved": false, "integrity": "sha512-vcfuiIxogLV4DlGBHIUOwI0IbrJ8HWPc4MU7HzviGeNho/UJDfi6B5p3sHeWIQ0KGIU0Jpxi5ZHxemQfLkkAwQ==", "dev": true, "requires": { @@ -12420,7 +12434,7 @@ }, "locate-path": { "version": "3.0.0", - "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-3.0.0.tgz", + "resolved": false, "integrity": "sha512-7AO748wWnIhNqAuaty2ZWHkQHRSNfPVIsPIfwEOWO22AmaoVrWavlOcMR5nzTLNYvp36X220/maaRsrec1G65A==", "dev": true, "requires": { @@ -12440,7 +12454,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": { @@ -12451,7 +12465,7 @@ }, "path-exists": { "version": "3.0.0", - "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-3.0.0.tgz", + "resolved": false, "integrity": "sha1-zg6+ql94yxiSXqfYENe1mwEP1RU=", "dev": true }, @@ -12463,7 +12477,7 @@ }, "pkg-dir": { "version": "3.0.0", - "resolved": "https://registry.npmjs.org/pkg-dir/-/pkg-dir-3.0.0.tgz", + "resolved": false, "integrity": "sha512-/E57AYkoeQ25qkxMj5PBOVgF8Kiu/h7cYS30Z5+R7WaiCCBfLq58ZI/dSeaEKb9WVJV5n/03QwrN3IeWIFllvw==", "dev": true, "requires": { @@ -12472,7 +12486,7 @@ }, "pump": { "version": "3.0.0", - "resolved": "https://registry.npmjs.org/pump/-/pump-3.0.0.tgz", + "resolved": false, "integrity": "sha512-LwZy+p3SFs1Pytd/jYct4wpv49HiYCqd9Rlc5ZVdk0V+8Yzv6jR5Blk3TRmPL1ft69TxP0IMZGJ+WPFU2BFhww==", "dev": true, "requires": { @@ -12488,13 +12502,13 @@ }, "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 }, "rimraf": { "version": "2.6.3", - "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.6.3.tgz", + "resolved": false, "integrity": "sha512-mwqeW5XsA2qAejG46gYdENaxXjx9onRNCfn7L0duuP4hCuTIi/QO7PDK07KJfp1d+izWPrzEJDcSqBa0OZQriA==", "dev": true, "requires": { diff --git a/src/datascience-ui/history-react/code.tsx b/src/datascience-ui/history-react/code.tsx index 73102d885ee5..d402aa3d28ee 100644 --- a/src/datascience-ui/history-react/code.tsx +++ b/src/datascience-ui/history-react/code.tsx @@ -43,6 +43,7 @@ interface ICodeState { export class Code extends React.Component { private subscriptions: monacoEditor.IDisposable[] = []; private lastCleanVersionId: number = 0; + private editorRef: React.RefObject = React.createRef(); constructor(prop: ICodeProps) { super(prop); @@ -102,6 +103,7 @@ export class Code extends React.Component { editorMounted={this.editorDidMount} options={options} openLink={this.props.openLink} + ref={this.editorRef} />
{this.getWatermarkString()}
@@ -135,9 +137,11 @@ export class Code extends React.Component { // Listen for model changes this.subscriptions.push(editor.onDidChangeModelContent(this.modelChanged)); - // List for key up/down events. - this.subscriptions.push(editor.onKeyDown(this.onKeyDown)); - this.subscriptions.push(editor.onKeyUp(this.onKeyUp)); + // List for key up/down events if not read only + if (!this.props.readOnly) { + this.subscriptions.push(editor.onKeyDown(this.onKeyDown)); + this.subscriptions.push(editor.onKeyUp(this.onKeyUp)); + } // Indicate we're ready this.props.onCreated(this.props.code, model!.id); @@ -203,8 +207,15 @@ export class Code extends React.Component { return ''; } + private isAutoCompleteOpen() : boolean { + if (this.editorRef.current) { + return this.editorRef.current.isSuggesting(); + } + return false; + } + private arrowUp(e: monacoEditor.IKeyboardEvent) { - if (this.state.editor && this.state.model) { + if (this.state.editor && this.state.model && !this.isAutoCompleteOpen()) { const cursor = this.state.editor.getPosition(); if (cursor && cursor.lineNumber === 1 && this.props.history) { const currentValue = this.getContents(); @@ -220,7 +231,7 @@ export class Code extends React.Component { } private arrowDown(e: monacoEditor.IKeyboardEvent) { - if (this.state.editor && this.state.model) { + if (this.state.editor && this.state.model && !this.isAutoCompleteOpen()) { const cursor = this.state.editor.getPosition(); if (cursor && cursor.lineNumber === this.state.model.getLineCount() && this.props.history) { const currentValue = this.getContents(); diff --git a/src/datascience-ui/react-common/monacoEditor.tsx b/src/datascience-ui/react-common/monacoEditor.tsx index c7e1fde47c6f..3f83cdfb3681 100644 --- a/src/datascience-ui/react-common/monacoEditor.tsx +++ b/src/datascience-ui/react-common/monacoEditor.tsx @@ -182,6 +182,15 @@ export class MonacoEditor extends React.Component= 2) { + const suggestWidget = this.widgetParent.firstChild.childNodes.item(1) as HTMLDivElement; + return suggestWidget && suggestWidget.className.includes('visible'); + } + return false; + } + private windowResized = () => { if (this.resizeTimer) { clearTimeout(this.resizeTimer);