Skip to content

Commit 79ec05c

Browse files
authored
Merge pull request microsoft#187189 from microsoft/rebornix/excessive-cheetah
Register the notebook remote save setting
2 parents 30fde0d + 3f96146 commit 79ec05c

File tree

4 files changed

+10
-72
lines changed

4 files changed

+10
-72
lines changed

src/vs/workbench/api/common/extHostNotebook.ts

Lines changed: 2 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -328,11 +328,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
328328
}
329329

330330
// validate write
331-
const statBeforeWrite = await this._validateWriteFile(uri, options);
332-
333-
if (!statBeforeWrite) {
334-
await this._mkdirp(uri);
335-
}
331+
await this._validateWriteFile(uri, options);
336332

337333
const data: vscode.NotebookData = {
338334
metadata: filter(document.apiNotebook.metadata, key => !(serializer.options?.transientDocumentMetadata ?? {})[key]),
@@ -377,20 +373,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
377373
}
378374

379375
private async _validateWriteFile(uri: URI, options: files.IWriteFileOptions) {
380-
// File system provider registered in Extension Host doesn't have unlock or atomic support
381-
// Validate via file stat meta data
382376
const stat = await this._extHostFileSystem.value.stat(uri);
383-
384-
// File cannot be directory
385-
if ((stat.type & files.FileType.Directory) !== 0) {
386-
throw new files.FileOperationError(localize('fileIsDirectoryWriteError', "Unable to write file '{0}' that is actually a directory", this._resourceForError(uri)), files.FileOperationResult.FILE_IS_DIRECTORY, options);
387-
}
388-
389-
// File cannot be readonly
390-
if ((stat.permissions ?? 0) & files.FilePermission.Readonly) {
391-
throw new files.FileOperationError(localize('err.readonly', "Unable to modify read-only file '{0}'", this._resourceForError(uri)), files.FileOperationResult.FILE_PERMISSION_DENIED);
392-
}
393-
394377
// Dirty write prevention
395378
if (
396379
typeof options?.mtime === 'number' && typeof options.etag === 'string' && options.etag !== files.ETAG_DISABLED &&
@@ -400,58 +383,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape {
400383
throw new files.FileOperationError(localize('fileModifiedError', "File Modified Since"), files.FileOperationResult.FILE_MODIFIED_SINCE, options);
401384
}
402385

403-
return stat;
404-
}
405-
406-
private async _mkdirp(uri: URI) {
407-
const providerExtUri = this._extHostFileSystem.getFileSystemProviderExtUri(uri.scheme);
408-
let directory = providerExtUri.dirname(uri);
409-
410-
const directoriesToCreate: string[] = [];
411-
412-
while (!providerExtUri.isEqual(directory, providerExtUri.dirname(directory))) {
413-
try {
414-
const stat = await this._extHostFileSystem.value.stat(directory);
415-
if ((stat.type & files.FileType.Directory) === 0) {
416-
throw new Error(localize('mkdirExistsError', "Unable to create folder '{0}' that already exists but is not a directory", this._resourceForError(directory)));
417-
}
418-
419-
break; // we have hit a directory that exists -> good
420-
} catch (error) {
421-
422-
// Bubble up any other error that is not file not found
423-
if (files.toFileSystemProviderErrorCode(error) !== files.FileSystemProviderErrorCode.FileNotFound) {
424-
throw error;
425-
}
426-
427-
// Upon error, remember directories that need to be created
428-
directoriesToCreate.push(providerExtUri.basename(directory));
429-
430-
// Continue up
431-
directory = providerExtUri.dirname(directory);
432-
}
433-
}
434-
435-
// Create directories as needed
436-
for (let i = directoriesToCreate.length - 1; i >= 0; i--) {
437-
directory = providerExtUri.joinPath(directory, directoriesToCreate[i]);
438-
439-
try {
440-
await this._extHostFileSystem.value.createDirectory(directory);
441-
} catch (error) {
442-
if (files.toFileSystemProviderErrorCode(error) !== files.FileSystemProviderErrorCode.FileExists) {
443-
// For mkdirp() we tolerate that the mkdir() call fails
444-
// in case the folder already exists. This follows node.js
445-
// own implementation of fs.mkdir({ recursive: true }) and
446-
// reduces the chances of race conditions leading to errors
447-
// if multiple calls try to create the same folders
448-
// As such, we only throw an error here if it is other than
449-
// the fact that the file already exists.
450-
// (see also https://github.com/microsoft/vscode/issues/89834)
451-
throw error;
452-
}
453-
}
454-
}
386+
return;
455387
}
456388

457389
private _resourceForError(uri: URI): string {

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -984,6 +984,11 @@ configurationRegistry.registerConfiguration({
984984
codeOutput: true
985985
},
986986
tags: ['notebookLayout']
987+
},
988+
[NotebookSetting.remoteSaving]: {
989+
markdownDescription: nls.localize('notebook.remoteSaving', "Enables the incremental saving of notebooks in Remote environment. When enabled, only the changes to the notebook are sent to the extension host, improving performance for large notebooks and slow network connections."),
990+
type: 'boolean',
991+
default: typeof product.quality === 'string' && product.quality !== 'stable' // only enable as default in insiders
987992
}
988993
}
989994
});

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,7 @@ export const NotebookSetting = {
952952
findScope: 'notebook.find.scope',
953953
logging: 'notebook.logging',
954954
confirmDeleteRunningCell: 'notebook.confirmDeleteRunningCell',
955+
remoteSaving: 'notebook.experimental.remoteSave'
955956
} as const;
956957

957958
export const enum CellStatusbarAlignment {

src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import { IWriteFileOptions, IFileStatWithMetadata } from 'vs/platform/files/comm
1818
import { IRevertOptions, ISaveOptions, IUntypedEditorInput } from 'vs/workbench/common/editor';
1919
import { EditorModel } from 'vs/workbench/common/editor/editorModel';
2020
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
21-
import { ICellDto2, INotebookEditorModel, INotebookLoadOptions, IResolvedNotebookEditorModel, NotebookCellsChangeType, NotebookData } from 'vs/workbench/contrib/notebook/common/notebookCommon';
21+
import { ICellDto2, INotebookEditorModel, INotebookLoadOptions, IResolvedNotebookEditorModel, NotebookCellsChangeType, NotebookData, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon';
2222
import { INotebookSerializer, INotebookService, SimpleNotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookService';
2323
import { IFilesConfigurationService } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService';
2424
import { IFileWorkingCopyModelConfiguration } from 'vs/workbench/services/workingCopy/common/fileWorkingCopy';
@@ -215,7 +215,7 @@ export class NotebookFileWorkingCopyModel extends Disposable implements IStoredF
215215
};
216216

217217
// Override save behavior to avoid transferring the buffer across the wire 3 times
218-
if (this._configurationService.getValue('notebook.experimental.remoteSave')) {
218+
if (this._configurationService.getValue(NotebookSetting.remoteSaving)) {
219219
this.save = async (options: IWriteFileOptions, token: CancellationToken) => {
220220
const serializer = await this.getNotebookSerializer();
221221

0 commit comments

Comments
 (0)