Repository navigation
Show the first key of a chord after a timeout - #44
Merged
Merged
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The timeout, cancellation, cross-process behavior, packaging change, and relevant edge cases are consistently implemented and tested.
Review effort: Balanced
Findings: None
What changed in this PR
Adds timeout-based flushing for incomplete keyboard chords so pending keys become visible without breaking completed shortcuts.
Changes:
- Adds per-subscription chord timeout and cross-process handling.
- Moves Konami keymap to an opt-in sample.
- Adds comprehensive accumulator and timeout tests.
| File | Description |
|---|---|
src/Carnac/App.xaml.cs |
Shares the UI scheduler service. |
src/Carnac.Logic/MessageProvider.cs |
Implements chord timeout handling. |
src/Carnac.Logic/ShortcutAccumulator.cs |
Adds pending-state detection and flushing. |
src/Carnac.Logic/Carnac.Logic.csproj |
Stops deploying the Konami keymap. |
src/Carnac.Logic/Keymaps/konami.yml |
Removes the active keymap. |
src/Carnac.Logic/Keymaps/samples/konami.yml.sample |
Adds an opt-in sample. |
src/Carnac.Tests/ChordTimeoutFacts.cs |
Tests timeout and lifecycle behavior. |
src/Carnac.Tests/ShortcutAccumulatorFacts.cs |
Tests accumulator flushing. |
src/Carnac.Tests/ShortcutProviderFacts.cs |
Tests Konami keymap packaging. |
src/Carnac.Tests/MessageProviderFacts.cs |
Supplies the test scheduler dependency. |
src/Carnac.Tests/KeyPresses.cs |
Adds a shared key-press fixture helper. |
src/Carnac.Tests/Carnac.Tests.csproj |
Includes the new test sources. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
doggy8088
force-pushed
the
keys/chord-timeout
branch
10 times, most recently
from
September 29, 2026 00:24
68e00c8 to
b5e57de
Compare
A key that could start a multi-key shortcut was held back until the next key arrived, so a single Up arrow was never shown (konami.yml applies to every process and starts with Up) and the first key of real chords such as Ctrl+M,B or Ctrl+K,Ctrl+C was delayed indefinitely. Add ShortcutAccumulator.Flush(), which completes a pending accumulator through the existing no-match path, and let MessageProvider schedule a flush ChordTimeout (1 s) after a key leaves the accumulator pending. The timeout runs on IConcurrencyService.MainThreadScheduler (the same thread the keyboard hook delivers on) and each key restarts it. A key from another process now flushes the held keys and is processed by a new accumulator instead of being dropped. The Scan seed shared by all subscribers is gone: pending state is per subscription. Stop shipping konami.yml as an active keymap; it becomes the opt-in Keymaps\samples\konami.yml.sample, which the installer's Keymaps\*.yml glob does not pick up. Refs #9 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
doggy8088
force-pushed
the
keys/chord-timeout
branch
from
October 9, 2026 09:58
b5e57de to
a041f2e
Compare
This was referenced Oct 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #9
Summary
A key that could start a multi-key shortcut was held back until the next key arrived, so a single Up arrow was never shown (konami.yml applies to every process and starts with Up) and the first key of real chords (Ctrl+M,B, Ctrl+K,Ctrl+C) stayed invisible until the next press.
ShortcutAccumulator.Flush()now completes a pending accumulator through the existing no-match path, andMessageProviderflushes itChordTimeout(1 s) after the last key, driven byIConcurrencyService.MainThreadSchedulerso it is testable withTestScheduler. konami.yml is no longer shipped active; it is an opt-in sample.Changes
Flush()(no-op when nothing is buffered or already completed) andIsPendingForOtherProcess(KeyPress).IConcurrencyService(all ctor arguments are null-checked), publicChordTimeout(1 s). TheScanover the accumulator was replaced by anObservable.Createthat keeps the accumulator per subscription (the oldScanseed was shared by all subscribers), schedules the flush after a key leaves the accumulator pending, restarts/cancels it on the next key, forwards OnError/OnCompleted and disposes the timer. State changes and OnNext calls are under a lock (key hook and timer normally share the UI thread; the lock plus a key counter make a late timer callback harmless).ShortcutAccumulator.Addflushed the held keys and silently dropped the new key.ConcurrencyService,KeyProvider,ShortcutProviderandMessageProviderare created inOnStartup(after the single-instance check) so that one ConcurrencyService is shared by the MessageProvider and the KeysController.Noneitem (not copied to the output, so neither the build output, the Inno installer'sKeymaps\*.ymlglob norDirectory.GetFiles("*.yml")see it).Testing
Not verified / deviations
KeysController.Start()fromOnStartupon the UI thread, so key events andMainThreadSchedulercallbacks share a thread; a lock protects the accumulator anyway.Keymaps\samplesin the repository.does_not_show_key_press_on_partial_matchtest relies on it).Stack
keys- layer 2 of 5. Base:keys/keymap-parser. Author-assessed confidence: 95%.