Merge pull request #229884 from microsoft/andreamah/issue229878

call `expand` as async

src/vs/workbench/contrib/search/browser/searchActionsFind.ts

@@ -99,8 +99,8 @@ registerAction2(class ExpandSelectedTreeCommandAction extends Action2 {
 		});
 	}
 
-	override run(accessor: any) {
-		expandSelectSubtree(accessor);
+	override async run(accessor: any) {
+		return expandSelectSubtree(accessor);
 	}
 });
 
@@ -298,13 +298,13 @@ registerAction2(class FindInWorkspaceAction extends Action2 {
 });
 
 //#region Helpers
-function expandSelectSubtree(accessor: ServicesAccessor) {
+async function expandSelectSubtree(accessor: ServicesAccessor) {
 	const viewsService = accessor.get(IViewsService);
 	const searchView = getSearchView(viewsService);
 	if (searchView) {
 		const viewer = searchView.getControl();
 		const selected = viewer.getFocus()[0];
-		viewer.expand(selected, true);
+		await viewer.expand(selected, true);
 	}
 }
 

src/vs/workbench/contrib/search/browser/searchActionsNav.ts

@@ -453,9 +453,7 @@ async function focusNextSearchResult(accessor: ServicesAccessor): Promise<any> {
 		return (editorService.activeEditorPane as SearchEditor).focusNextResult();
 	}
 
-	return openSearchView(accessor.get(IViewsService)).then(searchView => {
-		searchView?.selectNextMatch();
-	});
+	return openSearchView(accessor.get(IViewsService)).then(searchView => searchView?.selectNextMatch());
 }
 
 async function focusPreviousSearchResult(accessor: ServicesAccessor): Promise<any> {
@@ -466,9 +464,7 @@ async function focusPreviousSearchResult(accessor: ServicesAccessor): Promise<an
 		return (editorService.activeEditorPane as SearchEditor).focusPreviousResult();
 	}
 
