From 287b4f85ca0a3c43795e944a1608ae2a14649100 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 1 Oct 2019 17:49:54 -0700 Subject: [PATCH 1/6] Add capability to support current file path for notebook root --- .vscode/launch.json | 3 +- news/1 Enhancements/4441.md | 1 + news/2 Fixes/7688.md | 1 + .../interactive-common/interactiveBase.ts | 10 ++-- .../interactive-ipynb/nativeEditor.ts | 8 +++ .../datascience/jupyter/jupyterNotebook.ts | 40 ++++++++++----- .../jupyter/jupyterServerFactory.ts | 5 +- .../datascience/jupyter/jupyterUtils.ts | 49 ++++++++++++++++++ .../jupyter/liveshare/guestJupyterNotebook.ts | 2 +- .../jupyter/liveshare/guestJupyterServer.ts | 3 +- .../jupyter/liveshare/hostJupyterNotebook.ts | 7 +-- .../jupyter/liveshare/hostJupyterServer.ts | 6 ++- .../jupyter/liveshare/serverCache.ts | 5 +- src/client/datascience/types.ts | 2 +- src/test/datascience/datascience.unit.test.ts | 51 ++++++++++++++----- src/test/datascience/execution.unit.test.ts | 2 +- 16 files changed, 156 insertions(+), 39 deletions(-) create mode 100644 news/1 Enhancements/4441.md create mode 100644 news/2 Fixes/7688.md create mode 100644 src/client/datascience/jupyter/jupyterUtils.ts diff --git a/.vscode/launch.json b/.vscode/launch.json index f4446d12dc44..74949856b246 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -8,7 +8,8 @@ "request": "launch", "runtimeExecutable": "${execPath}", "args": [ - "--extensionDevelopmentPath=${workspaceFolder}" + "--extensionDevelopmentPath=${workspaceFolder}", + "D:\\Training\\SnakePython\\sub\\simple.py" ], "stopOnEntry": false, "smartStep": true, 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. diff --git a/src/client/datascience/interactive-common/interactiveBase.ts b/src/client/datascience/interactive-common/interactiveBase.ts index 413970ba89cc..ebd865a2f22b 100644 --- a/src/client/datascience/interactive-common/interactiveBase.ts +++ b/src/client/datascience/interactive-common/interactiveBase.ts @@ -443,6 +443,12 @@ export abstract class InteractiveBase extends WebViewHost { + if (file !== Identifiers.EmptyFileName && this.notebook) { + await this.notebook.setLaunchingFile(file); + } + } + protected getNotebook(): INotebook | undefined { return this.notebook; } @@ -493,9 +499,7 @@ export abstract class InteractiveBase extends WebViewHost { + // For the native editor, use our own file as the path + const notebook = this.getNotebook(); + if (this.fileSystem.fileExists(this.file.fsPath) && notebook) { + await notebook.setLaunchingFile(this.file.fsPath); + } + } + protected sendCellsToWebView(cells: ICell[]) { // Filter out sysinfo messages. Don't want to show those const filtered = cells.filter(c => c.data.cell_type !== 'messages'); diff --git a/src/client/datascience/jupyter/jupyterNotebook.ts b/src/client/datascience/jupyter/jupyterNotebook.ts index 6872e8f0109d..8ddb286b3591 100644 --- a/src/client/datascience/jupyter/jupyterNotebook.ts +++ b/src/client/datascience/jupyter/jupyterNotebook.ts @@ -12,7 +12,7 @@ import * as uuid from 'uuid/v4'; import { Disposable, Event, EventEmitter, Uri } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; -import { ILiveShareApi } from '../../common/application/types'; +import { ILiveShareApi, IWorkspaceService } from '../../common/application/types'; import { Cancellation, CancellationError } from '../../common/cancellation'; import { traceError, traceInfo, traceWarning } from '../../common/logger'; import { IConfigurationService, IDisposableRegistry } from '../../common/types'; @@ -36,6 +36,7 @@ import { INotebookServerLaunchInfo, InterruptResult } from '../types'; +import { expandFileVariable } from './jupyterUtils'; class CellSubscriber { private deferred: Deferred = createDeferred(); @@ -139,6 +140,7 @@ export class JupyterNotebookBase implements INotebook { private ranInitialSetup = false; private _resource: Uri; private _disposed: boolean = false; + private _workingDirectory: string | undefined; constructor( _liveShare: ILiveShareApi, // This is so the liveshare mixin works @@ -149,7 +151,8 @@ export class JupyterNotebookBase implements INotebook { private launchInfo: INotebookServerLaunchInfo, private loggers: INotebookExecutionLogger[], resource: Uri, - private getDisposedError: () => Error + private getDisposedError: () => Error, + private workspace: IWorkspaceService ) { this.sessionStartTime = Date.now(); this._resource = resource; @@ -183,13 +186,11 @@ export class JupyterNotebookBase implements INotebook { return; } this.ranInitialSetup = true; + this._workingDirectory = undefined; try { // When we start our notebook initial, change to our workspace or user specified root directory - if (this.launchInfo && this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) { - traceInfo(`Changing directory for ${this.resource.toString()}`); - await this.changeDirectoryIfPossible(this.launchInfo.workingDir); - } + await this.updateWorkingDirectory(); const settings = this.configService.getSettings().datascience; const matplobInit = !settings || settings.enablePlotViewer ? CodeSnippits.MatplotLibInitSvg : CodeSnippits.MatplotLibInitPng; @@ -249,12 +250,9 @@ export class JupyterNotebookBase implements INotebook { return deferred.promise; } - public async setInitialDirectory(directory: string): Promise { - // If we launched local and have no working directory call this on add code to change directory - if (this.launchInfo && !this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) { - await this.changeDirectoryIfPossible(directory); - this.launchInfo.workingDir = directory; - } + public setLaunchingFile(file: string): Promise { + // Update our working directory if we don't have one set already + return this.updateWorkingDirectory(file); } public executeObservable(code: string, file: string, line: number, id: string, silent: boolean = false): Observable { @@ -587,6 +585,24 @@ export class JupyterNotebookBase implements INotebook { }); } + private async updateWorkingDirectory(launchingFile?: string): Promise { + if (this.launchInfo && this.launchInfo.connectionInfo.localLaunch && !this._workingDirectory) { + // See what our working dir is supposed to be + const suggested = this.launchInfo.workingDir; + if (suggested && await fs.pathExists(suggested)) { + // We should use the launch info directory. It trumps the possible dir + this._workingDirectory = suggested; + return this.changeDirectoryIfPossible(this._workingDirectory); + } else if (launchingFile && await fs.pathExists(launchingFile)) { + // Combine the working directory with this file if possible. + this._workingDirectory = expandFileVariable(this.launchInfo.workingDir, launchingFile, this.workspace); + if (this._workingDirectory) { + return this.changeDirectoryIfPossible(this._workingDirectory); + } + } + } + } + private changeDirectoryIfPossible = async (directory: string): Promise => { if (this.launchInfo && this.launchInfo.connectionInfo.localLaunch && await fs.pathExists(directory)) { await this.executeSilently(`%cd "${directory}"`); diff --git a/src/client/datascience/jupyter/jupyterServerFactory.ts b/src/client/datascience/jupyter/jupyterServerFactory.ts index 9570bc3ecc45..877e9f34adf1 100644 --- a/src/client/datascience/jupyter/jupyterServerFactory.ts +++ b/src/client/datascience/jupyter/jupyterServerFactory.ts @@ -9,7 +9,7 @@ import { Uri } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import * as vsls from 'vsls/vscode'; -import { ILiveShareApi } from '../../common/application/types'; +import { ILiveShareApi, IWorkspaceService } from '../../common/application/types'; import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../common/types'; import { IConnection, @@ -36,6 +36,7 @@ type JupyterServerClassType = { disposableRegistry: IDisposableRegistry, configService: IConfigurationService, sessionManager: IJupyterSessionManagerFactory, + workspaceService: IWorkspaceService, loggers: INotebookExecutionLogger[] ): IJupyterServerInterface; }; @@ -55,6 +56,7 @@ export class JupyterServerFactory implements INotebookServer, ILiveShareHasRole @inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry, @inject(IConfigurationService) configService: IConfigurationService, @inject(IJupyterSessionManagerFactory) sessionManager: IJupyterSessionManagerFactory, + @inject(IWorkspaceService) workspaceService: IWorkspaceService, @multiInject(INotebookExecutionLogger) @optional() loggers: INotebookExecutionLogger[] | undefined) { this.serverFactory = new RoleBasedFactory( liveShare, @@ -66,6 +68,7 @@ export class JupyterServerFactory implements INotebookServer, ILiveShareHasRole disposableRegistry, configService, sessionManager, + workspaceService, loggers ? loggers : [] ); } diff --git a/src/client/datascience/jupyter/jupyterUtils.ts b/src/client/datascience/jupyter/jupyterUtils.ts new file mode 100644 index 000000000000..5dc95196925b --- /dev/null +++ b/src/client/datascience/jupyter/jupyterUtils.ts @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. +'use strict'; +import '../../common/extensions'; + +import * as path from 'path'; +import { Uri } from 'vscode'; + +import { IWorkspaceService } from '../../common/application/types'; + +// tslint:disable: no-invalid-template-strings +export function expandFileVariable(fileVariable: string | undefined, file: string, workspace: IWorkspaceService): string | undefined { + const folder = workspace.getWorkspaceFolder(Uri.file(file)); + + // See here for the variable list: https://code.visualstudio.com/docs/editor/variables-reference + switch (fileVariable) { + case '${file}': + case '${fileDirname}': + default: + // Just use the path of the file + return path.dirname(file); + + case '${relativeFile}': + if (folder) { + return path.relative(folder.uri.fsPath, file); + } + break; + + case '${relativeFileDirname}': + if (folder) { + return path.relative(folder.uri.fsPath, path.dirname(file)); + } + break; + + case '${cwd}': + return process.cwd(); + + case '${workspaceFolder}': + if (folder) { + return folder.uri.fsPath; + } + break; + + case '${execPath}': + break; + } + + return undefined; +} diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts index afe03505c396..d2c8a572f6f2 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterNotebook.ts @@ -87,7 +87,7 @@ export class GuestJupyterNotebook return deferred.promise; } - public setInitialDirectory(_directory: string): Promise { + public setLaunchingFile(_directory: string): Promise { // Ignore this command on this side return Promise.resolve(); } diff --git a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts index 4acea881452c..9e4fa81361fa 100644 --- a/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/guestJupyterServer.ts @@ -6,7 +6,7 @@ import { Uri } from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import * as vsls from 'vsls/vscode'; -import { ILiveShareApi } from '../../../common/application/types'; +import { ILiveShareApi, IWorkspaceService } from '../../../common/application/types'; import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types'; import { createDeferred, Deferred } from '../../../common/utils/async'; import * as localize from '../../../common/utils/localize'; @@ -38,6 +38,7 @@ export class GuestJupyterServer private disposableRegistry: IDisposableRegistry, private configService: IConfigurationService, _sessionManager: IJupyterSessionManagerFactory, + _workspaceService: IWorkspaceService, _loggers: INotebookExecutionLogger[] ) { super(liveShare); diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts index 863e3de18c59..dd54843a5946 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterNotebook.ts @@ -10,7 +10,7 @@ import * as vscode from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import * as vsls from 'vsls/vscode'; -import { ILiveShareApi } from '../../../common/application/types'; +import { ILiveShareApi, IWorkspaceService } from '../../../common/application/types'; import { traceError } from '../../../common/logger'; import { IConfigurationService, IDisposableRegistry } from '../../../common/types'; import { createDeferred } from '../../../common/utils/async'; @@ -50,9 +50,10 @@ export class HostJupyterNotebook launchInfo: INotebookServerLaunchInfo, loggers: INotebookExecutionLogger[], resource: vscode.Uri, - getDisposedError: () => Error + getDisposedError: () => Error, + workspace: IWorkspaceService ) { - super(liveShare, session, configService, disposableRegistry, owner, launchInfo, loggers, resource, getDisposedError); + super(liveShare, session, configService, disposableRegistry, owner, launchInfo, loggers, resource, getDisposedError, workspace); } public dispose = async (): Promise => { diff --git a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts index 64ea465df7a1..6986ad168338 100644 --- a/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts +++ b/src/client/datascience/jupyter/liveshare/hostJupyterServer.ts @@ -8,7 +8,7 @@ import * as vscode from 'vscode'; import { CancellationToken } from 'vscode-jsonrpc'; import * as vsls from 'vsls/vscode'; -import { ILiveShareApi } from '../../../common/application/types'; +import { ILiveShareApi, IWorkspaceService } from '../../../common/application/types'; import { traceInfo } from '../../../common/logger'; import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../../common/types'; import * as localize from '../../../common/utils/localize'; @@ -46,6 +46,7 @@ export class HostJupyterServer disposableRegistry: IDisposableRegistry, configService: IConfigurationService, sessionManager: IJupyterSessionManagerFactory, + private workspaceService: IWorkspaceService, loggers: INotebookExecutionLogger[]) { super(liveShare, asyncRegistry, disposableRegistry, configService, sessionManager, loggers); } @@ -162,7 +163,8 @@ export class HostJupyterServer launchInfo, loggers, resource, - this.getDisposedError.bind(this)); + this.getDisposedError.bind(this), + this.workspaceService); // Wait for it to be ready traceInfo(`Waiting for idle ${this.id}`); diff --git a/src/client/datascience/jupyter/liveshare/serverCache.ts b/src/client/datascience/jupyter/liveshare/serverCache.ts index a3b05f880f0f..714b02287fe7 100644 --- a/src/client/datascience/jupyter/liveshare/serverCache.ts +++ b/src/client/datascience/jupyter/liveshare/serverCache.ts @@ -102,7 +102,7 @@ export class ServerCache implements IAsyncDisposable { // User setting is absolute and doesn't exist, use workspace workingDir = workspaceFolderPath; } - } else { + } else if (!fileRoot.includes('${')) { // fileRoot is a relative path, combine it with the workspace folder const combinedPath = path.join(workspaceFolderPath, fileRoot); if (await this.fileSystem.directoryExists(combinedPath)) { @@ -112,6 +112,9 @@ export class ServerCache implements IAsyncDisposable { // Combined path doesn't exist, use workspace workingDir = workspaceFolderPath; } + } else { + // fileRoot is a variable that hasn't been expanded + workingDir = fileRoot; } } return workingDir; diff --git a/src/client/datascience/types.ts b/src/client/datascience/types.ts index d0856618a417..712f2add8d42 100644 --- a/src/client/datascience/types.ts +++ b/src/client/datascience/types.ts @@ -95,7 +95,7 @@ export interface INotebook extends IAsyncDisposable { restartKernel(timeoutInMs: number): Promise; waitForIdle(timeoutInMs: number): Promise; interruptKernel(timeoutInMs: number): Promise; - setInitialDirectory(directory: string): Promise; + setLaunchingFile(file: string): Promise; getSysInfo(): Promise; setMatplotLibStyle(useDark: boolean): Promise; } diff --git a/src/test/datascience/datascience.unit.test.ts b/src/test/datascience/datascience.unit.test.ts index 2b9874470471..00570203e32d 100644 --- a/src/test/datascience/datascience.unit.test.ts +++ b/src/test/datascience/datascience.unit.test.ts @@ -2,13 +2,19 @@ // Licensed under the MIT License. 'use strict'; import { assert } from 'chai'; +import { anything, instance, mock, when } from 'ts-mockito'; +import { Uri } from 'vscode'; +import { WorkspaceService } from '../../client/common/application/workspace'; +import { IsWindows } from '../../client/common/types'; import { generateCells } from '../../client/datascience/cellFactory'; import { formatStreamText, stripComments } from '../../client/datascience/common'; +import { expandFileVariable } from '../../client/datascience/jupyter/jupyterUtils'; import { InputHistory } from '../../datascience-ui/interactive-common/inputHistory'; // tslint:disable: max-func-body-length suite('Data Science Tests', () => { + const workspaceService = mock(WorkspaceService); test('formatting stream text', async () => { assert.equal(formatStreamText('\rExecute\rExecute 1'), 'Execute 1'); @@ -21,6 +27,27 @@ suite('Data Science Tests', () => { assert.equal(formatStreamText('\rExecute\rExecute\nExecute 10\rExecute 11\r\n'), 'Execute\nExecute 11\n'); }); + // tslint:disable: no-invalid-template-strings + test('expanding file variables', async () => { + const uri = Uri.file('test/bar'); + const folder = { index: 0, name: '', uri }; + when(workspaceService.hasWorkspaceFolders).thenReturn(true); + when(workspaceService.workspaceFolders).thenReturn([folder]); + when(workspaceService.getWorkspaceFolder(anything())).thenReturn(folder); + const inst = instance(workspaceService); + const relativeFilePath = IsWindows ? '..\\xyz\\bip\\foo.baz' : '../xyz/bip/foo.baz'; + const relativeFileDir = IsWindows ? '..\\xyz\\bip' : '../xyz/bip'; + + assert.equal(expandFileVariable(undefined, 'bar/foo.baz', inst), 'bar'); + assert.equal(expandFileVariable(undefined, 'bar/bip/foo.baz', inst), 'bar/bip'); + assert.equal(expandFileVariable('${file}', 'bar/bip/foo.baz', inst), 'bar/bip'); + assert.equal(expandFileVariable('${fileDirName}', 'bar/bip/foo.baz', inst), 'bar/bip'); + assert.equal(expandFileVariable('${relativeFile}', 'test/xyz/bip/foo.baz', inst), relativeFilePath); + assert.equal(expandFileVariable('${relativeFileDirname}', 'test/xyz/bip/foo.baz', inst), relativeFileDir); + assert.equal(expandFileVariable('${cwd}', 'test/xyz/bip/foo.baz', inst), process.cwd()); + assert.equal(expandFileVariable('${workspaceFolder}', 'test/xyz/bip/foo.baz', inst), 'test/bar'); + }); + test('input history', async () => { let history = new InputHistory(); history.add('1', true); @@ -110,8 +137,8 @@ suite('Data Science Tests', () => { assert.equal(cells[0].data.cell_type, 'markdown', 'Markdown cell not generated'); assert.equal(cells[0].data.source.length, 2, 'Lines for cell not emitted'); -// tslint:disable-next-line: no-multiline-string -const multilineCode = `#%% + // tslint:disable-next-line: no-multiline-string + const multilineCode = `#%% myvar = """ # Lorem Ipsum Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam eget varius ligula, eget fermentum mauris. @@ -121,8 +148,8 @@ Sed mattis dui diam, et blandit augue mattis vestibulum. Suspendisse ornare interdum velit. Suspendisse potenti. Morbi molestie lacinia sapien nec porttitor. Nam at vestibulum nisi. """`; -// tslint:disable-next-line: no-multiline-string -const multilineTwo = `#%% + // tslint:disable-next-line: no-multiline-string + const multilineTwo = `#%% """ # Lorem Ipsum Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam eget varius ligula, eget fermentum mauris. @@ -141,9 +168,9 @@ Morbi molestie lacinia sapien nec porttitor. Nam at vestibulum nisi. assert.equal(cells.length, 1, 'code cell multline failed'); assert.equal(cells[0].data.cell_type, 'code', 'Code cell not generated'); assert.equal(cells[0].data.source.length, 10, 'Lines for cell not emitted'); -// tslint:disable-next-line: no-multiline-string + // tslint:disable-next-line: no-multiline-string assert.equal(cells[0].data.source[9], `""" print('bob')`, 'Lines for cell not emitted'); -// tslint:disable-next-line: no-multiline-string + // tslint:disable-next-line: no-multiline-string const multilineMarkdown = `#%% [markdown] # ## Block of Interest # @@ -171,8 +198,8 @@ Morbi molestie lacinia sapien nec porttitor. Nam at vestibulum nisi. assert.equal(cells[0].data.source.length, 20, 'Lines for cell not emitted'); assert.equal(cells[0].data.source[17], ' - Item 1-a-3-c\n', 'Lines for markdown not emitted'); -// tslint:disable-next-line: no-multiline-string -const multilineQuoteWithOtherDelimiter = `#%% [markdown] + // tslint:disable-next-line: no-multiline-string + const multilineQuoteWithOtherDelimiter = `#%% [markdown] ''' ### Take a look 2. Item 2 @@ -186,7 +213,7 @@ const multilineQuoteWithOtherDelimiter = `#%% [markdown] assert.equal(cells[0].data.source[2], '""" Not a comment delimiter', 'Lines for markdown not emitted'); // tslint:disable-next-line: no-multiline-string -const multilineQuoteInFunc = `#%% + const multilineQuoteInFunc = `#%% import requests def download(url, filename): """ utility function to download a file """ @@ -201,8 +228,8 @@ def download(url, filename): assert.equal(cells[0].data.source.length, 9, 'Lines for cell not emitted'); assert.equal(cells[0].data.source[3], ' """ utility function to download a file """\n', 'Lines for cell not emitted'); -// tslint:disable-next-line: no-multiline-string -const multilineMarkdownWithCell = `#%% [markdown] + // tslint:disable-next-line: no-multiline-string + const multilineMarkdownWithCell = `#%% [markdown] # # Define a simple class class Pizza(object): def __init__(self, size, toppings, price, rating): @@ -227,6 +254,6 @@ class Pizza(object): assert.equal(nonComments, '', 'Multline comment is not being stripped'); nonComments = stripComments(multilineQuoteInFunc); assert.equal(nonComments.splitLines().length, 6, 'Splitting quote in func wrong number of lines'); - }); + }); }); diff --git a/src/test/datascience/execution.unit.test.ts b/src/test/datascience/execution.unit.test.ts index 67ada4bb6c1f..36386c68cbc9 100644 --- a/src/test/datascience/execution.unit.test.ts +++ b/src/test/datascience/execution.unit.test.ts @@ -94,7 +94,7 @@ class MockJupyterNotebook implements INotebook { public waitForIdle(): Promise { throw new Error('Method not implemented'); } - public setInitialDirectory(_directory: string): Promise { + public setLaunchingFile(_file: string): Promise { throw new Error('Method not implemented'); } From f0b64a9082c7d90267ed0bf55a166610cc23cbb2 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Tue, 1 Oct 2019 17:53:07 -0700 Subject: [PATCH 2/6] Put launch.json back --- .vscode/launch.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.vscode/launch.json b/.vscode/launch.json index 74949856b246..f4446d12dc44 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -8,8 +8,7 @@ "request": "launch", "runtimeExecutable": "${execPath}", "args": [ - "--extensionDevelopmentPath=${workspaceFolder}", - "D:\\Training\\SnakePython\\sub\\simple.py" + "--extensionDevelopmentPath=${workspaceFolder}" ], "stopOnEntry": false, "smartStep": true, From 524e7b7541f2d5587727e39bd483a180c0d1892b Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 2 Oct 2019 09:37:07 -0700 Subject: [PATCH 3/6] Use system variables instead of creating separate resolver --- .../checks/invalidPythonPathInDebugger.ts | 3 +- src/client/common/configSettings.ts | 2 +- .../common/variables/systemVariables.ts | 63 +++++++++++++++++-- .../datascience/jupyter/jupyterNotebook.ts | 4 +- .../datascience/jupyter/jupyterUtils.ts | 43 +++---------- .../configuration/providers/djangoLaunch.ts | 3 +- .../configuration/providers/pyramidLaunch.ts | 3 +- src/test/datascience/datascience.unit.test.ts | 23 ++++--- 8 files changed, 84 insertions(+), 60 deletions(-) diff --git a/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts b/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts index 9d63a2ffcc76..0fc6bcbcc5e5 100644 --- a/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts +++ b/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts @@ -109,8 +109,7 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService await this.messageService.handle(diagnostic, { commandPrompts }); } protected resolveVariables(pythonPath: string, resource: Uri | undefined): string { - const workspaceFolder = resource ? this.workspace.getWorkspaceFolder(resource) : undefined; - const systemVariables = new SystemVariables(workspaceFolder ? workspaceFolder.uri.fsPath : undefined); + const systemVariables = new SystemVariables(resource, this.workspace); return systemVariables.resolveAny(pythonPath); } private getCommandPrompts(diagnostic: IDiagnostic): { prompt: string; command?: IDiagnosticCommand }[] { diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index e483f3629c27..35bb5721a46d 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -131,7 +131,7 @@ export class PythonSettings implements IPythonSettings { // tslint:disable-next-line:cyclomatic-complexity max-func-body-length protected update(pythonSettings: WorkspaceConfiguration) { const workspaceRoot = this.workspaceRoot.fsPath; - const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined); + const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot, this.workspace); // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion this.pythonPath = systemVariables.resolveAny(pythonSettings.get('pythonPath'))!; diff --git a/src/client/common/variables/systemVariables.ts b/src/client/common/variables/systemVariables.ts index 217247fd71f2..514416acb146 100644 --- a/src/client/common/variables/systemVariables.ts +++ b/src/client/common/variables/systemVariables.ts @@ -2,15 +2,17 @@ * Copyright (c) Microsoft Corporation. All rights reserved. * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ - 'use strict'; - import * as Path from 'path'; +import { Range, Uri } from 'vscode'; + +import { IDocumentManager, IWorkspaceService } from '../application/types'; import * as Types from '../utils/sysTypes'; import { IStringDictionary, ISystemVariables } from './types'; + /* tslint:disable:rule1 no-any no-unnecessary-callback-wrapper jsdoc-format no-for-in prefer-const no-increment-decrement */ -export abstract class AbstractSystemVariables implements ISystemVariables { +abstract class AbstractSystemVariables implements ISystemVariables { public resolve(value: string): string; public resolve(value: string[]): string[]; @@ -93,14 +95,25 @@ export abstract class AbstractSystemVariables implements ISystemVariables { export class SystemVariables extends AbstractSystemVariables { private _workspaceFolder: string; private _workspaceFolderName: string; + private _filePath: string | undefined; + private _lineNumber: number | undefined; + private _selectedText: string | undefined; + private _execPath: string; - constructor(workspaceFolder?: string) { + constructor(fileOrFolder: Uri | string | undefined, workspace?: IWorkspaceService, documentManager?: IDocumentManager) { super(); - this._workspaceFolder = typeof workspaceFolder === 'string' ? workspaceFolder : __dirname; + const workspaceFolder = workspace && (typeof fileOrFolder !== 'string') ? workspace.getWorkspaceFolder(fileOrFolder) : undefined; + this._workspaceFolder = workspaceFolder ? workspaceFolder.uri.fsPath : fileOrFolder as string || __dirname; this._workspaceFolderName = Path.basename(this._workspaceFolder); Object.keys(process.env).forEach(key => { (this as any as Record)[`env:${key}`] = (this as any as Record)[`env.${key}`] = process.env[key]; }); + this._filePath = (typeof fileOrFolder !== 'string') && fileOrFolder ? fileOrFolder.fsPath : undefined; + if (documentManager && documentManager.activeTextEditor) { + this._lineNumber = documentManager.activeTextEditor.selection.anchor.line + 1; + this._selectedText = documentManager.activeTextEditor.document.getText(new Range(documentManager.activeTextEditor.selection.start, documentManager.activeTextEditor.selection.end)); + } + this._execPath = process.execPath; } public get cwd(): string { @@ -122,4 +135,44 @@ export class SystemVariables extends AbstractSystemVariables { public get workspaceFolderBasename(): string { return this._workspaceFolderName; } + + public get file(): string | undefined { + return this._filePath; + } + + public get relativeFile(): string | undefined { + return this.file ? Path.relative(this._workspaceFolder, this.file) : undefined; + } + + public get relativeFileDirname(): string | undefined { + return this.relativeFile ? Path.dirname(this.relativeFile) : undefined; + } + + public get fileBasename(): string | undefined { + return this.file ? Path.basename(this.file) : undefined; + } + + public get fileBasenameNoExtension(): string | undefined { + return this.file ? Path.parse(this.file).name : undefined; + } + + public get fileDirname(): string | undefined { + return this.file ? Path.dirname(this.file) : undefined; + } + + public get fileExtname(): string | undefined { + return this.file ? Path.extname(this.file) : undefined; + } + + public get lineNumber(): number | undefined { + return this._lineNumber; + } + + public get selectedText(): string | undefined { + return this._selectedText; + } + + public get execPath(): string { + return this._execPath; + } } diff --git a/src/client/datascience/jupyter/jupyterNotebook.ts b/src/client/datascience/jupyter/jupyterNotebook.ts index 8ddb286b3591..f27fdc4b8031 100644 --- a/src/client/datascience/jupyter/jupyterNotebook.ts +++ b/src/client/datascience/jupyter/jupyterNotebook.ts @@ -36,7 +36,7 @@ import { INotebookServerLaunchInfo, InterruptResult } from '../types'; -import { expandFileVariable } from './jupyterUtils'; +import { expandWorkingDir } from './jupyterUtils'; class CellSubscriber { private deferred: Deferred = createDeferred(); @@ -595,7 +595,7 @@ export class JupyterNotebookBase implements INotebook { return this.changeDirectoryIfPossible(this._workingDirectory); } else if (launchingFile && await fs.pathExists(launchingFile)) { // Combine the working directory with this file if possible. - this._workingDirectory = expandFileVariable(this.launchInfo.workingDir, launchingFile, this.workspace); + this._workingDirectory = expandWorkingDir(this.launchInfo.workingDir, launchingFile, this.workspace); if (this._workingDirectory) { return this.changeDirectoryIfPossible(this._workingDirectory); } diff --git a/src/client/datascience/jupyter/jupyterUtils.ts b/src/client/datascience/jupyter/jupyterUtils.ts index 5dc95196925b..00802dff732a 100644 --- a/src/client/datascience/jupyter/jupyterUtils.ts +++ b/src/client/datascience/jupyter/jupyterUtils.ts @@ -7,43 +7,14 @@ import * as path from 'path'; import { Uri } from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; +import { SystemVariables } from '../../common/variables/systemVariables'; -// tslint:disable: no-invalid-template-strings -export function expandFileVariable(fileVariable: string | undefined, file: string, workspace: IWorkspaceService): string | undefined { - const folder = workspace.getWorkspaceFolder(Uri.file(file)); - - // See here for the variable list: https://code.visualstudio.com/docs/editor/variables-reference - switch (fileVariable) { - case '${file}': - case '${fileDirname}': - default: - // Just use the path of the file - return path.dirname(file); - - case '${relativeFile}': - if (folder) { - return path.relative(folder.uri.fsPath, file); - } - break; - - case '${relativeFileDirname}': - if (folder) { - return path.relative(folder.uri.fsPath, path.dirname(file)); - } - break; - - case '${cwd}': - return process.cwd(); - - case '${workspaceFolder}': - if (folder) { - return folder.uri.fsPath; - } - break; - - case '${execPath}': - break; +export function expandWorkingDir(workingDir: string | undefined, launchingFile: string, workspace: IWorkspaceService): string { + if (workingDir) { + const variables = new SystemVariables(Uri.file(launchingFile), workspace); + return variables.resolve(workingDir); } - return undefined; + // No working dir, just use the path of the launching file. + return path.dirname(launchingFile); } diff --git a/src/client/debugger/extension/configuration/providers/djangoLaunch.ts b/src/client/debugger/extension/configuration/providers/djangoLaunch.ts index f17650b53bbb..d9cbb1beec64 100644 --- a/src/client/debugger/extension/configuration/providers/djangoLaunch.ts +++ b/src/client/debugger/extension/configuration/providers/djangoLaunch.ts @@ -72,8 +72,7 @@ export class DjangoLaunchDebugConfigurationProvider implements IDebugConfigurati return; } protected resolveVariables(pythonPath: string, resource: Uri | undefined): string { - const workspaceFolder = resource ? this.workspace.getWorkspaceFolder(resource) : undefined; - const systemVariables = new SystemVariables(workspaceFolder ? workspaceFolder.uri.fsPath : undefined); + const systemVariables = new SystemVariables(resource, this.workspace); return systemVariables.resolveAny(pythonPath); } diff --git a/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts b/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts index 9dd070f9d9e7..8c2ca3d9a7a8 100644 --- a/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts +++ b/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts @@ -76,8 +76,7 @@ export class PyramidLaunchDebugConfigurationProvider implements IDebugConfigurat } } protected resolveVariables(pythonPath: string, resource: Uri | undefined): string { - const workspaceFolder = resource ? this.workspace.getWorkspaceFolder(resource) : undefined; - const systemVariables = new SystemVariables(workspaceFolder ? workspaceFolder.uri.fsPath : undefined); + const systemVariables = new SystemVariables(resource, this.workspace); return systemVariables.resolveAny(pythonPath); } diff --git a/src/test/datascience/datascience.unit.test.ts b/src/test/datascience/datascience.unit.test.ts index 00570203e32d..3145173c3258 100644 --- a/src/test/datascience/datascience.unit.test.ts +++ b/src/test/datascience/datascience.unit.test.ts @@ -9,7 +9,7 @@ import { WorkspaceService } from '../../client/common/application/workspace'; import { IsWindows } from '../../client/common/types'; import { generateCells } from '../../client/datascience/cellFactory'; import { formatStreamText, stripComments } from '../../client/datascience/common'; -import { expandFileVariable } from '../../client/datascience/jupyter/jupyterUtils'; +import { expandWorkingDir } from '../../client/datascience/jupyter/jupyterUtils'; import { InputHistory } from '../../datascience-ui/interactive-common/inputHistory'; // tslint:disable: max-func-body-length @@ -28,7 +28,9 @@ suite('Data Science Tests', () => { }); // tslint:disable: no-invalid-template-strings - test('expanding file variables', async () => { + test('expanding file variables', async function () { + // tslint:disable-next-line: no-invalid-this + this.timeout(10000); const uri = Uri.file('test/bar'); const folder = { index: 0, name: '', uri }; when(workspaceService.hasWorkspaceFolders).thenReturn(true); @@ -38,14 +40,15 @@ suite('Data Science Tests', () => { const relativeFilePath = IsWindows ? '..\\xyz\\bip\\foo.baz' : '../xyz/bip/foo.baz'; const relativeFileDir = IsWindows ? '..\\xyz\\bip' : '../xyz/bip'; - assert.equal(expandFileVariable(undefined, 'bar/foo.baz', inst), 'bar'); - assert.equal(expandFileVariable(undefined, 'bar/bip/foo.baz', inst), 'bar/bip'); - assert.equal(expandFileVariable('${file}', 'bar/bip/foo.baz', inst), 'bar/bip'); - assert.equal(expandFileVariable('${fileDirName}', 'bar/bip/foo.baz', inst), 'bar/bip'); - assert.equal(expandFileVariable('${relativeFile}', 'test/xyz/bip/foo.baz', inst), relativeFilePath); - assert.equal(expandFileVariable('${relativeFileDirname}', 'test/xyz/bip/foo.baz', inst), relativeFileDir); - assert.equal(expandFileVariable('${cwd}', 'test/xyz/bip/foo.baz', inst), process.cwd()); - assert.equal(expandFileVariable('${workspaceFolder}', 'test/xyz/bip/foo.baz', inst), 'test/bar'); + assert.equal(expandWorkingDir(undefined, 'bar/foo.baz', inst), 'bar'); + assert.equal(expandWorkingDir(undefined, 'bar/bip/foo.baz', inst), 'bar/bip'); + assert.equal(expandWorkingDir('${file}', 'bar/bip/foo.baz', inst), 'bar/bip/foo.baz'); + assert.equal(expandWorkingDir('${fileDirname}', 'bar/bip/foo.baz', inst), 'bar/bip'); + assert.equal(expandWorkingDir('${relativeFile}', 'test/xyz/bip/foo.baz', inst), relativeFilePath); + assert.equal(expandWorkingDir('${relativeFileDirname}', 'test/xyz/bip/foo.baz', inst), relativeFileDir); + assert.equal(expandWorkingDir('${cwd}', 'test/xyz/bip/foo.baz', inst), 'test/bar'); + assert.equal(expandWorkingDir('${workspaceFolder}', 'test/xyz/bip/foo.baz', inst), 'test/bar'); + assert.equal(expandWorkingDir('${cwd}-${file}', 'bar/bip/foo.baz', inst), 'test/bar-bar/bip/foo.baz'); }); test('input history', async () => { From 6633a2a21a6624a8d05b220f2787bcb68627573e Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 2 Oct 2019 10:35:00 -0700 Subject: [PATCH 4/6] Fix mistake in config settings with passing in a uri instead of a string --- src/client/common/configSettings.ts | 2 +- .../common/variables/systemVariables.ts | 6 +-- .../configSettings.unit.test.ts | 42 ++++++++++++++++++- 3 files changed, 45 insertions(+), 5 deletions(-) diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 35bb5721a46d..8333aba2259e 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -131,7 +131,7 @@ export class PythonSettings implements IPythonSettings { // tslint:disable-next-line:cyclomatic-complexity max-func-body-length protected update(pythonSettings: WorkspaceConfiguration) { const workspaceRoot = this.workspaceRoot.fsPath; - const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot, this.workspace); + const systemVariables: SystemVariables = new SystemVariables(workspaceRoot, this.workspace); // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion this.pythonPath = systemVariables.resolveAny(pythonSettings.get('pythonPath'))!; diff --git a/src/client/common/variables/systemVariables.ts b/src/client/common/variables/systemVariables.ts index 514416acb146..d264632107b4 100644 --- a/src/client/common/variables/systemVariables.ts +++ b/src/client/common/variables/systemVariables.ts @@ -105,15 +105,15 @@ export class SystemVariables extends AbstractSystemVariables { const workspaceFolder = workspace && (typeof fileOrFolder !== 'string') ? workspace.getWorkspaceFolder(fileOrFolder) : undefined; this._workspaceFolder = workspaceFolder ? workspaceFolder.uri.fsPath : fileOrFolder as string || __dirname; this._workspaceFolderName = Path.basename(this._workspaceFolder); - Object.keys(process.env).forEach(key => { - (this as any as Record)[`env:${key}`] = (this as any as Record)[`env.${key}`] = process.env[key]; - }); this._filePath = (typeof fileOrFolder !== 'string') && fileOrFolder ? fileOrFolder.fsPath : undefined; if (documentManager && documentManager.activeTextEditor) { this._lineNumber = documentManager.activeTextEditor.selection.anchor.line + 1; this._selectedText = documentManager.activeTextEditor.document.getText(new Range(documentManager.activeTextEditor.selection.start, documentManager.activeTextEditor.selection.end)); } this._execPath = process.execPath; + Object.keys(process.env).forEach(key => { + (this as any as Record)[`env:${key}`] = (this as any as Record)[`env.${key}`] = process.env[key]; + }); } public get cwd(): string { diff --git a/src/test/common/configSettings/configSettings.unit.test.ts b/src/test/common/configSettings/configSettings.unit.test.ts index 7bca3adc6a5b..7153912e1c24 100644 --- a/src/test/common/configSettings/configSettings.unit.test.ts +++ b/src/test/common/configSettings/configSettings.unit.test.ts @@ -5,7 +5,7 @@ // tslint:disable:no-any -import { expect } from 'chai'; +import { assert, expect } from 'chai'; import * as path from 'path'; import * as TypeMoq from 'typemoq'; // tslint:disable-next-line:no-require-imports @@ -243,4 +243,44 @@ suite('Python Settings', async () => { } config.verifyAll(); }); + test('File env variables remain in settings', () => { + expected.datascience = { + allowImportFromNotebook: true, + jupyterLaunchTimeout: 20000, + jupyterLaunchRetries: 3, + enabled: true, + jupyterServerURI: 'local', + // tslint:disable-next-line: no-invalid-template-strings + notebookFileRoot: '${FileDirname}', + changeDirOnImportExport: true, + useDefaultConfigForJupyter: true, + jupyterInterruptTimeout: 10000, + searchForJupyter: true, + showCellInputCode: true, + collapseCellInputCodeByDefault: true, + allowInput: true, + maxOutputSize: 400, + errorBackgroundColor: '#FFFFFF', + sendSelectionToInteractiveWindow: false, + showJupyterVariableExplorer: true, + variableExplorerExclude: 'module;function;builtin_function_or_method', + codeRegularExpression: '', + markdownRegularExpression: '', + enablePlotViewer: true, + runStartupCommands: '', + debugJustMyCode: true + }; + expected.pythonPath = 'python3'; + // tslint:disable-next-line:no-any + expected.experiments = { + enabled: false + }; + initializeConfig(expected); + config.setup(c => c.get('experiments')) + .returns(() => expected.experiments) + .verifiable(TypeMoq.Times.once()); + + settings.update(config.object); + assert.equal(expected.datascience.notebookFileRoot, settings.datascience.notebookFileRoot); + }); }); From b96c5fe1abdfc85f4d98a3e96248d9a7add18e94 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 2 Oct 2019 10:38:21 -0700 Subject: [PATCH 5/6] Make arguments more explicit --- .../diagnostics/checks/invalidPythonPathInDebugger.ts | 2 +- src/client/common/configSettings.ts | 2 +- src/client/common/variables/systemVariables.ts | 8 ++++---- src/client/datascience/jupyter/jupyterUtils.ts | 2 +- .../extension/configuration/providers/djangoLaunch.ts | 2 +- .../extension/configuration/providers/pyramidLaunch.ts | 2 +- .../debugger/extension/configuration/resolvers/base.ts | 4 ++-- .../debugger/extension/hooks/childProcessAttachService.ts | 2 +- src/test/common/configSettings.test.ts | 2 +- 9 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts b/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts index 0fc6bcbcc5e5..7d01f35df940 100644 --- a/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts +++ b/src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts @@ -109,7 +109,7 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService await this.messageService.handle(diagnostic, { commandPrompts }); } protected resolveVariables(pythonPath: string, resource: Uri | undefined): string { - const systemVariables = new SystemVariables(resource, this.workspace); + const systemVariables = new SystemVariables(resource, undefined, this.workspace); return systemVariables.resolveAny(pythonPath); } private getCommandPrompts(diagnostic: IDiagnostic): { prompt: string; command?: IDiagnosticCommand }[] { diff --git a/src/client/common/configSettings.ts b/src/client/common/configSettings.ts index 8333aba2259e..baabae7159d5 100644 --- a/src/client/common/configSettings.ts +++ b/src/client/common/configSettings.ts @@ -131,7 +131,7 @@ export class PythonSettings implements IPythonSettings { // tslint:disable-next-line:cyclomatic-complexity max-func-body-length protected update(pythonSettings: WorkspaceConfiguration) { const workspaceRoot = this.workspaceRoot.fsPath; - const systemVariables: SystemVariables = new SystemVariables(workspaceRoot, this.workspace); + const systemVariables: SystemVariables = new SystemVariables(undefined, workspaceRoot, this.workspace); // tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion this.pythonPath = systemVariables.resolveAny(pythonSettings.get('pythonPath'))!; diff --git a/src/client/common/variables/systemVariables.ts b/src/client/common/variables/systemVariables.ts index d264632107b4..6b75adf302f0 100644 --- a/src/client/common/variables/systemVariables.ts +++ b/src/client/common/variables/systemVariables.ts @@ -100,12 +100,12 @@ export class SystemVariables extends AbstractSystemVariables { private _selectedText: string | undefined; private _execPath: string; - constructor(fileOrFolder: Uri | string | undefined, workspace?: IWorkspaceService, documentManager?: IDocumentManager) { + constructor(file: Uri | undefined, rootFolder: string | undefined, workspace?: IWorkspaceService, documentManager?: IDocumentManager) { super(); - const workspaceFolder = workspace && (typeof fileOrFolder !== 'string') ? workspace.getWorkspaceFolder(fileOrFolder) : undefined; - this._workspaceFolder = workspaceFolder ? workspaceFolder.uri.fsPath : fileOrFolder as string || __dirname; + const workspaceFolder = workspace && file ? workspace.getWorkspaceFolder(file) : undefined; + this._workspaceFolder = workspaceFolder ? workspaceFolder.uri.fsPath : rootFolder || __dirname; this._workspaceFolderName = Path.basename(this._workspaceFolder); - this._filePath = (typeof fileOrFolder !== 'string') && fileOrFolder ? fileOrFolder.fsPath : undefined; + this._filePath = file ? file.fsPath : undefined; if (documentManager && documentManager.activeTextEditor) { this._lineNumber = documentManager.activeTextEditor.selection.anchor.line + 1; this._selectedText = documentManager.activeTextEditor.document.getText(new Range(documentManager.activeTextEditor.selection.start, documentManager.activeTextEditor.selection.end)); diff --git a/src/client/datascience/jupyter/jupyterUtils.ts b/src/client/datascience/jupyter/jupyterUtils.ts index 00802dff732a..06980b60859f 100644 --- a/src/client/datascience/jupyter/jupyterUtils.ts +++ b/src/client/datascience/jupyter/jupyterUtils.ts @@ -11,7 +11,7 @@ import { SystemVariables } from '../../common/variables/systemVariables'; export function expandWorkingDir(workingDir: string | undefined, launchingFile: string, workspace: IWorkspaceService): string { if (workingDir) { - const variables = new SystemVariables(Uri.file(launchingFile), workspace); + const variables = new SystemVariables(Uri.file(launchingFile), undefined, workspace); return variables.resolve(workingDir); } diff --git a/src/client/debugger/extension/configuration/providers/djangoLaunch.ts b/src/client/debugger/extension/configuration/providers/djangoLaunch.ts index d9cbb1beec64..236969bba9fe 100644 --- a/src/client/debugger/extension/configuration/providers/djangoLaunch.ts +++ b/src/client/debugger/extension/configuration/providers/djangoLaunch.ts @@ -72,7 +72,7 @@ export class DjangoLaunchDebugConfigurationProvider implements IDebugConfigurati return; } protected resolveVariables(pythonPath: string, resource: Uri | undefined): string { - const systemVariables = new SystemVariables(resource, this.workspace); + const systemVariables = new SystemVariables(resource, undefined, this.workspace); return systemVariables.resolveAny(pythonPath); } diff --git a/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts b/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts index 8c2ca3d9a7a8..60be96c2fefb 100644 --- a/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts +++ b/src/client/debugger/extension/configuration/providers/pyramidLaunch.ts @@ -76,7 +76,7 @@ export class PyramidLaunchDebugConfigurationProvider implements IDebugConfigurat } } protected resolveVariables(pythonPath: string, resource: Uri | undefined): string { - const systemVariables = new SystemVariables(resource, this.workspace); + const systemVariables = new SystemVariables(resource, undefined, this.workspace); return systemVariables.resolveAny(pythonPath); } diff --git a/src/client/debugger/extension/configuration/resolvers/base.ts b/src/client/debugger/extension/configuration/resolvers/base.ts index c93a19efac4e..f0ae10c47558 100644 --- a/src/client/debugger/extension/configuration/resolvers/base.ts +++ b/src/client/debugger/extension/configuration/resolvers/base.ts @@ -65,7 +65,7 @@ export abstract class BaseConfigurationResolver im return; } if (debugConfiguration.envFile && (workspaceFolder || debugConfiguration.cwd)) { - const systemVariables = new SystemVariables((workspaceFolder ? workspaceFolder.fsPath : undefined) || debugConfiguration.cwd); + const systemVariables = new SystemVariables(undefined, (workspaceFolder ? workspaceFolder.fsPath : undefined) || debugConfiguration.cwd); debugConfiguration.envFile = systemVariables.resolveAny(debugConfiguration.envFile); } } @@ -112,7 +112,7 @@ export abstract class BaseConfigurationResolver im ]; } else { // Expand ${workspaceFolder} variable first if necessary. - const systemVariables = new SystemVariables(defaultLocalRoot); + const systemVariables = new SystemVariables(undefined, defaultLocalRoot); pathMappings = pathMappings.map(({ localRoot: mappedLocalRoot, remoteRoot }) => ({ localRoot: systemVariables.resolveAny(mappedLocalRoot), // TODO: Apply to remoteRoot too? diff --git a/src/client/debugger/extension/hooks/childProcessAttachService.ts b/src/client/debugger/extension/hooks/childProcessAttachService.ts index baadce796a15..4795fd644095 100644 --- a/src/client/debugger/extension/hooks/childProcessAttachService.ts +++ b/src/client/debugger/extension/hooks/childProcessAttachService.ts @@ -73,7 +73,7 @@ export class ChildProcessAttachService implements IChildProcessAttachService { // We cannot expect the debugger to assume remote root is the same as the cwd, // As debugger doesn't necessarily know whether the process being attached to is // a child process or not. - const systemVariables = new SystemVariables(config.workspaceFolder); + const systemVariables = new SystemVariables(undefined, config.workspaceFolder); const localRoot = config.cwd && config.cwd.length > 0 ? systemVariables.resolveAny(config.cwd) : config.workspaceFolder; config.pathMappings = [ { remoteRoot: '.', localRoot } diff --git a/src/test/common/configSettings.test.ts b/src/test/common/configSettings.test.ts index 066ed1613a56..cf05e838bcaa 100644 --- a/src/test/common/configSettings.test.ts +++ b/src/test/common/configSettings.test.ts @@ -14,7 +14,7 @@ suite('Configuration Settings', () => { setup(initialize); test('Check Values', done => { - const systemVariables: SystemVariables = new SystemVariables(workspaceRoot); + const systemVariables: SystemVariables = new SystemVariables(undefined, workspaceRoot); // tslint:disable-next-line:no-any const pythonConfig = vscode.workspace.getConfiguration('python', null as any as vscode.Uri); const pythonSettings = getExtensionSettings(vscode.Uri.file(workspaceRoot)); From 8d9d385c7ba0fb2823e0881555f775249e753b72 Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Wed, 2 Oct 2019 10:58:47 -0700 Subject: [PATCH 6/6] Fix IS_WINDOWS to work on unit tests --- src/test/datascience/datascience.unit.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/datascience/datascience.unit.test.ts b/src/test/datascience/datascience.unit.test.ts index 3145173c3258..38cbfaa36550 100644 --- a/src/test/datascience/datascience.unit.test.ts +++ b/src/test/datascience/datascience.unit.test.ts @@ -6,7 +6,7 @@ import { anything, instance, mock, when } from 'ts-mockito'; import { Uri } from 'vscode'; import { WorkspaceService } from '../../client/common/application/workspace'; -import { IsWindows } from '../../client/common/types'; +import { IS_WINDOWS } from '../../client/common/platform/constants'; import { generateCells } from '../../client/datascience/cellFactory'; import { formatStreamText, stripComments } from '../../client/datascience/common'; import { expandWorkingDir } from '../../client/datascience/jupyter/jupyterUtils'; @@ -37,8 +37,8 @@ suite('Data Science Tests', () => { when(workspaceService.workspaceFolders).thenReturn([folder]); when(workspaceService.getWorkspaceFolder(anything())).thenReturn(folder); const inst = instance(workspaceService); - const relativeFilePath = IsWindows ? '..\\xyz\\bip\\foo.baz' : '../xyz/bip/foo.baz'; - const relativeFileDir = IsWindows ? '..\\xyz\\bip' : '../xyz/bip'; + const relativeFilePath = IS_WINDOWS ? '..\\xyz\\bip\\foo.baz' : '../xyz/bip/foo.baz'; + const relativeFileDir = IS_WINDOWS ? '..\\xyz\\bip' : '../xyz/bip'; assert.equal(expandWorkingDir(undefined, 'bar/foo.baz', inst), 'bar'); assert.equal(expandWorkingDir(undefined, 'bar/bip/foo.baz', inst), 'bar/bip');