Skip to content

Commit ec2f179

Browse files
rchiodorchiodo
authored
Add capability to support current file path for notebook root (#7724)
* Add capability to support current file path for notebook root * Put launch.json back * Use system variables instead of creating separate resolver * Fix mistake in config settings with passing in a uri instead of a string * Make arguments more explicit * Fix IS_WINDOWS to work on unit tests
1 parent e0f29f6 commit ec2f179

24 files changed

Lines changed: 235 additions & 55 deletions

File tree

‎news/1 Enhancements/4441.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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.

‎news/2 Fixes/7688.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
When there's no workspace open, use the directory of the opened file as the root directory for a jupyter session.

‎src/client/application/diagnostics/checks/invalidPythonPathInDebugger.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ export class InvalidPythonPathInDebuggerService extends BaseDiagnosticsService
109109
await this.messageService.handle(diagnostic, { commandPrompts });
110110
}
111111
protected resolveVariables(pythonPath: string, resource: Uri | undefined): string {
112-
const workspaceFolder = resource ? this.workspace.getWorkspaceFolder(resource) : undefined;
113-
const systemVariables = new SystemVariables(workspaceFolder ? workspaceFolder.uri.fsPath : undefined);
112+
const systemVariables = new SystemVariables(resource, undefined, this.workspace);
114113
return systemVariables.resolveAny(pythonPath);
115114
}
116115
private getCommandPrompts(diagnostic: IDiagnostic): { prompt: string; command?: IDiagnosticCommand }[] {

‎src/client/common/configSettings.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ export class PythonSettings implements IPythonSettings {
131131
// tslint:disable-next-line:cyclomatic-complexity max-func-body-length
132132
protected update(pythonSettings: WorkspaceConfiguration) {
133133
const workspaceRoot = this.workspaceRoot.fsPath;
134-
const systemVariables: SystemVariables = new SystemVariables(this.workspaceRoot ? this.workspaceRoot.fsPath : undefined);
134+
const systemVariables: SystemVariables = new SystemVariables(undefined, workspaceRoot, this.workspace);
135135

136136
// tslint:disable-next-line:no-backbone-get-set-outside-model no-non-null-assertion
137137
this.pythonPath = systemVariables.resolveAny(pythonSettings.get<string>('pythonPath'))!;

‎src/client/common/variables/systemVariables.ts‎

Lines changed: 58 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,17 @@
22
* Copyright (c) Microsoft Corporation. All rights reserved.
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
5-
65
'use strict';
7-
86
import * as Path from 'path';
7+
import { Range, Uri } from 'vscode';
8+
9+
import { IDocumentManager, IWorkspaceService } from '../application/types';
910
import * as Types from '../utils/sysTypes';
1011
import { IStringDictionary, ISystemVariables } from './types';
12+
1113
/* tslint:disable:rule1 no-any no-unnecessary-callback-wrapper jsdoc-format no-for-in prefer-const no-increment-decrement */
1214

13-
export abstract class AbstractSystemVariables implements ISystemVariables {
15+
abstract class AbstractSystemVariables implements ISystemVariables {
1416

1517
public resolve(value: string): string;
1618
public resolve(value: string[]): string[];
@@ -93,11 +95,22 @@ export abstract class AbstractSystemVariables implements ISystemVariables {
9395
export class SystemVariables extends AbstractSystemVariables {
9496
private _workspaceFolder: string;
9597
private _workspaceFolderName: string;
98+
private _filePath: string | undefined;
99+
private _lineNumber: number | undefined;
100+
private _selectedText: string | undefined;
101+
private _execPath: string;
96102

97-
constructor(workspaceFolder?: string) {
103+
constructor(file: Uri | undefined, rootFolder: string | undefined, workspace?: IWorkspaceService, documentManager?: IDocumentManager) {
98104
super();
99-
this._workspaceFolder = typeof workspaceFolder === 'string' ? workspaceFolder : __dirname;
105+
const workspaceFolder = workspace && file ? workspace.getWorkspaceFolder(file) : undefined;
106+
this._workspaceFolder = workspaceFolder ? workspaceFolder.uri.fsPath : rootFolder || __dirname;
100107
this._workspaceFolderName = Path.basename(this._workspaceFolder);
108+
this._filePath = file ? file.fsPath : undefined;
109+
if (documentManager && documentManager.activeTextEditor) {
110+
this._lineNumber = documentManager.activeTextEditor.selection.anchor.line + 1;
111+
this._selectedText = documentManager.activeTextEditor.document.getText(new Range(documentManager.activeTextEditor.selection.start, documentManager.activeTextEditor.selection.end));
112+
}
113+
this._execPath = process.execPath;
101114
Object.keys(process.env).forEach(key => {
102115
(this as any as Record<string, string | undefined>)[`env:${key}`] = (this as any as Record<string, string | undefined>)[`env.${key}`] = process.env[key];
103116
});
@@ -122,4 +135,44 @@ export class SystemVariables extends AbstractSystemVariables {
122135
public get workspaceFolderBasename(): string {
123136
return this._workspaceFolderName;
124137
}
138+
139+
public get file(): string | undefined {
140+
return this._filePath;
141+
}
142+
143+
public get relativeFile(): string | undefined {
144+
return this.file ? Path.relative(this._workspaceFolder, this.file) : undefined;
145+
}
146+
147+
public get relativeFileDirname(): string | undefined {
148+
return this.relativeFile ? Path.dirname(this.relativeFile) : undefined;
149+
}
150+
151+
public get fileBasename(): string | undefined {
152+
return this.file ? Path.basename(this.file) : undefined;
153+
}
154+
155+
public get fileBasenameNoExtension(): string | undefined {
156+
return this.file ? Path.parse(this.file).name : undefined;
157+
}
158+
159+
public get fileDirname(): string | undefined {
160+
return this.file ? Path.dirname(this.file) : undefined;
161+
}
162+
163+
public get fileExtname(): string | undefined {
164+
return this.file ? Path.extname(this.file) : undefined;
165+
}
166+
167+
public get lineNumber(): number | undefined {
168+
return this._lineNumber;
169+
}
170+
171+
public get selectedText(): string | undefined {
172+
return this._selectedText;
173+
}
174+
175+
public get execPath(): string {
176+
return this._execPath;
177+
}
125178
}

‎src/client/datascience/interactive-common/interactiveBase.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -443,6 +443,12 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
443443
}
444444
}
445445

446+
protected async setLaunchingFile(file: string): Promise<void> {
447+
if (file !== Identifiers.EmptyFileName && this.notebook) {
448+
await this.notebook.setLaunchingFile(file);
449+
}
450+
}
451+
446452
protected getNotebook(): INotebook | undefined {
447453
return this.notebook;
448454
}
@@ -493,9 +499,7 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
493499
if (this.notebook) {
494500
// Before we try to execute code make sure that we have an initial directory set
495501
// Normally set via the workspace, but we might not have one here if loading a single loose file
496-
if (file !== Identifiers.EmptyFileName) {
497-
await this.notebook.setInitialDirectory(path.dirname(file));
498-
}
502+
await this.setLaunchingFile(file);
499503

500504
if (debug) {
501505
// Attach our debugger

‎src/client/datascience/interactive-ipynb/nativeEditor.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,14 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
294294
return this._file;
295295
}
296296

297+
protected async setLaunchingFile(_file: string): Promise<void> {
298+
// For the native editor, use our own file as the path
299+
const notebook = this.getNotebook();
300+
if (this.fileSystem.fileExists(this.file.fsPath) && notebook) {
301+
await notebook.setLaunchingFile(this.file.fsPath);
302+
}
303+
}
304+
297305
protected sendCellsToWebView(cells: ICell[]) {
298306
// Filter out sysinfo messages. Don't want to show those
299307
const filtered = cells.filter(c => c.data.cell_type !== 'messages');

‎src/client/datascience/jupyter/jupyterNotebook.ts‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import * as uuid from 'uuid/v4';
1212
import { Disposable, Event, EventEmitter, Uri } from 'vscode';
1313
import { CancellationToken } from 'vscode-jsonrpc';
1414

15-
import { ILiveShareApi } from '../../common/application/types';
15+
import { ILiveShareApi, IWorkspaceService } from '../../common/application/types';
1616
import { Cancellation, CancellationError } from '../../common/cancellation';
1717
import { traceError, traceInfo, traceWarning } from '../../common/logger';
1818
import { IConfigurationService, IDisposableRegistry } from '../../common/types';
@@ -36,6 +36,7 @@ import {
3636
INotebookServerLaunchInfo,
3737
InterruptResult
3838
} from '../types';
39+
import { expandWorkingDir } from './jupyterUtils';
3940

4041
class CellSubscriber {
4142
private deferred: Deferred<CellState> = createDeferred<CellState>();
@@ -139,6 +140,7 @@ export class JupyterNotebookBase implements INotebook {
139140
private ranInitialSetup = false;
140141
private _resource: Uri;
141142
private _disposed: boolean = false;
143+
private _workingDirectory: string | undefined;
142144

143145
constructor(
144146
_liveShare: ILiveShareApi, // This is so the liveshare mixin works
@@ -149,7 +151,8 @@ export class JupyterNotebookBase implements INotebook {
149151
private launchInfo: INotebookServerLaunchInfo,
150152
private loggers: INotebookExecutionLogger[],
151153
resource: Uri,
152-
private getDisposedError: () => Error
154+
private getDisposedError: () => Error,
155+
private workspace: IWorkspaceService
153156
) {
154157
this.sessionStartTime = Date.now();
155158
this._resource = resource;
@@ -183,13 +186,11 @@ export class JupyterNotebookBase implements INotebook {
183186
return;
184187
}
185188
this.ranInitialSetup = true;
189+
this._workingDirectory = undefined;
186190

187191
try {
188192
// When we start our notebook initial, change to our workspace or user specified root directory
189-
if (this.launchInfo && this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) {
190-
traceInfo(`Changing directory for ${this.resource.toString()}`);
191-
await this.changeDirectoryIfPossible(this.launchInfo.workingDir);
192-
}
193+
await this.updateWorkingDirectory();
193194

194195
const settings = this.configService.getSettings().datascience;
195196
const matplobInit = !settings || settings.enablePlotViewer ? CodeSnippits.MatplotLibInitSvg : CodeSnippits.MatplotLibInitPng;
@@ -249,12 +250,9 @@ export class JupyterNotebookBase implements INotebook {
249250
return deferred.promise;
250251
}
251252

252-
public async setInitialDirectory(directory: string): Promise<void> {
253-
// If we launched local and have no working directory call this on add code to change directory
254-
if (this.launchInfo && !this.launchInfo.workingDir && this.launchInfo.connectionInfo.localLaunch) {
255-
await this.changeDirectoryIfPossible(directory);
256-
this.launchInfo.workingDir = directory;
257-
}
253+
public setLaunchingFile(file: string): Promise<void> {
254+
// Update our working directory if we don't have one set already
255+
return this.updateWorkingDirectory(file);
258256
}
259257

260258
public executeObservable(code: string, file: string, line: number, id: string, silent: boolean = false): Observable<ICell[]> {
@@ -587,6 +585,24 @@ export class JupyterNotebookBase implements INotebook {
587585
});
588586
}
589587

588+
private async updateWorkingDirectory(launchingFile?: string): Promise<void> {
589+
if (this.launchInfo && this.launchInfo.connectionInfo.localLaunch && !this._workingDirectory) {
590+
// See what our working dir is supposed to be
591+
const suggested = this.launchInfo.workingDir;
592+
if (suggested && await fs.pathExists(suggested)) {
593+
// We should use the launch info directory. It trumps the possible dir
594+
this._workingDirectory = suggested;
595+
return this.changeDirectoryIfPossible(this._workingDirectory);
596+
} else if (launchingFile && await fs.pathExists(launchingFile)) {
597+
// Combine the working directory with this file if possible.
598+
this._workingDirectory = expandWorkingDir(this.launchInfo.workingDir, launchingFile, this.workspace);
599+
if (this._workingDirectory) {
600+
return this.changeDirectoryIfPossible(this._workingDirectory);
601+
}
602+
}
603+
}
604+
}
605+
590606
private changeDirectoryIfPossible = async (directory: string): Promise<void> => {
591607
if (this.launchInfo && this.launchInfo.connectionInfo.localLaunch && await fs.pathExists(directory)) {
592608
await this.executeSilently(`%cd "${directory}"`);

‎src/client/datascience/jupyter/jupyterServerFactory.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { Uri } from 'vscode';
99
import { CancellationToken } from 'vscode-jsonrpc';
1010
import * as vsls from 'vsls/vscode';
1111

12-
import { ILiveShareApi } from '../../common/application/types';
12+
import { ILiveShareApi, IWorkspaceService } from '../../common/application/types';
1313
import { IAsyncDisposableRegistry, IConfigurationService, IDisposableRegistry } from '../../common/types';
1414
import {
1515
IConnection,
@@ -36,6 +36,7 @@ type JupyterServerClassType = {
3636
disposableRegistry: IDisposableRegistry,
3737
configService: IConfigurationService,
3838
sessionManager: IJupyterSessionManagerFactory,
39+
workspaceService: IWorkspaceService,
3940
loggers: INotebookExecutionLogger[]
4041
): IJupyterServerInterface;
4142
};
@@ -55,6 +56,7 @@ export class JupyterServerFactory implements INotebookServer, ILiveShareHasRole
5556
@inject(IAsyncDisposableRegistry) asyncRegistry: IAsyncDisposableRegistry,
5657
@inject(IConfigurationService) configService: IConfigurationService,
5758
@inject(IJupyterSessionManagerFactory) sessionManager: IJupyterSessionManagerFactory,
59+
@inject(IWorkspaceService) workspaceService: IWorkspaceService,
5860
@multiInject(INotebookExecutionLogger) @optional() loggers: INotebookExecutionLogger[] | undefined) {
5961
this.serverFactory = new RoleBasedFactory<IJupyterServerInterface, JupyterServerClassType>(
6062
liveShare,
@@ -66,6 +68,7 @@ export class JupyterServerFactory implements INotebookServer, ILiveShareHasRole
6668
disposableRegistry,
6769
configService,
6870
sessionManager,
71+
workspaceService,
6972
loggers ? loggers : []
7073
);
7174
}

‎src/client/datascience/jupyter/jupyterUtils.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
'use strict';
4+
import '../../common/extensions';
5+
6+
import * as path from 'path';
7+
import { Uri } from 'vscode';
8+
9+
import { IWorkspaceService } from '../../common/application/types';
10+
import { SystemVariables } from '../../common/variables/systemVariables';
11+
12+
export function expandWorkingDir(workingDir: string | undefined, launchingFile: string, workspace: IWorkspaceService): string {
13+
if (workingDir) {
14+
const variables = new SystemVariables(Uri.file(launchingFile), undefined, workspace);
15+
return variables.resolve(workingDir);
16+
}
17+
18+
// No working dir, just use the path of the launching file.
19+
return path.dirname(launchingFile);
20+
}

0 commit comments

Comments
 (0)