Skip to content

Commit 5737c7e

Browse files
authored
allow to reuse a menu id and only to create one when none with the id exists (microsoft#155042)
fixes microsoft#155030
1 parent 3f4cc87 commit 5737c7e

File tree

3 files changed

+25
-5
lines changed

3 files changed

+25
-5
lines changed

src/vs/platform/actions/common/actions.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export function isISubmenuItem(item: IMenuItem | ISubmenuItem): item is ISubmenu
4545

4646
export class MenuId {
4747

48-
private static readonly _idPool = new Set<string>();
48+
private static readonly _instances = new Map<string, MenuId>();
4949

5050
static readonly CommandPalette = new MenuId('CommandPalette');
5151
static readonly DebugBreakpointsContext = new MenuId('DebugBreakpointsContext');
@@ -162,14 +162,25 @@ export class MenuId {
162162
static readonly NewFile = new MenuId('NewFile');
163163
static readonly MergeToolbar = new MenuId('MergeToolbar');
164164

165+
/**
166+
* Create or reuse a `MenuId` with the given identifier
167+
*/
168+
static for(identifier: string): MenuId {
169+
return MenuId._instances.get(identifier) ?? new MenuId(identifier);
170+
}
165171

166172
readonly id: string;
167173

174+
/**
175+
* Create a new `MenuId` with the unique identifier. Will throw if a menu
176+
* with the identifier already exists, use `MenuId.for(ident)` or a unique
177+
* identifier
178+
*/
168179
constructor(identifier: string) {
169-
if (MenuId._idPool.has(identifier)) {
170-
throw new Error(`Duplicate menu identifier ${identifier}`);
180+
if (MenuId._instances.has(identifier)) {
181+
throw new TypeError(`MenuId with identifier '${identifier}' already exists. Use MenuId.for(ident) or a unique identifier`);
171182
}
172-
MenuId._idPool.add(identifier);
183+
MenuId._instances.set(identifier, this);
173184
this.id = identifier;
174185
}
175186
}

src/vs/platform/actions/test/common/menuService.test.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,13 @@ suite('MenuService', function () {
202202
assert.strictEqual(foundA, true);
203203
assert.strictEqual(foundB, true);
204204
});
205+
206+
test('Extension contributed submenus missing with errors in output #155030', function () {
207+
208+
const id = generateUuid();
209+
const menu = new MenuId(id);
210+
211+
assert.throws(() => new MenuId(id));
212+
assert.ok(menu === MenuId.for(id));
213+
});
205214
});

src/vs/workbench/services/actions/common/menusExtensionPoint.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ submenusExtensionPoint.setHandler(extensions => {
717717
}
718718

719719
const item: IRegisteredSubmenu = {
720-
id: new MenuId(`api:${submenuInfo.id}`),
720+
id: MenuId.for(`api:${submenuInfo.id}`),
721721
label: submenuInfo.label,
722722
icon: absoluteIcon
723723
};

0 commit comments

Comments
 (0)