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
UnbindChord/GetChord/HasChordBinding(commandId) throw on a blank id once a profile is active, but return false/null otherwise #129
The active-profile overloads in Keybinding/Services/KeybindingService.cs don't guard commandId:
GetChord(commandId) (~line 93)
UnbindChord(commandId) (~line 131)
HasChordBinding(commandId) (~line 253)
They forward to Profile.GetChord, Profile.RemoveChord and Profile.HasChord, which throw ArgumentException on whitespace. The (profileId, commandId) overloads check string.IsNullOrWhiteSpace(commandId) and return null or false.
Reproduction
svc.UnbindChord(" ");// no active profile -> falsesvc.SetActiveProfile("p");svc.UnbindChord(" ");// -> throws ArgumentExceptionsvc.GetChord(" ");// -> throws ArgumentExceptionsvc.GetChord("p"," ");// -> null
This was reproduced with a temporary MSTest test.
Why it matters
The result for the same input depends on whether a profile happens to be active, which is hidden state. The documented contract for UnbindChord is "false if it didn't exist or no active profile", and a caller that follows it can crash. The one-argument and two-argument overloads also disagree for identical inputs.
This is separate from #123. That issue is about a Command whose id has surrounding whitespace; this one is about blank ids passed to the lookup and unbind methods.
Suggested fix
Add the same if (string.IsNullOrWhiteSpace(commandId)) return null/false; guard that the profile-id overloads use.
Acceptance: for a blank commandId, the one-argument overloads return the same result as the two-argument ones, whether or not a profile is active.
Priority: Low. The trigger is a blank command id, and the result depends on hidden state (whether a profile is active). A caller following the documented UnbindChord contract can still crash. The fix is a one-line guard in three places.
Area / suggested assignee: The active-profile overloads GetChord, UnbindChord and HasChordBinding in Services/KeybindingService.cs (around :93, :131 and :253). Maintainer: @matt-edmondson.
Next steps: Add the IsNullOrWhiteSpace(commandId) guard the two-argument overloads already use. Add tests that call each one-argument overload with and without an active profile.
Activity
matt-edmondson commentedon Sep 27, 2026
Triage
UnbindChordcontract can still crash. The fix is a one-line guard in three places.GetChord,UnbindChordandHasChordBindinginServices/KeybindingService.cs(around:93,:131and:253). Maintainer: @matt-edmondson.Commandwhose id has surrounding whitespace) and ParsePhrase("") throws "Phrase must contain at least one chord (Parameter 'sequence')" from its own empty-input guard #128 (ParsePhrase("")input validation). All three are about how blank or whitespace input is handled, and could be fixed together in one input-validation pass.Note(NoteName)normalization) and doesn't touch these overloads.Next steps: Add the
IsNullOrWhiteSpace(commandId)guard the two-argument overloads already use. Add tests that call each one-argument overload with and without an active profile.Generated by Claude Code