refactor(commandhandler): replace the menu-echo class-name check with a flag - #163
Merged
Merged
Conversation
… a flag The dispatcher decided whether to echo a menu-triggered command back to chat by comparing command.__class__.__name__ against a hardcoded tuple of two names, so any new command needing that behavior silently wouldn't get it unless someone remembered to edit the generic dispatcher. Added an echo_from_menu class attribute on BaseCommand (default False), enabled on VoiceTrigger and MyInstantsSearch, and checked polymorphically. Random subclasses VoiceTrigger and would have inherited the flag, which the old exact-name check never granted it, so it is explicitly set back to False to keep behavior identical. Bumps VERSION to 1.8.17. Closes #158 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Dolly132 <109222243+Dolly132@users.noreply.github.com>
Rushaway
commented
Sep 5, 2026
Rushaway
left a comment
Rushaway
left a comment
Member
Author
There was a problem hiding this comment.
Self-review: the interesting bit is Random -- it inherits VoiceTrigger, so a naive flag would have silently enabled menu-echo for it where the old string check didn't. Pinned it to False to keep this a true no-op refactor, and verified equivalence across all 25 command classes by resolving the attribute through the inheritance chain rather than assuming. Flagging it explicitly since it's a judgment call you may want to reverse.
12 tasks
Dolly132
approved these changes
Sep 6, 2026
Dolly132
left a comment
Dolly132
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, would be helpful when adding new menu-based commands in the future instead of having to edit CommandHandler.py again.
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
2 participants
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.
Summary
CommandHandlerdecided whether to echo a menu-triggered command back to chat viacommand.__class__.__name__ in ("VoiceTrigger", "MyInstantsSearch")— a string comparison buried in the generic dispatcher. Any new command needing that behavior silently wouldn't get it unless someone remembered to edit this tuple.echo_from_menuclass attribute onBaseCommand(defaultFalse), set toTrueonVoiceTriggerandMyInstantsSearch, and the dispatcher now checks it polymorphically.One subtlety worth a look
RandomsubclassesVoiceTrigger, so it would have inheritedecho_from_menu = True— a behavior change the old exact-class-name check never granted it. Since this is a refactor, I setRandom.echo_from_menu = Falseexplicitly to keep runtime behavior byte-identical.If you think
Randomshould echo when played from a menu (arguably it should — it plays a sound like any other trigger), that's a one-line follow-up, but it's a deliberate product decision rather than something to slip in under a refactor.Closes #158
Test plan
BaseCommandsubclasses via AST, resolvingecho_from_menuthrough the inheritance chain, and compared against the old__name__ in (...)predicate: zero mismatches — semantics are identical,Randomincluded.🤖 Generated with Claude Code