Skip to content

Commit 5315151

Browse files
RtlZeroMemoryRtlZeroMemory
committed
feat: distinguish shifted letter keybindings
Refs #318.
1 parent b82fabd commit 5315151

File tree

6 files changed

+199
-12
lines changed

6 files changed

+199
-12
lines changed

‎packages/core/src/app/__tests__/shortcutEnforcement.test.ts‎

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { assert, describe, test } from "@rezi-ui/testkit";
22
import type { RuntimeBackend } from "../../backend.js";
33
import type { ZrevEvent } from "../../events.js";
44
import { ui } from "../../index.js";
5-
import { ZR_MOD_CTRL } from "../../keybindings/keyCodes.js";
5+
import { ZR_MOD_CTRL, ZR_MOD_SHIFT } from "../../keybindings/keyCodes.js";
66
import { DEFAULT_TERMINAL_CAPS } from "../../terminalCaps.js";
77
import { defaultTheme } from "../../theme/defaultTheme.js";
88
import type { CommandItem, CommandSource } from "../../widgets/types.js";
@@ -571,6 +571,118 @@ describe("widget shortcut enforcement contracts", () => {
571571
}
572572
});
573573

574+
test("uppercase text bindings are distinct from lowercase text bindings", async () => {
575+
const backend = new StubBackend();
576+
const hits: string[] = [];
577+
const app = createApp({ backend, initialState: 0 });
578+
579+
app.keys({
580+
j: () => {
581+
hits.push("lower");
582+
},
583+
J: () => {
584+
hits.push("upper");
585+
},
586+
});
587+
app.view(() => ui.text("ready"));
588+
589+
await app.start();
590+
try {
591+
await pushEvents(backend, [{ kind: "resize", timeMs: 1, cols: 40, rows: 12 }]);
592+
await settleNextFrame(backend);
593+
594+
await pushEvents(backend, [
595+
{ kind: "text", timeMs: 2, codepoint: 106 },
596+
{ kind: "text", timeMs: 3, codepoint: 74 },
597+
]);
598+
assert.deepEqual(hits, ["lower", "upper"]);
599+
} finally {
600+
await app.stop();
601+
}
602+
});
603+
604+
test("paired shift key and text only fire uppercase keybindings once", async () => {
605+
const backend = new StubBackend();
606+
const inputValues: Array<readonly [string, number]> = [];
607+
let uppercaseHits = 0;
608+
const app = createApp({ backend, initialState: "" });
609+
610+
app.keys({
611+
J: () => {
612+
uppercaseHits++;
613+
},
614+
});
615+
app.view((value) =>
616+
ui.input({
617+
id: "name",
618+
value,
619+
onInput: (next, cursor) => {
620+
inputValues.push([next, cursor]);
621+
app.update(() => next);
622+
},
623+
}),
624+
);
625+
626+
await app.start();
627+
try {
628+
await pushEvents(backend, [{ kind: "resize", timeMs: 1, cols: 40, rows: 12 }]);
629+
await settleNextFrame(backend);
630+
631+
await pushEvents(backend, [{ kind: "key", timeMs: 2, key: 3, mods: 0, action: "down" }]);
632+
await settleNextFrame(backend);
633+
await pushEvents(backend, [
634+
{ kind: "key", timeMs: 3, key: 74, mods: ZR_MOD_SHIFT, action: "down" },
635+
{ kind: "text", timeMs: 3, codepoint: 74 },
636+
]);
637+
638+
assert.equal(uppercaseHits, 1);
639+
assert.equal(inputValues.length, 0);
640+
} finally {
641+
await app.stop();
642+
}
643+
});
644+
645+
test("paired shift key still routes text when no uppercase binding consumes it", async () => {
646+
const backend = new StubBackend();
647+
const inputValues: Array<readonly [string, number]> = [];
648+
let lowercaseHits = 0;
649+
const app = createApp({ backend, initialState: "" });
650+
651+
app.keys({
652+
j: () => {
653+
lowercaseHits++;
654+
},
655+
});
656+
app.view((value) =>
657+
ui.input({
658+
id: "name",
659+
value,
660+
onInput: (next, cursor) => {
661+
inputValues.push([next, cursor]);
662+
app.update(() => next);
663+
},
664+
}),
665+
);
666+
667+
await app.start();
668+
try {
669+
await pushEvents(backend, [{ kind: "resize", timeMs: 1, cols: 40, rows: 12 }]);
670+
await settleNextFrame(backend);
671+
672+
await pushEvents(backend, [{ kind: "key", timeMs: 2, key: 3, mods: 0, action: "down" }]);
673+
await settleNextFrame(backend);
674+
await pushEvents(backend, [
675+
{ kind: "key", timeMs: 3, key: 74, mods: ZR_MOD_SHIFT, action: "down" },
676+
{ kind: "text", timeMs: 3, codepoint: 74 },
677+
]);
678+
679+
assert.equal(lowercaseHits, 0);
680+
assert.deepEqual(inputValues[inputValues.length - 1], ["J", 1]);
681+
} finally {
682+
await app.stop();
683+
}
684+
});
685+
574686
test("invalid shortcut strings are ignored without crashing", () => {
575687
const backend = createNoopBackend();
576688
const renderer = new WidgetRenderer<void>({ backend, requestRender: () => {} });

‎packages/core/src/app/createApp/eventLoop.ts‎

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { describeThrown } from "../../debug/describeThrown.js";
44
import type { UiEvent } from "../../events.js";
55
import type { KeyContext, KeybindingManagerState } from "../../keybindings/index.js";
66
import { resetChordState, routeKeyEvent } from "../../keybindings/index.js";
7-
import { ZR_MOD_CTRL } from "../../keybindings/keyCodes.js";
7+
import { ZR_MOD_CTRL, ZR_MOD_SHIFT } from "../../keybindings/keyCodes.js";
88
import { perfMarkEnd, perfMarkStart, perfNow } from "../../perf/perf.js";
99
import type { EventTimeUnwrapState } from "../../protocol/types.js";
1010
import { parseEventBatchV1 } from "../../protocol/zrev_v1.js";
@@ -15,6 +15,7 @@ import type { ResolvedAppConfig } from "./config.js";
1515
import { DIRTY_LAYOUT, DIRTY_RENDER, DIRTY_VIEW } from "./dirtyPlan.js";
1616
import {
1717
type AppKeybindingHelpers,
18+
codepointToImplicitTextMods,
1819
codepointToCtrlKeyCode,
1920
codepointToKeyCode,
2021
} from "./keybindings.js";
@@ -166,12 +167,25 @@ export function createEventLoop<S>(options: CreateEventLoopOptions<S>): AppEvent
166167
const droppedBatches = batch.droppedBatches;
167168

168169
try {
170+
let pendingShiftTextPair:
171+
| Readonly<{ codepoint: number; keybindingConsumed: boolean; timeMs: number }>
172+
| null = null;
173+
169174
if (engineTruncated || droppedBatches > 0) {
170175
if (!options.emit({ kind: "overrun", engineTruncated, droppedBatches })) return;
171176
if (options.getRuntimeState() !== "Running") return;
172177
}
173178

174179
for (const ev of parsed.value.events) {
180+
const pairedShiftText =
181+
pendingShiftTextPair !== null &&
182+
ev.kind === "text" &&
183+
ev.timeMs === pendingShiftTextPair.timeMs &&
184+
ev.codepoint === pendingShiftTextPair.codepoint
185+
? pendingShiftTextPair
186+
: null;
187+
pendingShiftTextPair = null;
188+
175189
if (ev.kind === "key" || ev.kind === "text" || ev.kind === "paste" || ev.kind === "mouse") {
176190
options.setInteractiveBudget(2);
177191
}
@@ -251,6 +265,8 @@ export function createEventLoop<S>(options: CreateEventLoopOptions<S>): AppEvent
251265
}
252266

253267
if (ev.kind === "key") {
268+
const shouldPairShiftText =
269+
ev.action === "down" && ev.mods === ZR_MOD_SHIFT && ev.key >= 65 && ev.key <= 90;
254270
const bypass = options.widgetRenderer.shouldBypassKeybindings(ev);
255271
if (!bypass) {
256272
const keyCtx: KeyContext<S> = Object.freeze({
@@ -272,22 +288,37 @@ export function createEventLoop<S>(options: CreateEventLoopOptions<S>): AppEvent
272288
return;
273289
}
274290
if (keyResult.consumed) {
291+
if (shouldPairShiftText) {
292+
pendingShiftTextPair = Object.freeze({
293+
codepoint: ev.key,
294+
keybindingConsumed: true,
295+
timeMs: ev.timeMs,
296+
});
297+
}
275298
options.noteBreadcrumbConsumptionPath("keybindings");
276299
continue;
277300
}
278301
}
302+
if (shouldPairShiftText) {
303+
pendingShiftTextPair = Object.freeze({
304+
codepoint: ev.key,
305+
keybindingConsumed: false,
306+
timeMs: ev.timeMs,
307+
});
308+
}
279309
}
280310

281311
if (ev.kind === "text") {
282312
const ctrlKeyCode = codepointToCtrlKeyCode(ev.codepoint);
283313
const shouldRouteCtrlText = ctrlKeyCode !== null;
284314
const shouldRoutePrintableText =
285315
!shouldRouteCtrlText && !options.widgetRenderer.hasActiveOverlay();
286-
if (shouldRouteCtrlText || shouldRoutePrintableText) {
316+
if (pairedShiftText === null && (shouldRouteCtrlText || shouldRoutePrintableText)) {
287317
const keyCode = shouldRouteCtrlText
288318
? ctrlKeyCode
289319
: codepointToKeyCode(ev.codepoint);
290-
const mods = shouldRouteCtrlText ? ZR_MOD_CTRL : 0;
320+
const mods =
321+
(shouldRouteCtrlText ? ZR_MOD_CTRL : 0) | codepointToImplicitTextMods(ev.codepoint);
291322
if (keyCode !== null) {
292323
const syntheticKeyEvent = {
293324
kind: "key" as const,
@@ -320,6 +351,10 @@ export function createEventLoop<S>(options: CreateEventLoopOptions<S>): AppEvent
320351
}
321352
}
322353
}
354+
if (pairedShiftText?.keybindingConsumed === true) {
355+
options.noteBreadcrumbConsumptionPath("keybindings");
356+
continue;
357+
}
323358
}
324359
}
325360

‎packages/core/src/app/createApp/keybindings.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
registerModes,
1212
removeBindingsBySource,
1313
} from "../../keybindings/index.js";
14+
import { ZR_MOD_SHIFT } from "../../keybindings/keyCodes.js";
1415
import { DIRTY_VIEW } from "./dirtyPlan.js";
1516

1617
const ROUTE_KEYBINDING_SOURCE = "__rezi:router";
@@ -61,6 +62,13 @@ export function codepointToCtrlKeyCode(codepoint: number): number | null {
6162
return null;
6263
}
6364

65+
export function codepointToImplicitTextMods(codepoint: number): number {
66+
if (codepoint >= 65 && codepoint <= 90) {
67+
return ZR_MOD_SHIFT;
68+
}
69+
return 0;
70+
}
71+
6472
export function computeKeybindingsEnabled<S>(
6573
state: KeybindingManagerState<KeyContext<S>>,
6674
): boolean {

‎packages/core/src/keybindings/__tests__/keybinding.parse.test.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,9 +212,18 @@ describe("parseKeySequence area A", () => {
212212
assert.deepEqual(mixed.value, lower.value);
213213
});
214214

215-
test("uppercase and lowercase letters parse to same key code", () => {
215+
test("bare uppercase letters parse as implicit shift+letter", () => {
216216
const upper = parseKeySequence("A");
217-
const lower = parseKeySequence("a");
217+
const explicit = parseKeySequence("shift+a");
218+
assert.equal(upper.ok, true);
219+
assert.equal(explicit.ok, true);
220+
if (!upper.ok || !explicit.ok) return;
221+
assert.deepEqual(upper.value, explicit.value);
222+
});
223+
224+
test("ctrl+A still parses identically to ctrl+a", () => {
225+
const upper = parseKeySequence("ctrl+A");
226+
const lower = parseKeySequence("ctrl+a");
218227
assert.equal(upper.ok, true);
219228
assert.equal(lower.ok, true);
220229
if (!upper.ok || !lower.ok) return;

‎packages/core/src/keybindings/__tests__/parser.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ describe("parseKeySequence", () => {
4141
assert.ok(key);
4242
if (!key) return;
4343
assert.equal(key.key, 65);
44+
assert.equal(key.mods.shift, true);
4445
});
4546

4647
test("digit", () => {
@@ -287,6 +288,17 @@ describe("parseKeySequence", () => {
287288
});
288289

289290
describe("case insensitivity", () => {
291+
test("bare uppercase letter implies shift", () => {
292+
const upper = parseKeySequence("J");
293+
const explicit = parseKeySequence("shift+j");
294+
295+
assert.equal(upper.ok, true);
296+
assert.equal(explicit.ok, true);
297+
if (!upper.ok || !explicit.ok) return;
298+
299+
assert.deepEqual(upper.value, explicit.value);
300+
});
301+
290302
test("CTRL+S equals ctrl+s", () => {
291303
const upper = parseKeySequence("CTRL+S");
292304
const lower = parseKeySequence("ctrl+s");

‎packages/core/src/keybindings/parser.ts‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,8 @@ function parseKeyPart(
3434
};
3535
}
3636

37-
const lower = part.toLowerCase();
38-
3937
// Split on "+" to get modifiers and key
40-
const pieces = lower.split("+");
38+
const pieces = part.split("+");
4139
if (pieces.length === 0) {
4240
return {
4341
ok: false,
@@ -63,11 +61,12 @@ function parseKeyPart(
6361
}
6462

6563
const isLast = i === pieces.length - 1;
64+
const pieceLower = piece.toLowerCase();
6665

67-
if (MODIFIER_NAMES.has(piece)) {
66+
if (MODIFIER_NAMES.has(pieceLower)) {
6867
// It's a modifier
6968
let modifier: "shift" | "ctrl" | "alt" | "meta" | null = null;
70-
switch (piece) {
69+
switch (pieceLower) {
7170
case "shift":
7271
modifier = "shift";
7372
shift = true;
@@ -144,14 +143,26 @@ function parseKeyPart(
144143

145144
// Look up the key code
146145
let keyCode: number | null = null;
146+
const keyNameLower = keyName.toLowerCase();
147147

148148
// First try named keys
149-
const namedCode = KEY_NAME_TO_CODE.get(keyName);
149+
const namedCode = KEY_NAME_TO_CODE.get(keyNameLower);
150150
if (namedCode !== undefined) {
151151
keyCode = namedCode;
152152
} else if (keyName.length === 1) {
153153
// Single character - convert to key code
154154
keyCode = charToKeyCode(keyName);
155+
const charCode = keyName.charCodeAt(0);
156+
if (
157+
charCode >= 65 &&
158+
charCode <= 90 &&
159+
!shift &&
160+
!ctrl &&
161+
!alt &&
162+
!meta
163+
) {
164+
shift = true;
165+
}
155166
}
156167

157168
if (keyCode === null) {

0 commit comments

Comments
 (0)