watcher - some cleanup (microsoft#210723)

Author: bpasero

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

@@ -135,14 +135,14 @@ export interface IRecursiveWatcherWithSubscribe extends IRecursiveWatcher {
 
 	/**
 	 * Subscribe to file events for the given path. The callback is called
-	 * whenever a file event occurs for the path. I fthe watcher failed,
+	 * whenever a file event occurs for the path. If the watcher failed,
 	 * the error parameter is set to `true`.
 	 *
 	 * @returns an `IDisposable` to stop listening to events or `undefined`
 	 * if no events can be watched for the path given the current set of
 	 * recursive watch requests.
 	 */
-	subscribe(path: string, callback: (error: boolean, change?: IFileChange) => void): IDisposable | undefined;
+	subscribe(path: string, callback: (error: true | null, change?: IFileChange) => void): IDisposable | undefined;
 }
 
 export interface IRecursiveWatcherOptions {

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

@@ -147,7 +147,12 @@ export class NodeJSFileWatcherLibrary extends Disposable {
 
 	private doWatchWithExistingWatcher(realPath: string, isDirectory: boolean, disposables: DisposableStore): boolean {
 		if (isDirectory) {
-			return false; // only supported for files where we have the full path known upfront
+			// TODO@bpasero recursive watcher re-use is currently not enabled
+			// for when folders are watched. this is because the dispatching
+			// in the recursive watcher for non-recurive requests is optimized
+			// for file changes  where we really only match on the exact path
+			// and not child paths.
+			return false;
 		}
 
 		const resource = URI.file(this.request.path);

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

@@ -782,7 +782,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
 		return true;
 	}
 
-	subscribe(path: string, callback: (error: boolean, change?: IFileChange) => void): IDisposable | undefined {
+	subscribe(path: string, callback: (error: true | null, change?: IFileChange) => void): IDisposable | undefined {
 		for (const watcher of this.watchers) {
 			if (watcher.failed) {
 				continue; // watcher has already failed
@@ -810,7 +810,7 @@ export class ParcelWatcher extends BaseWatcher implements IRecursiveWatcherWithS
 				callback(true /* error */);
 			}));
 			disposables.add(Event.once(watcher.onDidFail)(() => callback(true /* error */)));
-			disposables.add(watcher.subscribe(path, change => callback(false, change)));
+			disposables.add(watcher.subscribe(path, change => callback(null, change)));
 
 			return disposables;
 		}

src/vs/platform/files/node/watcher/watcherStats.ts

@@ -38,19 +38,31 @@ export function computeStats(
 	lines.push(`- I/O Handles Impact:     total: ${recursiveRequestsStatus.polling + nonRecursiveRequestsStatus.polling + recursiveWatcherStatus.active + nonRecursiveWatcherStatus.active}`);
 
 	lines.push(`\n[Recursive Requests (${allRecursiveRequests.length}, suspended: ${recursiveRequestsStatus.suspended}, polling: ${recursiveRequestsStatus.polling})]:`);
+	const recursiveRequestLines: string[] = [];
 	for (const request of [nonSuspendedRecursiveRequests, suspendedPollingRecursiveRequests, suspendedNonPollingRecursiveRequests].flat()) {
-		fillRequestStats(lines, request, recursiveWatcher);
+		fillRequestStats(recursiveRequestLines, request, recursiveWatcher);
 	}
+	lines.push(...alignTextColumns(recursiveRequestLines));
 
-	fillRecursiveWatcherStats(lines, recursiveWatcher);
+	const recursiveWatcheLines: string[] = [];
+	fillRecursiveWatcherStats(recursiveWatcheLines, recursiveWatcher);
+	lines.push(...alignTextColumns(recursiveWatcheLines));
 
 	lines.push(`\n[Non-Recursive Requests (${allNonRecursiveRequests.length}, suspended: ${nonRecursiveRequestsStatus.suspended}, polling: ${nonRecursiveRequestsStatus.polling})]:`);
+	const nonRecursiveRequestLines: string[] = [];
 	for (const request of [nonSuspendedNonRecursiveRequests, suspendedPollingNonRecursiveRequests, suspendedNonPollingNonRecursiveRequests].flat()) {
-		fillRequestStats(lines, request, nonRecursiveWatcher);
+		fillRequestStats(nonRecursiveRequestLines, request, nonRecursiveWatcher);
 	}
+	lines.push(...alignTextColumns(nonRecursiveRequestLines));
 
-	fillNonRecursiveWatcherStats(lines, nonRecursiveWatcher);
+	const nonRecursiveWatcheLines: string[] = [];
+	fillNonRecursiveWatcherStats(nonRecursiveWatcheLines, nonRecursiveWatcher);
+	lines.push(...alignTextColumns(nonRecursiveWatcheLines));
 
+	return `\n\n[File Watcher] request stats:\n\n${lines.join('\n')}\n\n`;
+}
+
+function alignTextColumns(lines: string[]) {
 	let maxLength = 0;
 	for (const line of lines) {
 		maxLength = Math.max(maxLength, line.split('\t')[0].length);
@@ -65,7 +77,7 @@ export function computeStats(
 		}
 	}
 
-	return `\n\n[File Watcher] request stats:\n\n${lines.join('\n')}\n\n`;
+	return lines;
 }
 
 function computeRequestStatus(requests: IUniversalWatchRequest[], watcher: ParcelWatcher | NodeJSWatcher): { suspended: number; polling: number } {