Allow unregistering registerSchemaContribution (#233733)

This makes `registerSchemaContribution` return a disposable which unregisters the contribution
This commit is contained in:
Matt Bierner 2025-06-10 11:04:30 -07:00 committed by GitHub
parent 47fb40495e
commit f5c3e1cf2a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 41 additions and 22 deletions

View File

@ -591,8 +591,8 @@ export class StandaloneKeybindingService extends AbstractKeybindingService {
return '';
}
public registerSchemaContribution(contribution: KeybindingsSchemaContribution): void {
// noop
public registerSchemaContribution(contribution: KeybindingsSchemaContribution): IDisposable {
return Disposable.None;
}
/**

View File

@ -11,7 +11,7 @@ import { Emitter, Event } from '../../../base/common/event.js';
import { IME } from '../../../base/common/ime.js';
import { KeyCode } from '../../../base/common/keyCodes.js';
import { Keybinding, ResolvedChord, ResolvedKeybinding, SingleModifierChord } from '../../../base/common/keybindings.js';
import { Disposable } from '../../../base/common/lifecycle.js';
import { Disposable, IDisposable } from '../../../base/common/lifecycle.js';
import * as nls from '../../../nls.js';
import { ICommandService } from '../../commands/common/commands.js';
@ -89,7 +89,7 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
public abstract resolveKeybinding(keybinding: Keybinding): ResolvedKeybinding[];
public abstract resolveKeyboardEvent(keyboardEvent: IKeyboardEvent): ResolvedKeybinding;
public abstract resolveUserBinding(userBinding: string): ResolvedKeybinding[];
public abstract registerSchemaContribution(contribution: KeybindingsSchemaContribution): void;
public abstract registerSchemaContribution(contribution: KeybindingsSchemaContribution): IDisposable;
public abstract _dumpDebugInfo(): string;
public abstract _dumpDebugInfoJSON(): string;

View File

@ -7,6 +7,7 @@ import { Event } from '../../../base/common/event.js';
import { IJSONSchema } from '../../../base/common/jsonSchema.js';
import { KeyCode } from '../../../base/common/keyCodes.js';
import { ResolvedKeybinding, Keybinding } from '../../../base/common/keybindings.js';
import { IDisposable } from '../../../base/common/lifecycle.js';
import { IContextKeyService, IContextKeyServiceTarget } from '../../contextkey/common/contextkey.js';
import { createDecorator } from '../../instantiation/common/instantiation.js';
import { ResolutionResult } from './keybindingResolver.js';
@ -101,7 +102,7 @@ export interface IKeybindingService {
*/
mightProducePrintableCharacter(event: IKeyboardEvent): boolean;
registerSchemaContribution(contribution: KeybindingsSchemaContribution): void;
registerSchemaContribution(contribution: KeybindingsSchemaContribution): IDisposable;
toggleLogging(): boolean;

View File

@ -5,7 +5,7 @@
import assert from 'assert';
import { KeyChord, KeyCode, KeyMod } from '../../../../base/common/keyCodes.js';
import { createSimpleKeybinding, ResolvedKeybinding, KeyCodeChord, Keybinding } from '../../../../base/common/keybindings.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { Disposable, IDisposable } from '../../../../base/common/lifecycle.js';
import { OS } from '../../../../base/common/platform.js';
import Severity from '../../../../base/common/severity.js';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../base/test/common/utils.js';
@ -93,8 +93,8 @@ suite('AbstractKeybindingService', () => {
return '';
}
public registerSchemaContribution() {
// noop
public registerSchemaContribution(): IDisposable {
return Disposable.None;
}
public enableKeybindingHoldMode() {

View File

@ -4,7 +4,8 @@
*--------------------------------------------------------------------------------------------*/
import { Event } from '../../../../base/common/event.js';
import { ResolvedKeybinding, KeyCodeChord, Keybinding } from '../../../../base/common/keybindings.js';
import { KeyCodeChord, Keybinding, ResolvedKeybinding } from '../../../../base/common/keybindings.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { OS } from '../../../../base/common/platform.js';
import { ContextKeyExpression, ContextKeyValue, IContextKey, IContextKeyChangeEvent, IContextKeyService, IContextKeyServiceTarget, IScopedContextKeyService } from '../../../contextkey/common/contextkey.js';
import { IKeybindingService, IKeyboardEvent } from '../../common/keybinding.js';
@ -168,6 +169,6 @@ export class MockKeybindingService implements IKeybindingService {
}
public registerSchemaContribution() {
// noop
return Disposable.None;
}
}

View File

@ -10,6 +10,7 @@ import * as browser from '../../../../base/browser/browser.js';
import { BrowserFeatures, KeyboardSupport } from '../../../../base/browser/canIUse.js';
import * as dom from '../../../../base/browser/dom.js';
import { printKeyboardEvent, printStandardKeyboardEvent, StandardKeyboardEvent } from '../../../../base/browser/keyboardEvent.js';
import { mainWindow } from '../../../../base/browser/window.js';
import { DeferredPromise, RunOnceScheduler } from '../../../../base/common/async.js';
import { Emitter, Event } from '../../../../base/common/event.js';
import { parse } from '../../../../base/common/json.js';
@ -18,13 +19,13 @@ import { UserSettingsLabelProvider } from '../../../../base/common/keybindingLab
import { KeybindingParser } from '../../../../base/common/keybindingParser.js';
import { Keybinding, KeyCodeChord, ResolvedKeybinding, ScanCodeChord } from '../../../../base/common/keybindings.js';
import { IMMUTABLE_CODE_TO_KEY_CODE, KeyCode, KeyCodeUtils, KeyMod, ScanCode, ScanCodeUtils } from '../../../../base/common/keyCodes.js';
import { Disposable, DisposableStore, IDisposable } from '../../../../base/common/lifecycle.js';
import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../../base/common/lifecycle.js';
import * as objects from '../../../../base/common/objects.js';
import { isMacintosh, OperatingSystem, OS } from '../../../../base/common/platform.js';
import { dirname } from '../../../../base/common/resources.js';
import { mainWindow } from '../../../../base/browser/window.js';
// platform
import { ILocalizedString, isLocalizedString } from '../../../../platform/action/common/action.js';
import { MenuRegistry } from '../../../../platform/actions/common/actions.js';
import { CommandsRegistry, ICommandService } from '../../../../platform/commands/common/commands.js';
import { ContextKeyExpr, ContextKeyExpression, IContextKey, IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js';
@ -44,17 +45,17 @@ import { INotificationService } from '../../../../platform/notification/common/n
import { Registry } from '../../../../platform/registry/common/platform.js';
import { ITelemetryService } from '../../../../platform/telemetry/common/telemetry.js';
import { IUriIdentityService } from '../../../../platform/uriIdentity/common/uriIdentity.js';
import { ILocalizedString, isLocalizedString } from '../../../../platform/action/common/action.js';
// workbench
import { remove } from '../../../../base/common/arrays.js';
import { commandsExtensionPoint } from '../../actions/common/menusExtensionPoint.js';
import { IExtensionService } from '../../extensions/common/extensions.js';
import { ExtensionMessageCollector, ExtensionsRegistry } from '../../extensions/common/extensionsRegistry.js';
import { IHostService } from '../../host/browser/host.js';
import { IUserDataProfileService } from '../../userDataProfile/common/userDataProfile.js';
import { IUserKeybindingItem, KeybindingIO, OutputBuilder } from '../common/keybindingIO.js';
import { IKeyboard, INavigatorWithKeyboard } from './navigatorKeyboard.js';
import { getAllUnboundCommands } from './unboundCommands.js';
import { IUserKeybindingItem, KeybindingIO, OutputBuilder } from '../common/keybindingIO.js';
import { IUserDataProfileService } from '../../userDataProfile/common/userDataProfile.js';
interface ContributedKeyBinding {
command: string;
@ -184,7 +185,10 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
private userKeybindings: UserKeybindings;
private isComposingGlobalContextKey: IContextKey<boolean>;
private _keybindingHoldMode: DeferredPromise<void> | null;
private readonly _contributions: KeybindingsSchemaContribution[] = [];
private readonly _contributions: Array<{
readonly listener?: IDisposable;
readonly contribution: KeybindingsSchemaContribution;
}> = [];
private readonly kbsJsonSchema: KeybindingsJsonSchema;
constructor(
@ -266,6 +270,13 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
}));
}
public override dispose(): void {
this._contributions.forEach(c => c.listener?.dispose());
this._contributions.length = 0;
super.dispose();
}
private _registerKeyListeners(window: Window): IDisposable {
const disposables = new DisposableStore();
@ -300,16 +311,22 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
return disposables;
}
public registerSchemaContribution(contribution: KeybindingsSchemaContribution): void {
this._contributions.push(contribution);
if (contribution.onDidChange) {
this._register(contribution.onDidChange(() => this.updateKeybindingsJsonSchema()));
}
public registerSchemaContribution(contribution: KeybindingsSchemaContribution): IDisposable {
const listener = contribution.onDidChange?.(() => this.updateKeybindingsJsonSchema());
const entry = { listener, contribution };
this._contributions.push(entry);
this.updateKeybindingsJsonSchema();
return toDisposable(() => {
listener?.dispose();
remove(this._contributions, entry);
this.updateKeybindingsJsonSchema();
});
}
private updateKeybindingsJsonSchema() {
this.kbsJsonSchema.updateSchema(this._contributions.flatMap(x => x.getSchemaAdditions()));
this.kbsJsonSchema.updateSchema(this._contributions.flatMap(x => x.contribution.getSchemaAdditions()));
}
private _printKeybinding(keybinding: Keybinding): string {