Skip to content

Commit

Permalink
Fix reveal in explorer race condition (#197291)
Browse files Browse the repository at this point in the history
Fixes #197268
  • Loading branch information
lramos15 authored Nov 2, 2023
1 parent 6d16777 commit 0e9376c
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 9 deletions.
10 changes: 8 additions & 2 deletions src/vs/workbench/contrib/files/browser/fileCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { OPEN_TO_SIDE_COMMAND_ID, COMPARE_WITH_SAVED_COMMAND_ID, SELECT_FOR_COMP
import { IFileDialogService } from 'vs/platform/dialogs/common/dialogs';
import { RemoveRootFolderAction } from 'vs/workbench/browser/actions/workspaceActions';
import { OpenEditorsView } from 'vs/workbench/contrib/files/browser/views/openEditorsView';
import { ExplorerView } from 'vs/workbench/contrib/files/browser/views/explorerView';

export const openWindowCommand = (accessor: ServicesAccessor, toOpen: IWindowOpenable[], options?: IOpenWindowOptions) => {
if (Array.isArray(toOpen)) {
Expand Down Expand Up @@ -326,13 +327,18 @@ CommandsRegistry.registerCommand({
const explorerService = accessor.get(IExplorerService);
const uri = getResourceForCommand(resource, accessor.get(IListService), accessor.get(IEditorService));


if (uri && contextService.isInsideWorkspace(uri)) {
const explorerView = await viewService.openView(VIEW_ID, false);
const explorerView = viewService.getViewWithId<ExplorerView>(VIEW_ID);
if (explorerView) {
const oldAutoReveal = explorerView.autoReveal;
// Disable autoreveal before revealing the explorer to prevent a race betwene auto reveal + selection
// Fixes #197268
explorerView.autoReveal = false;
await viewService.openView<ExplorerView>(VIEW_ID, false);
explorerView.setExpanded(true);
await explorerService.select(uri, 'force');
explorerView.focus();
explorerView.autoReveal = oldAutoReveal;
}
} else {
const openEditorsView = await viewService.openView(OpenEditorsView.ID, false);
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/files/browser/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export interface IExplorerService {
export const IExplorerService = createDecorator<IExplorerService>('explorerService');

export interface IExplorerView {
autoReveal: boolean | 'force' | 'focusNoScroll';
getContext(respectMultiSelection: boolean): ExplorerItem[];
refresh(recursive: boolean, item?: ExplorerItem): Promise<void>;
selectResource(resource: URI | undefined, reveal?: boolean | string): Promise<void>;
Expand Down
27 changes: 20 additions & 7 deletions src/vs/workbench/contrib/files/browser/views/explorerView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ export class ExplorerView extends ViewPane implements IExplorerView {
private horizontalScrolling: boolean | undefined;

private dragHandler!: DelayedDragHandler;
private autoReveal: boolean | 'force' | 'focusNoScroll' = false;
private _autoReveal: boolean | 'force' | 'focusNoScroll' = false;
private decorationsProvider: ExplorerDecorationsProvider | undefined;
private readonly delegate: IExplorerViewContainerDelegate | undefined;

Expand Down Expand Up @@ -227,6 +227,14 @@ export class ExplorerView extends ViewPane implements IExplorerView {
this.explorerService.registerView(this);
}

get autoReveal() {
return this._autoReveal;
}

set autoReveal(autoReveal: boolean | 'force' | 'focusNoScroll') {
this._autoReveal = autoReveal;
}

get name(): string {
return this.labelService.getWorkspaceLabel(this.contextService.getWorkspace());
}
Expand Down Expand Up @@ -320,7 +328,7 @@ export class ExplorerView extends ViewPane implements IExplorerView {
this.tree.domFocus();

const focused = this.tree.getFocus();
if (focused.length === 1 && this.autoReveal) {
if (focused.length === 1 && this._autoReveal) {
this.tree.reveal(focused[0], 0.5);
}
}
Expand Down Expand Up @@ -373,8 +381,8 @@ export class ExplorerView extends ViewPane implements IExplorerView {
}
}

private selectActiveFile(reveal = this.autoReveal): void {
if (this.autoReveal) {
private async selectActiveFile(reveal = this._autoReveal): Promise<void> {
if (this._autoReveal) {
const activeFile = EditorResourceAccessor.getCanonicalUri(this.editorService.activeEditor, { supportSideBySide: SideBySideEditor.PRIMARY });

if (activeFile) {
Expand All @@ -384,7 +392,7 @@ export class ExplorerView extends ViewPane implements IExplorerView {
// No action needed, active file is already focused and selected
return;
}
this.explorerService.select(activeFile, reveal);
return this.explorerService.select(activeFile, reveal);
}
}
}
Expand Down Expand Up @@ -536,7 +544,7 @@ export class ExplorerView extends ViewPane implements IExplorerView {
private onConfigurationUpdated(event: IConfigurationChangeEvent | undefined): void {
if (!event || event.affectsConfiguration('explorer.autoReveal')) {
const configuration = this.configurationService.getValue<IFilesConfiguration>();
this.autoReveal = configuration?.explorer?.autoReveal;
this._autoReveal = configuration?.explorer?.autoReveal;
}

// Push down config updates to components of viewer
Expand Down Expand Up @@ -750,7 +758,7 @@ export class ExplorerView extends ViewPane implements IExplorerView {
}
}

public async selectResource(resource: URI | undefined, reveal = this.autoReveal, retry = 0): Promise<void> {
public async selectResource(resource: URI | undefined, reveal = this._autoReveal, retry = 0): Promise<void> {
// do no retry more than once to prevent infinite loops in cases of inconsistent model
if (retry === 2) {
return;
Expand All @@ -760,6 +768,11 @@ export class ExplorerView extends ViewPane implements IExplorerView {
return;
}

// If something is refreshing the explorer, we must await it or else a selection race condition can occur
if (this.setTreeInputPromise) {
await this.setTreeInputPromise;
}

// Expand all stats in the parent chain.
let item: ExplorerItem | null = this.explorerService.findClosestRoot(resource);

Expand Down

0 comments on commit 0e9376c

Please sign in to comment.