Skip to content

Commit de320f4

Browse files
mjbvzmjbvz
committed
Use a proper Map for ICommandsMap
Changes the `ICommandsMap` type to be a proper map. This can help catch type errors and also gets us closer to being able to disable the tsconfig suppressImplicitAnyIndexErrors workaround
1 parent 612901a commit de320f4

8 files changed

Lines changed: 43 additions & 47 deletions

File tree

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

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -121,72 +121,67 @@ export interface IMenuService {
121121
createMenu(id: MenuId, scopedKeybindingService: IContextKeyService): IMenu;
122122
}
123123

124+
export type ICommandsMap = Map<string, ICommandAction>;
125+
124126
export interface IMenuRegistry {
125127
addCommand(userCommand: ICommandAction): IDisposable;
126-
getCommand(id: string): ICommandAction;
128+
getCommand(id: string): ICommandAction | undefined;
127129
getCommands(): ICommandsMap;
128130
appendMenuItem(menu: MenuId, item: IMenuItem | ISubmenuItem): IDisposable;
129131
getMenuItems(loc: MenuId): Array<IMenuItem | ISubmenuItem>;
130132
readonly onDidChangeMenu: Event<MenuId>;
131133
}
132134

133-
export interface ICommandsMap {
134-
[id: string]: ICommandAction;
135-
}
136-
137135
export const MenuRegistry: IMenuRegistry = new class implements IMenuRegistry {
138136

139-
private readonly _commands: { [id: string]: ICommandAction } = Object.create(null);
140-
private readonly _menuItems: { [loc: number]: Array<IMenuItem | ISubmenuItem> } = Object.create(null);
137+
private readonly _commands = new Map<string, ICommandAction>();
138+
private readonly _menuItems = new Map<number, Array<IMenuItem | ISubmenuItem>>();
141139
private readonly _onDidChangeMenu = new Emitter<MenuId>();
142140

143141
readonly onDidChangeMenu: Event<MenuId> = this._onDidChangeMenu.event;
144142

145143
addCommand(command: ICommandAction): IDisposable {
146-
this._commands[command.id] = command;
144+
this._commands.set(command.id, command);
147145
this._onDidChangeMenu.fire(MenuId.CommandPalette);
148146
return {
149147
dispose: () => {
150-
if (delete this._commands[command.id]) {
148+
if (this._commands.delete(command.id)) {
151149
this._onDidChangeMenu.fire(MenuId.CommandPalette);
152150
}
153151
}
154152
};
155153
}
156154

157-
getCommand(id: string): ICommandAction {
158-
return this._commands[id];
155+
getCommand(id: string): ICommandAction | undefined {
156+
return this._commands.get(id);
159157
}
160158

161159
getCommands(): ICommandsMap {
162-
const result: ICommandsMap = Object.create(null);
163-
for (const key in this._commands) {
164-
result[key] = this.getCommand(key);
165-
}
166-
return result;
160+
return new Map<string, ICommandAction>(this._commands.entries());
167161
}
168162

169163
appendMenuItem(id: MenuId, item: IMenuItem | ISubmenuItem): IDisposable {
170-
let array = this._menuItems[id];
164+
let array = this._menuItems.get(id);
171165
if (!array) {
172-
this._menuItems[id] = array = [item];
166+
array = [item];
167+
this._menuItems.set(id, array);
173168
} else {
174169
array.push(item);
175170
}
176171
this._onDidChangeMenu.fire(id);
177172
return {
178173
dispose: () => {
179-
const idx = array.indexOf(item);
174+
const idx = array!.indexOf(item);
180175
if (idx >= 0) {
181-
array.splice(idx, 1);
176+
array!.splice(idx, 1);
182177
this._onDidChangeMenu.fire(id);
183178
}
184179
}
185180
};
186181
}
187182

188183
getMenuItems(id: MenuId): Array<IMenuItem | ISubmenuItem> {
189-
const result = (this._menuItems[id] || []).slice(0);
184+
const result = (this._menuItems.get(id) || []).slice(0);
190185

191186
if (id === MenuId.CommandPalette) {
192187
// CommandPalette is special because it shows
@@ -207,9 +202,9 @@ export const MenuRegistry: IMenuRegistry = new class implements IMenuRegistry {
207202
set.add(alt.id);
208203
}
209204
}
210-
for (let id in this._commands) {
205+
for (const [id, command] of this._commands) {
211206
if (!set.has(id)) {
212-
result.push({ command: this._commands[id] });
207+
result.push({ command });
213208
}
214209
}
215210
}

‎src/vs/platform/commands/common/commands.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,7 @@ export interface ICommandService {
2222
executeCommand<T = any>(commandId: string, ...args: any[]): Promise<T | undefined>;
2323
}
2424

25-
export interface ICommandsMap {
26-
[id: string]: ICommand;
27-
}
25+
export type ICommandsMap = Map<string, ICommand>;
2826

2927
export interface ICommandHandler {
3028
(accessor: ServicesAccessor, ...args: any[]): void;
@@ -122,10 +120,13 @@ export const CommandsRegistry: ICommandRegistry = new class implements ICommandR
122120
}
123121

124122
getCommands(): ICommandsMap {
125-
const result: ICommandsMap = Object.create(null);
126-
this._commands.forEach((value, key) => {
127-
result[key] = this.getCommand(key)!;
128-
});
123+
const result = new Map<string, ICommand>();
124+
for (const key of this._commands.keys()) {
125+
const command = this.getCommand(key);
126+
if (command) {
127+
result.set(key, command);
128+
}
129+
}
129130
return result;
130131
}
131132
};

‎src/vs/platform/commands/test/commands.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ suite('Command Tests', function () {
6868
}
6969
});
7070

71-
CommandsRegistry.getCommands()['test'].handler.apply(undefined, [undefined!, 'string']);
72-
CommandsRegistry.getCommands()['test2'].handler.apply(undefined, [undefined!, 'string']);
73-
assert.throws(() => CommandsRegistry.getCommands()['test3'].handler.apply(undefined, [undefined!, 'string']));
74-
assert.equal(CommandsRegistry.getCommands()['test3'].handler.apply(undefined, [undefined!, 1]), true);
71+
CommandsRegistry.getCommands().get('test')!.handler.apply(undefined, [undefined!, 'string']);
72+
CommandsRegistry.getCommands().get('test2')!.handler.apply(undefined, [undefined!, 'string']);
73+
assert.throws(() => CommandsRegistry.getCommands().get('test3')!.handler.apply(undefined, [undefined!, 'string']));
74+
assert.equal(CommandsRegistry.getCommands().get('test3')!.handler.apply(undefined, [undefined!, 1]), true);
7575

7676
});
7777
});

