Trim the id in the Command(CommandId) constructor as the string one does - #138
Merged
Merged
Conversation
…oes [patch] CommandRegistry keys commands on Command.Id but trims the id every lookup is given, so a Command built from a CommandId with surrounding whitespace registered under a key no lookup could produce: it showed up in listings but could never be found, bound or unregistered. The CommandId constructor now trims like the string constructor, and rejects a whitespace-only id. Fixes #123 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk
|
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
1 participant
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.
Fixes #123
What was wrong
Command(string id, …)trims the id, butCommand(CommandId id, …)stored it as given.CommandRegistrykeys its dictionary oncommand.Iduntrimmed, whileIsCommandRegistered,GetCommandandUnregisterCommandtrim the id they are given before looking it up. So a command built fromCommandId.Create(" file.save ")registered and showed up in listings, but could never be found, bound or unregistered.Change
CommandIdconstructor now trims the id in the same place the string constructor does. An id that is already trimmed is kept as the same instance, following theNote(NoteName)pattern.CommandIdnow throwsArgumentException, which the constructor's XML doc already promised.CommandId's regex is still not enforced.Tests
CommandIdWhitespaceTestshas 4 tests. They check that the id is trimmed, that the command equals one built with the string constructor, that a whitespace-only id is rejected, and that a padded command can be registered, found, bound and unregistered. All 4 fail when the fix is reverted. Full suite: 106/106 pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01X65VbCUfvG15D4o8afrvpk
Generated by Claude Code