Skip to content

Commit

Permalink
resolves #89723
Browse files Browse the repository at this point in the history 
fyi @sandy081 @isidorn
  • Loading branch information
@sbatten
sbatten committed Feb 5, 2020
1 parent 967aab8 commit 34f2579
Show file tree
Hide file tree
Showing 15 changed files with 52 additions and 66 deletions.
8 changes: 4 additions & 4 deletions src/vs/workbench/browser/actions/navigationActions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ abstract class BaseNavigationAction extends Action {
return true;
}

protected navigateToPanel(): IPanel | boolean {
protected async navigateToPanel(): Promise<IPanel | boolean> {
if (!this.layoutService.isVisible(Parts.PANEL_PART)) {
return false;
}
Expand All @@ -75,7 +75,7 @@ abstract class BaseNavigationAction extends Action {

const activePanelId = activePanel.getId();

const res = this.panelService.openPanel(activePanelId, true);
const res = await this.panelService.openPanel(activePanelId, true);
if (!res) {
return false;
}
Expand Down Expand Up @@ -191,7 +191,7 @@ class NavigateRightAction extends BaseNavigationAction {
}

if (!isPanelPositionDown) {
return Promise.resolve(this.navigateToPanel());
return this.navigateToPanel();
}

if (!isSidebarPositionLeft) {
Expand Down Expand Up @@ -270,7 +270,7 @@ class NavigateDownAction extends BaseNavigationAction {
}

if (isPanelPositionDown) {
return Promise.resolve(this.navigateToPanel());
return this.navigateToPanel();
}

return Promise.resolve(false);
Expand Down
6 changes: 2 additions & 4 deletions src/vs/workbench/browser/panel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,12 @@ export abstract class TogglePanelAction extends Action {
super(id, label, cssClass);
}

run(): Promise<any> {
async run(): Promise<any> {
if (this.isPanelFocused()) {
this.layoutService.setPanelHidden(true);
} else {
this.panelService.openPanel(this.panelId, true);
await this.panelService.openPanel(this.panelId, true);
}

return Promise.resolve();
}

private isPanelActive(): boolean {
Expand Down
12 changes: 5 additions & 7 deletions src/vs/workbench/browser/parts/panel/panelActions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,10 +182,9 @@ export class PanelActivityAction extends ActivityAction {
super(activity);
}

run(event: any): Promise<any> {
this.panelService.openPanel(this.activity.id, true);
async run(event: any): Promise<any> {
await this.panelService.openPanel(this.activity.id, true);
this.activate();
return Promise.resolve();
}

setActivity(activity: IActivity): void {
Expand Down Expand Up @@ -225,11 +224,11 @@ export class SwitchPanelViewAction extends Action {
super(id, name);
}

run(offset: number): Promise<any> {
async run(offset: number): Promise<any> {
const pinnedPanels = this.panelService.getPinnedPanels();
const activePanel = this.panelService.getActivePanel();
if (!activePanel) {
return Promise.resolve();
return;
}
let targetPanelId: string | undefined;
for (let i = 0; i < pinnedPanels.length; i++) {
Expand All @@ -239,9 +238,8 @@ export class SwitchPanelViewAction extends Action {
}
}
if (typeof targetPanelId === 'string') {
this.panelService.openPanel(targetPanelId, true);
await this.panelService.openPanel(targetPanelId, true);
}
return Promise.resolve();
}
}

Expand Down
10 changes: 5 additions & 5 deletions src/vs/workbench/browser/parts/panel/panelPart.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export class PanelPart extends CompositePart<Panel> implements IPanelService {
this.compositeBar = this._register(this.instantiationService.createInstance(CompositeBar, this.getCachedPanels(), {
icon: false,
orientation: ActionsOrientation.HORIZONTAL,
openComposite: (compositeId: string) => Promise.resolve(this.openPanel(compositeId, true)),
openComposite: (compositeId: string) => this.openPanel(compositeId, true),
getActivityAction: (compositeId: string) => this.getCompositeActions(compositeId).activityAction,
getCompositePinnedAction: (compositeId: string) => this.getCompositeActions(compositeId).pinnedAction,
getOnCompositeClickAction: (compositeId: string) => this.instantiationService.createInstance(PanelActivityAction, assertIsDefined(this.getPanel(compositeId))),
Expand Down Expand Up @@ -355,7 +355,7 @@ export class PanelPart extends CompositePart<Panel> implements IPanelService {
}
}

openPanel(id: string, focus?: boolean): Panel | undefined {
doOpenPanel(id: string, focus?: boolean): Panel | undefined {
if (this.blockOpeningPanel) {
return undefined; // Workaround against a potential race condition
}
Expand All @@ -373,15 +373,15 @@ export class PanelPart extends CompositePart<Panel> implements IPanelService {
return this.openComposite(id, focus);
}

async openPanelAsync(id?: string, focus?: boolean): Promise<Panel | undefined> {
async openPanel(id?: string, focus?: boolean): Promise<Panel | undefined> {
if (typeof id === 'string' && this.getPanel(id)) {
return this.openPanel(id, focus);
return this.doOpenPanel(id, focus);
}

await this.extensionService.whenInstalledExtensionsRegistered();

if (typeof id === 'string' && this.getPanel(id)) {
return this.openPanel(id, focus);
return this.doOpenPanel(id, focus);
}

return undefined;
Expand Down
2 changes: 1 addition & 1 deletion src/vs/workbench/browser/parts/views/views.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ export class ViewsService extends Disposable implements IViewsService {
if (location === ViewContainerLocation.Sidebar) {
return this.viewletService.openViewlet(compositeId, focus);
} else if (location === ViewContainerLocation.Panel) {
return this.panelService.openPanel(compositeId, focus) as IPaneComposite;
return this.panelService.openPanel(compositeId, focus) as Promise<IPaneComposite>;
}
return undefined;
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/browser/workbench.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,9 @@ export class Workbench extends Layout {
restorePromises.push((async () => {
mark('willRestorePanel');

const panel = await panelService.openPanelAsync(this.state.panel.panelToRestore);
const panel = await panelService.openPanel(this.state.panel.panelToRestore!);
if (!panel) {
panelService.openPanel(Registry.as<PanelRegistry>(PanelExtensions.Panels).getDefaultPanelId()); // fallback to default panel as needed
await panelService.openPanel(Registry.as<PanelRegistry>(PanelExtensions.Panels).getDefaultPanelId()); // fallback to default panel as needed
}

mark('didRestorePanel');
Expand Down
36 changes: 18 additions & 18 deletions src/vs/workbench/contrib/bulkEdit/browser/bulkEdit.contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ import { IEditorInput } from 'vs/workbench/common/editor';
import type { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
import { CancellationTokenSource } from 'vs/base/common/cancellation';

function getBulkEditPane(panelService: IPanelService): BulkEditPane | undefined {
async function getBulkEditPane(panelService: IPanelService): Promise<BulkEditPane | undefined> {

This comment has been minimized.

Sign in to view

Copy link
@sbatten

sbatten Feb 5, 2020

Author Member

fyi @jrieken

This comment has been minimized.

Sign in to view

Copy link
@sandy081

sandy081 Feb 5, 2020

Member

Does this should be replaced with IViewsService.getActiveViewWithId ?

This comment has been minimized.

Sign in to view

Copy link
@sbatten

sbatten Feb 5, 2020

Author Member

so I believe the intent of this call is to force the view to be active which wouldn't happen with getActiveViewWithId but you are right that I should call openView on that service instead.

This comment has been minimized.

Sign in to view

Copy link
@sandy081

sandy081 Feb 5, 2020

Member

Because at the end of the day getActiveViewWithId is doing the same - getting the view from the container. But going forward the container need not be in panel.

This comment has been minimized.

Sign in to view

Copy link
@sbatten

sbatten Feb 5, 2020

Author Member

getActiveViewWithId will return the view iff it is active, otherwise null. openPanel and openView will first open the view guaranteeing it is active.

This comment has been minimized.

Sign in to view

Copy link
@sandy081

sandy081 Feb 5, 2020

Member

Yes, you are right. I misread the line that opens the panel. Agreed it has to be openView call.
let view: ViewPane | undefined;
const activePanel = panelService.openPanel(BulkEditPane.ID, true);
const activePanel = await panelService.openPanel(BulkEditPane.ID, true);
if (activePanel instanceof PaneCompositePanel) {
view = activePanel.getViewPaneContainer().getView(BulkEditPane.ID);
}
Expand All @@ -51,11 +51,11 @@ class UXState {
this._activePanel = _panelService.getActivePanel()?.getId();
}

restore(): void {
async restore(): Promise<void> {

// (1) restore previous panel
if (typeof this._activePanel === 'string') {
this._panelService.openPanel(this._activePanel);
await this._panelService.openPanel(this._activePanel);
} else {
this._panelService.hideActivePanel();
}
Expand Down Expand Up @@ -124,7 +124,7 @@ class BulkEditPreviewContribution {

// the actual work...
try {
const view = getBulkEditPane(this._panelService);
const view = await getBulkEditPane(this._panelService);
if (!view) {
return edit;
}
Expand All @@ -139,7 +139,7 @@ class BulkEditPreviewContribution {
} finally {
// restore UX state
if (this._activeSession === session) {
this._activeSession.uxState.restore();
await this._activeSession.uxState.restore();
this._activeSession.cts.dispose();
this._ctxEnabled.set(false);
this._activeSession = undefined;
Expand Down Expand Up @@ -174,9 +174,9 @@ registerAction2(class ApplyAction extends Action2 {
});
}

run(accessor: ServicesAccessor): any {
async run(accessor: ServicesAccessor): Promise<any> {
const panelService = accessor.get(IPanelService);
const view = getBulkEditPane(panelService);
const view = await getBulkEditPane(panelService);
if (view) {
view.accept();
}
Expand All @@ -203,9 +203,9 @@ registerAction2(class DiscardAction extends Action2 {
});
}

run(accessor: ServicesAccessor): void | Promise<void> {
async run(accessor: ServicesAccessor): Promise<void> {
const panelService = accessor.get(IPanelService);
const view = getBulkEditPane(panelService);
const view = await getBulkEditPane(panelService);
if (view) {
view.discard();
}
Expand Down Expand Up @@ -234,9 +234,9 @@ registerAction2(class ToggleAction extends Action2 {
});
}

run(accessor: ServicesAccessor): void | Promise<void> {
async run(accessor: ServicesAccessor): Promise<void> {
const panelService = accessor.get(IPanelService);
const view = getBulkEditPane(panelService);
const view = await getBulkEditPane(panelService);
if (view) {
view.toggleChecked();
}
Expand All @@ -263,9 +263,9 @@ registerAction2(class GroupByFile extends Action2 {
});
}

run(accessor: ServicesAccessor): void | Promise<void> {
async run(accessor: ServicesAccessor): Promise<void> {
const panelService = accessor.get(IPanelService);
const view = getBulkEditPane(panelService);
const view = await getBulkEditPane(panelService);
if (view) {
view.groupByFile();
}
Expand All @@ -290,9 +290,9 @@ registerAction2(class GroupByType extends Action2 {
});
}

run(accessor: ServicesAccessor): void | Promise<void> {
async run(accessor: ServicesAccessor): Promise<void> {
const panelService = accessor.get(IPanelService);
const view = getBulkEditPane(panelService);
const view = await getBulkEditPane(panelService);
if (view) {
view.groupByType();
}
Expand All @@ -316,9 +316,9 @@ registerAction2(class ToggleGrouping extends Action2 {
});
}

run(accessor: ServicesAccessor): void | Promise<void> {
async run(accessor: ServicesAccessor): Promise<void> {
const panelService = accessor.get(IPanelService);
const view = getBulkEditPane(panelService);
const view = await getBulkEditPane(panelService);
if (view) {
view.toggleGrouping();
}
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/comments/browser/commentsPanel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,11 +223,11 @@ export class CommentsPanel extends Panel {

CommandsRegistry.registerCommand({
id: 'workbench.action.focusCommentsPanel',
handler: (accessor) => {
handler: async (accessor) => {
const panelService = accessor.get(IPanelService);
const panels = panelService.getPanels();
if (panels.some(panelIdentifier => panelIdentifier.id === COMMENTS_PANEL_ID)) {
panelService.openPanel(COMMENTS_PANEL_ID, true);
await panelService.openPanel(COMMENTS_PANEL_ID, true);
}
}
});
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/debug/browser/debugTaskRunner.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export class DebugTaskRunner {
return TaskRunResult.Success;
}
if (onTaskErrors === 'showErrors') {
this.panelService.openPanel(Constants.MARKERS_PANEL_ID);
await this.panelService.openPanel(Constants.MARKERS_PANEL_ID);
return Promise.resolve(TaskRunResult.Failure);
}

Expand Down Expand Up @@ -97,7 +97,7 @@ export class DebugTaskRunner {
return TaskRunResult.Success;
}

this.panelService.openPanel(Constants.MARKERS_PANEL_ID);
await this.panelService.openPanel(Constants.MARKERS_PANEL_ID);
return Promise.resolve(TaskRunResult.Failure);
} catch (err) {
await onError(err.message, [this.taskService.configureAction()]);
Expand Down
8 changes: 4 additions & 4 deletions src/vs/workbench/contrib/markers/browser/markers.contribution.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ KeybindingsRegistry.registerCommandAndKeybindingRule({
weight: KeybindingWeight.WorkbenchContrib,
when: undefined,
primary: undefined,
handler: (accessor, args: any) => {
accessor.get(IPanelService).openPanel(Constants.MARKERS_PANEL_ID);
handler: async (accessor, args: any) => {
await accessor.get(IPanelService).openPanel(Constants.MARKERS_PANEL_ID);
}
});

Expand Down Expand Up @@ -314,13 +314,13 @@ MenuRegistry.appendMenuItem(MenuId.MenubarViewMenu, {
order: 4
});

CommandsRegistry.registerCommand('workbench.actions.view.toggleProblems', accessor => {
CommandsRegistry.registerCommand('workbench.actions.view.toggleProblems', async (accessor) => {
const panelService = accessor.get(IPanelService);
const panel = accessor.get(IPanelService).getActivePanel();
if (panel && panel.getId() === Constants.MARKERS_PANEL_ID) {
panelService.hideActivePanel();
} else {
panelService.openPanel(Constants.MARKERS_PANEL_ID, true);
await panelService.openPanel(Constants.MARKERS_PANEL_ID, true);
}
});

Expand Down
3 changes: 1 addition & 2 deletions src/vs/workbench/contrib/markers/browser/markersViewActions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ export class ShowProblemsPanelAction extends Action {
}

public run(): Promise<any> {
this.panelService.openPanel(Constants.MARKERS_PANEL_ID, true);
return Promise.resolve();
return this.panelService.openPanel(Constants.MARKERS_PANEL_ID, true);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,12 +413,12 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
return this.runShowTasks();
});

CommandsRegistry.registerCommand('workbench.action.tasks.toggleProblems', () => {
CommandsRegistry.registerCommand('workbench.action.tasks.toggleProblems', async () => {
const panel = this.panelService.getActivePanel();
if (panel && panel.getId() === Constants.MARKERS_PANEL_ID) {
this.layoutService.setPanelHidden(true);
} else {
this.panelService.openPanel(Constants.MARKERS_PANEL_ID, true);
await this.panelService.openPanel(Constants.MARKERS_PANEL_ID, true);
}
});

Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/terminal/browser/terminalService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -413,10 +413,10 @@ export class TerminalService implements ITerminalService {
}

public showPanel(focus?: boolean): Promise<void> {
return new Promise<void>((complete) => {
return new Promise<void>(async (complete) => {
const panel = this._panelService.getActivePanel();
if (!panel || panel.getId() !== TERMINAL_PANEL_ID) {
this._panelService.openPanel(TERMINAL_PANEL_ID, focus);
await this._panelService.openPanel(TERMINAL_PANEL_ID, focus);
if (focus) {
// Do the focus call asynchronously as going through the
// command palette will force editor focus
Expand Down
7 changes: 1 addition & 6 deletions src/vs/workbench/services/panel/common/panelService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,7 @@ export interface IPanelService {
/**
* Opens a panel with the given identifier and pass keyboard focus to it if specified.
*/
openPanel(id: string, focus?: boolean): IPanel | undefined;

/**
* Opens a panel with the given identifier and pass keyboard focus to it if specified.
*/
openPanelAsync(id?: string, focus?: boolean): Promise<IPanel | undefined>;
openPanel(id?: string, focus?: boolean): Promise<IPanel | undefined>;

/**
* Returns the current active panel or null if none
Expand Down
6 changes: 1 addition & 5 deletions src/vs/workbench/test/browser/workbenchTestServices.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -520,11 +520,7 @@ export class TestPanelService implements IPanelService {
onDidPanelOpen = new Emitter<{ panel: IPanel, focus: boolean }>().event;
onDidPanelClose = new Emitter<IPanel>().event;

openPanel(id: string, focus?: boolean): undefined {
return undefined;
}

async openPanelAsync(id?: string, focus?: boolean): Promise<undefined> {
async openPanel(id?: string, focus?: boolean): Promise<undefined> {
return undefined;
}

Expand Down

0 comments on commit 34f2579

Please sign in to comment.