Skip to content

Commit 4ea73f7

Browse files
authored
Merge pull request #671 from CodinGame/lmn/fix-global-picker-without-global-keybindings
fix global picker without global keybindings
2 parents a3fc78f + a1c2899 commit 4ea73f7

File tree

2 files changed

+361
-0
lines changed

2 files changed

+361
-0
lines changed
Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,321 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: =?UTF-8?q?Lo=C3=AFc=20Mangeonjean?= <loic@coderpad.io>
3+
Date: Wed, 6 Aug 2025 18:38:26 +0200
4+
Subject: [PATCH] refactor: make editors register themself
5+
6+
instead of scanning them in the keybinding service
7+
---
8+
.../widget/codeEditor/codeEditorWidget.ts | 6 ++
9+
.../codeEditor/embeddedCodeEditorWidget.ts | 6 +-
10+
.../widget/diffEditor/diffEditorWidget.ts | 4 +
11+
.../diffEditor/embeddedDiffEditorWidget.ts | 6 +-
12+
.../browser/standaloneCodeEditor.ts | 4 +-
13+
.../standalone/browser/standaloneServices.ts | 79 +++++++------------
14+
.../common/abstractKeybindingService.ts | 4 +
15+
.../platform/keybinding/common/keybinding.ts | 2 +
16+
.../comments/browser/simpleCommentEditor.ts | 4 +-
17+
9 files changed, 57 insertions(+), 58 deletions(-)
18+
19+
diff --git a/src/vs/editor/browser/widget/codeEditor/codeEditorWidget.ts b/src/vs/editor/browser/widget/codeEditor/codeEditorWidget.ts
20+
index f8893bb7ddb..3defd1d0f35 100644
21+
--- a/src/vs/editor/browser/widget/codeEditor/codeEditorWidget.ts
22+
+++ b/src/vs/editor/browser/widget/codeEditor/codeEditorWidget.ts
23+
@@ -62,6 +62,7 @@ import { IThemeService, registerThemingParticipant } from '../../../../platform/
24+
import { MenuId } from '../../../../platform/actions/common/actions.js';
25+
import { TextModelEditReason, EditReasons } from '../../../common/textModelEditReason.js';
26+
import { TextEdit } from '../../../common/core/edits/textEdit.js';
27+
+import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js';
28+
29+
export class CodeEditorWidget extends Disposable implements editorBrowser.ICodeEditor {
30+
31+
@@ -268,6 +269,7 @@ export class CodeEditorWidget extends Disposable implements editorBrowser.ICodeE
32+
@IAccessibilityService accessibilityService: IAccessibilityService,
33+
@ILanguageConfigurationService private readonly languageConfigurationService: ILanguageConfigurationService,
34+
@ILanguageFeaturesService languageFeaturesService: ILanguageFeaturesService,
35+
+ @IKeybindingService private readonly keybindingService: IKeybindingService,
36+
) {
37+
super();
38+
codeEditorService.willCreateCodeEditor();
39+
@@ -387,6 +389,10 @@ export class CodeEditorWidget extends Disposable implements editorBrowser.ICodeE
40+
}));
41+
42+
this._codeEditorService.addCodeEditor(this);
43+
+
44+
+ if (!this.getOption(EditorOption.inDiffEditor)) {
45+
+ this._register(this.keybindingService.registerContainer(this.getContainerDomNode()));
46+
+ }
47+
}
48+
49+
public writeScreenReaderContent(reason: string): void {
50+
diff --git a/src/vs/editor/browser/widget/codeEditor/embeddedCodeEditorWidget.ts b/src/vs/editor/browser/widget/codeEditor/embeddedCodeEditorWidget.ts
51+
index 3852374d394..bdcb4ff9fd4 100644
52+
--- a/src/vs/editor/browser/widget/codeEditor/embeddedCodeEditorWidget.ts
53+
+++ b/src/vs/editor/browser/widget/codeEditor/embeddedCodeEditorWidget.ts
54+
@@ -16,6 +16,7 @@ import { IContextKeyService } from '../../../../platform/contextkey/common/conte
55+
import { IInstantiationService, ServicesAccessor } from '../../../../platform/instantiation/common/instantiation.js';
56+
import { INotificationService } from '../../../../platform/notification/common/notification.js';
57+
import { IThemeService } from '../../../../platform/theme/common/themeService.js';
58+
+import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js';
59+
60+
export class EmbeddedCodeEditorWidget extends CodeEditorWidget {
61+
private readonly _parentEditor: ICodeEditor;
62+
@@ -34,9 +35,10 @@ export class EmbeddedCodeEditorWidget extends CodeEditorWidget {
63+
@INotificationService notificationService: INotificationService,
64+
@IAccessibilityService accessibilityService: IAccessibilityService,
65+
@ILanguageConfigurationService languageConfigurationService: ILanguageConfigurationService,
66+
- @ILanguageFeaturesService languageFeaturesService: ILanguageFeaturesService
67+
+ @ILanguageFeaturesService languageFeaturesService: ILanguageFeaturesService,
68+
+ @IKeybindingService keybindingService: IKeybindingService,
69+
) {
70+
- super(domElement, { ...parentEditor.getRawOptions(), overflowWidgetsDomNode: parentEditor.getOverflowWidgetsDomNode() }, codeEditorWidgetOptions, instantiationService, codeEditorService, commandService, contextKeyService, themeService, notificationService, accessibilityService, languageConfigurationService, languageFeaturesService);
71+
+ super(domElement, { ...parentEditor.getRawOptions(), overflowWidgetsDomNode: parentEditor.getOverflowWidgetsDomNode() }, codeEditorWidgetOptions, instantiationService, codeEditorService, commandService, contextKeyService, themeService, notificationService, accessibilityService, languageConfigurationService, languageFeaturesService, keybindingService);
72+
73+
this._parentEditor = parentEditor;
74+
this._overwriteOptions = options;
75+
diff --git a/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts
76+
index fbc898a0a10..d7cae92af87 100644
77+
--- a/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts
78+
+++ b/src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts
79+
@@ -14,6 +14,7 @@ import { AccessibilitySignal, IAccessibilitySignalService } from '../../../../pl
80+
import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js';
81+
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
82+
import { ServiceCollection } from '../../../../platform/instantiation/common/serviceCollection.js';
83+
+import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js';
84+
import { bindContextKey } from '../../../../platform/observable/common/platformObservableUtils.js';
85+
import { IEditorProgressService } from '../../../../platform/progress/common/progress.js';
86+
import { IDiffEditorOptions } from '../../../common/config/editorOptions.js';
87+
@@ -100,6 +101,7 @@ export class DiffEditorWidget extends DelegatingEditor implements IDiffEditor {
88+
@ICodeEditorService codeEditorService: ICodeEditorService,
89+
@IAccessibilitySignalService private readonly _accessibilitySignalService: IAccessibilitySignalService,
90+
@IEditorProgressService private readonly _editorProgressService: IEditorProgressService,
91+
+ @IKeybindingService private readonly keybindingService: IKeybindingService,
92+
) {
93+
super();
94+
this.elements = h('div.monaco-diff-editor.side-by-side', { style: { position: 'relative', height: '100%' } }, [
95+
@@ -416,6 +418,8 @@ export class DiffEditorWidget extends DelegatingEditor implements IDiffEditor {
96+
this._register(autorun(reader => {
97+
this._options.setModel(this._diffModel.read(reader));
98+
}));
99+
+
100+
+ this._register(this.keybindingService.registerContainer(this.getContainerDomNode()));
101+
}
102+
103+
public getViewWidth(): number {
104+
diff --git a/src/vs/editor/browser/widget/diffEditor/embeddedDiffEditorWidget.ts b/src/vs/editor/browser/widget/diffEditor/embeddedDiffEditorWidget.ts
105+
index ce41de7bc70..916c923585c 100644
106+
--- a/src/vs/editor/browser/widget/diffEditor/embeddedDiffEditorWidget.ts
107+
+++ b/src/vs/editor/browser/widget/diffEditor/embeddedDiffEditorWidget.ts
108+
@@ -12,6 +12,7 @@ import { IAccessibilitySignalService } from '../../../../platform/accessibilityS
109+
import { IContextKeyService } from '../../../../platform/contextkey/common/contextkey.js';
110+
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
111+
import { IEditorProgressService } from '../../../../platform/progress/common/progress.js';
112+
+import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js';
113+
export class EmbeddedDiffEditorWidget extends DiffEditorWidget {
114+
115+
private readonly _parentEditor: ICodeEditor;
116+
@@ -26,9 +27,10 @@ export class EmbeddedDiffEditorWidget extends DiffEditorWidget {
117+
@IInstantiationService instantiationService: IInstantiationService,
118+
@ICodeEditorService codeEditorService: ICodeEditorService,
119+
@IAccessibilitySignalService accessibilitySignalService: IAccessibilitySignalService,
120+
- @IEditorProgressService editorProgressService: IEditorProgressService
121+
+ @IEditorProgressService editorProgressService: IEditorProgressService,
122+
+ @IKeybindingService keybindingService: IKeybindingService,
123+
) {
124+
- super(domElement, parentEditor.getRawOptions(), codeEditorWidgetOptions, contextKeyService, instantiationService, codeEditorService, accessibilitySignalService, editorProgressService);
125+
+ super(domElement, parentEditor.getRawOptions(), codeEditorWidgetOptions, contextKeyService, instantiationService, codeEditorService, accessibilitySignalService, editorProgressService, keybindingService);
126+
127+
this._parentEditor = parentEditor;
128+
this._overwriteOptions = options;
129+
diff --git a/src/vs/editor/standalone/browser/standaloneCodeEditor.ts b/src/vs/editor/standalone/browser/standaloneCodeEditor.ts
130+
index ca9736c46f1..cab8bb8a1a1 100644
131+
--- a/src/vs/editor/standalone/browser/standaloneCodeEditor.ts
132+
+++ b/src/vs/editor/standalone/browser/standaloneCodeEditor.ts
133+
@@ -283,7 +283,7 @@ export class StandaloneCodeEditor extends CodeEditorWidget implements IStandalon
134+
) {
135+
const options = { ..._options };
136+
options.ariaLabel = options.ariaLabel || StandaloneCodeEditorNLS.editorViewAccessibleLabel;
137+
- super(domElement, options, { isStandaloneEditor: true }, instantiationService, codeEditorService, commandService, contextKeyService, themeService, notificationService, accessibilityService, languageConfigurationService, languageFeaturesService);
138+
+ super(domElement, options, { isStandaloneEditor: true }, instantiationService, codeEditorService, commandService, contextKeyService, themeService, notificationService, accessibilityService, languageConfigurationService, languageFeaturesService, keybindingService);
139+
140+
if (keybindingService instanceof StandaloneKeybindingService) {
141+
this._standaloneKeybindingService = keybindingService;
142+
@@ -507,6 +507,7 @@ export class StandaloneDiffEditor2 extends DiffEditorWidget implements IStandalo
143+
@IEditorProgressService editorProgressService: IEditorProgressService,
144+
@IClipboardService clipboardService: IClipboardService,
145+
@IAccessibilitySignalService accessibilitySignalService: IAccessibilitySignalService,
146+
+ @IKeybindingService keybindingService: IKeybindingService,
147+
) {
148+
const options = { ..._options };
149+
updateConfigurationService(configurationService, options, true);
150+
@@ -527,6 +528,7 @@ export class StandaloneDiffEditor2 extends DiffEditorWidget implements IStandalo
151+
codeEditorService,
152+
accessibilitySignalService,
153+
editorProgressService,
154+
+ keybindingService,
155+
);
156+
157+
this._configurationService = configurationService;
158+
diff --git a/src/vs/editor/standalone/browser/standaloneServices.ts b/src/vs/editor/standalone/browser/standaloneServices.ts
159+
index 8093d795a47..48013c189bd 100644
160+
--- a/src/vs/editor/standalone/browser/standaloneServices.ts
161+
+++ b/src/vs/editor/standalone/browser/standaloneServices.ts
162+
@@ -53,8 +53,6 @@ import { basename } from '../../../base/common/resources.js';
163+
import { ICodeEditorService } from '../../browser/services/codeEditorService.js';
164+
import { ConsoleLogger, ILoggerService, ILogService, NullLoggerService } from '../../../platform/log/common/log.js';
165+
import { IWorkspaceTrustManagementService, IWorkspaceTrustTransitionParticipant, IWorkspaceTrustUriInfo } from '../../../platform/workspace/common/workspaceTrust.js';
166+
-import { EditorOption } from '../../common/config/editorOptions.js';
167+
-import { ICodeEditor, IDiffEditor } from '../../browser/editorBrowser.js';
168+
import { IContextMenuService, IContextViewDelegate, IContextViewService, IOpenContextView } from '../../../platform/contextview/browser/contextView.js';
169+
import { ContextViewService } from '../../../platform/contextview/browser/contextViewService.js';
170+
import { LanguageService } from '../../common/services/languageService.js';
171+
@@ -421,66 +419,43 @@ export class StandaloneKeybindingService extends AbstractKeybindingService {
172+
this._cachedResolver = null;
173+
this._dynamicKeybindings = [];
174+
this._domNodeListeners = [];
175+
+ }
176+
177+
- const addContainer = (domNode: HTMLElement) => {
178+
- const disposables = new DisposableStore();
179+
+ override registerContainer(container: HTMLElement): IDisposable {
180+
+ const disposables = new DisposableStore();
181+
182+
- // for standard keybindings
183+
- disposables.add(dom.addDisposableListener(domNode, dom.EventType.KEY_DOWN, (e: KeyboardEvent) => {
184+
- const keyEvent = new StandardKeyboardEvent(e);
185+
- const shouldPreventDefault = this._dispatch(keyEvent, keyEvent.target);
186+
- if (shouldPreventDefault) {
187+
- keyEvent.preventDefault();
188+
- keyEvent.stopPropagation();
189+
- }
190+
- }));
191+
-
192+
- // for single modifier chord keybindings (e.g. shift shift)
193+
- disposables.add(dom.addDisposableListener(domNode, dom.EventType.KEY_UP, (e: KeyboardEvent) => {
194+
- const keyEvent = new StandardKeyboardEvent(e);
195+
- const shouldPreventDefault = this._singleModifierDispatch(keyEvent, keyEvent.target);
196+
- if (shouldPreventDefault) {
197+
- keyEvent.preventDefault();
198+
- }
199+
- }));
200+
+ // for standard keybindings
201+
+ disposables.add(dom.addDisposableListener(container, dom.EventType.KEY_DOWN, (e: KeyboardEvent) => {
202+
+ const keyEvent = new StandardKeyboardEvent(e);
203+
+ const shouldPreventDefault = this._dispatch(keyEvent, keyEvent.target);
204+
+ if (shouldPreventDefault) {
205+
+ keyEvent.preventDefault();
206+
+ keyEvent.stopPropagation();
207+
+ }
208+
+ }));
209+
210+
- this._domNodeListeners.push(new DomNodeListeners(domNode, disposables));
211+
- };
212+
- const removeContainer = (domNode: HTMLElement) => {
213+
+ // for single modifier chord keybindings (e.g. shift shift)
214+
+ disposables.add(dom.addDisposableListener(container, dom.EventType.KEY_UP, (e: KeyboardEvent) => {
215+
+ const keyEvent = new StandardKeyboardEvent(e);
216+
+ const shouldPreventDefault = this._singleModifierDispatch(keyEvent, keyEvent.target);
217+
+ if (shouldPreventDefault) {
218+
+ keyEvent.preventDefault();
219+
+ }
220+
+ }));
221+
+
222+
+ this._domNodeListeners.push(new DomNodeListeners(container, disposables));
223+
+
224+
+ disposables.add(toDisposable(() => {
225+
for (let i = 0; i < this._domNodeListeners.length; i++) {
226+
const domNodeListeners = this._domNodeListeners[i];
227+
- if (domNodeListeners.domNode === domNode) {
228+
+ if (domNodeListeners.domNode === container) {
229+
this._domNodeListeners.splice(i, 1);
230+
domNodeListeners.dispose();
231+
}
232+
}
233+
- };
234+
-
235+
- const addCodeEditor = (codeEditor: ICodeEditor) => {
236+
- if (codeEditor.getOption(EditorOption.inDiffEditor)) {
237+
- return;
238+
- }
239+
- addContainer(codeEditor.getContainerDomNode());
240+
- };
241+
- const removeCodeEditor = (codeEditor: ICodeEditor) => {
242+
- if (codeEditor.getOption(EditorOption.inDiffEditor)) {
243+
- return;
244+
- }
245+
- removeContainer(codeEditor.getContainerDomNode());
246+
- };
247+
- this._register(codeEditorService.onCodeEditorAdd(addCodeEditor));
248+
- this._register(codeEditorService.onCodeEditorRemove(removeCodeEditor));
249+
- codeEditorService.listCodeEditors().forEach(addCodeEditor);
250+
+ }));
251+
252+
- const addDiffEditor = (diffEditor: IDiffEditor) => {
253+
- addContainer(diffEditor.getContainerDomNode());
254+
- };
255+
- const removeDiffEditor = (diffEditor: IDiffEditor) => {
256+
- removeContainer(diffEditor.getContainerDomNode());
257+
- };
258+
- this._register(codeEditorService.onDiffEditorAdd(addDiffEditor));
259+
- this._register(codeEditorService.onDiffEditorRemove(removeDiffEditor));
260+
- codeEditorService.listDiffEditors().forEach(addDiffEditor);
261+
+ return disposables;
262+
}
263+
264+
public addDynamicKeybinding(command: string, keybinding: number, handler: ICommandHandler, when: ContextKeyExpression | undefined): IDisposable {
265+
diff --git a/src/vs/platform/keybinding/common/abstractKeybindingService.ts b/src/vs/platform/keybinding/common/abstractKeybindingService.ts
266+
index 7678f6c893f..3859431103c 100644
267+
--- a/src/vs/platform/keybinding/common/abstractKeybindingService.ts
268+
+++ b/src/vs/platform/keybinding/common/abstractKeybindingService.ts
269+
@@ -80,6 +80,10 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
270+
this._logging = false;
271+
}
272+
273+
+ registerContainer(container: HTMLElement): IDisposable {
274+
+ return Disposable.None;
275+
+ }
276+
+
277+
public override dispose(): void {
278+
super.dispose();
279+
}
280+
diff --git a/src/vs/platform/keybinding/common/keybinding.ts b/src/vs/platform/keybinding/common/keybinding.ts
281+
index 72f0d6c9640..a5e37dc4885 100644
282+
--- a/src/vs/platform/keybinding/common/keybinding.ts
283+
+++ b/src/vs/platform/keybinding/common/keybinding.ts
284+
@@ -45,6 +45,8 @@ export interface IKeybindingService {
285+
286+
readonly inChordMode: boolean;
287+
288+
+ registerContainer(container: HTMLElement): IDisposable;
289+
+
290+
onDidUpdateKeybindings: Event<void>;
291+
292+
/**
293+
diff --git a/src/vs/workbench/contrib/comments/browser/simpleCommentEditor.ts b/src/vs/workbench/contrib/comments/browser/simpleCommentEditor.ts
294+
index f11377c917a..62338521f52 100644
295+
--- a/src/vs/workbench/contrib/comments/browser/simpleCommentEditor.ts
296+
+++ b/src/vs/workbench/contrib/comments/browser/simpleCommentEditor.ts
297+
@@ -39,6 +39,7 @@ import { MenuId } from '../../../../platform/actions/common/actions.js';
298+
import { ContentHoverController } from '../../../../editor/contrib/hover/browser/contentHoverController.js';
299+
import { GlyphHoverController } from '../../../../editor/contrib/hover/browser/glyphHoverController.js';
300+
import { PlaceholderTextContribution } from '../../../../editor/contrib/placeholderText/browser/placeholderTextContribution.js';
301+
+import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js';
302+
303+
export const ctxCommentEditorFocused = new RawContextKey<boolean>('commentEditorFocused', false);
304+
export const MIN_EDITOR_HEIGHT = 5 * 18;
305+
@@ -66,6 +67,7 @@ export class SimpleCommentEditor extends CodeEditorWidget {
306+
@IAccessibilityService accessibilityService: IAccessibilityService,
307+
@ILanguageConfigurationService languageConfigurationService: ILanguageConfigurationService,
308+
@ILanguageFeaturesService languageFeaturesService: ILanguageFeaturesService,
309+
+ @IKeybindingService keybindingService: IKeybindingService,
310+
) {
311+
const codeEditorWidgetOptions: ICodeEditorWidgetOptions = {
312+
contributions: <IEditorContributionDescription[]>[
313+
@@ -91,7 +93,7 @@ export class SimpleCommentEditor extends CodeEditorWidget {
314+
contextMenuId: MenuId.SimpleEditorContext
315+
};
316+
317+
- super(domElement, options, codeEditorWidgetOptions, instantiationService, codeEditorService, commandService, scopedContextKeyService, themeService, notificationService, accessibilityService, languageConfigurationService, languageFeaturesService);
318+
+ super(domElement, options, codeEditorWidgetOptions, instantiationService, codeEditorService, commandService, scopedContextKeyService, themeService, notificationService, accessibilityService, languageConfigurationService, languageFeaturesService, keybindingService);
319+
320+
this._commentEditorFocused = ctxCommentEditorFocused.bindTo(scopedContextKeyService);
321+
this._commentEditorEmpty = CommentContextKeys.commentIsEmpty.bindTo(scopedContextKeyService);

0 commit comments

Comments
 (0)