-
Notifications
You must be signed in to change notification settings - Fork 124
Add hook example #478
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
Add hook example #478
Conversation
Codecov Report
@@ Coverage Diff @@
## master #478 +/- ##
=======================================
Coverage 90.96% 90.96%
=======================================
Files 11 11
Lines 2868 2868
=======================================
Hits 2609 2609
Misses 259 259Continue to review full report at Codecov.
|
tleonhardt
left a comment
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.
Consider my couple comments, but everything looks good.
examples/hooks.py
Outdated
| last = first + 10 | ||
|
|
||
| for x in range(first, last): | ||
| self.poutput(x) |
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.
Recommend converting x from an int to a str first since poutput is intended to receive a str (even though this will work):
self.poutput(str(x))
examples/hooks.py
Outdated
| def abbrev_hook(self, data: cmd2.plugin.PostparsingData) -> cmd2.plugin.PostparsingData: | ||
| """Accept unique abbreviated commands""" | ||
| target = 'do_' + data.statement.command | ||
| if not target in dir(self): |
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.
Recommend testing for membership using:
if target not in dir(self):| super().__init__(*args, **kwargs) | ||
|
|
||
| # register three hooks | ||
| self.register_postparsing_hook(self.add_whitespace_hook) |
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.
Interesting use case for multiple postparsing hooks. I like the example, thanks for creating it.
| last = first + 10 | ||
|
|
||
| for x in range(first, last): | ||
| self.poutput(str(x)) |
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.
Everything looks good. TravisCI seems to be having some issues today. I restored one of the jobs since it failed for no good reason. This should be ready to merge once CI completes.
Closes #477.