-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add mixin of commands on ctx #15
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15 +/- ##
===========================================
+ Coverage 68.13% 79.88% +11.74%
===========================================
Files 7 7
Lines 408 502 +94
Branches 66 78 +12
===========================================
+ Hits 278 401 +123
+ Misses 128 100 -28
+ Partials 2 1 -1 ☔ View full report in Codecov by Sentry. |
If the merge commands function is not to clear on what it's doing, just let me know, I can refactor it and can add it as a commands method |
105d79a
to
63bc0fa
Compare
Removed pollution from commit history |
41a7eaf
to
dc3586c
Compare
Test added for merging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work. Thank you so much!
Is something missing for this to be merge? I can work on #16 if you think its appropriated |
2ca44ee
to
2c2934f
Compare
3c704df
to
db51f08
Compare
Now it should be good. Merging main into this branch resulted in duplicate commits, since I merged the fixLocalization branch into this one before. Got a little messy along the way, but it's clean and okay now. (I re-checked that it's working on the telegram client) |
Summary
setMyCommands
function to be able to mergeSetMyCommandsParams
from two or more different sources, based on the fact thattoSingleScopeArgs
group them by language. Merge function its basically a reducer that concatlang.commands
arrays of the same language and returns an array ofsetMyCommandsParams
. It does not break scoping since it's performed after thetoSingleScopeArgs
function. If an empty array is passed, it does not throw, if only one Commands instance is passed it treats it ok.result
variable might be better to change it toacc_Array
or something in those linesPD: I closed the other PR since it was a little messy on the branch managment end, this one is ok.
PD2: I can refactor this into a MixedCommands class that extends the Commands Class if that suits more the style.
Another way would be to add this as a method to the Commands Class, but I don't know good of an idea would be to mix them before
toSingleScopeArgs
in a method likecommands.mergeWith(otherCommands)
because it would effectively duplicate commands registration if someone use it on let say commands A and B, and register both intobot.use
, registering all at once on A (B was merged into A), and then re-register B commands. Even in that case they would simply overwrite themselves I think so, no?eg:
snippet from the gif (localization in the admin command... 🥴)