From 86b64aa4bc0afb6973d9394f9044bd4fff29a088 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Fri, 7 Feb 2025 10:43:11 +0100 Subject: [PATCH] files - abandon watcher correlation API proposal (#239906) * files - abandon watcher correlation API proposal * fix compile --- .../workspace.watcher.test.ts | 27 +--------- .../common/extensionsApiProposals.ts | 3 -- src/vs/platform/files/common/fileService.ts | 2 +- src/vs/platform/files/common/files.ts | 4 +- .../mainThreadFileSystemEventService.ts | 5 +- .../workbench/api/common/extHost.api.impl.ts | 21 ++------ .../common/extHostFileSystemEventService.ts | 13 +++-- .../extHostFileSystemEventService.test.ts | 4 +- ...code.proposed.createFileSystemWatcher.d.ts | 51 ------------------- 9 files changed, 21 insertions(+), 109 deletions(-) delete mode 100644 src/vscode-dts/vscode.proposed.createFileSystemWatcher.d.ts diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.watcher.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.watcher.test.ts index 7859ae40682..4a07b78c6d9 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/workspace.watcher.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/workspace.watcher.test.ts @@ -48,7 +48,7 @@ suite('vscode API - workspace-watcher', () => { assertNoRpc(); }); - test('createFileSystemWatcher (old style)', async function () { + test('createFileSystemWatcher', async function () { // Non-recursive let watchUri = vscode.Uri.from({ scheme: 'watcherTest', path: '/somePath/folder' }); @@ -68,29 +68,4 @@ suite('vscode API - workspace-watcher', () => { assert.strictEqual(request.uri.toString(), watchUri.toString()); assert.strictEqual(request.options.recursive, true); }); - - test('createFileSystemWatcher (new style)', async function () { - - // Non-recursive - let watchUri = vscode.Uri.from({ scheme: 'watcherTest', path: '/somePath/folder' }); - const watcher = vscode.workspace.createFileSystemWatcher(new vscode.RelativePattern(watchUri, '*.txt'), { excludes: ['testing'], ignoreChangeEvents: true }); - let request = await onDidWatchPromise(); - - assert.strictEqual(request.uri.toString(), watchUri.toString()); - assert.strictEqual(request.options.recursive, false); - assert.strictEqual(request.options.excludes.length, 1); - assert.strictEqual(request.options.excludes[0], 'testing'); - - watcher.dispose(); - - // Recursive - watchUri = vscode.Uri.from({ scheme: 'watcherTest', path: '/somePath/folder' }); - vscode.workspace.createFileSystemWatcher(new vscode.RelativePattern(watchUri, '**/*.txt'), { excludes: ['testing'], ignoreCreateEvents: true }); - request = await onDidWatchPromise(); - - assert.strictEqual(request.uri.toString(), watchUri.toString()); - assert.strictEqual(request.options.recursive, true); - assert.strictEqual(request.options.excludes.length, 1); - assert.strictEqual(request.options.excludes[0], 'testing'); - }); }); diff --git a/src/vs/platform/extensions/common/extensionsApiProposals.ts b/src/vs/platform/extensions/common/extensionsApiProposals.ts index bf5936b2a70..93d6088e742 100644 --- a/src/vs/platform/extensions/common/extensionsApiProposals.ts +++ b/src/vs/platform/extensions/common/extensionsApiProposals.ts @@ -146,9 +146,6 @@ const _allApiProposals = { contribViewsWelcome: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribViewsWelcome.d.ts', }, - createFileSystemWatcher: { - proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.createFileSystemWatcher.d.ts', - }, customEditorMove: { proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.customEditorMove.d.ts', }, diff --git a/src/vs/platform/files/common/fileService.ts b/src/vs/platform/files/common/fileService.ts index 2c0bfe1369c..978cac73b67 100644 --- a/src/vs/platform/files/common/fileService.ts +++ b/src/vs/platform/files/common/fileService.ts @@ -1114,7 +1114,7 @@ export class FileService extends Disposable implements IFileService { private static WATCHER_CORRELATION_IDS = 0; - createWatcher(resource: URI, options: IWatchOptionsWithoutCorrelation): IFileSystemWatcher { + createWatcher(resource: URI, options: IWatchOptionsWithoutCorrelation & { recursive: false }): IFileSystemWatcher { return this.watch(resource, { ...options, // Explicitly set a correlation id so that file events that originate diff --git a/src/vs/platform/files/common/files.ts b/src/vs/platform/files/common/files.ts index b1bc8113abf..bec0de5c3f9 100644 --- a/src/vs/platform/files/common/files.ts +++ b/src/vs/platform/files/common/files.ts @@ -240,8 +240,10 @@ export interface IFileService { * * The watcher runs correlated and thus, file events will be reported on the returned * `IFileSystemWatcher` and not on the generic `IFileService.onDidFilesChange` event. + * + * Note: only non-recursive file watching supports event correlation for now. */ - createWatcher(resource: URI, options: IWatchOptionsWithoutCorrelation): IFileSystemWatcher; + createWatcher(resource: URI, options: IWatchOptionsWithoutCorrelation & { recursive: false }): IFileSystemWatcher; /** * Allows to start a watcher that reports file/folder change events on the provided resource. diff --git a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts index 572acf83917..83e2221d0a1 100644 --- a/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts +++ b/src/vs/workbench/api/browser/mainThreadFileSystemEventService.ts @@ -228,11 +228,12 @@ export class MainThreadFileSystemEventService implements MainThreadFileSystemEve } // Correlated file watching: use an exclusive `createWatcher()` - if (correlate) { + // Note: currently not enabled for extensions (but leaving in in case of future usage) + if (correlate && !opts.recursive) { this._logService.trace(`MainThreadFileSystemEventService#$watch(): request to start watching correlated (extension: ${extensionId}, path: ${uri.toString(true)}, recursive: ${opts.recursive}, session: ${session}, excludes: ${JSON.stringify(opts.excludes)}, includes: ${JSON.stringify(opts.includes)})`); const watcherDisposables = new DisposableStore(); - const subscription = watcherDisposables.add(this._fileService.createWatcher(uri, opts)); + const subscription = watcherDisposables.add(this._fileService.createWatcher(uri, { ...opts, recursive: false })); watcherDisposables.add(subscription.onDidChange(event => { this._proxy.$onFileEvent({ session, diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 789d30aec57..64cbb2e6778 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1009,22 +1009,11 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I return extHostBulkEdits.applyWorkspaceEdit(edit, extension, metadata); }, createFileSystemWatcher: (pattern, optionsOrIgnoreCreate, ignoreChange?, ignoreDelete?): vscode.FileSystemWatcher => { - let options: FileSystemWatcherCreateOptions | undefined = undefined; - - if (optionsOrIgnoreCreate && typeof optionsOrIgnoreCreate !== 'boolean') { - checkProposedApiEnabled(extension, 'createFileSystemWatcher'); - options = { - ...optionsOrIgnoreCreate, - correlate: true - }; - } else { - options = { - ignoreCreateEvents: Boolean(optionsOrIgnoreCreate), - ignoreChangeEvents: Boolean(ignoreChange), - ignoreDeleteEvents: Boolean(ignoreDelete), - correlate: false - }; - } + const options: FileSystemWatcherCreateOptions = { + ignoreCreateEvents: Boolean(optionsOrIgnoreCreate), + ignoreChangeEvents: Boolean(ignoreChange), + ignoreDeleteEvents: Boolean(ignoreDelete), + }; return extHostFileSystemEvent.createFileSystemWatcher(extHostWorkspace, configProvider, extension, pattern, options); }, diff --git a/src/vs/workbench/api/common/extHostFileSystemEventService.ts b/src/vs/workbench/api/common/extHostFileSystemEventService.ts index ff4892239e9..9bacc4ae0da 100644 --- a/src/vs/workbench/api/common/extHostFileSystemEventService.ts +++ b/src/vs/workbench/api/common/extHostFileSystemEventService.ts @@ -22,13 +22,9 @@ import { rtrim } from '../../../base/common/strings.js'; import { normalizeWatcherPattern } from '../../../platform/files/common/watcher.js'; export interface FileSystemWatcherCreateOptions { - readonly correlate: boolean; - readonly ignoreCreateEvents?: boolean; readonly ignoreChangeEvents?: boolean; readonly ignoreDeleteEvents?: boolean; - - readonly excludes?: string[]; } class FileSystemWatcher implements vscode.FileSystemWatcher { @@ -78,7 +74,10 @@ class FileSystemWatcher implements vscode.FileSystemWatcher { // 1.84.x introduces new proposed API for a watcher to set exclude // rules. In these cases, we turn the file watcher into correlation // mode and ignore any event that does not match the correlation ID. - const excludeUncorrelatedEvents = options.correlate; + // + // Update (Feb 2025): proposal is discontinued, so the previous + // `options.correlate` is always `false`. + const excludeUncorrelatedEvents = false; const subscription = dispatcher(events => { if (typeof events.session === 'number' && events.session !== this.session) { @@ -115,7 +114,7 @@ class FileSystemWatcher implements vscode.FileSystemWatcher { } }); - this._disposable = Disposable.from(this.ensureWatching(mainContext, workspace, configuration, extension, globPattern, options, options.correlate), this._onDidCreate, this._onDidChange, this._onDidDelete, subscription); + this._disposable = Disposable.from(this.ensureWatching(mainContext, workspace, configuration, extension, globPattern, options, false), this._onDidCreate, this._onDidChange, this._onDidDelete, subscription); } private ensureWatching(mainContext: IMainContext, workspace: IExtHostWorkspace, configuration: ExtHostConfigProvider, extension: IExtensionDescription, globPattern: string | IRelativePatternDto, options: FileSystemWatcherCreateOptions, correlate: boolean | undefined): Disposable { @@ -136,7 +135,7 @@ class FileSystemWatcher implements vscode.FileSystemWatcher { recursive = true; // only watch recursively if pattern indicates the need for it } - const excludes = options.excludes ?? []; + const excludes = []; let includes: Array | undefined = undefined; let filter: FileChangeFilter | undefined; diff --git a/src/vs/workbench/api/test/browser/extHostFileSystemEventService.test.ts b/src/vs/workbench/api/test/browser/extHostFileSystemEventService.test.ts index 6ffae58bc79..3aa7c61a2f8 100644 --- a/src/vs/workbench/api/test/browser/extHostFileSystemEventService.test.ts +++ b/src/vs/workbench/api/test/browser/extHostFileSystemEventService.test.ts @@ -22,13 +22,13 @@ suite('ExtHostFileSystemEventService', () => { drain: undefined! }; - const watcher1 = new ExtHostFileSystemEventService(protocol, new NullLogService(), undefined!).createFileSystemWatcher(undefined!, undefined!, undefined!, '**/somethingInteresting', { correlate: false }); + const watcher1 = new ExtHostFileSystemEventService(protocol, new NullLogService(), undefined!).createFileSystemWatcher(undefined!, undefined!, undefined!, '**/somethingInteresting', {}); assert.strictEqual(watcher1.ignoreChangeEvents, false); assert.strictEqual(watcher1.ignoreCreateEvents, false); assert.strictEqual(watcher1.ignoreDeleteEvents, false); watcher1.dispose(); - const watcher2 = new ExtHostFileSystemEventService(protocol, new NullLogService(), undefined!).createFileSystemWatcher(undefined!, undefined!, undefined!, '**/somethingBoring', { ignoreCreateEvents: true, ignoreChangeEvents: true, ignoreDeleteEvents: true, correlate: false }); + const watcher2 = new ExtHostFileSystemEventService(protocol, new NullLogService(), undefined!).createFileSystemWatcher(undefined!, undefined!, undefined!, '**/somethingBoring', { ignoreCreateEvents: true, ignoreChangeEvents: true, ignoreDeleteEvents: true }); assert.strictEqual(watcher2.ignoreChangeEvents, true); assert.strictEqual(watcher2.ignoreCreateEvents, true); assert.strictEqual(watcher2.ignoreDeleteEvents, true); diff --git a/src/vscode-dts/vscode.proposed.createFileSystemWatcher.d.ts b/src/vscode-dts/vscode.proposed.createFileSystemWatcher.d.ts deleted file mode 100644 index d138471f2af..00000000000 --- a/src/vscode-dts/vscode.proposed.createFileSystemWatcher.d.ts +++ /dev/null @@ -1,51 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -// https://github.com/microsoft/vscode/issues/169724 @bpasero - -declare module 'vscode' { - - export interface FileSystemWatcherOptions { - - /** - * Ignore when files have been created. - */ - readonly ignoreCreateEvents?: boolean; - - /** - * Ignore when files have been changed. - */ - readonly ignoreChangeEvents?: boolean; - - /** - * Ignore when files have been deleted. - */ - readonly ignoreDeleteEvents?: boolean; - - /** - * An optional set of glob patterns to exclude from watching. - * Glob patterns are always matched relative to the watched folder. - */ - readonly excludes: string[]; - } - - export namespace workspace { - - /** - * A variant of {@link workspace.createFileSystemWatcher} that optionally allows to specify - * a set of glob patterns to exclude from watching. - * - * It provides the following advantages over the other {@link workspace.createFileSystemWatcher} - * method: - * - the configured excludes from `files.watcherExclude` setting are NOT applied - * - requests for recursive file watchers inside the opened workspace are NOT ignored - * - the watcher is ONLY notified for events from this request and not from any other watcher - * - * As such, this method is prefered in cases where you want full control over the watcher behavior - * without being impacted by settings or other watchers that are installed. - */ - export function createFileSystemWatcher(pattern: RelativePattern, options?: FileSystemWatcherOptions): FileSystemWatcher; - } -}