From 8966a159dbc95028513c1f0581dc28748a0873c3 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Fri, 12 Jun 2026 08:49:20 +0200 Subject: [PATCH] require a clean release for double-tap modifier detection (T-409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing Shift+; opened quick-open instead of a colon. Two flaws in the T-341 detector: it counted a tap on the Shift keydown (so a chorded press could complete the gesture before the chord key arrived), and it relied on the chorded key bubbling to the root KeyboardListener to break the gesture — but a focused editor or text field consumes that event, so the tracker never saw it. The tracker now models press/release: a tap is a press with no other key going down while the modifier is held, and the gesture fires on the second clean release. The root shell feeds it from a HardwareKeyboard handler, which observes every event before focus dispatch regardless of who consumes it, and treats a modifier pressed while a non-modifier is already held as a chord. Co-Authored-By: Claude Fable 5 --- .pql/changelog/ticket_history/2026-06.sql | 29 ++++++ .pql/changelog/tickets/2026-06.sql | 41 +++++++++ CHANGELOG.md | 8 ++ lib/kernel/src/keymap/modifier_tap.dart | 69 +++++++++----- lib/src/shell/root_shell.dart | 48 ++++++---- test/app_test.dart | 38 ++++++++ test/kernel/src/keymap/modifier_tap_test.dart | 90 +++++++++++++++---- 7 files changed, 264 insertions(+), 59 deletions(-) diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index 46782e00..e4adc2a8 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -4279,3 +4279,32 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang - Cross-pane angle: the ex line is GLOBAL under vim.normal (works with tree/conversation focused — :q closes the focused tab via editor.close fallback to active workspace tab; keep v1 simple: editor-targeted only, document it). Done when: : opens the overlay from any pane under the vim preset; the v1 table works with widget tests; unknown commands don''t execute anything; ZZ saves+closes.', NULL, '2026-06-12 03:22:28', '2026-06-12 03:22:28', '2026-06-12 03:22:28', NULL, '267348e926541d1c7b5e55c3c1ef6219', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VTK2MYQQ173MSJN6E1DM', 'description', NULL, 'User report: typing /model in the Claude conversation view does nothing useful — it is forwarded to the session''s stream-json stdin like a plain message, and the CLI''s interactive /model picker only exists in its own TUI. Clide must own it (same class as /clear,/resume,/fork — T-156). + +Interaction design: +- `/model ` → set the session model directly to (accept aliases like sonnet/opus and full ids). +- `/model` (bare) → show a model picker in the interaction zone — replaces the composer while open, like ToolPromptCard (D-78); list selectable via keyboard (numbers/arrows + Enter), Esc cancels back to the composer. + +Implementation map (from code exploration): +- Add ''model'' to kClideOwnedCommands in lib/builtin/claude/src/slash_commands.dart and handle it in ClaudePane._send (lib/builtin/claude/src/claude_pane.dart). +- Add StreamJsonSession.setModel(String) following the setPermissionMode control_request pattern (lib/builtin/claude/src/stream_json_session.dart) — subtype set_model; optimistically merge SessionStatus(model: …) so the status bar updates. +- Model list for the bare-picker: query the CLI via the supported_models-style control request if available (verify exact subtype/shapes against the installed CLI), falling back to a static alias list. +- Picker widget swaps in via the existing pending-interaction slot in ClaudePane; reuse composer focus/draft preservation (the draft must survive the swap). + +Acceptance: +- `/model sonnet` switches the live session model; status bar reflects it on the next status merge. +- bare `/model` opens the picker; choosing an entry sets the model; Esc restores the composer with the draft intact. +- `/model` is never forwarded to the session as message text. +- Unit tests in test/builtin/claude/ for the parsing (slash_commands_test.dart), the pane interception, and the picker widget.', NULL, '2026-06-12 06:40:53', '2026-06-12 06:40:53', '2026-06-12 06:40:53', NULL, '86aaba4cdf1d5e3c054af341977c5315', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VYR84023Z5XFEX9DS0S0', 'description', NULL, 'Regression from T-341 (double-tap-modifier shortcuts, shipped in 2.4.0). User report: pressing shift+; to type a colon in the editor or the Claude composer no longer types '':'' — the quick-open finder opens instead. + +Likely cause: the double-Shift tap detector counts a Shift press/release as a "tap" even when another key was chorded while Shift was held. Typing '':'' is shift-down, '';'', shift-up; two colons (or a colon shortly after any shifted character) within the tap window then reads as shift,shift → "Search Everywhere" fires and may also swallow the keystroke. + +Fix: a modifier press only qualifies as a tap if NO other key goes down between the modifier''s keydown and keyup. Any chorded key must invalidate the pending tap (and reset the double-tap sequence state). + +Acceptance: +- Typing `::` rapidly in the editor and in the Claude composer produces two colons, never quick-open. +- Shifted typing in general (capitals, symbols) never triggers double-tap bindings. +- Genuine double-Shift (two bare taps within the window) still opens quick-open in all four presets. +- Regression test covering chorded-Shift-then-Shift-tap sequences.', NULL, '2026-06-12 06:40:54', '2026-06-12 06:40:54', '2026-06-12 06:40:54', NULL, '4f7eddbb58a3551d208dcd03541bed5d', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VYR84023Z5XFEX9DS0S0', 'status', 'backlog', 'in_progress', NULL, '2026-06-12 06:40:59', '2026-06-12 06:40:59', '2026-06-12 06:40:59', NULL, '6f856737a3d115b0a8dd510d2051047d', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index 2b731694..6131247d 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -4572,3 +4572,44 @@ INSERT INTO tickets (record_id, type, parent_record_id, title, description, stat - Cross-pane angle: the ex line is GLOBAL under vim.normal (works with tree/conversation focused — :q closes the focused tab via editor.close fallback to active workspace tab; keep v1 simple: editor-targeted only, document it). Done when: : opens the overlay from any pane under the vim preset; the v1 table works with widget tests; unknown commands don''t execute anything; ZZ saves+closes.', 'backlog', 'low', NULL, NULL, NULL, '2026-06-12 03:22:10', '2026-06-12 03:22:28', NULL, 'f5eeef51f090bf48c73133940d5348f9', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VTK2MYQQ173MSJN6E1DM', 'task', '06FB0TNQM5TWC00GW0P3X02HZW', 'intercept /model in the Claude pane: arg sets model, bare shows picker', NULL, 'backlog', 'medium', NULL, NULL, NULL, '2026-06-12 06:40:25', '2026-06-12 06:40:25', NULL, 'f08ac88045e15ed480efe59a127dcedc', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VYR84023Z5XFEX9DS0S0', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'shift+; opens quick-open instead of typing colon — double-Shift detector fires on chorded Shift', NULL, 'backlog', 'high', NULL, NULL, NULL, '2026-06-12 06:40:26', '2026-06-12 06:40:26', NULL, 'fc7bbb1351f2b1b3cc279aa466009381', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VTK2MYQQ173MSJN6E1DM', 'task', '06FB0TNQM5TWC00GW0P3X02HZW', 'intercept /model in the Claude pane: arg sets model, bare shows picker', 'User report: typing /model in the Claude conversation view does nothing useful — it is forwarded to the session''s stream-json stdin like a plain message, and the CLI''s interactive /model picker only exists in its own TUI. Clide must own it (same class as /clear,/resume,/fork — T-156). + +Interaction design: +- `/model ` → set the session model directly to (accept aliases like sonnet/opus and full ids). +- `/model` (bare) → show a model picker in the interaction zone — replaces the composer while open, like ToolPromptCard (D-78); list selectable via keyboard (numbers/arrows + Enter), Esc cancels back to the composer. + +Implementation map (from code exploration): +- Add ''model'' to kClideOwnedCommands in lib/builtin/claude/src/slash_commands.dart and handle it in ClaudePane._send (lib/builtin/claude/src/claude_pane.dart). +- Add StreamJsonSession.setModel(String) following the setPermissionMode control_request pattern (lib/builtin/claude/src/stream_json_session.dart) — subtype set_model; optimistically merge SessionStatus(model: …) so the status bar updates. +- Model list for the bare-picker: query the CLI via the supported_models-style control request if available (verify exact subtype/shapes against the installed CLI), falling back to a static alias list. +- Picker widget swaps in via the existing pending-interaction slot in ClaudePane; reuse composer focus/draft preservation (the draft must survive the swap). + +Acceptance: +- `/model sonnet` switches the live session model; status bar reflects it on the next status merge. +- bare `/model` opens the picker; choosing an entry sets the model; Esc restores the composer with the draft intact. +- `/model` is never forwarded to the session as message text. +- Unit tests in test/builtin/claude/ for the parsing (slash_commands_test.dart), the pane interception, and the picker widget.', 'backlog', 'medium', NULL, NULL, NULL, '2026-06-12 06:40:25', '2026-06-12 06:40:53', NULL, 'dfd74b1e6c5c14326837dfc2e3ae67fa', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VYR84023Z5XFEX9DS0S0', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'shift+; opens quick-open instead of typing colon — double-Shift detector fires on chorded Shift', 'Regression from T-341 (double-tap-modifier shortcuts, shipped in 2.4.0). User report: pressing shift+; to type a colon in the editor or the Claude composer no longer types '':'' — the quick-open finder opens instead. + +Likely cause: the double-Shift tap detector counts a Shift press/release as a "tap" even when another key was chorded while Shift was held. Typing '':'' is shift-down, '';'', shift-up; two colons (or a colon shortly after any shifted character) within the tap window then reads as shift,shift → "Search Everywhere" fires and may also swallow the keystroke. + +Fix: a modifier press only qualifies as a tap if NO other key goes down between the modifier''s keydown and keyup. Any chorded key must invalidate the pending tap (and reset the double-tap sequence state). + +Acceptance: +- Typing `::` rapidly in the editor and in the Claude composer produces two colons, never quick-open. +- Shifted typing in general (capitals, symbols) never triggers double-tap bindings. +- Genuine double-Shift (two bare taps within the window) still opens quick-open in all four presets. +- Regression test covering chorded-Shift-then-Shift-tap sequences.', 'backlog', 'high', NULL, NULL, NULL, '2026-06-12 06:40:26', '2026-06-12 06:40:54', NULL, 'faecb0d66ba124ee017b89ab14c3c332', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FBN3VYR84023Z5XFEX9DS0S0', 'bug', '06FB0TNQM5TWC00GW0P3X02HZW', 'shift+; opens quick-open instead of typing colon — double-Shift detector fires on chorded Shift', 'Regression from T-341 (double-tap-modifier shortcuts, shipped in 2.4.0). User report: pressing shift+; to type a colon in the editor or the Claude composer no longer types '':'' — the quick-open finder opens instead. + +Likely cause: the double-Shift tap detector counts a Shift press/release as a "tap" even when another key was chorded while Shift was held. Typing '':'' is shift-down, '';'', shift-up; two colons (or a colon shortly after any shifted character) within the tap window then reads as shift,shift → "Search Everywhere" fires and may also swallow the keystroke. + +Fix: a modifier press only qualifies as a tap if NO other key goes down between the modifier''s keydown and keyup. Any chorded key must invalidate the pending tap (and reset the double-tap sequence state). + +Acceptance: +- Typing `::` rapidly in the editor and in the Claude composer produces two colons, never quick-open. +- Shifted typing in general (capitals, symbols) never triggers double-tap bindings. +- Genuine double-Shift (two bare taps within the window) still opens quick-open in all four presets. +- Regression test covering chorded-Shift-then-Shift-tap sequences.', 'in_progress', 'high', NULL, NULL, NULL, '2026-06-12 06:40:26', '2026-06-12 06:40:59', NULL, 'bfbf8ed99c6f9a9920a341d8a470a23e', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 25f892b4..8a881f4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,14 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. ## [Unreleased] +### Fixed + +- **`Shift+;` types a colon again — double-Shift no longer fires on chorded + Shift.** The double-tap detector counted any Shift press as a tap, even + mid-chord, and never saw keys the focused editor consumed; a tap now + requires a clean press-and-release, observed at the raw-keyboard level. + (T-409) + ## [2.4.0] — 2026-06-12 ### Added diff --git a/lib/kernel/src/keymap/modifier_tap.dart b/lib/kernel/src/keymap/modifier_tap.dart index 76233696..e51b1339 100644 --- a/lib/kernel/src/keymap/modifier_tap.dart +++ b/lib/kernel/src/keymap/modifier_tap.dart @@ -1,10 +1,16 @@ /// Detects a double-tapped bare modifier (e.g. JetBrains "Search /// Everywhere" = double-Shift). (T-341) /// -/// Headless and clock-injected: the caller (the global key handler) passes -/// the event time so it neither reads a clock nor consumes events. Feed it -/// every [KeyDownEvent]: a bare modifier press via [tap], any other key via -/// [reset] (an intervening key breaks the gesture, e.g. `Shift a Shift`). +/// A "tap" is a clean press-and-release: no other key may go down while the +/// modifier is held, otherwise the press was a chord (`Shift+;` typing a +/// colon) and must not count (T-409). The gesture therefore completes on the +/// second clean *release*, never on a key-down — at down time it's unknowable +/// whether the press will stay bare. +/// +/// Headless and clock-injected: the caller (the root shell's raw-keyboard +/// handler) passes the event time so it neither reads a clock nor consumes +/// events. Feed every [KeyDownEvent] to [down] and every [KeyUpEvent] to +/// [up], passing the event's [KeyModifier] (null for non-modifier keys). library; import 'key_chord.dart'; @@ -12,33 +18,50 @@ import 'key_chord.dart'; class ModifierTapTracker { ModifierTapTracker({this.window = const Duration(milliseconds: 350)}); - /// Max gap between the two taps to count as a double-tap. + /// Max gap between the two tap releases to count as a double-tap. final Duration window; - KeyModifier? _last; - DateTime? _lastAt; + /// Modifier currently held whose press is still bare (no chorded key yet). + KeyModifier? _pressing; - /// Record a bare-modifier press at [now]. Returns the modifier when this - /// press completes a double-tap of the *same* modifier within [window]; - /// otherwise records it as the first tap and returns null. - KeyModifier? tap(KeyModifier m, DateTime now) { - final last = _last; - final lastAt = _lastAt; - if (last == m && lastAt != null) { - final gap = now.difference(lastAt); + /// Modifier of the last completed clean tap, arming the double-tap. + KeyModifier? _armed; + DateTime? _armedAt; + + /// Record a key press. A non-modifier key ([mod] == null) — or any key + /// landing while a modifier is already held — is a chord: it dirties the + /// held press and breaks the armed gesture. + void down(KeyModifier? mod) { + if (mod == null || _pressing != null) { + _pressing = null; + _disarm(); + return; + } + _pressing = mod; + } + + /// Record a key release at [now]. Returns the modifier when this release + /// completes a double-tap: the second clean tap of the *same* modifier + /// within [window] of the first tap's release. + KeyModifier? up(KeyModifier? mod, DateTime now) { + if (mod == null) return null; + final pressing = _pressing; + _pressing = null; + if (pressing != mod) return null; // press went dirty (chorded) or stale + if (_armed == mod && _armedAt != null) { + final gap = now.difference(_armedAt!); if (gap >= Duration.zero && gap <= window) { - reset(); - return m; + _disarm(); + return mod; } } - _last = m; - _lastAt = now; + _armed = mod; + _armedAt = now; return null; } - /// Break the gesture — any non-modifier key press resets the tracker. - void reset() { - _last = null; - _lastAt = null; + void _disarm() { + _armed = null; + _armedAt = null; } } diff --git a/lib/src/shell/root_shell.dart b/lib/src/shell/root_shell.dart index 05dfac68..da2eb635 100644 --- a/lib/src/shell/root_shell.dart +++ b/lib/src/shell/root_shell.dart @@ -25,8 +25,10 @@ class RootShellState extends State { late final FocusNode _keyFocus; final MenuBarController _menuBar = MenuBarController(); // Detects double-tapped bare modifiers (e.g. double-Shift → quick-open, - // JetBrains "Search Everywhere"). Bare modifiers never resolve as a single - // chord, so this is the only path that handles them (T-341). + // JetBrains "Search Everywhere"). Fed from a HardwareKeyboard handler, not + // the focus tree: a focused editor consumes the chorded key of `Shift+;`, + // so the gesture must observe every event to know a press wasn't bare + // (T-341, T-409). final ModifierTapTracker _modTap = ModifierTapTracker(); @override @@ -34,10 +36,12 @@ class RootShellState extends State { super.initState(); _keyFocus = FocusNode()..requestFocus(); widget.services.textZoom.addListener(_onZoom); + HardwareKeyboard.instance.addHandler(_onRawKey); } @override void dispose() { + HardwareKeyboard.instance.removeHandler(_onRawKey); widget.services.textZoom.removeListener(_onZoom); _menuBar.dispose(); _keyFocus.dispose(); @@ -156,26 +160,36 @@ class RootShellState extends State { void _onKey(KeyEvent event) { if (_handleMenuMnemonic(event)) return; - // Double-tapped bare modifier (e.g. double-Shift → quick-open). Handle - // it here because a bare modifier never forms a single chord — an - // intervening non-modifier key breaks the gesture (T-341). - if (event is KeyDownEvent) { - final mod = KeyChord.modifierForLogicalKey(event.logicalKey); - if (mod != null) { - if (_modTap.tap(mod, DateTime.now()) != null) { - final seq = [KeyChord.bareModifier(mod), KeyChord.bareModifier(mod)]; - final tapIntent = widget.services.keymap.resolveSequence(seq); - if (tapIntent != null) _dispatchIntent(tapIntent); - } - return; // a bare modifier resolves nothing else - } - _modTap.reset(); - } final intent = widget.services.keymap.resolveEvent(event, HardwareKeyboard.instance); if (intent == null) return; _dispatchIntent(intent); } + /// Double-tapped bare modifier (e.g. double-Shift → quick-open). Observed + /// at the HardwareKeyboard level — before focus dispatch and regardless of + /// who consumes the event — so a chorded key the focused editor swallows + /// (the `;` of `Shift+;`) still dirties the press (T-341, T-409). Fires on + /// the second clean *release*; never consumes anything. + bool _onRawKey(KeyEvent event) { + if (event is KeyDownEvent) { + var mod = KeyChord.modifierForLogicalKey(event.logicalKey); + // A modifier pressed while a non-modifier is already held (rolled + // `a`+Shift) is a chord, not a tap. + if (mod != null && _nonModifierHeld()) mod = null; + _modTap.down(mod); + } else if (event is KeyUpEvent) { + final mod = _modTap.up(KeyChord.modifierForLogicalKey(event.logicalKey), DateTime.now()); + if (mod != null) { + final seq = [KeyChord.bareModifier(mod), KeyChord.bareModifier(mod)]; + final tapIntent = widget.services.keymap.resolveSequence(seq); + if (tapIntent != null) _dispatchIntent(tapIntent); + } + } + return false; + } + + bool _nonModifierHeld() => HardwareKeyboard.instance.logicalKeysPressed.any((k) => KeyChord.modifierForLogicalKey(k) == null); + void _dispatchIntent(Intent intent) { // Try the focused context first so feature widgets (palette, editor, …) // get a chance to handle their own intents; fall back to the app root's diff --git a/test/app_test.dart b/test/app_test.dart index 3805a422..439cbe21 100644 --- a/test/app_test.dart +++ b/test/app_test.dart @@ -354,6 +354,44 @@ void main() { expect(tester.takeException(), isNull); }); + testWidgets('double-tapped bare Shift opens quick-open (T-341)', (tester) async { + await pumpApp(tester); + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + await tester.pump(); + expect(f.services.quickOpen.isOpen, isTrue); + expect(tester.takeException(), isNull); + }); + + testWidgets('typing colons (Shift+;) never triggers quick-open (T-409)', (tester) async { + await pumpApp(tester); + // Two rapid `:` keystrokes — the chorded `;` dirties each Shift press. + for (var i = 0; i < 2; i++) { + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.semicolon); + await tester.sendKeyUpEvent(LogicalKeyboardKey.semicolon); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + } + await tester.pump(); + expect(f.services.quickOpen.isOpen, isFalse); + expect(tester.takeException(), isNull); + }); + + testWidgets('a bare Shift tap followed by a Shift chord does not fire (T-409)', (tester) async { + await pumpApp(tester); + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); // clean tap arms + await tester.sendKeyDownEvent(LogicalKeyboardKey.shiftLeft); + await tester.sendKeyDownEvent(LogicalKeyboardKey.semicolon); // chord — old code fired on the down + await tester.sendKeyUpEvent(LogicalKeyboardKey.semicolon); + await tester.sendKeyUpEvent(LogicalKeyboardKey.shiftLeft); + await tester.pump(); + expect(f.services.quickOpen.isOpen, isFalse); + expect(tester.takeException(), isNull); + }); + testWidgets('file.closeWorkspace command closes the active project', (tester) async { final repo = Directory.current.path; await tester.runAsync(() async => f.services.project.open(repo)); diff --git a/test/kernel/src/keymap/modifier_tap_test.dart b/test/kernel/src/keymap/modifier_tap_test.dart index c19c26d7..17b82dec 100644 --- a/test/kernel/src/keymap/modifier_tap_test.dart +++ b/test/kernel/src/keymap/modifier_tap_test.dart @@ -1,5 +1,5 @@ /// Unit tests for ModifierTapTracker — double-tapped bare-modifier -/// detection (T-341). +/// detection (T-341), clean-release semantics (T-409). library; import 'package:clide/kernel/src/keymap/key_chord.dart'; @@ -12,45 +12,97 @@ void main() { final t0 = DateTime(2026, 1, 1, 12); DateTime at(int ms) => t0.add(Duration(milliseconds: ms)); + // A clean tap: bare press + release. + KeyModifier? tap(ModifierTapTracker t, KeyModifier m, int ms) { + t.down(m); + return t.up(m, at(ms)); + } + group('ModifierTapTracker', () { - test('two taps of the same modifier within the window fire', () { + test('two clean taps of the same modifier within the window fire', () { final t = ModifierTapTracker(window: const Duration(milliseconds: 350)); - expect(t.tap(KeyModifier.shift, at(0)), isNull); // first tap arms - expect(t.tap(KeyModifier.shift, at(200)), KeyModifier.shift); // double-tap + expect(tap(t, KeyModifier.shift, 0), isNull); // first tap arms + expect(tap(t, KeyModifier.shift, 200), KeyModifier.shift); // double-tap }); test('the second tap just outside the window does not fire', () { final t = ModifierTapTracker(window: const Duration(milliseconds: 350)); - expect(t.tap(KeyModifier.shift, at(0)), isNull); - expect(t.tap(KeyModifier.shift, at(400)), isNull); // too slow + expect(tap(t, KeyModifier.shift, 0), isNull); + expect(tap(t, KeyModifier.shift, 400), isNull); // too slow }); test('a slow second tap re-arms, so a prompt third tap fires', () { final t = ModifierTapTracker(window: const Duration(milliseconds: 350)); - expect(t.tap(KeyModifier.shift, at(0)), isNull); - expect(t.tap(KeyModifier.shift, at(500)), isNull); // re-arms from here - expect(t.tap(KeyModifier.shift, at(600)), KeyModifier.shift); + expect(tap(t, KeyModifier.shift, 0), isNull); + expect(tap(t, KeyModifier.shift, 500), isNull); // re-arms from here + expect(tap(t, KeyModifier.shift, 600), KeyModifier.shift); }); test('different modifiers never form a double-tap', () { final t = ModifierTapTracker(); - expect(t.tap(KeyModifier.shift, at(0)), isNull); - expect(t.tap(KeyModifier.ctrl, at(100)), isNull); // ctrl != shift + expect(tap(t, KeyModifier.shift, 0), isNull); + expect(tap(t, KeyModifier.ctrl, 100), isNull); // ctrl != shift }); - test('an intervening key (reset) breaks the gesture', () { + test('an intervening key between taps breaks the gesture', () { final t = ModifierTapTracker(); - expect(t.tap(KeyModifier.shift, at(0)), isNull); - t.reset(); // e.g. a letter was pressed: Shift a Shift - expect(t.tap(KeyModifier.shift, at(100)), isNull); + expect(tap(t, KeyModifier.shift, 0), isNull); + t.down(null); // a letter: Shift a Shift + t.up(null, at(50)); + expect(tap(t, KeyModifier.shift, 100), isNull); }); test('firing consumes the pair — a third tap re-arms, not re-fires', () { final t = ModifierTapTracker(); - expect(t.tap(KeyModifier.shift, at(0)), isNull); - expect(t.tap(KeyModifier.shift, at(100)), KeyModifier.shift); // fires + resets - expect(t.tap(KeyModifier.shift, at(150)), isNull); // back to arming - expect(t.tap(KeyModifier.shift, at(200)), KeyModifier.shift); + expect(tap(t, KeyModifier.shift, 0), isNull); + expect(tap(t, KeyModifier.shift, 100), KeyModifier.shift); // fires + resets + expect(tap(t, KeyModifier.shift, 150), isNull); // back to arming + expect(tap(t, KeyModifier.shift, 200), KeyModifier.shift); + }); + + // T-409 regression: a chorded press (Shift+; typing a colon) is not a tap. + test('a key chorded onto a held modifier dirties the press', () { + final t = ModifierTapTracker(); + t.down(KeyModifier.shift); + t.down(null); // `;` while Shift held — typing `:` + t.up(null, at(50)); + expect(t.up(KeyModifier.shift, at(80)), isNull); // dirty press, no tap + }); + + test('typing two colons rapidly never fires', () { + final t = ModifierTapTracker(); + for (final base in [0, 120]) { + t.down(KeyModifier.shift); + t.down(null); + t.up(null, at(base + 40)); + expect(t.up(KeyModifier.shift, at(base + 60)), isNull); + } + }); + + test('a chorded press also breaks an armed first tap', () { + final t = ModifierTapTracker(); + expect(tap(t, KeyModifier.shift, 0), isNull); // clean tap arms + t.down(KeyModifier.shift); + t.down(null); // Shift+; — chord, must disarm + t.up(null, at(40)); + expect(t.up(KeyModifier.shift, at(60)), isNull); + // The next single clean tap re-arms but must not fire either. + expect(tap(t, KeyModifier.shift, 100), isNull); + }); + + test('a second modifier chorded onto the first is not a tap', () { + final t = ModifierTapTracker(); + t.down(KeyModifier.shift); + t.down(KeyModifier.ctrl); // ctrl while shift held + expect(t.up(KeyModifier.ctrl, at(30)), isNull); + expect(t.up(KeyModifier.shift, at(50)), isNull); + }); + + test('a release without a tracked press is ignored', () { + final t = ModifierTapTracker(); + expect(t.up(KeyModifier.shift, at(0)), isNull); // stale release + expect(tap(t, KeyModifier.shift, 50), isNull); // arms normally after + expect(tap(t, KeyModifier.shift, 150), KeyModifier.shift); }); }); }