improve handling of file delete events

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

@@ -9,6 +9,7 @@ import paths = require('vs/base/common/paths');
 import URI from 'vs/base/common/uri';
 import glob = require('vs/base/common/glob');
 import events = require('vs/base/common/events');
+import { isLinux } from 'vs/base/common/platform';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 
 export const IFileService = createDecorator<IFileService>('fileService');
@@ -191,19 +192,6 @@ export class FileChangesEvent extends events.Event {
 			return false;
 		}
 
-		return this.containsAny([resource], type);
-	}
-
-	/**
-	 * Returns true if this change event contains any of the provided files with the given change type. In case of
-	 * type DELETED, this method will also return true if a folder got deleted that is the parent of any of the
-	 * provided file paths.
-	 */
-	public containsAny(resources: URI[], type: FileChangeType): boolean {
-		if (!resources || !resources.length) {
-			return false;
-		}
-
 		return this._changes.some((change) => {
 			if (change.type !== type) {
 				return false;
@@ -211,22 +199,10 @@ export class FileChangesEvent extends events.Event {
 
 			// For deleted also return true when deleted folder is parent of target path
 			if (type === FileChangeType.DELETED) {
-				return resources.some((a: URI) => {
-					if (!a) {
-						return false;
-					}
-
-					return paths.isEqualOrParent(a.fsPath, change.resource.fsPath);
-				});
+				return isEqual(resource.fsPath, change.resource.fsPath) || isParent(resource.fsPath, change.resource.fsPath);
 			}
 
-			return resources.some((a: URI) => {
-				if (!a) {
-					return false;
-				}
-
-				return a.fsPath === change.resource.fsPath;
-			});
+			return isEqual(resource.fsPath, change.resource.fsPath);
 		});
 	}
 
@@ -283,6 +259,19 @@ export class FileChangesEvent extends events.Event {
 	}
 }
 
+export function isEqual(path1: string, path2: string) {
+	const identityEquals = (path1 === path2);
+	if (isLinux || identityEquals) {
+		return identityEquals;
+	}
+
+	return path1.toLowerCase() === path2.toLowerCase();
+}
+
+export function isParent(path: string, candidate: string): boolean {
+	return path.indexOf(candidate + paths.nativeSep) === 0;
+}
+
 export interface IBaseStat {
 
 	/**

src/vs/platform/files/test/events.test.ts

@@ -39,24 +39,24 @@ suite('Workbench Events', () => {
 
 	test('File Changes Event', function () {
 		let changes = [
-			{ resource: URI.file(Paths.join('C:\\', '/foo/updated.txt')), type: 0 },
-			{ resource: URI.file(Paths.join('C:\\', '/foo/otherupdated.txt')), type: 0 },
-			{ resource: URI.file(Paths.join('C:\\', '/added.txt')), type: 1 },
-			{ resource: URI.file(Paths.join('C:\\', '/bar/deleted.txt')), type: 2 },
-			{ resource: URI.file(Paths.join('C:\\', '/bar/folder')), type: 2 }
+			{ resource: URI.file(Paths.join('C:\\', '/foo/updated.txt')), type: Files.FileChangeType.UPDATED },
+			{ resource: URI.file(Paths.join('C:\\', '/foo/otherupdated.txt')), type: Files.FileChangeType.UPDATED },
+			{ resource: URI.file(Paths.join('C:\\', '/added.txt')), type: Files.FileChangeType.ADDED },
+			{ resource: URI.file(Paths.join('C:\\', '/bar/deleted.txt')), type: Files.FileChangeType.DELETED },
+			{ resource: URI.file(Paths.join('C:\\', '/bar/folder')), type: Files.FileChangeType.DELETED }
 		];
 
 		let r1 = new FileChangesEvent(changes);
 
-		assert(!r1.contains(toResource('/foo'), 0));
-		assert(r1.contains(toResource('/foo/updated.txt'), 0));
-		assert(!r1.contains(toResource('/foo/updated.txt'), 1));
-		assert(!r1.contains(toResource('/foo/updated.txt'), 2));
+		assert(!r1.contains(toResource('/foo'), Files.FileChangeType.UPDATED));
+		assert(r1.contains(toResource('/foo/updated.txt'), Files.FileChangeType.UPDATED));
+		assert(!r1.contains(toResource('/foo/updated.txt'), Files.FileChangeType.ADDED));
+		assert(!r1.contains(toResource('/foo/updated.txt'), Files.FileChangeType.DELETED));
 
-		assert(r1.contains(toResource('/bar/folder'), 2));
-		assert(r1.contains(toResource('/bar/folder/somefile'), 2));
-		assert(r1.contains(toResource('/bar/folder/somefile/test.txt'), 2));
-		assert(!r1.contains(toResource('/bar/folder2/somefile'), 2));
+		assert(r1.contains(toResource('/bar/folder'), Files.FileChangeType.DELETED));
+		assert(r1.contains(toResource('/bar/folder/somefile'), Files.FileChangeType.DELETED));
+		assert(r1.contains(toResource('/bar/folder/somefile/test.txt'), Files.FileChangeType.DELETED));
+		assert(!r1.contains(toResource('/bar/folder2/somefile'), Files.FileChangeType.DELETED));
 
 		assert.strictEqual(5, r1.changes.length);
 		assert.strictEqual(1, r1.getAdded().length);

src/vs/workbench/parts/files/common/editors/fileEditorInput.ts

@@ -10,7 +10,7 @@ import labels = require('vs/base/common/labels');
 import URI from 'vs/base/common/uri';
 import { EditorModel, EncodingMode, ConfirmResult } from 'vs/workbench/common/editor';
 import { BinaryEditorModel } from 'vs/workbench/common/editor/binaryEditorModel';
-import { IFileOperationResult, FileOperationResult, FileChangesEvent, EventType } from 'vs/platform/files/common/files';
+import { IFileOperationResult, FileOperationResult, FileChangesEvent, EventType, FileChangeType } from 'vs/platform/files/common/files';
 import { BINARY_FILE_EDITOR_ID, TEXT_FILE_EDITOR_ID, FILE_EDITOR_INPUT_ID, FileEditorInput as CommonFileEditorInput } from 'vs/workbench/parts/files/common/files';
 import { ITextFileService, AutoSaveMode, ModelState, TextFileModelChangeEvent, LocalFileChangeEvent } from 'vs/workbench/services/textfile/common/textfiles';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
@@ -78,9 +78,9 @@ export class FileEditorInput extends CommonFileEditorInput {
 	}
 
 	private onFileChanges(e: FileChangesEvent): void {
-		e.getDeleted().forEach(deleted => {
-			this.disposeIfRelated(deleted.resource);
-		});
+		if (e.gotDeleted()) {
+			this.disposeIfRelated(e);
+		}
 	}
 
 	private onDirtyStateChange(e: TextFileModelChangeEvent): void {
@@ -207,7 +207,7 @@ export class FileEditorInput extends CommonFileEditorInput {
 		});
 	}
 
-	private disposeIfRelated(resource: URI, movedTo?: URI): void {
+	private disposeIfRelated(arg1: URI | FileChangesEvent, movedTo?: URI): void {
 		if (this.isDirty()) {
 			return; // we never dispose dirty files
 		}
@@ -219,9 +219,14 @@ export class FileEditorInput extends CommonFileEditorInput {
 			return;
 		}
 
-		// Check if path is identical or path is a folder that the content is inside
-		if (paths.isEqualOrParent(this.resource.toString(), resource.toString())) {
-			this.historyService.remove(this);
+		let matches = false;
+		if (arg1 instanceof FileChangesEvent) {
+			matches = arg1.contains(this.resource, FileChangeType.DELETED);
+		} else {
+			matches = paths.isEqualOrParent(this.resource.toString(), arg1.toString());
+		}
+
+		if (matches) {
 			this.dispose();
 		}
 	}

src/vs/workbench/parts/files/common/editors/fileEditorTracker.ts

@@ -175,55 +175,27 @@ export class FileEditorTracker implements IWorkbenchContribution {
 		return input instanceof FileEditorInput && input.getResource().toString() === resource.toString();
 	}
 
-	private getMatchingFileEditorInputFromDiff(input: DiffEditorInput, deletedResource: URI): FileEditorInput;
-	private getMatchingFileEditorInputFromDiff(input: DiffEditorInput, updatedFiles: FileChangesEvent): FileEditorInput;
-	private getMatchingFileEditorInputFromDiff(input: DiffEditorInput, arg: any): FileEditorInput {
+	private getMatchingFileEditorInputFromDiff(input: DiffEditorInput, e: FileChangesEvent): FileEditorInput {
 
 		// First try modifiedInput
 		const modifiedInput = input.modifiedInput;
-		const res = this.getMatchingFileEditorInputFromInput(modifiedInput, arg);
+		const res = this.getMatchingFileEditorInputFromInput(modifiedInput, e);
 		if (res) {
 			return res;
 		}
 
 		// Second try originalInput
-		return this.getMatchingFileEditorInputFromInput(input.originalInput, arg);
+		return this.getMatchingFileEditorInputFromInput(input.originalInput, e);
 	}
 
-	private getMatchingFileEditorInputFromInput(input: EditorInput, deletedResource: URI): FileEditorInput;
-	private getMatchingFileEditorInputFromInput(input: EditorInput, updatedFiles: FileChangesEvent): FileEditorInput;
-	private getMatchingFileEditorInputFromInput(input: EditorInput, arg: any): FileEditorInput {
-		if (input instanceof FileEditorInput) {
-			if (arg instanceof URI) {
-				const deletedResource = <URI>arg;
-				if (this.containsResource(input, deletedResource)) {
-					return input;
-				}
-			} else {
-				const updatedFiles = <FileChangesEvent>arg;
-				if (updatedFiles.contains(input.getResource(), FileChangeType.UPDATED)) {
-					return input;
-				}
-			}
+	private getMatchingFileEditorInputFromInput(input: EditorInput, e: FileChangesEvent): FileEditorInput {
+		if (input instanceof FileEditorInput && e.contains(input.getResource(), FileChangeType.UPDATED)) {
+			return input;
 		}
 
 		return null;
 	}
 
-	private containsResource(input: FileEditorInput, resource: URI): boolean;
-	private containsResource(input: EditorInput, resource: URI): boolean {
-		let fileResource: URI;
-		if (input instanceof FileEditorInput) {
-			fileResource = input.getResource();
-		}
-
-		if (paths.isEqualOrParent(fileResource.fsPath, resource.fsPath)) {
-			return true;
-		}
-
-		return false;
-	}
-
 	public dispose(): void {
 		this.toUnbind = dispose(this.toUnbind);
 	}

src/vs/workbench/parts/files/common/explorerViewModel.ts

@@ -7,9 +7,8 @@
 
 import assert = require('vs/base/common/assert');
 import URI from 'vs/base/common/uri';
-import { isLinux } from 'vs/base/common/platform';
 import paths = require('vs/base/common/paths');
-import { IFileStat } from 'vs/platform/files/common/files';
+import { IFileStat, isEqual, isParent } from 'vs/platform/files/common/files';
 
 export enum StatType {
 	FILE,
@@ -257,7 +256,7 @@ export class FileStat implements IFileStat {
 	public find(resource: URI): FileStat {
 
 		// Return if path found
-		if (this.fileResourceEquals(resource, this.resource)) {
+		if (isEqual(resource.toString(), this.resource.toString())) {
 			return this;
 		}
 
@@ -269,26 +268,17 @@ export class FileStat implements IFileStat {
 		for (let i = 0; i < this.children.length; i++) {
 			const child = this.children[i];
 
-			if (this.fileResourceEquals(resource, child.resource)) {
+			if (isEqual(resource.toString(), child.resource.toString())) {
 				return child;
 			}
 
-			if (child.isDirectory && paths.isEqualOrParent(resource.fsPath, child.resource.fsPath)) {
+			if (child.isDirectory && isParent(resource.fsPath, child.resource.fsPath)) {
 				return child.find(resource);
 			}
 		}
 
 		return null; //Unable to find
 	}
-
-	private fileResourceEquals(r1: URI, r2: URI) {
-		const identityEquals = (r1.toString() === r2.toString());
-		if (isLinux || identityEquals) {
-			return identityEquals;
-		}
-
-		return r1.toString().toLowerCase() === r2.toString().toLowerCase();
-	}
 }
 
 /* A helper that can be used to show a placeholder when creating a new stat */

src/vs/workbench/services/files/node/watcher/common.ts

@@ -6,8 +6,7 @@
 'use strict';
 
 import uri from 'vs/base/common/uri';
-import paths = require('vs/base/common/paths');
-import { FileChangeType, FileChangesEvent } from 'vs/platform/files/common/files';
+import { FileChangeType, FileChangesEvent, isParent } from 'vs/platform/files/common/files';
 
 export interface IRawFileChange {
 	type: FileChangeType;
@@ -106,7 +105,7 @@ class EventNormalizer {
 		}).sort((e1, e2) => {
 			return e1.path.length - e2.path.length; // shortest path first
 		}).filter(e => {
-			if (deletedPaths.some(d => this.isParent(e.path, d))) {
+			if (deletedPaths.some(d => isParent(e.path, d))) {
 				return false; // DELETE is ignored if parent is deleted already
 			}
 
@@ -116,8 +115,4 @@ class EventNormalizer {
 			return true;
 		}).concat(addedChangeEvents);
 	}
-
-	private isParent(p: string, candidate: string): boolean {
-		return p.indexOf(candidate + paths.nativeSep) === 0;
-	}
 }
\ No newline at end of file

src/vs/workbench/services/history/browser/history.ts

@@ -9,7 +9,6 @@ import { TPromise } from 'vs/base/common/winjs.base';
 import errors = require('vs/base/common/errors');
 import platform = require('vs/base/common/platform');
 import nls = require('vs/nls');
-import paths = require('vs/base/common/paths');
 import URI from 'vs/base/common/uri';
 import product from 'vs/platform/product';
 import { IEditor as IBaseEditor } from 'vs/platform/editor/common/editor';
@@ -18,7 +17,7 @@ import { BaseTextEditor } from 'vs/workbench/browser/parts/editor/textEditor';
 import { IWorkbenchEditorService } from 'vs/workbench/services/editor/common/editorService';
 import { IEventService } from 'vs/platform/event/common/event';
 import { IHistoryService } from 'vs/workbench/services/history/common/history';
-import { FileChangesEvent, EventType } from 'vs/platform/files/common/files';
+import { FileChangesEvent, EventType, FileChangeType } from 'vs/platform/files/common/files';
 import { Selection } from 'vs/editor/common/core/selection';
 import { IEditorInput, ITextEditorOptions, IResourceInput } from 'vs/platform/editor/common/editor';
 import { IWorkspaceContextService } from 'vs/platform/workspace/common/workspace';
@@ -290,9 +289,9 @@ export class HistoryService extends BaseHistoryService implements IHistoryServic
 	}
 
 	private onFileChanges(e: FileChangesEvent): void {
-		e.getDeleted().forEach(deleted => {
-			this.remove(deleted.resource); // remove from history files that got deleted or moved
-		});
+		if (e.gotDeleted()) {
+			this.remove(e); // remove from history files that got deleted or moved
+		}
 	}
 
 	private onEditorClosed(event: IGroupEvent): void {
@@ -420,18 +419,18 @@ export class HistoryService extends BaseHistoryService implements IHistoryServic
 	}
 
 	public remove(input: IEditorInput | IResourceInput): void;
-	public remove(input: URI): void;
-	public remove(arg1: IEditorInput | IResourceInput | URI): void {
+	public remove(input: FileChangesEvent): void;
+	public remove(arg1: IEditorInput | IResourceInput | FileChangesEvent): void {
 		this.removeFromHistory(arg1);
 		this.removeFromStack(arg1);
 		this.removeFromRecentlyClosedFiles(arg1);
 	}
 
-	private removeFromHistory(input: IEditorInput | IResourceInput | URI, index?: number): void {
+	private removeFromHistory(arg1: IEditorInput | IResourceInput | FileChangesEvent, index?: number): void {
 		this.ensureLoaded();
 
 		if (typeof index !== 'number') {
-			index = this.indexOf(input);
+			index = this.indexOf(arg1);
 		}
 
 		if (index >= 0) {
@@ -439,10 +438,10 @@ export class HistoryService extends BaseHistoryService implements IHistoryServic
 		}
 	}
 
-	private indexOf(input: IEditorInput | IResourceInput | URI): number {
+	private indexOf(arg1: IEditorInput | IResourceInput | FileChangesEvent): number {
 		for (let i = 0; i < this.history.length; i++) {
 			const entry = this.history[i];
-			if (this.matches(input, entry)) {
+			if (this.matches(arg1, entry)) {
 				return i;
 			}
 		}
@@ -582,9 +581,9 @@ export class HistoryService extends BaseHistoryService implements IHistoryServic
 		return s1.startLineNumber === s2.startLineNumber; // we consider the history entry same if we are on the same line
 	}
 
-	private removeFromStack(input: IEditorInput | IResourceInput | URI): void {
+	private removeFromStack(arg1: IEditorInput | IResourceInput | FileChangesEvent): void {
 		this.stack.forEach((e, i) => {
-			if (this.matches(input, e.input)) {
+			if (this.matches(arg1, e.input)) {
 				this.stack.splice(i, 1);
 				if (this.index >= i) {
 					this.index--; // reduce index if the element is before index
@@ -593,9 +592,9 @@ export class HistoryService extends BaseHistoryService implements IHistoryServic
 		});
 	}
 
-	private removeFromRecentlyClosedFiles(input: IEditorInput | IResourceInput | URI): void {
+	private removeFromRecentlyClosedFiles(arg1: IEditorInput | IResourceInput | FileChangesEvent): void {
 		this.recentlyClosedFiles.forEach((e, i) => {
-			if (this.matchesFile(e.resource, input)) {
+			if (this.matchesFile(e.resource, arg1)) {
 				this.recentlyClosedFiles.splice(i, 1);
 			}
 		});
@@ -613,47 +612,47 @@ export class HistoryService extends BaseHistoryService implements IHistoryServic
 		return group.getEditors().some(e => this.matchesFile(resource, e));
 	}
 
-	private matches(inputA: IEditorInput | IResourceInput | URI, inputB: IEditorInput | IResourceInput): boolean {
-		if (inputA instanceof URI) {
+	private matches(arg1: IEditorInput | IResourceInput | FileChangesEvent, inputB: IEditorInput | IResourceInput): boolean {
+		if (arg1 instanceof FileChangesEvent) {
 			if (inputB instanceof EditorInput) {
 				return false; // we only support this for IResourceInput
 			}
 
 			const resourceInputB = inputB as IResourceInput;
 
-			return resourceInputB && paths.isEqualOrParent(resourceInputB.resource.toString(), inputA.toString());
+			return arg1.contains(resourceInputB.resource, FileChangeType.DELETED);
 		}
 
-		if (inputA instanceof EditorInput && inputB instanceof EditorInput) {
-			return inputA.matches(inputB);
+		if (arg1 instanceof EditorInput && inputB instanceof EditorInput) {
+			return arg1.matches(inputB);
 		}
 
-		if (inputA instanceof EditorInput) {
-			return this.matchesFile((inputB as IResourceInput).resource, inputA);
+		if (arg1 instanceof EditorInput) {
+			return this.matchesFile((inputB as IResourceInput).resource, arg1);
 		}
 
 		if (inputB instanceof EditorInput) {
-			return this.matchesFile((inputA as IResourceInput).resource, inputB);
+			return this.matchesFile((arg1 as IResourceInput).resource, inputB);
 		}
 
-		const resourceInputA = inputA as IResourceInput;
+		const resourceInputA = arg1 as IResourceInput;
 		const resourceInputB = inputB as IResourceInput;
 
 		return resourceInputA && resourceInputB && resourceInputA.resource.toString() === resourceInputB.resource.toString();
 	}
 
-	private matchesFile(resource: URI, input: IEditorInput | IResourceInput | URI): boolean {
-		if (input instanceof URI) {
-			return paths.isEqualOrParent(resource.toString(), input.toString());
+	private matchesFile(resource: URI, arg2: IEditorInput | IResourceInput | FileChangesEvent): boolean {
+		if (arg2 instanceof FileChangesEvent) {
+			return arg2.contains(resource, FileChangeType.DELETED);
 		}
 
-		if (input instanceof EditorInput) {
-			const fileInput = asFileEditorInput(input);
+		if (arg2 instanceof EditorInput) {
+			const fileInput = asFileEditorInput(arg2);
 
 			return fileInput && fileInput.getResource().toString() === resource.toString();
 		}
 
-		const resourceInput = input as IResourceInput;
+		const resourceInput = arg2 as IResourceInput;
 
 		return resourceInput && resourceInput.resource.toString() === resource.toString();
 	}