You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
KeybindingConfiguration's AutoSave, AutoSaveIntervalMilliseconds and DefaultProfileId/Name have no effect: nothing in the library reads IKeybindingConfiguration #145
IKeybindingConfiguration (Keybinding/Contracts/IKeybindingConfiguration.cs) and its implementation KeybindingConfiguration (Keybinding/Models/KeybindingConfiguration.cs) are public API that promise behaviour:
AutoSave - "whether to auto-save changes"
AutoSaveIntervalMilliseconds - "the auto-save interval in milliseconds (0 to disable)"
DefaultProfileId - "the default profile ID to use when no profile is active"
DefaultProfileName
No code in the library consumes any of them. grep -rn "AutoSave\|IKeybindingConfiguration\|DefaultProfileId" Keybinding/ finds only the interface and the class itself:
KeybindingManager has no constructor or property that takes an IKeybindingConfiguration; it only ever saves when the caller invokes SaveAsync().
KeybindingManager.CreateDefaultProfile hard-codes "default" / "Default" as parameter defaults rather than reading the configuration.
ServiceCollectionExtensions.AddKeybinding* never registers or resolves IKeybindingConfiguration.
The default constructor and the constructor both default AutoSave to true, which suggests saving happens automatically.
Failure scenario
Following USAGE_EXAMPLES.md ("Using Configuration Objects" and "Configuration with Options Pattern"):
varconfig=newKeybindingConfiguration("./data",autoSave:true,autoSaveIntervalMilliseconds:3000);varmanager=newKeybindingManager(newCommandRegistry(),newProfileManager(),newJsonKeybindingRepository(config.DataDirectory));awaitmanager.InitializeAsync();manager.CreateDefaultProfile();manager.Keybindings.BindChord("file.save",Chord.Parse("Ctrl+S"));// app exits without an explicit SaveAsync()
The caller reasonably expects the binding to have been auto-saved within 3 seconds. Nothing is written to disk, so every change made since start-up is lost on exit. Likewise, a DefaultProfileId of "my-default" is never used: CreateDefaultProfile() still creates "default", and nothing falls back to the configured profile when none is active.
Suggested fix
Pick one, and make the docs match:
Wire it up: give KeybindingManager a constructor (and DI registration) that accepts IKeybindingConfiguration; use DefaultProfileId/DefaultProfileName in CreateDefaultProfile when no arguments are passed; when AutoSave is true and the interval is > 0, save after mutations on a debounced timer (disposed in Dispose), or
Retire it: mark AutoSave, AutoSaveIntervalMilliseconds, DefaultProfileId and DefaultProfileName[Obsolete] (or remove them in a major release) and drop the auto-save examples from USAGE_EXAMPLES.md.
Acceptance criteria
Either a configured AutoSave/interval demonstrably persists changes without an explicit SaveAsync() call (covered by a test), and CreateDefaultProfile() honours DefaultProfileId/DefaultProfileName; or those members are obsolete/removed and no documentation implies they do anything.
Category: Bug (public API and USAGE_EXAMPLES.md promise behaviour that nothing implements)
Priority: Medium. A caller who trusts AutoSave = true (the default) loses every binding change made since start-up on exit, and gets no error. The workaround is easy (call SaveAsync()), and any app that already saves explicitly is unaffected, so it isn't High.
Area / suggested assignment:Keybinding/Contracts/IKeybindingConfiguration.cs, Keybinding/Models/KeybindingConfiguration.cs, KeybindingManager (CreateDefaultProfile), ServiceCollectionExtensions, and USAGE_EXAMPLES.md.
Suggested next step: choose between wiring it up and retiring it before anyone writes code. Given #93, retiring AutoSave/AutoSaveIntervalMilliseconds with [Obsolete] now and fixing the docs is the cheaper, safer option. DefaultProfileId/DefaultProfileName are simple to honour in CreateDefaultProfile() either way. Wiring up auto-save should wait until #142 merges, so the timer's save runs under the same lock as mutations.
Activity
matt-edmondson commentedon Sep 28, 2026
Triage
USAGE_EXAMPLES.mdpromise behaviour that nothing implements)AutoSave = true(the default) loses every binding change made since start-up on exit, and gets no error. The workaround is easy (callSaveAsync()), and any app that already saves explicitly is unaffected, so it isn't High.Keybinding/Contracts/IKeybindingConfiguration.cs,Keybinding/Models/KeybindingConfiguration.cs,KeybindingManager(CreateDefaultProfile),ServiceCollectionExtensions, andUSAGE_EXAMPLES.md.ktsu.AppDataStorage). If that lands, it changes where an auto-save timer would live.Suggested next step: choose between wiring it up and retiring it before anyone writes code. Given #93, retiring
AutoSave/AutoSaveIntervalMillisecondswith[Obsolete]now and fixing the docs is the cheaper, safer option.DefaultProfileId/DefaultProfileNameare simple to honour inCreateDefaultProfile()either way. Wiring up auto-save should wait until #142 merges, so the timer's save runs under the same lock as mutations.Generated by Claude Code