Skip to content

Commit d56dbf3

Browse files
austin3dickeyaustin3dickey
authored
Add notebook.workingDirectory setting (#8558)
<!-- Thank you for submitting a pull request. If this is your first pull request you can find information about contributing here: * https://github.com/posit-dev/positron/blob/main/CONTRIBUTING.md We recommend synchronizing your branch with the latest changes in the main branch by either pulling or rebasing. --> <!-- Describe briefly what problem this pull request resolves, or what new feature it introduces. Include screenshots of any new or altered UI. Link to any GitHub issues but avoid "magic" keywords that will automatically close the issue. If there are any details about your approach that are unintuitive or you want to draw attention to, please describe them here. --> Addresses #7988. Adds a `notebook.workingDirectory` setting and hides the `jupyter.notebookFileRoot` setting, which doesn't work in Positron. ### Release Notes <!-- Optionally, replace `N/A` with text to be included in the next release notes. The `N/A` bullets are ignored. If you refer to one or more Positron issues, these issues are used to collect information about the feature or bugfix, such as the relevant language pack as determined by Github labels of type `lang: `. The note will automatically be tagged with the language. These notes are typically filled by the Positron team. If you are an external contributor, you may ignore this section. --> #### New Features - Added `notebook.workingDirectory` setting, which can be used to set the default working directory for notebooks (#7988) #### Bug Fixes - N/A ### QA Notes <!-- Positron team members: please add relevant e2e test tags, so the tests can be run when you open this pull request. - Instructions: https://github.com/posit-dev/positron/blob/main/test/e2e/README.md#pull-requests-and-test-tags - Available tags: https://github.com/posit-dev/positron/blob/main/test/e2e/infra/test-runner/test-tags.ts --> @:notebooks @:vscode-settings @:console @:extensions @:interpreter @:win I added some unit tests and e2e tests! <!-- Add additional information for QA on how to validate the change, paying special attention to the level of risk, adjacent areas that could be affected by the change, and any important contextual information not present in the linked issues. --> Some things to try: - The `jupyter.notebookFileRoot` setting should no longer exist - Open a workspace, and a new notebook in a subdirectory of that workspace, with a cell containing `%pwd`, and see its value for different values of the new setting (note you need to close/open the notebook for the change to take effect): - (unset) - `${workspaceFolder}` - `${fileDirname}` - `${userHome}` - `/some/path/that/exists` - `/some/path/that/doesnt/exist` - whatever else you want to try (variables are defined [here](https://github.com/posit-dev/positron/blob/adcc385b17e3246c5439ea23c01eaf551b57403d/src/vs/workbench/services/configurationResolver/common/configurationResolver.ts#L77)) - The Console and Terminal working directories shouldn't change - Filename completions should correspond to the correct working directory - This should work for R (`getwd()`) or Python (`os.getcwd()`) notebooks, and for VSC or Positron notebooks
1 parent 2c589dd commit d56dbf3

File tree

15 files changed

+410
-36
lines changed

15 files changed

+410
-36
lines changed

‎extensions/positron-python/python_files/posit/positron/positron_jedilsp.py‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#
2-
# Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
2+
# Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
33
# Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
#
55

@@ -11,7 +11,6 @@
1111
import threading
1212
import warnings
1313
from functools import lru_cache
14-
from pathlib import Path
1514
from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, Type, Union, cast
1615

1716
from comm.base_comm import BaseComm
@@ -248,7 +247,7 @@ class HelpTopicRequest:
248247
class PositronInitializationOptions:
249248
"""Positron-specific language server initialization options."""
250249

251-
notebook_path: Optional[Path] = attrs.field(default=None)
250+
working_directory: Optional[str] = attrs.field(default=None)
252251

253252

254253
class PositronJediLanguageServerProtocol(JediLanguageServerProtocol):
@@ -285,10 +284,7 @@ def lsp_initialize(self, params: InitializeParams) -> InitializeResult:
285284
server.show_message_log(msg, msg_type=MessageType.Error)
286285
initialization_options = PositronInitializationOptions()
287286

288-
# If a notebook path was provided, set the project path to the notebook's parent.
289-
# See https://github.com/posit-dev/positron/issues/5948.
290-
notebook_path = initialization_options.notebook_path
291-
path = notebook_path.parent if notebook_path else self._server.workspace.root_path
287+
path = initialization_options.working_directory or self._server.workspace.root_path
292288

293289
# Create the Jedi Project.
294290
# Note that this overwrites a Project already created in the parent class.

‎extensions/positron-python/python_files/posit/positron/tests/test_positron_jedilsp.py‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#
2-
# Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
2+
# Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
33
# Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
#
55

@@ -103,7 +103,7 @@ def _reduce_debounce_time(monkeypatch):
103103
def create_server(
104104
namespace: Optional[Dict[str, Any]] = None,
105105
root_path: Optional[Path] = None,
106-
notebook_path: Optional[Path] = None,
106+
working_directory: Optional[str] = None,
107107
) -> PositronJediLanguageServer:
108108
# Create a server.
109109
server = create_positron_server()
@@ -128,7 +128,7 @@ def create_server(
128128
# to test deserialization too.
129129
"positron": cattrs.unstructure(
130130
PositronInitializationOptions(
131-
notebook_path=notebook_path,
131+
working_directory=working_directory,
132132
)
133133
),
134134
},
@@ -476,7 +476,7 @@ def assert_has_path_completion(
476476
expected_completion: str,
477477
chars_from_end=1,
478478
root_path: Optional[Path] = None,
479-
notebook_path: Optional[Path] = None,
479+
working_directory: Optional[str] = None,
480480
):
481481
# Replace separators for testing cross-platform.
482482
source = source.replace("/", os.path.sep)
@@ -486,7 +486,7 @@ def assert_has_path_completion(
486486
if os.name == "nt":
487487
expected_completion = expected_completion.replace("/", "\\" + os.path.sep)
488488

489-
server = create_server(root_path=root_path, notebook_path=notebook_path)
489+
server = create_server(root_path=root_path, working_directory=working_directory)
490490
text_document = create_text_document(server, TEST_DOCUMENT_URI, source)
491491
character = len(source) - chars_from_end
492492
completions = _completions(server, text_document, character)
@@ -721,14 +721,31 @@ def test_notebook_path_completions(tmp_path) -> None:
721721
notebook_parent = tmp_path / "notebooks"
722722
notebook_parent.mkdir()
723723

724-
notebook_path = notebook_parent / "notebook.ipynb"
725-
726724
# Create a file in the notebook's parent.
727725
file_to_complete = notebook_parent / "data.csv"
728726
file_to_complete.write_text("")
729727

730728
assert_has_path_completion(
731-
'""', file_to_complete.name, root_path=tmp_path, notebook_path=notebook_path
729+
'""', file_to_complete.name, root_path=tmp_path, working_directory=str(notebook_parent)
730+
)
731+
732+
733+
def test_notebook_path_completions_different_wd(tmp_path) -> None:
734+
notebook_parent = tmp_path / "notebooks"
735+
notebook_parent.mkdir()
736+
737+
# Make a different working directory.
738+
working_directory = tmp_path / "different-working-directory"
739+
working_directory.mkdir()
740+
741+
# Create files in the notebook's parent and the working directory.
742+
bad_file = notebook_parent / "bad-data.csv"
743+
bad_file.write_text("")
744+
good_file = working_directory / "good-data.csv"
745+
good_file.write_text("")
746+
747+
assert_has_path_completion(
748+
'""', good_file.name, root_path=tmp_path, working_directory=working_directory
732749
)
733750

734751

‎extensions/positron-python/src/client/positron/lsp.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*---------------------------------------------------------------------------------------------
2-
* Copyright (C) 2023-2024 Posit Software, PBC. All rights reserved.
2+
* Copyright (C) 2023-2025 Posit Software, PBC. All rights reserved.
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55
import * as vscode from 'vscode';
@@ -90,7 +90,7 @@ export class PythonLsp implements vscode.Disposable {
9090
return out.promise;
9191
};
9292

93-
const { notebookUri } = this._metadata;
93+
const { notebookUri, workingDirectory } = this._metadata;
9494

9595
// If this client belongs to a notebook, set the document selector to only include that notebook.
9696
// Otherwise, this is the main client for this language, so set the document selector to include
@@ -127,7 +127,7 @@ export class PythonLsp implements vscode.Disposable {
127127
// If this server is for a notebook, set the notebook path option.
128128
if (notebookUri) {
129129
this._clientOptions.initializationOptions.positron = {
130-
notebook_path: notebookUri.fsPath,
130+
working_directory: workingDirectory,
131131
};
132132
}
133133

‎extensions/positron-supervisor/src/KallichoreSession.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -330,19 +330,10 @@ export class KallichoreSession implements JupyterLanguageRuntimeSession {
330330
this._kernelSpec = kernelSpec;
331331
const varActions = await this.buildEnvVarActions(false);
332332

333-
// Prepare the working directory; use the workspace root if available,
334-
// otherwise the home directory
335-
let workingDir = vscode.workspace.workspaceFolders?.[0].uri.fsPath || os.homedir();
336-
337-
// If we have a notebook URI, use its parent directory as the working
338-
// directory instead. Note that not all notebooks have valid on-disk
339-
// URIs since they may be transient or not yet saved; for these, we fall
340-
// back to the workspace root or home directory.
341-
if (this.metadata.notebookUri?.fsPath) {
342-
const notebookPath = this.metadata.notebookUri.fsPath;
343-
if (fs.existsSync(notebookPath)) {
344-
workingDir = path.dirname(notebookPath);
345-
}
333+
let workingDir = this.metadata.workingDirectory;
334+
if (!workingDir) {
335+
// Use the workspace root if available, otherwise the home directory
336+
workingDir = vscode.workspace.workspaceFolders?.[0].uri.fsPath || os.homedir();
346337
}
347338

348339
// Form the command-line arguments to the kernel process

‎src/positron-dts/positron.d.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,9 @@ declare module 'positron' {
540540

541541
/** The URI of the notebook document associated with the session, if any */
542542
readonly notebookUri?: vscode.Uri;
543+
544+
/** The starting working directory of the session, if any */
545+
readonly workingDirectory?: string;
543546
}
544547

545548
/**

‎src/vs/workbench/api/common/configurationExtensionPoint.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ const IGNORED_JUPYTER_CONFIGURATION_PROPERTIES = new Set([
4444
'jupyter.interactiveWindow.textEditor.executeSelection',
4545
'jupyter.interactiveWindow.textEditor.magicCommandsAsComments',
4646
'jupyter.interactiveWindow.viewColumn',
47+
'jupyter.notebookFileRoot',
4748
]);
4849
// --- End Positron ---
4950
const jsonRegistry = Registry.as<IJSONContributionRegistry>(JSONExtensions.JSONContribution);

‎src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@ import { IModelService } from '../../../../editor/common/services/model.js';
1515
import { ILanguageSelection, ILanguageService } from '../../../../editor/common/languages/language.js';
1616
import { ITextModelContentProvider, ITextModelService } from '../../../../editor/common/services/resolverService.js';
1717
import * as nls from '../../../../nls.js';
18+
// --- Start Positron ---
19+
// eslint-disable-next-line no-duplicate-imports
20+
import { ConfigurationScope } from '../../../../platform/configuration/common/configurationRegistry.js';
21+
// --- End Positron ---
1822
import { Extensions, IConfigurationPropertySchema, IConfigurationRegistry } from '../../../../platform/configuration/common/configurationRegistry.js';
1923
import { SyncDescriptor } from '../../../../platform/instantiation/common/descriptors.js';
2024
import { InstantiationType, registerSingleton } from '../../../../platform/instantiation/common/extensions.js';
@@ -1311,6 +1315,14 @@ configurationRegistry.registerConfiguration({
13111315
type: 'string',
13121316
default: '',
13131317
tags: ['notebookLayout']
1318+
},
1319+
// --- Start Positron ---
1320+
[NotebookSetting.workingDirectory]: {
1321+
markdownDescription: nls.localize('notebook.workingDirectory', "Default working directory for notebook kernels. Supports [variables](https://code.visualstudio.com/docs/reference/variables-reference) like `${workspaceFolder}`. If this setting doesn't resolve to an existing directory, it defaults to the notebook file's directory. Any change to this setting will apply to future opened notebooks."),
1322+
type: 'string',
1323+
default: '',
1324+
scope: ConfigurationScope.RESOURCE
13141325
}
1326+
// --- End Positron ---
13151327
}
13161328
});

‎src/vs/workbench/contrib/notebook/common/notebookCommon.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,9 @@ export const NotebookSetting = {
10651065
outputBackupSizeLimit: 'notebook.backup.sizeLimit',
10661066
multiCursor: 'notebook.multiCursor.enabled',
10671067
markupFontFamily: 'notebook.markup.fontFamily',
1068+
// --- Start Positron ---
1069+
workingDirectory: 'notebook.workingDirectory',
1070+
// --- End Positron ---
10681071
} as const;
10691072

10701073
export const enum CellStatusbarAlignment {

‎src/vs/workbench/services/runtimeSession/common/runtimeSession.ts‎

Lines changed: 82 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ import { INotificationService, Severity } from '../../../../platform/notificatio
2727
import { localize } from '../../../../nls.js';
2828
import { UiClientInstance } from '../../languageRuntime/common/languageRuntimeUiClient.js';
2929
import { IWorkbenchEnvironmentService } from '../../environment/common/environmentService.js';
30+
import { IConfigurationResolverService } from '../../configurationResolver/common/configurationResolver.js';
31+
import { IWorkspaceContextService } from '../../../../platform/workspace/common/workspace.js';
32+
import { NotebookSetting } from '../../../contrib/notebook/common/notebookCommon.js';
33+
import { IFileService } from '../../../../platform/files/common/files.js';
34+
import { dirname } from '../../../../base/common/path.js';
3035

3136
/**
3237
* The maximum number of active sessions a user can have running at a time.
@@ -170,7 +175,10 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
170175
@IExtensionService private readonly _extensionService: IExtensionService,
171176
@IStorageService private readonly _storageService: IStorageService,
172177
@IUpdateService private readonly _updateService: IUpdateService,
173-
@IWorkbenchEnvironmentService private readonly _environmentService: IWorkbenchEnvironmentService
178+
@IWorkbenchEnvironmentService private readonly _environmentService: IWorkbenchEnvironmentService,
179+
@IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService,
180+
@IWorkspaceContextService private readonly _workspaceContextService: IWorkspaceContextService,
181+
@IFileService private readonly _fileService: IFileService,
174182
) {
175183

176184
super();
@@ -387,6 +395,70 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
387395
return Array.from(this._activeSessionsBySessionId.values());
388396
}
389397

398+
/**
399+
* Validates that a path is an existing directory.
400+
*
401+
* @param path The path to validate
402+
* @returns A promise that resolves to true if the path is a directory and exists,
403+
* false otherwise.
404+
*/
405+
private async isValidDirectory(path: string): Promise<boolean> {
406+
try {
407+
const stat = await this._fileService.stat(URI.file(path));
408+
if (!stat.isDirectory) {
409+
this._logService.warn(`${NotebookSetting.workingDirectory}: Path '${path}' exists but is not a directory`);
410+
return false;
411+
}
412+
return true;
413+
} catch (error) {
414+
this._logService.warn(`${NotebookSetting.workingDirectory}: Path '${path}' does not exist or is not accessible:`, error);
415+
return false;
416+
}
417+
}
418+
419+
/**
420+
* Resolves the working directory for a notebook based on its URI and the setting.
421+
* If the setting doesn't resolve to an existing directory, use the notebook's directory.
422+
* If the directory doesn't exist, return undefined.
423+
*
424+
* @param notebookUri The URI of the notebook
425+
* @returns The resolved working directory or undefined if it doesn't exist
426+
*/
427+
private async resolveNotebookWorkingDirectory(notebookUri: URI): Promise<string | undefined> {
428+
// The default value is the notebook's parent directory, if it exists.
429+
let defaultValue: string | undefined;
430+
const notebookParent = dirname(notebookUri.fsPath);
431+
if (await this.isValidDirectory(notebookParent)) {
432+
defaultValue = notebookParent;
433+
}
434+
435+
const configValue = this._configurationService.getValue<string>(
436+
NotebookSetting.workingDirectory, { resource: notebookUri }
437+
);
438+
if (!configValue || configValue.trim() === '') {
439+
return defaultValue;
440+
}
441+
const workspaceFolder = this._workspaceContextService.getWorkspaceFolder(notebookUri);
442+
443+
// Resolve the variables in the setting
444+
let resolvedValue: string;
445+
try {
446+
resolvedValue = await this._configurationResolverService.resolveAsync(
447+
workspaceFolder || undefined, configValue
448+
);
449+
} catch (error) {
450+
this._logService.warn(`${NotebookSetting.workingDirectory}: Failed to resolve variables in '${configValue}':`, error);
451+
return defaultValue;
452+
}
453+
454+
// Check if the result is a directory that exists
455+
if (await this.isValidDirectory(resolvedValue)) {
456+
return resolvedValue;
457+
} else {
458+
return defaultValue;
459+
}
460+
}
461+
390462
/**
391463
* Select a session for the provided runtime.
392464
*
@@ -1535,10 +1607,18 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
15351607
}
15361608

15371609
const sessionId = this.generateNewSessionId(runtimeMetadata, sessionMode === LanguageRuntimeSessionMode.Notebook);
1610+
1611+
// Resolve the working directory configuration
1612+
let workingDirectory: string | undefined;
1613+
if (notebookUri) {
1614+
workingDirectory = await this.resolveNotebookWorkingDirectory(notebookUri);
1615+
}
1616+
15381617
const sessionMetadata: IRuntimeSessionMetadata = {
15391618
sessionId,
15401619
sessionMode,
15411620
notebookUri,
1621+
workingDirectory,
15421622
createdTimestamp: Date.now(),
15431623
startReason: source
15441624
};
@@ -1645,7 +1725,7 @@ export class RuntimeSessionService extends Disposable implements IRuntimeSession
16451725
* @param runtime The runtime to get the manager for.
16461726
* @returns The session manager that manages the runtime.
16471727
*
1648-
* Throws an errror if no session manager is found for the runtime.
1728+
* Throws an error if no session manager is found for the runtime.
16491729
*/
16501730
private async getManagerForRuntime(runtime: ILanguageRuntimeMetadata): Promise<ILanguageRuntimeSessionManager> {
16511731
// Look for the session manager that manages the runtime.

‎src/vs/workbench/services/runtimeSession/common/runtimeSessionService.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,9 @@ export interface IRuntimeSessionMetadata {
8585
/** The notebook associated with the session, if any */
8686
readonly notebookUri: URI | undefined;
8787

88+
/** The starting working directory of the session, if any */
89+
readonly workingDirectory?: string;
90+
8891
/**
8992
* A timestamp (in milliseconds since the Epoch) representing the time at
9093
* which the runtime session was created.

0 commit comments

Comments
 (0)