‎src/vs/platform/keybinding/common/keybindingResolver.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -334,10 +334,10 @@ export class KeybindingResolver {
334334
}
335335
unboundCommands.push(id);
336336
};
337-
for (const id in MenuRegistry.getCommands()) {
337+
for (const id of MenuRegistry.getCommands().keys()) {
338338
addCommand(id, true);
339339
}
340-
for (const id in CommandsRegistry.getCommands()) {
340+
for (const id of CommandsRegistry.getCommands().keys()) {
341341
addCommand(id, false);
342342
}
343343

‎src/vs/workbench/api/browser/mainThreadCommands.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,9 @@ export class MainThreadCommands implements MainThreadCommandsShape {
3636
return this._proxy.$getContributedCommandHandlerDescriptions().then(result => {
3737
// add local commands
3838
const commands = CommandsRegistry.getCommands();
39-
for (let id in commands) {
40-
let { description } = commands[id];
41-
if (description) {
42-
result[id] = description;
39+
for (const [id, command] of commands) {
40+
if (command.description) {
41+
result[id] = command.description;
4342
}
4443
}
4544

@@ -79,7 +78,7 @@ export class MainThreadCommands implements MainThreadCommandsShape {
7978
}
8079

8180
$getCommands(): Promise<string[]> {
82-
return Promise.resolve(Object.keys(CommandsRegistry.getCommands()));
81+
return Promise.resolve([...CommandsRegistry.getCommands().keys()]);
8382
}
8483
}
8584

‎src/vs/workbench/services/keybinding/browser/keybindingService.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -675,8 +675,8 @@ function updateSchema() {
675675
};
676676

677677
const allCommands = CommandsRegistry.getCommands();
678-
for (let commandId in allCommands) {
679-
const commandDescription = allCommands[commandId].description;
678+
for (const [commandId, command] of allCommands) {
679+
const commandDescription = command.description;
680680

681681
addKnownCommand(commandId, commandDescription ? commandDescription.description : undefined);
682682

@@ -704,7 +704,7 @@ function updateSchema() {
704704
}
705705

706706
const menuCommands = MenuRegistry.getCommands();
707-
for (let commandId in menuCommands) {
707+
for (const commandId of menuCommands.keys()) {
708708
addKnownCommand(commandId);
709709
}
710710
}

‎src/vs/workbench/services/preferences/common/keybindingsEditorModel.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ export class KeybindingsEditorModel extends EditorModel {
210210
}
211211

212212
private static toKeybindingEntry(command: string, keybindingItem: ResolvedKeybindingItem, workbenchActionsRegistry: IWorkbenchActionRegistry, editorActions: { [id: string]: string; }): IKeybindingItem {
213-
const menuCommand = MenuRegistry.getCommand(command);
213+
const menuCommand = MenuRegistry.getCommand(command)!;
214214
const editorActionLabel = editorActions[command];
215215
return <IKeybindingItem>{
216216
keybinding: keybindingItem.resolvedKeybinding,

‎src/vs/workbench/test/electron-browser/api/extHostApiCommands.test.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,11 @@ suite('ExtHostLanguageFeatureCommands', function () {
6868
instantiationService.stub(ICommandService, {
6969
_serviceBrand: undefined,
7070
executeCommand(id: string, args: any): any {
71-
if (!CommandsRegistry.getCommands()[id]) {
71+
const command = CommandsRegistry.getCommands().get(id);
72+
if (!command) {
7273
return Promise.reject(new Error(id + ' NOT known'));
7374
}
74-
let { handler } = CommandsRegistry.getCommands()[id];
75+
const { handler } = command;
7576
return Promise.resolve(instantiationService.invokeFunction(handler, args));
7677
}
7778
});

0 commit comments

Comments
 (0)