Skip to content

Commit b826b37

Browse files
bpaserobpasero
committed
Allow createFileSystemWatcher to bypass files.watcherExclude for flat watching (fix microsoft#146066)
1 parent a70083c commit b826b37

File tree

11 files changed

+134
-35
lines changed

11 files changed

+134
-35
lines changed

‎src/vs/platform/files/common/diskFileSystemProvider.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ export abstract class AbstractDiskFileSystemProvider extends Disposable {
6969
private watchUniversal(resource: URI, opts: IWatchOptions): IDisposable {
7070

7171
// Add to list of paths to watch universally
72-
const pathToWatch: IUniversalWatchRequest = { path: this.toFilePath(resource), excludes: opts.excludes, recursive: opts.recursive };
72+
const pathToWatch: IUniversalWatchRequest = { path: this.toFilePath(resource), excludes: opts.excludes, includes: opts.includes, recursive: opts.recursive };
7373
const remove = insert(this.universalPathsToWatch, pathToWatch);
7474

7575
// Trigger update
@@ -150,7 +150,7 @@ export abstract class AbstractDiskFileSystemProvider extends Disposable {
150150
private watchNonRecursive(resource: URI, opts: IWatchOptions): IDisposable {
151151

152152
// Add to list of paths to watch non-recursively
153-
const pathToWatch: INonRecursiveWatchRequest = { path: this.toFilePath(resource), excludes: opts.excludes, recursive: false };
153+
const pathToWatch: INonRecursiveWatchRequest = { path: this.toFilePath(resource), excludes: opts.excludes, includes: opts.includes, recursive: false };
154154
const remove = insert(this.nonRecursivePathsToWatch, pathToWatch);
155155

156156
// Trigger update

‎src/vs/platform/files/common/files.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,9 +428,16 @@ export interface IWatchOptions {
428428
readonly recursive: boolean;
429429

430430
/**
431-
* A set of paths to exclude from watching.
431+
* A set of glob patterns or paths to exclude from watching.
432432
*/
433433
excludes: string[];
434+
435+
/**
436+
* An optional set of glob patterns or paths to include for
437+
* watching. If not provided, all paths are considered for
438+
* events.
439+
*/
440+
includes?: string[];
434441
}
435442

436443
export const enum FileSystemProviderCapabilities {

‎src/vs/platform/files/common/watcher.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ interface IWatchRequest {
2525
* A set of glob patterns or paths to exclude from watching.
2626
*/
2727
excludes: string[];
28+
29+
/**
30+
* An optional set of glob patterns or paths to include for
31+
* watching. If not provided, all paths are considered for
32+
* events.
33+
*/
34+
includes?: string[];
2835
}
2936

3037
export interface INonRecursiveWatchRequest extends IWatchRequest {

‎src/vs/platform/files/node/watcher/nodejs/nodejsWatcher.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,19 +49,19 @@ export class NodeJSWatcher extends Disposable implements INonRecursiveWatcher {
4949
return true; // not yet watching that path
5050
}
5151

52-
// Re-watch path if excludes have changed
53-
return !equals(watcher.request.excludes, request.excludes);
52+
// Re-watch path if excludes or includes have changed
53+
return !equals(watcher.request.excludes, request.excludes) || !equals(watcher.request.includes, request.includes);
5454
});
5555

5656
// Gather paths that we should stop watching
5757
const pathsToStopWatching = Array.from(this.watchers.values()).filter(({ request }) => {
58-
return !normalizedRequests.find(normalizedRequest => normalizedRequest.path === request.path && equals(normalizedRequest.excludes, request.excludes));
58+
return !normalizedRequests.find(normalizedRequest => normalizedRequest.path === request.path && equals(normalizedRequest.excludes, request.excludes) && equals(normalizedRequest.includes, request.includes));
5959
}).map(({ request }) => request.path);
6060

6161
// Logging
6262

6363
if (requestsToStartWatching.length) {
64-
this.trace(`Request to start watching: ${requestsToStartWatching.map(request => `${request.path} (excludes: ${request.excludes.length > 0 ? request.excludes : '<none>'})`).join(',')}`);
64+
this.trace(`Request to start watching: ${requestsToStartWatching.map(request => `${request.path} (excludes: ${request.excludes.length > 0 ? request.excludes : '<none>'}, includes: ${request.includes && request.includes.length > 0 ? request.includes : '<all>'})`).join(',')}`);
6565
}
6666

6767
if (pathsToStopWatching.length) {

‎src/vs/platform/files/node/watcher/nodejs/nodejsWatcherLib.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ export class NodeJSFileWatcherLibrary extends Disposable {
4848
private fileChangesBuffer: IDiskFileChange[] = [];
4949

5050
private readonly excludes = this.request.excludes.map(exclude => parse(exclude));
51+
private readonly includes = this.request.includes?.map(include => parse(include));
5152

5253
private readonly cts = new CancellationTokenSource();
5354

@@ -317,14 +318,14 @@ export class NodeJSFileWatcherLibrary extends Disposable {
317318

318319
// File still exists, so emit as change event and reapply the watcher
319320
if (fileExists) {
320-
this.onFileChange({ path: this.request.path, type: FileChangeType.UPDATED }, true /* skip excludes (file is explicitly watched) */);
321+
this.onFileChange({ path: this.request.path, type: FileChangeType.UPDATED }, true /* skip excludes/includes (file is explicitly watched) */);
321322

322323
disposables.add(await this.doWatch(path, false));
323324
}
324325

325326
// File seems to be really gone, so emit a deleted event and dispose
326327
else {
327-
const eventPromise = this.onFileChange({ path: this.request.path, type: FileChangeType.DELETED }, true /* skip excludes (file is explicitly watched) */);
328+
const eventPromise = this.onFileChange({ path: this.request.path, type: FileChangeType.DELETED }, true /* skip excludes/includes (file is explicitly watched) */);
328329

329330
// Important to await the event delivery
330331
// before disposing the watcher, otherwise
@@ -342,7 +343,7 @@ export class NodeJSFileWatcherLibrary extends Disposable {
342343

343344
// File changed
344345
else {
345-
this.onFileChange({ path: this.request.path, type: FileChangeType.UPDATED }, true /* skip excludes (file is explicitly watched) */);
346+
this.onFileChange({ path: this.request.path, type: FileChangeType.UPDATED }, true /* skip excludes/includes (file is explicitly watched) */);
346347
}
347348
}
348349
});
@@ -358,7 +359,7 @@ export class NodeJSFileWatcherLibrary extends Disposable {
358359
});
359360
}
360361

361-
private async onFileChange(event: IDiskFileChange, skipExcludes = false): Promise<void> {
362+
private async onFileChange(event: IDiskFileChange, skipIncludeExcludeChecks = false): Promise<void> {
362363
if (this.cts.token.isCancellationRequested) {
363364
return;
364365
}
@@ -368,10 +369,14 @@ export class NodeJSFileWatcherLibrary extends Disposable {
368369
this.trace(`${event.type === FileChangeType.ADDED ? '[ADDED]' : event.type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${event.path}`);
369370
}
370371

371-
// Add to buffer unless ignored (not if explicitly disabled)
372-
if (!skipExcludes && this.excludes.some(exclude => exclude(event.path))) {
372+
// Add to buffer unless excluded or not included (not if explicitly disabled)
373+
if (!skipIncludeExcludeChecks && this.excludes.some(exclude => exclude(event.path))) {
373374
if (this.verboseLogging) {
374-
this.trace(` >> ignored ${event.path}`);
375+
this.trace(` >> ignored (excluded) ${event.path}`);
376+
}
377+
} else if (!skipIncludeExcludeChecks && this.includes && this.includes.length > 0 && !this.includes.some(include => include(event.path))) {
378+
if (this.verboseLogging) {
379+
this.trace(` >> ignored (not included) ${event.path}`);
375380
}
376381
} else {
377382
this.fileChangesBuffer.push(event);

‎src/vs/platform/files/node/watcher/parcel/parcelWatcher.ts

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -128,15 +128,16 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
128128
return true; // not yet watching that path
129129
}
130130

131-
// Re-watch path if excludes have changed or polling interval
132-
return !equals(watcher.request.excludes, request.excludes) || watcher.request.pollingInterval !== request.pollingInterval;
131+
// Re-watch path if excludes/includes have changed or polling interval
132+
return !equals(watcher.request.excludes, request.excludes) || !equals(watcher.request.includes, request.includes) || watcher.request.pollingInterval !== request.pollingInterval;
133133
});
134134

135135
// Gather paths that we should stop watching
136136
const pathsToStopWatching = Array.from(this.watchers.values()).filter(({ request }) => {
137137
return !normalizedRequests.find(normalizedRequest => {
138138
return normalizedRequest.path === request.path &&
139139
equals(normalizedRequest.excludes, request.excludes) &&
140+
equals(normalizedRequest.includes, request.includes) &&
140141
normalizedRequest.pollingInterval === request.pollingInterval;
141142

142143
});
@@ -145,7 +146,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
145146
// Logging
146147

147148
if (requestsToStartWatching.length) {
148-
this.trace(`Request to start watching: ${requestsToStartWatching.map(request => `${request.path} (excludes: ${request.excludes.length > 0 ? request.excludes : '<none>'})`).join(',')}`);
149+
this.trace(`Request to start watching: ${requestsToStartWatching.map(request => `${request.path} (excludes: ${request.excludes.length > 0 ? request.excludes : '<none>'}, includes: ${request.includes && request.includes.length > 0 ? request.includes : '<all>'})`).join(',')}`);
149150
}
150151

151152
if (pathsToStopWatching.length) {
@@ -283,8 +284,9 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
283284
// Path checks for symbolic links / wrong casing
284285
const { realPath, realPathDiffers, realPathLength } = this.normalizePath(request);
285286

286-
// Warm up exclude patterns for usage
287+
// Warm up exclude/include patterns for usage
287288
const excludePatterns = request.excludes.map(exclude => parse(exclude));
289+
const includePatterns = request.includes?.map(include => parse(include));
288290

289291
const ignore = this.toExcludePaths(realPath, watcher.request.excludes);
290292

@@ -308,7 +310,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
308310
}
309311

310312
// Handle & emit events
311-
this.onParcelEvents(parcelEvents, watcher, excludePatterns, realPathDiffers, realPathLength);
313+
this.onParcelEvents(parcelEvents, watcher, excludePatterns, includePatterns, realPathDiffers, realPathLength);
312314
}
313315

314316
// Store a snapshot of files to the snapshot file
@@ -352,8 +354,9 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
352354
// Path checks for symbolic links / wrong casing
353355
const { realPath, realPathDiffers, realPathLength } = this.normalizePath(request);
354356

355-
// Warm up exclude patterns for usage
357+
// Warm up exclude/include patterns for usage
356358
const excludePatterns = request.excludes.map(exclude => parse(exclude));
359+
const includePatterns = request.includes?.map(include => parse(include));
357360

358361
const ignore = this.toExcludePaths(realPath, watcher.request.excludes);
359362
parcelWatcher.subscribe(realPath, (error, parcelEvents) => {
@@ -370,7 +373,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
370373
}
371374

372375
// Handle & emit events
373-
this.onParcelEvents(parcelEvents, watcher, excludePatterns, realPathDiffers, realPathLength);
376+
this.onParcelEvents(parcelEvents, watcher, excludePatterns, includePatterns, realPathDiffers, realPathLength);
374377
}, {
375378
backend: ParcelWatcher.PARCEL_WATCHER_BACKEND,
376379
ignore
@@ -385,13 +388,13 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
385388
});
386389
}
387390

388-
private onParcelEvents(parcelEvents: parcelWatcher.Event[], watcher: IParcelWatcherInstance, excludes: ParsedPattern[], realPathDiffers: boolean, realPathLength: number): void {
391+
private onParcelEvents(parcelEvents: parcelWatcher.Event[], watcher: IParcelWatcherInstance, excludes: ParsedPattern[], includes: ParsedPattern[] | undefined, realPathDiffers: boolean, realPathLength: number): void {
389392
if (parcelEvents.length === 0) {
390393
return;
391394
}
392395

393396
// Check for excludes
394-
const rawEvents = this.handleExcludes(parcelEvents, excludes);
397+
const rawEvents = this.handleExcludeIncludes(parcelEvents, excludes, includes);
395398

396399
// Normalize events: handle NFC normalization and symlinks
397400
const { events: normalizedEvents, rootDeleted } = this.normalizeEvents(rawEvents, watcher.request, realPathDiffers, realPathLength);
@@ -411,7 +414,7 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
411414
}
412415
}
413416

414-
private handleExcludes(parcelEvents: parcelWatcher.Event[], excludes: ParsedPattern[]): IDiskFileChange[] {
417+
private handleExcludeIncludes(parcelEvents: parcelWatcher.Event[], excludes: ParsedPattern[], includes: ParsedPattern[] | undefined): IDiskFileChange[] {
415418
const events: IDiskFileChange[] = [];
416419

417420
for (const { path, type: parcelEventType } of parcelEvents) {
@@ -420,9 +423,14 @@ export class ParcelWatcher extends Disposable implements IRecursiveWatcher {
420423
this.trace(`${type === FileChangeType.ADDED ? '[ADDED]' : type === FileChangeType.DELETED ? '[DELETED]' : '[CHANGED]'} ${path}`);
421424
}
422425

426+
// Add to buffer unless excluded or not included (not if explicitly disabled)
423427
if (excludes.some(exclude => exclude(path))) {
424428
if (this.verboseLogging) {
425-
this.trace(` >> ignored ${path}`);
429+
this.trace(` >> ignored (excluded) ${path}`);
430+
}
431+
} else if (includes && includes.length > 0 && !includes.some(include => include(path))) {
432+
if (this.verboseLogging) {
433+
this.trace(` >> ignored (not included) ${path}`);
426434
}
427435
} else {
428436
events.push({ type, path });

‎src/vs/platform/files/test/browser/fileService.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,16 @@ suite('File Service', () => {
121121
const resource3 = URI.parse('test://foo/bar3');
122122
const watcher3Disposable1 = service.watch(resource3);
123123
const watcher3Disposable2 = service.watch(resource3, { recursive: true, excludes: [] });
124+
const watcher3Disposable3 = service.watch(resource3, { recursive: false, excludes: [], includes: [] });
124125

125126
await timeout(0); // service.watch() is async
126127
assert.strictEqual(disposeCounter, 0);
127128
watcher3Disposable1.dispose();
128129
assert.strictEqual(disposeCounter, 1);
129130
watcher3Disposable2.dispose();
130131
assert.strictEqual(disposeCounter, 2);
132+
watcher3Disposable3.dispose();
133+
assert.strictEqual(disposeCounter, 3);
131134

132135
service.dispose();
133136
});

‎src/vs/platform/files/test/node/nodejsWatcher.integrationTest.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,26 @@ import { NodeJSWatcher } from 'vs/platform/files/node/watcher/nodejs/nodejsWatch
372372
return basicCrudTest(filePath, true);
373373
});
374374

375+
test('includes can be updated (folder watch)', async function () {
376+
await watcher.watch([{ path: testDir, excludes: [], includes: ['nothing'], recursive: false }]);
377+
await watcher.watch([{ path: testDir, excludes: [], recursive: false }]);
378+
379+
return basicCrudTest(join(testDir, 'files-includes.txt'));
380+
});
381+
382+
test('non-includes are ignored (file watch)', async function () {
383+
const filePath = join(testDir, 'lorem.txt');
384+
await watcher.watch([{ path: filePath, excludes: [], includes: ['nothing'], recursive: false }]);
385+
386+
return basicCrudTest(filePath, true);
387+
});
388+
389+
test('includes are supported (folder watch)', async function () {
390+
await watcher.watch([{ path: testDir, excludes: [], includes: ['**/files-includes.txt'], recursive: false }]);
391+
392+
return basicCrudTest(join(testDir, 'files-includes.txt'));
393+
});
394+
375395
(isWindows /* windows: cannot create file symbolic link without elevated context */ ? test.skip : test)('symlink support (folder watch)', async function () {
376396
const link = join(testDir, 'deep-linked');
377397
const linkTarget = join(testDir, 'deep');
@@ -495,5 +515,3 @@ import { NodeJSWatcher } from 'vs/platform/files/node/watcher/nodejs/nodejsWatch
495515
return watchPromise;
496516
});
497517
});
498-
499-
// TODO test for excludes? subsequent updates to rewatch like parcel?

‎src/vs/platform/files/test/node/parcelWatcher.integrationTest.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ import { ltrim } from 'vs/base/common/strings';
155155
}
156156

157157
test('basics', async function () {
158-
await watcher.watch([{ path: testDir, excludes: [], recursive: true }]);
158+
await watcher.watch([{ path: testDir, excludes: [], recursive: true }]); //
159159

160160
// New file
161161
const newFilePath = join(testDir, 'deep', 'newFile.txt');
@@ -437,6 +437,19 @@ import { ltrim } from 'vs/base/common/strings';
437437
return basicCrudTest(join(testDir, 'deep', 'newFile.txt'));
438438
});
439439

440+
test('subsequent watch updates watchers (includes)', async function () {
441+
await watcher.watch([{ path: testDir, excludes: [], includes: ['nothing'], recursive: true }]);
442+
await watcher.watch([{ path: testDir, excludes: [], recursive: true }]);
443+
444+
return basicCrudTest(join(testDir, 'deep', 'newFile.txt'));
445+
});
446+
447+
test('includes are supported', async function () {
448+
await watcher.watch([{ path: testDir, excludes: [], includes: ['**/deep/**'], recursive: true }]);
449+
450+
return basicCrudTest(join(testDir, 'deep', 'newFile.txt'));
451+
});
452+
440453
(isWindows /* windows: cannot create file symbolic link without elevated context */ ? test.skip : test)('symlink support (root)', async function () {
441454
const link = join(testDir, 'deep-linked');
442455
const linkTarget = join(testDir, 'deep');

‎src/vs/workbench/api/browser/mainThreadFileSystem.ts

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,17 +165,22 @@ export class MainThreadFileSystem implements MainThreadFileSystemShape {
165165

166166
$watch(extensionId: string, session: number, resource: UriComponents, opts: IWatchOptions): void {
167167
const uri = URI.revive(resource);
168-
169-
// refuse to watch anything that is already watched via
170-
// our workspace watchers
171-
if (this._contextService.isInsideWorkspace(uri)) {
168+
const isInsideWorkspace = this._contextService.isInsideWorkspace(uri);
169+
170+
// Refuse to watch anything that is already watched via
171+
// our workspace watchers in case the request is a
172+
// recursive file watcher.
173+
// Still allow for non-recursive watch requests as a way
174+
// to bypass configured exlcude rules though
175+
// (see https://github.com/microsoft/vscode/issues/146066)
176+
if (isInsideWorkspace && opts.recursive) {
172177
this._logService.trace(`MainThreadFileSystem#$watch(): ignoring request to start watching because path is inside workspace (extension: ${extensionId}, path: ${uri.toString(true)}, recursive: ${opts.recursive}, session: ${session})`);
173178
return;
174179
}
175180

176181
this._logService.trace(`MainThreadFileSystem#$watch(): request to start watching (extension: ${extensionId}, path: ${uri.toString(true)}, recursive: ${opts.recursive}, session: ${session})`);
177182

178-
// automatically add `files.watcherExclude` patterns when watching
183+
// Automatically add `files.watcherExclude` patterns when watching
179184
// recursively to give users a chance to configure exclude rules
180185
// for reducing the overhead of watching recursively
181186
if (opts.recursive) {
@@ -189,6 +194,35 @@ export class MainThreadFileSystem implements MainThreadFileSystemShape {
189194
}
190195
}
191196

197+
// Non-recursive watching inside the workspace will overlap with
198+
// our standard workspace watchers. To prevent duplicate events,
199+
// we only want to include events for files that are otherwise
200+
// excluded via `files.watcherExclude`. As such, we configure
201+
// to include each configured exclude pattern so that only those
202+
// events are reported that are otherwise excluded.
203+
else if (isInsideWorkspace) {
204+
const config = this._configurationService.getValue<IFilesConfiguration>();
205+
if (config.files?.watcherExclude) {
206+
for (const key in config.files.watcherExclude) {
207+
if (config.files.watcherExclude[key] === true) {
208+
if (!opts.includes) {
209+
opts.includes = [];
210+
}
211+
212+
opts.includes.push(key);
213+
}
214+
}
215+
}
216+
217+
// Still ignore watch request if there are actually no configured
218+
// exclude rules, because in that case our default recursive watcher
219+
// should be able to take care of all events.
220+
if (!opts.includes || opts.includes.length === 0) {
221+
this._logService.trace(`MainThreadFileSystem#$watch(): ignoring request to start watching because path is inside workspace and no excludes are configured (extension: ${extensionId}, path: ${uri.toString(true)}, recursive: ${opts.recursive}, session: ${session})`);
222+
return;
223+
}
224+
}
225+
192226
const subscription = this._fileService.watch(uri, opts);
193227
this._watches.set(session, subscription);
194228
}

0 commit comments

Comments
 (0)