Skip to content

Commit 780d15a

Browse files
isidornisidorn
authored
Merge pull request microsoft#66657 from Microsoft/sandy081/66653
Support deregistering a composite
2 parents e71cf22 + 7587bec commit 780d15a

16 files changed

Lines changed: 172 additions & 214 deletions

File tree

‎src/tsconfig.strictNullChecks.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -672,7 +672,6 @@
672672
"./vs/workbench/parts/search/browser/patternInputWidget.ts",
673673
"./vs/workbench/parts/search/browser/replaceContributions.ts",
674674
"./vs/workbench/parts/search/browser/replaceService.ts",
675-
"./vs/workbench/parts/search/browser/searchViewLocationUpdater.ts",
676675
"./vs/workbench/parts/search/common/constants.ts",
677676
"./vs/workbench/parts/search/common/queryBuilder.ts",
678677
"./vs/workbench/parts/search/common/replace.ts",

‎src/vs/workbench/api/electron-browser/mainThreadComments.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,16 @@ import { IEditorService } from 'vs/workbench/services/editor/common/editorServic
1212
import { ExtHostCommentsShape, ExtHostContext, IExtHostContext, MainContext, MainThreadCommentsShape, CommentProviderFeatures } from '../node/extHost.protocol';
1313

1414
import { ICommentService } from 'vs/workbench/parts/comments/electron-browser/commentService';
15-
import { COMMENTS_PANEL_ID } from 'vs/workbench/parts/comments/electron-browser/commentsPanel';
15+
import { COMMENTS_PANEL_ID, CommentsPanel, COMMENTS_PANEL_TITLE } from 'vs/workbench/parts/comments/electron-browser/commentsPanel';
1616
import { IPanelService } from 'vs/workbench/services/panel/common/panelService';
1717
import { URI } from 'vs/base/common/uri';
1818
import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
1919
import { generateUuid } from 'vs/base/common/uuid';
2020
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
2121
import { ICommentsConfiguration } from 'vs/workbench/parts/comments/electron-browser/comments.contribution';
2222
import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
23+
import { Registry } from 'vs/platform/registry/common/platform';
24+
import { PanelRegistry, Extensions as PanelExtensions, PanelDescriptor } from 'vs/workbench/browser/panel';
2325

