-
Notifications
You must be signed in to change notification settings - Fork 32
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: added ability to override mirai parsers and action_parsers. #81
feat: added ability to override mirai parsers and action_parsers. #81
Conversation
Hey @khanjasir90, The analyze check is failing due to an unused import. Please check it. https://github.com/BuildMirai/mirai/actions/runs/12608379968/job/35142662480?pr=81 |
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.
Thanks for this great work 🎉
Please check my suggestions.
@@ -16,24 +16,14 @@ class MiraiRegistry { | |||
|
|||
bool register(MiraiParser parser) { |
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.
@khanjasir90 Can we add an optional param called "bool override". And the parser will be overridden only of the value is true.
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.
Sure, added the same @divyanshub024
return true; | ||
} | ||
_miraiActionParsers[type] = parser; | ||
return true; | ||
} | ||
|
||
Future<dynamic> registerAll(List<MiraiParser> parsers) { |
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.
Also, please add the same logic to registerAll
and registerAllActions
functions.
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.
Update the registerAll
and update registerAllActions
functions with the same logic
Hey @khanjasir90, There is some formatting issue. Can you please run |
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.
Thanks for this amazing PR @khanjasir90 🥇
Description
Related Issues
Closes #17
Type of Change