Member
Closes #2885. supersedes & closes #2890
in hindsight the review process of #2890 revealed to me that it might be better to
- properly split PrefixHandler from CommandHandler
- not have the commends updatable via setters. For now I completely removed that. If users actually use that & ask for it, we cann add proper method
update_commands()etc. IMO
Checklist for PRs
- Added
.. versionadded:: version,.. versionchanged:: versionor.. deprecated:: versionto the docstrings for user facing changes (for methods/class descriptions, arguments and attributes) - Created new or adapted existing unit tests
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question:
Comment on lines +75 to +77
| Warning: | ||
| When setting :paramref:`block` to :obj:`False`, you cannot rely on adding custom | ||
| attributes to :class:`telegram.ext.CallbackContext`. See its docs for more info. |
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm, what is this warning doing here? Is it saying that the custom .args attribute which this handler adds wont be there?
Member Author
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you have handlers in multiple groups and you do context.custom_arg = foo in a lower group, you cannot rely on context.custom_arg being available in the higher group. We have this warning in all handlers.
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests should be updated + .rst file renaming needed
Start pulling apart CH & PH
Split CH & PH
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a nitpick
Member
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm