Refactor: softDispatch doesn't return null (#181891)

* fix `softDispatch` returning `NoMatchingKb` vs null

See #179528 (comment)

* refactor: `softDispatch` doesn't return null

* Adopt softDispatch changes

---------

Co-authored-by: Alex Dima <alexdima@microsoft.com>

src/vs/editor/contrib/hover/browser/hover.ts

@@ -206,7 +206,7 @@ export class ModesHoverController implements IEditorContribution {
 
 		const resolvedKeyboardEvent = this._keybindingService.softDispatch(e, this._editor.getDomNode());
 		// If the beginning of a multi-chord keybinding is pressed, or the command aims to focus the hover, set the variable to true, otherwise false
-		const mightTriggerFocus = (resolvedKeyboardEvent?.kind === ResultKind.MoreChordsNeeded || (resolvedKeyboardEvent && resolvedKeyboardEvent.kind === ResultKind.KbFound && resolvedKeyboardEvent.commandId === 'editor.action.showHover' && this._contentWidget?.isVisible()));
+		const mightTriggerFocus = (resolvedKeyboardEvent.kind === ResultKind.MoreChordsNeeded || (resolvedKeyboardEvent.kind === ResultKind.KbFound && resolvedKeyboardEvent.commandId === 'editor.action.showHover' && this._contentWidget?.isVisible()));
 
 		if (e.keyCode !== KeyCode.Ctrl && e.keyCode !== KeyCode.Alt && e.keyCode !== KeyCode.Meta && e.keyCode !== KeyCode.Shift
 			&& !mightTriggerFocus) {

src/vs/platform/keybinding/common/abstractKeybindingService.ts

@@ -17,7 +17,7 @@ import * as nls from 'vs/nls';
 import { ICommandService } from 'vs/platform/commands/common/commands';
 import { IContextKeyService, IContextKeyServiceTarget } from 'vs/platform/contextkey/common/contextkey';
 import { IKeybindingService, IKeyboardEvent, KeybindingsSchemaContribution } from 'vs/platform/keybinding/common/keybinding';
-import { ResolutionResult, KeybindingResolver, ResultKind } from 'vs/platform/keybinding/common/keybindingResolver';
+import { ResolutionResult, KeybindingResolver, ResultKind, NoMatchingKb } from 'vs/platform/keybinding/common/keybindingResolver';
 import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem';
 import { ILogService } from 'vs/platform/log/common/log';
 import { INotificationService } from 'vs/platform/notification/common/notification';
@@ -138,18 +138,18 @@ export abstract class AbstractKeybindingService extends Disposable implements IK
 
 	// TODO@ulugbekna: update namings to align with `_doDispatch`
 	// TODO@ulugbekna: this fn doesn't seem to take into account single-modifier keybindings, eg `shift shift`
-	public softDispatch(e: IKeyboardEvent, target: IContextKeyServiceTarget): ResolutionResult | null {
+	public softDispatch(e: IKeyboardEvent, target: IContextKeyServiceTarget): ResolutionResult {
 		this._log(`/ Soft dispatching keyboard event`);
 		const keybinding = this.resolveKeyboardEvent(e);
 		if (keybinding.hasMultipleChords()) {
-			console.warn('Unexpected keyboard event mapped to multiple chords');
-			return null;
+			console.warn('keyboard event should not be mapped to multiple chords');
+			return NoMatchingKb;
 		}
 		const [firstChord,] = keybinding.getDispatchChords();
 		if (firstChord === null) {
 			// cannot be dispatched, probably only modifier keys
 			this._log(`\\ Keyboard event cannot be dispatched`);
-			return null;
+			return NoMatchingKb;
 		}
 
 		const contextValue = this._contextKeyService.getContext(target);

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

@@ -63,7 +63,7 @@ export interface IKeybindingService {
 	/**
 	 * Resolve and dispatch `keyboardEvent`, but do not invoke the command or change inner state.
 	 */
-	softDispatch(keyboardEvent: IKeyboardEvent, target: IContextKeyServiceTarget): ResolutionResult | null;
+	softDispatch(keyboardEvent: IKeyboardEvent, target: IContextKeyServiceTarget): ResolutionResult;
 
 	dispatchByUserSettingsLabel(userSettingsLabel: string, target: IContextKeyServiceTarget): void;
 

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

@@ -27,7 +27,7 @@ export type ResolutionResult =
 
 // util definitions to make working with the above types easier within this module:
 
-const NoMatchingKb: ResolutionResult = { kind: ResultKind.NoMatchingKb };
+export const NoMatchingKb: ResolutionResult = { kind: ResultKind.NoMatchingKb };
 const MoreChordsNeeded: ResolutionResult = { kind: ResultKind.MoreChordsNeeded };
 function KbFound(commandId: string | null, commandArgs: any, isBubble: boolean): ResolutionResult {
 	return { kind: ResultKind.KbFound, commandId, commandArgs, isBubble };

src/vs/platform/keybinding/test/common/mockKeybindingService.ts

@@ -8,7 +8,7 @@ import { ResolvedKeybinding, KeyCodeChord, Keybinding } from 'vs/base/common/key
 import { OS } from 'vs/base/common/platform';
 import { ContextKeyExpression, ContextKeyValue, IContextKey, IContextKeyChangeEvent, IContextKeyService, IContextKeyServiceTarget, IScopedContextKeyService } from 'vs/platform/contextkey/common/contextkey';
 import { IKeybindingService, IKeyboardEvent } from 'vs/platform/keybinding/common/keybinding';
-import { ResolutionResult } from 'vs/platform/keybinding/common/keybindingResolver';
+import { NoMatchingKb, ResolutionResult } from 'vs/platform/keybinding/common/keybindingResolver';
 import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem';
 import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayoutResolvedKeybinding';
 
@@ -135,8 +135,8 @@ export class MockKeybindingService implements IKeybindingService {
 		return 0;
 	}
 
-	public softDispatch(keybinding: IKeyboardEvent, target: IContextKeyServiceTarget): ResolutionResult | null {
-		return null;
+	public softDispatch(keybinding: IKeyboardEvent, target: IContextKeyServiceTarget): ResolutionResult {
+		return NoMatchingKb;
 	}
 
 	public dispatchByUserSettingsLabel(userSettingsLabel: string, target: IContextKeyServiceTarget): void {

src/vs/platform/list/browser/listService.ts

@@ -810,13 +810,13 @@ function createKeyboardNavigationEventFilter(keybindingService: IKeybindingServi
 
 		const result = keybindingService.softDispatch(event, event.target);
 
-		if (result?.kind === ResultKind.MoreChordsNeeded) {
+		if (result.kind === ResultKind.MoreChordsNeeded) {
 			inMultiChord = true;
 			return false;
 		}
 
 		inMultiChord = false;
-		return result === null || result.kind === ResultKind.NoMatchingKb;
+		return result.kind === ResultKind.NoMatchingKb;
 	};
 }
 

src/vs/workbench/browser/actions/developerActions.ts

@@ -260,7 +260,7 @@ class ToggleScreencastModeAction extends Action2 {
 			const shortcut = keybindingService.softDispatch(event, event.target);
 
 			// Hide the single arrow key pressed
-			if (shortcut && shortcut.kind === ResultKind.KbFound && shortcut.commandId && configurationService.getValue('screencastMode.hideSingleEditorCursorMoves') && (
+			if (shortcut.kind === ResultKind.KbFound && shortcut.commandId && configurationService.getValue('screencastMode.hideSingleEditorCursorMoves') && (
 				['cursorLeft', 'cursorRight', 'cursorUp', 'cursorDown'].includes(shortcut.commandId))
 			) {
 				return;
@@ -324,8 +324,8 @@ class ToggleScreencastModeAction extends Action2 {
 		ToggleScreencastModeAction.disposable = disposables;
 	}
 
-	private _isKbFound(resolutionResult: ResolutionResult | null): resolutionResult is { kind: ResultKind.KbFound; commandId: string | null; commandArgs: any; isBubble: boolean } {
-		return resolutionResult !== null && resolutionResult.kind === ResultKind.KbFound;
+	private _isKbFound(resolutionResult: ResolutionResult): resolutionResult is { kind: ResultKind.KbFound; commandId: string | null; commandArgs: any; isBubble: boolean } {
+		return resolutionResult.kind === ResultKind.KbFound;
 	}
 }
 

src/vs/workbench/browser/parts/dialogs/dialogHandler.ts

@@ -128,7 +128,7 @@ export class BrowserDialogHandler extends AbstractDialogHandler {
 				type: this.getDialogType(type),
 				keyEventProcessor: (event: StandardKeyboardEvent) => {
 					const resolved = this.keybindingService.softDispatch(event, this.layoutService.container);
-					if (resolved && resolved.kind === ResultKind.KbFound && resolved.commandId) {
+					if (resolved.kind === ResultKind.KbFound && resolved.commandId) {
 						if (BrowserDialogHandler.ALLOWABLE_COMMANDS.indexOf(resolved.commandId) === -1) {
 							EventHelper.stop(event, true);
 						}

src/vs/workbench/contrib/terminal/browser/terminalInstance.ts

@@ -926,7 +926,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 			// Respect chords if the allowChords setting is set and it's not Escape. Escape is
 			// handled specially for Zen Mode's Escape, Escape chord, plus it's important in
 			// terminals generally
-			const isValidChord = resolveResult?.kind === ResultKind.MoreChordsNeeded && this._configHelper.config.allowChords && event.key !== 'Escape';
+			const isValidChord = resolveResult.kind === ResultKind.MoreChordsNeeded && this._configHelper.config.allowChords && event.key !== 'Escape';
 			if (this._keybindingService.inChordMode || isValidChord) {
 				event.preventDefault();
 				return false;
@@ -946,7 +946,7 @@ export class TerminalInstance extends Disposable implements ITerminalInstance {
 
 			// for keyboard events that resolve to commands described
 			// within commandsToSkipShell, either alert or skip processing by xterm.js
-			if (resolveResult && resolveResult.kind === ResultKind.KbFound && resolveResult.commandId && this._skipTerminalCommands.some(k => k === resolveResult.commandId) && !this._configHelper.config.sendKeybindingsToShell) {
+			if (resolveResult.kind === ResultKind.KbFound && resolveResult.commandId && this._skipTerminalCommands.some(k => k === resolveResult.commandId) && !this._configHelper.config.sendKeybindingsToShell) {
 				// don't alert when terminal is opened or closed
 				if (this._storageService.getBoolean(SHOW_TERMINAL_CONFIG_PROMPT_KEY, StorageScope.APPLICATION, true) &&
 					this._hasHadInput &&

src/vs/workbench/services/hover/browser/hoverService.ts

@@ -115,7 +115,7 @@ export class HoverService implements IHoverService {
 		}
 		const event = new StandardKeyboardEvent(e);
 		const keybinding = this._keybindingService.resolveKeyboardEvent(event);
-		if (keybinding.getSingleModifierDispatchChords().some(value => !!value) || this._keybindingService.softDispatch(event, event.target)?.kind !== ResultKind.NoMatchingKb) {
+		if (keybinding.getSingleModifierDispatchChords().some(value => !!value) || this._keybindingService.softDispatch(event, event.target).kind !== ResultKind.NoMatchingKb) {
 			return;
 		}
 		if (hideOnKeyDown && (!this._currentHoverOptions?.trapFocus || e.key !== 'Tab')) {

src/vs/workbench/services/progress/browser/progressService.ts

@@ -557,7 +557,7 @@ export class ProgressService extends Disposable implements IProgressService {
 					disableDefaultAction: options.sticky,
 					keyEventProcessor: (event: StandardKeyboardEvent) => {
 						const resolved = this.keybindingService.softDispatch(event, this.layoutService.container);
-						if (resolved && resolved.kind === ResultKind.KbFound && resolved.commandId) {
+						if (resolved.kind === ResultKind.KbFound && resolved.commandId) {
 							if (!allowableCommands.includes(resolved.commandId)) {
 								EventHelper.stop(event, true);
 							}