What's wrong
The two Command constructors store the id differently:
Command(string id, …) trims the id.Command(CommandId id, …) (Keybinding/Models/Command.cs:18-23) stores Id = id as given.
CommandRegistry.RegisterCommand keys its dictionary on command.Id untrimmed (Keybinding/Services/CommandRegistry.cs, TryAdd(command.Id, …)). But IsCommandRegistered, GetCommand and UnregisterCommand all trim the id they are given before looking it up. So a command whose CommandId has leading or trailing whitespace is stored under a key that no lookup can ever produce.
CommandId's documented pattern (^[a-z0-9]+(\.[a-z0-9]+)*$) is not enforced either, so CommandId.Create(" file.save ") succeeds.
Reproduction
This was executed as a scratch MSTest:
reg.RegisterCommand(new Command(CommandId.Create(" file.save "), CommandName.Create("Save"))); // true
reg.IsCommandRegistered("file.save"); // False
reg.IsCommandRegistered(" file.save "); // False (lookup trims, stored key doesn't)
reg.GetAllCommands(); // contains "[ file.save ]"
svc.BindChord(" file.save ", Chord.Parse("Ctrl+S")); // False
reg.UnregisterCommand(" file.save "); // FalseAfter a save and reload, the repository rebuilds the command with the string constructor, so the id comes back trimmed. The in-memory state and the reloaded state disagree.
Why it matters
Command ids often come from configuration files or plugin manifests, where stray whitespace is common. The registration looks successful and the command shows up in listings, but it can never be bound, executed or removed, and nothing reports an error.
Suggested fix / acceptance criteria
- Normalize in one place: trim in the
Command(CommandId, …) constructor, or key CommandRegistry on command.Id.ToString().Trim().
new Command(CommandId.Create(" file.save "), …) then behaves exactly like new Command(" file.save ", …).- Enforcing
CommandId's regex is not recommended as the fix: the similar NoteName pattern would reject + and ,, which the Ctrl++ / Ctrl+, support relies on.
What's wrong
The two
Commandconstructors store the id differently:Command(string id, …)trims the id.Command(CommandId id, …)(Keybinding/Models/Command.cs:18-23) storesId = idas given.CommandRegistry.RegisterCommandkeys its dictionary oncommand.Iduntrimmed (Keybinding/Services/CommandRegistry.cs,TryAdd(command.Id, …)). ButIsCommandRegistered,GetCommandandUnregisterCommandall trim the id they are given before looking it up. So a command whoseCommandIdhas leading or trailing whitespace is stored under a key that no lookup can ever produce.CommandId's documented pattern (^[a-z0-9]+(\.[a-z0-9]+)*$) is not enforced either, soCommandId.Create(" file.save ")succeeds.Reproduction
This was executed as a scratch MSTest:
After a save and reload, the repository rebuilds the command with the string constructor, so the id comes back trimmed. The in-memory state and the reloaded state disagree.
Why it matters
Command ids often come from configuration files or plugin manifests, where stray whitespace is common. The registration looks successful and the command shows up in listings, but it can never be bound, executed or removed, and nothing reports an error.
Suggested fix / acceptance criteria
Command(CommandId, …)constructor, or keyCommandRegistryoncommand.Id.ToString().Trim().new Command(CommandId.Create(" file.save "), …)then behaves exactly likenew Command(" file.save ", …).CommandId's regex is not recommended as the fix: the similarNoteNamepattern would reject+and,, which theCtrl++/Ctrl+,support relies on.