-	return openSearchView(accessor.get(IViewsService)).then(searchView => {
-		searchView?.selectPreviousMatch();
-	});
+	return openSearchView(accessor.get(IViewsService)).then(searchView => searchView?.selectPreviousMatch());
 }
 
 async function findOrReplaceInFiles(accessor: ServicesAccessor, expandSearchReplaceWidget: boolean): Promise<any> {

src/vs/workbench/contrib/search/browser/searchActionsRemoveReplace.ts

@@ -113,7 +113,7 @@ registerAction2(class RemoveAction extends Action2 {
 		let nextFocusElement;
 		const shouldRefocusMatch = shouldRefocus(elementsToRemove, focusElement);
 		if (focusElement && shouldRefocusMatch) {
-			nextFocusElement = getElementToFocusAfterRemoved(viewer, focusElement, elementsToRemove);
+			nextFocusElement = await getElementToFocusAfterRemoved(viewer, focusElement, elementsToRemove);
 		}
 
 		const searchResult = searchView.searchResult;
@@ -126,7 +126,7 @@ registerAction2(class RemoveAction extends Action2 {
 
 		if (focusElement && shouldRefocusMatch) {
 			if (!nextFocusElement) {
-				nextFocusElement = getLastNodeFromSameType(viewer, focusElement);
+				nextFocusElement = await getLastNodeFromSameType(viewer, focusElement);
 			}
 
 			if (nextFocusElement && !arrayContainsElementOrParent(nextFocusElement, elementsToRemove)) {
@@ -281,7 +281,7 @@ async function performReplace(accessor: ServicesAccessor,
 	}
 	let nextFocusElement;
 	if (focusElement) {
-		nextFocusElement = getElementToFocusAfterRemoved(viewer, focusElement, elementsToReplace);
+		nextFocusElement = await getElementToFocusAfterRemoved(viewer, focusElement, elementsToReplace);
 	}
 
 	const searchResult = viewlet?.searchResult;
@@ -294,7 +294,7 @@ async function performReplace(accessor: ServicesAccessor,
 
 	if (focusElement) {
 		if (!nextFocusElement) {
-			nextFocusElement = getLastNodeFromSameType(viewer, focusElement);
+			nextFocusElement = await getLastNodeFromSameType(viewer, focusElement);
 		}
 
 		if (nextFocusElement) {
@@ -370,17 +370,17 @@ function compareLevels(elem1: RenderableMatch, elem2: RenderableMatch) {
 /**
  * Returns element to focus after removing the given element
  */
-export function getElementToFocusAfterRemoved(viewer: WorkbenchCompressibleAsyncDataTree<SearchResult, RenderableMatch>, element: RenderableMatch, elementsToRemove: RenderableMatch[]): RenderableMatch | undefined {
+export async function getElementToFocusAfterRemoved(viewer: WorkbenchCompressibleAsyncDataTree<SearchResult, RenderableMatch>, element: RenderableMatch, elementsToRemove: RenderableMatch[]): Promise<RenderableMatch | undefined> {
 	const navigator: ITreeNavigator<any> = viewer.navigate(element);
 	if (element instanceof FolderMatch) {
 		while (!!navigator.next() && (!(navigator.current() instanceof FolderMatch) || arrayContainsElementOrParent(navigator.current(), elementsToRemove))) { }
 	} else if (element instanceof FileMatch) {
 		while (!!navigator.next() && (!(navigator.current() instanceof FileMatch) || arrayContainsElementOrParent(navigator.current(), elementsToRemove))) {
-			viewer.expand(navigator.current());
+			await viewer.expand(navigator.current());
 		}
 	} else {
 		while (navigator.next() && (!(navigator.current() instanceof Match) || arrayContainsElementOrParent(navigator.current(), elementsToRemove))) {
-			viewer.expand(navigator.current());
+			await viewer.expand(navigator.current());
 		}
 	}
 	return navigator.current();
@@ -389,13 +389,16 @@ export function getElementToFocusAfterRemoved(viewer: WorkbenchCompressibleAsync
 /***
  * Finds the last element in the tree with the same type as `element`
  */
-export function getLastNodeFromSameType(viewer: WorkbenchCompressibleAsyncDataTree<SearchResult, RenderableMatch>, element: RenderableMatch): RenderableMatch | undefined {
+export async function getLastNodeFromSameType(viewer: WorkbenchCompressibleAsyncDataTree<SearchResult, RenderableMatch>, element: RenderableMatch): Promise<RenderableMatch | undefined> {
 	let lastElem: RenderableMatch | null = viewer.lastVisibleElement ?? null;
 
 	while (lastElem) {
 		const compareVal = compareLevels(element, lastElem);
 		if (compareVal === -1) {
-			viewer.expand(lastElem);
+			const expanded = await viewer.expand(lastElem);
+			if (!expanded) {
+				return lastElem;
+			}
 			lastElem = viewer.lastVisibleElement;
 		} else if (compareVal === 1) {
 			const potentialLastElem = viewer.getParentElement(lastElem);

src/vs/workbench/contrib/search/browser/searchActionsTopBar.ts

@@ -127,7 +127,7 @@ registerAction2(class ExpandAllAction extends Action2 {
 			}]
 		});
 	}
-	run(accessor: ServicesAccessor, ...args: any[]) {
+	async run(accessor: ServicesAccessor, ...args: any[]) {
 		return expandAll(accessor);
 	}
 });
@@ -214,7 +214,7 @@ const clearHistoryCommand: ICommandHandler = accessor => {
 	searchHistoryService.clearHistory();
 };
 
-function expandAll(accessor: ServicesAccessor) {
+async function expandAll(accessor: ServicesAccessor) {
 	const viewsService = accessor.get(IViewsService);
 	const searchView = getSearchView(viewsService);
 	if (searchView) {
@@ -224,7 +224,7 @@ function expandAll(accessor: ServicesAccessor) {
 			if (searchView.model.hasAIResults) {
 				viewer.expandAll();
 			} else {
-				viewer.expand(searchView.model.searchResult.plainTextSearchResult, true);
+				await viewer.expand(searchView.model.searchResult.plainTextSearchResult, true);
 			}
 		} else {
 			viewer.expandAll();

src/vs/workbench/contrib/search/browser/searchView.ts

@@ -394,17 +394,17 @@ export class SearchView extends ViewPane {
 
 		asyncResults.then((complete) => {
 			clearTimeout(slowTimer);
-			this.onSearchComplete(progressComplete, undefined, undefined, complete);
+			return this.onSearchComplete(progressComplete, undefined, undefined, complete);
 		}, (e) => {
 			clearTimeout(slowTimer);
-			this.onSearchError(e, progressComplete, undefined, undefined);
+			return this.onSearchError(e, progressComplete, undefined, undefined);
 		});
 
 		const collapseResults = this.searchConfig.collapseResults;
 		if (collapseResults !== 'alwaysCollapse' && this.viewModel.searchResult.matches().length === 1) {
 			const onlyMatch = this.viewModel.searchResult.matches()[0];
 			if (onlyMatch.count() < 50) {
-				this.tree.expand(onlyMatch);
+				await this.tree.expand(onlyMatch);
 			}
 		}
 	}
@@ -1017,7 +1017,7 @@ export class SearchView extends ViewPane {
 		return false;
 	}
 
-	selectNextMatch(): void {
+	async selectNextMatch(): Promise<void> {
 		if (!this.hasSearchResults()) {
 			return;
 		}
@@ -1027,7 +1027,7 @@ export class SearchView extends ViewPane {
 		// Expand the initial selected node, if needed
 		if (selected && !(selected instanceof Match)) {
 			if (this.tree.isCollapsed(selected)) {
-				this.tree.expand(selected);
+				await this.tree.expand(selected);
 			}
 		}
 
@@ -1041,7 +1041,7 @@ export class SearchView extends ViewPane {
 		// Expand until first child is a Match
 		while (next && !(next instanceof Match)) {
 			if (this.tree.isCollapsed(next)) {
-				this.tree.expand(next);
+				await this.tree.expand(next);
 			}
 
 			// Select the first child
@@ -1062,7 +1062,7 @@ export class SearchView extends ViewPane {
 		}
 	}
 
-	selectPreviousMatch(): void {
+	async selectPreviousMatch(): Promise<void> {
 		if (!this.hasSearchResults()) {
 			return;
 		}
@@ -1089,7 +1089,7 @@ export class SearchView extends ViewPane {
 			if (!nextItem) {
 				break;
 			}
-			this.tree.expand(prev);
+			await this.tree.expand(prev);
 			navigator = this.tree.navigate(nextItem); // recreate navigator because modifying the tree can invalidate it
 			prev = nextItem ? navigator.previous() : navigator.last(); // select last child
 		}
@@ -1642,7 +1642,7 @@ export class SearchView extends ViewPane {
 		}
 	}
 
-	private onSearchComplete(progressComplete: () => void, excludePatternText?: string, includePatternText?: string, completed?: ISearchComplete) {
+	private async onSearchComplete(progressComplete: () => void, excludePatternText?: string, includePatternText?: string, completed?: ISearchComplete) {
 
 		this.state = SearchUIState.Idle;
 
@@ -1656,7 +1656,7 @@ export class SearchView extends ViewPane {
 		if (collapseResults !== 'alwaysCollapse' && this.viewModel.searchResult.matches().length === 1) {
 			const onlyMatch = this.viewModel.searchResult.matches()[0];
 			if (onlyMatch.count() < 50) {
-				this.tree.expand(onlyMatch);
+				await this.tree.expand(onlyMatch);
 			}
 		}
 
@@ -1745,7 +1745,7 @@ export class SearchView extends ViewPane {
 		this.reLayout();
 	}
 
-	private onSearchError(e: any, progressComplete: () => void, excludePatternText?: string, includePatternText?: string, completed?: ISearchComplete) {
+	private async onSearchError(e: any, progressComplete: () => void, excludePatternText?: string, includePatternText?: string, completed?: ISearchComplete) {
 		this.state = SearchUIState.Idle;
 		if (errors.isCancellationError(e)) {
 			return this.onSearchComplete(progressComplete, excludePatternText, includePatternText, completed);
@@ -1787,10 +1787,10 @@ export class SearchView extends ViewPane {
 		const result = this.viewModel.addAIResults();
 		return result.then((complete) => {
 			clearTimeout(slowTimer);
-			this.onSearchComplete(progressComplete, excludePatternText, includePatternText, complete);
+			return this.onSearchComplete(progressComplete, excludePatternText, includePatternText, complete);
 		}, (e) => {
 			clearTimeout(slowTimer);
-			this.onSearchError(e, progressComplete, excludePatternText, includePatternText);
+			return this.onSearchError(e, progressComplete, excludePatternText, includePatternText);
 		});
 
 	}
@@ -1822,10 +1822,10 @@ export class SearchView extends ViewPane {
 		const result = this.viewModel.search(query);
 		return result.asyncResults.then((complete) => {
 			clearTimeout(slowTimer);
-			this.onSearchComplete(progressComplete, excludePatternText, includePatternText, complete);
+			return this.onSearchComplete(progressComplete, excludePatternText, includePatternText, complete);
 		}, (e) => {
 			clearTimeout(slowTimer);
-			this.onSearchError(e, progressComplete, excludePatternText, includePatternText);
+			return this.onSearchError(e, progressComplete, excludePatternText, includePatternText);
 		});
 	}
 

src/vs/workbench/contrib/search/test/browser/searchActions.test.ts

@@ -42,28 +42,28 @@ suite('Search Actions', () => {
 		instantiationService.dispose();
 	});
 
-	test('get next element to focus after removing a match when it has next sibling file', function () {
+	test('get next element to focus after removing a match when it has next sibling file', async function () {
 		const fileMatch1 = aFileMatch();
 		const fileMatch2 = aFileMatch();
 		const data = [fileMatch1, aMatch(fileMatch1), aMatch(fileMatch1), fileMatch2, aMatch(fileMatch2), aMatch(fileMatch2)];
 		const tree = aTree(data);
 		const target = data[2];
 
-		const actual = getElementToFocusAfterRemoved(tree, target, [target]);
+		const actual = await getElementToFocusAfterRemoved(tree, target, [target]);
 		assert.strictEqual(data[4], actual);
 	});
 
-	test('get next element to focus after removing a match when it is the only match', function () {
+	test('get next element to focus after removing a match when it is the only match', async function () {
 		const fileMatch1 = aFileMatch();
 		const data = [fileMatch1, aMatch(fileMatch1)];
 		const tree = aTree(data);
 		const target = data[1];
 
-		const actual = getElementToFocusAfterRemoved(tree, target, [target]);
+		const actual = await getElementToFocusAfterRemoved(tree, target, [target]);
 		assert.strictEqual(undefined, actual);
 	});
 
-	test('get next element to focus after removing a file match when it has next sibling', function () {
+	test('get next element to focus after removing a file match when it has next sibling', async function () {
 		const fileMatch1 = aFileMatch();
 		const fileMatch2 = aFileMatch();
 		const fileMatch3 = aFileMatch();
@@ -71,40 +71,40 @@ suite('Search Actions', () => {
 		const tree = aTree(data);
 		const target = data[2];
 
-		const actual = getElementToFocusAfterRemoved(tree, target, []);
+		const actual = await getElementToFocusAfterRemoved(tree, target, []);
 		assert.strictEqual(data[4], actual);
 	});
 
-	test('Find last FileMatch in Tree', function () {
+	test('Find last FileMatch in Tree', async function () {
 		const fileMatch1 = aFileMatch();
 		const fileMatch2 = aFileMatch();
 		const fileMatch3 = aFileMatch();
 		const data = [fileMatch1, aMatch(fileMatch1), fileMatch2, aMatch(fileMatch2), fileMatch3, aMatch(fileMatch3)];
 		const tree = aTree(data);
 
-		const actual = getLastNodeFromSameType(tree, fileMatch1);
+		const actual = await getLastNodeFromSameType(tree, fileMatch1);
 		assert.strictEqual(fileMatch3, actual);
 	});
 
-	test('Find last Match in Tree', function () {
+	test('Find last Match in Tree', async function () {
 		const fileMatch1 = aFileMatch();
 		const fileMatch2 = aFileMatch();
 		const fileMatch3 = aFileMatch();
 		const data = [fileMatch1, aMatch(fileMatch1), fileMatch2, aMatch(fileMatch2), fileMatch3, aMatch(fileMatch3)];
 		const tree = aTree(data);
 
-		const actual = getLastNodeFromSameType(tree, aMatch(fileMatch1));
+		const actual = await getLastNodeFromSameType(tree, aMatch(fileMatch1));
 		assert.strictEqual(data[5], actual);
 	});
 
-	test('get next element to focus after removing a file match when it is only match', function () {
+	test('get next element to focus after removing a file match when it is only match', async function () {
 		const fileMatch1 = aFileMatch();
 		const data = [fileMatch1, aMatch(fileMatch1)];
 		const tree = aTree(data);
 		const target = data[0];
 		// const testObject: ReplaceAction = instantiationService.createInstance(ReplaceAction, tree, target, null);
 
-		const actual = getElementToFocusAfterRemoved(tree, target, []);
+		const actual = await getElementToFocusAfterRemoved(tree, target, []);
 		assert.strictEqual(undefined, actual);
 	});