2426
export class MainThreadDocumentCommentProvider implements modes.DocumentCommentProvider {
2527
private _proxy: ExtHostCommentsShape;
@@ -133,11 +135,16 @@ export class MainThreadComments extends Disposable implements MainThreadComments
133135
this._handlers.set(handle, providerId);
134136

135137
const commentsPanelAlreadyConstructed = this._panelService.getPanels().some(panel => panel.id === COMMENTS_PANEL_ID);
136-
this._panelService.setPanelEnablement(COMMENTS_PANEL_ID, true);
138+
Registry.as<PanelRegistry>(PanelExtensions.Panels).registerPanel(new PanelDescriptor(
139+
CommentsPanel,
140+
COMMENTS_PANEL_ID,
141+
COMMENTS_PANEL_TITLE,
142+
'commentsPanel',
143+
10
144+
));
137145

138146
const openPanel = this._configurationService.getValue<ICommentsConfiguration>('comments').openPanel;
139147

140-
141148
if (openPanel === 'neverOpen') {
142149
this.registerOpenPanelListener(commentsPanelAlreadyConstructed);
143150
}
@@ -180,7 +187,7 @@ export class MainThreadComments extends Disposable implements MainThreadComments
180187
$unregisterWorkspaceCommentProvider(handle: number): void {
181188
this._workspaceProviders.delete(handle);
182189
if (this._workspaceProviders.size === 0) {
183-
this._panelService.setPanelEnablement(COMMENTS_PANEL_ID, false);
190+
Registry.as<PanelRegistry>(PanelExtensions.Panels).deregisterPanel(COMMENTS_PANEL_ID);
184191

185192
if (this._openPanelListener) {
186193
this._openPanelListener.dispose();

‎src/vs/workbench/browser/composite.ts‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,6 @@ export abstract class Composite extends Component implements IComposite {
215215
*/
216216
export abstract class CompositeDescriptor<T extends Composite> {
217217

218-
public enabled: boolean = true;
219-
220218
constructor(
221219
private readonly ctor: IConstructorSignature0<T>,
222220
public readonly id: string,
@@ -236,6 +234,9 @@ export abstract class CompositeRegistry<T extends Composite> extends Disposable
236234
private readonly _onDidRegister: Emitter<CompositeDescriptor<T>> = this._register(new Emitter<CompositeDescriptor<T>>());
237235
get onDidRegister(): Event<CompositeDescriptor<T>> { return this._onDidRegister.event; }
238236

237+
private readonly _onDidDeregister: Emitter<CompositeDescriptor<T>> = this._register(new Emitter<CompositeDescriptor<T>>());
238+
get onDidDeregister(): Event<CompositeDescriptor<T>> { return this._onDidDeregister.event; }
239+
239240
private composites: CompositeDescriptor<T>[] = [];
240241

241242
protected registerComposite(descriptor: CompositeDescriptor<T>): void {
@@ -247,6 +248,16 @@ export abstract class CompositeRegistry<T extends Composite> extends Disposable
247248
this._onDidRegister.fire(descriptor);
248249
}
249250

251+
protected deregisterComposite(id: string): void {
252+
const descriptor = this.compositeById(id);
253+
if (descriptor === null) {
254+
return;
255+
}
256+
257+
this.composites.splice(this.composites.indexOf(descriptor), 1);
258+
this._onDidDeregister.fire(descriptor);
259+
}
260+
250261
getComposite(id: string): CompositeDescriptor<T> | null {
251262
return this.compositeById(id);
252263
}

‎src/vs/workbench/browser/panel.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ export class PanelRegistry extends CompositeRegistry<Panel> {
3434
super.registerComposite(descriptor);
3535
}
3636

37+
/**
38+
* Deregisters a panel to the platform.
39+
*/
40+
deregisterPanel(id: string): void {
41+
super.deregisterComposite(id);
42+
}
43+
3744
/**
3845
* Returns an array of registered panels known to the platform.
3946
*/

‎src/vs/workbench/browser/parts/activitybar/activitybarPart.ts‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,25 +93,19 @@ export class ActivitybarPart extends Part {
9393
}));
9494

9595
this.registerListeners();
96-
this.onDidRegisterViewlets(viewletService.getAllViewlets());
96+
this.onDidRegisterViewlets(viewletService.getViewlets());
9797
}
9898

9999
private registerListeners(): void {
100100

101101
this._register(this.viewletService.onDidViewletRegister(viewlet => this.onDidRegisterViewlets([viewlet])));
102+
this._register(this.viewletService.onDidViewletDeregister(({ id }) => this.removeComposite(id, true)));
102103

103104
// Activate viewlet action on opening of a viewlet
104105
this._register(this.viewletService.onDidViewletOpen(viewlet => this.onDidViewletOpen(viewlet)));
105106

106107
// Deactivate viewlet action on close
107108
this._register(this.viewletService.onDidViewletClose(viewlet => this.compositeBar.deactivateComposite(viewlet.getId())));
108-
this._register(this.viewletService.onDidViewletEnablementChange(({ id, enabled }) => {
109-
if (enabled) {
110-
this.compositeBar.addComposite(this.viewletService.getViewlet(id));
111-
} else {
112-
this.removeComposite(id, true);
113-
}
114-
}));
115109

116110
let disposables: IDisposable[] = [];
117111
this._register(this.extensionService.onDidRegisterExtensions(() => {
@@ -125,7 +119,7 @@ export class ActivitybarPart extends Part {
125119

126120
private onDidRegisterExtensions(): void {
127121
this.removeNotExistingComposites();
128-
for (const viewlet of this.viewletService.getAllViewlets()) {
122+
for (const viewlet of this.viewletService.getViewlets()) {
129123
this.enableCompositeActions(viewlet);
130124
const viewContainer = this.getViewContainer(viewlet.id);
131125
if (viewContainer) {
@@ -299,7 +293,7 @@ export class ActivitybarPart extends Part {
299293
}
300294

301295
private removeNotExistingComposites(): void {
302-
const viewlets = this.viewletService.getAllViewlets();
296+
const viewlets = this.viewletService.getViewlets();
303297
for (const { id } of this.cachedViewlets) {
304298
if (viewlets.every(viewlet => viewlet.id !== id)) {
305299
this.removeComposite(id, false);
@@ -395,7 +389,7 @@ export class ActivitybarPart extends Part {
395389
private saveCachedViewlets(): void {
396390
const state: ICachedViewlet[] = [];
397391
const compositeItems = this.compositeBar.getCompositeBarItems();
398-
const allViewlets = this.viewletService.getAllViewlets();
392+
const allViewlets = this.viewletService.getViewlets();
399393
for (const compositeItem of compositeItems) {
400394
const viewContainer = this.getViewContainer(compositeItem.id);
401395
const viewlet = allViewlets.filter(({ id }) => id === compositeItem.id)[0];

‎src/vs/workbench/browser/parts/compositePart.ts‎

Lines changed: 43 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import 'vs/css!./media/compositepart';
77
import * as nls from 'vs/nls';
88
import { defaultGenerator } from 'vs/base/common/idGenerator';
9-
import { IDisposable, dispose } from 'vs/base/common/lifecycle';
9+
import { IDisposable, dispose, toDisposable } from 'vs/base/common/lifecycle';
1010
import * as strings from 'vs/base/common/strings';
1111
import { Emitter } from 'vs/base/common/event';
1212
import * as errors from 'vs/base/common/errors';
@@ -46,20 +46,24 @@ export interface ICompositeTitleLabel {
4646
updateStyles(): void;
4747
}
4848

49+
interface CompositeItem {
50+
composite: Composite;
51+
disposable: IDisposable;
52+
progressService: IProgressService;
53+
}
54+
4955
export abstract class CompositePart<T extends Composite> extends Part {
5056

5157
protected _onDidCompositeOpen = this._register(new Emitter<{ composite: IComposite, focus: boolean }>());
5258
protected _onDidCompositeClose = this._register(new Emitter<IComposite>());
5359

5460
protected toolBar: ToolBar;
5561

56-
private instantiatedCompositeListeners: IDisposable[];
5762
private mapCompositeToCompositeContainer: { [compositeId: string]: HTMLElement; };
5863
private mapActionsBindingToComposite: { [compositeId: string]: () => void; };
59-
private mapProgressServiceToComposite: { [compositeId: string]: IProgressService; };
6064
private activeComposite: Composite | null;
6165
private lastActiveCompositeId: string;
62-
private instantiatedComposites: Composite[];
66+
private instantiatedCompositeItems: Map<string, CompositeItem>;
6367
private titleLabel: ICompositeTitleLabel;
6468
private progressBar: ProgressBar;
6569
private contentAreaSize: Dimension;
@@ -86,12 +90,10 @@ export abstract class CompositePart<T extends Composite> extends Part {
8690
) {
8791
super(id, options, themeService, storageService);
8892

89-
this.instantiatedCompositeListeners = [];
9093
this.mapCompositeToCompositeContainer = {};
9194
this.mapActionsBindingToComposite = {};
92-
this.mapProgressServiceToComposite = {};
9395
this.activeComposite = null;
94-
this.instantiatedComposites = [];
96+
this.instantiatedCompositeItems = new Map<string, CompositeItem>();
9597
this.lastActiveCompositeId = storageService.get(activeCompositeSettingsKey, StorageScope.WORKSPACE, this.defaultCompositeId);
9698
}
9799

@@ -159,10 +161,9 @@ export abstract class CompositePart<T extends Composite> extends Part {
159161
protected createComposite(id: string, isActive?: boolean): Composite {
160162

161163
// Check if composite is already created
162-
for (const composite of this.instantiatedComposites) {
163-
if (composite.getId() === id) {
164-
return composite;
165-
}
164+
const compositeItem = this.instantiatedCompositeItems.get(id);
165+
if (compositeItem) {
166+
return compositeItem.composite;
166167
}
167168

168169
// Instantiate composite from registry otherwise
@@ -172,13 +173,13 @@ export abstract class CompositePart<T extends Composite> extends Part {
172173
const compositeInstantiationService = this.instantiationService.createChild(new ServiceCollection([IProgressService, progressService]));
173174

174175
const composite = compositeDescriptor.instantiate(compositeInstantiationService);
175-
this.mapProgressServiceToComposite[composite.getId()] = progressService;
176+
const disposables: IDisposable[] = [];
176177

177178
// Remember as Instantiated
178-
this.instantiatedComposites.push(composite);
179+
this.instantiatedCompositeItems.set(id, { composite, disposable: toDisposable(() => dispose(disposables)), progressService });
179180

180181
// Register to title area update events from the composite
181-
this.instantiatedCompositeListeners.push(composite.onTitleAreaUpdate(() => this.onTitleAreaUpdate(composite.getId())));
182+
composite.onTitleAreaUpdate(() => this.onTitleAreaUpdate(composite.getId()), this, disposables);
182183

183184
return composite;
184185
}
@@ -219,9 +220,9 @@ export abstract class CompositePart<T extends Composite> extends Part {
219220
}
220221

221222
// Report progress for slow loading composites (but only if we did not create the composites before already)
222-
const progressService = this.mapProgressServiceToComposite[composite.getId()];
223-
if (progressService && !compositeContainer) {
224-
this.mapProgressServiceToComposite[composite.getId()].showWhile(Promise.resolve(), this.partService.isRestored() ? 800 : 3200 /* less ugly initial startup */);
223+
const compositeItem = this.instantiatedCompositeItems.get(composite.getId());
224+
if (compositeItem && !compositeContainer) {
225+
compositeItem.progressService.showWhile(Promise.resolve(), this.partService.isRestored() ? 800 : 3200 /* less ugly initial startup */);
225226
}
226227

227228
// Fill Content and Actions
@@ -446,8 +447,9 @@ export abstract class CompositePart<T extends Composite> extends Part {
446447
return contentContainer;
447448
}
448449

449-
getProgressIndicator(id: string): IProgressService {
450-
return this.mapProgressServiceToComposite[id];
450+
getProgressIndicator(id: string): IProgressService | null {
451+
const compositeItem = this.instantiatedCompositeItems.get(id);
452+
return compositeItem ? compositeItem.progressService : null;
451453
}
452454

453455
protected getActions(): IAction[] {
@@ -476,17 +478,33 @@ export abstract class CompositePart<T extends Composite> extends Part {
476478
return sizes;
477479
}
478480

481+
protected removeComposite(compositeId: string): boolean {
482+
if (this.activeComposite && this.activeComposite.getId() === compositeId) {
483+
// do not remove active compoiste
484+
return false;
485+
}
486+
487+
delete this.mapCompositeToCompositeContainer[compositeId];
488+
delete this.mapActionsBindingToComposite[compositeId];
489+
const compositeItem = this.instantiatedCompositeItems.get(compositeId);
490+
if (compositeItem) {
491+
compositeItem.composite.dispose();
492+
dispose(compositeItem.disposable);
493+
this.instantiatedCompositeItems.delete(compositeId);
494+
}
495+
return true;
496+
}
497+
479498
dispose(): void {
480499
this.mapCompositeToCompositeContainer = null!; // StrictNullOverride: nulling out ok in dispose
481-
this.mapProgressServiceToComposite = null!; // StrictNullOverride: nulling out ok in dispose
482500
this.mapActionsBindingToComposite = null!; // StrictNullOverride: nulling out ok in dispose
483501

484-
for (const composite of this.instantiatedComposites) {
485-
composite.dispose();
486-
}
502+
this.instantiatedCompositeItems.forEach(compositeItem => {
503+
compositeItem.composite.dispose();
504+
dispose(compositeItem.disposable);
505+
});
487506

488-
this.instantiatedComposites = [];
489-
this.instantiatedCompositeListeners = dispose(this.instantiatedCompositeListeners);
507+
this.instantiatedCompositeItems.clear();
490508

491509
super.dispose();
492510
}

0 commit comments

Comments
 (0)