This PR adds an API inspired by the standard library's cmd module. #135
Conversation
- lastcmd + emptyline() - precmd() - postcmd() - default precmd casts arguments to their annotation type if an annotation was provided
Codecov Report
@@ Coverage Diff @@
## master #135 +/- ##
==========================================
- Coverage 92% 86.81% -5.19%
==========================================
Files 4 4
Lines 300 364 +64
Branches 39 48 +9
==========================================
+ Hits 276 316 +40
- Misses 16 35 +19
- Partials 8 13 +5
Continue to review full report at Codecov.
|
| help_template = '{cmd_name} {arg_list}\n {doc}\n' | ||
| help_short_template = ' {cmd_name}{cmd_arg_sep}{arg_list}: {doc_firstline}' # noqa | ||
|
|
||
| _sin = None # type: IO[str] |
There was a problem hiding this comment.
This gave me problems further down the road, where mypy does not realise that within the do_* methods, _sout is guaranteed to be set to an instance of IO[str], so it errors out because None doesn't have a write method.
| @@ -141,6 +153,72 @@ def _server(self) -> None: | |||
| except (socket.timeout, OSError): | |||
| continue | |||
| 8000 |
|
||
| def _interactive_loop(self, sin: IO[str], sout: IO[str]) -> None: | |||
| """Main interactive loop of the monitor""" | |||
| self._sin = sin | |||
There was a problem hiding this comment.
why do we need expose self._sin at class level?
There was a problem hiding this comment.
just to avoid do_where(self, sin: IO[str], sout: IO[str], taskid: int) having extra agrs? How we can ensure that _sin/_sout are set? may be add check in _command_dispatch
There was a problem hiding this comment.
Yes, avoiding passing the same IO arguments everywhere was the idea.
Given how the Monitor class is structured, the do_* methods are only called from within _command_dispatch, which itself is only called from within a running _interactive_loop. This loop sets the IO attributes at the beginning, and unsets them in the finally clause at the end. So they are guaranteed to be set for do_* methods.
If you dislike this, I'm open to reverting this.
aiomonitor/monitor.py
Outdated
| if str(param).startswith('*'): | ||
| yield from (type_(arg) for arg in ia) # type: ignore | ||
| else: | ||
| yield type_(next(ia)) # type: ignore |
There was a problem hiding this comment.
we expect StopIteration only here?
There was a problem hiding this comment.
We iterate over the functions annotation for its parameters. Within that, we manually iterate over the given arguments (with the ia = iter(args) and next(ia) construct; I don't use zip since those two sequences may have arbitrary length difference). Now, when when we are in the else clause of the if (meaning we have not yet reached *args), we could be in the situation where a further parameter exists, but no argument is given to that parameter. Since we might have do_* methods with optional, non-star arguments, we must ignore a StopIteration from this call to next.
I probably should add this as a comment to the code.
aiomonitor/monitor.py
Outdated
| yield from (type_(arg) for arg in ia) # type: ignore | ||
| else: | ||
| yield type_(next(ia)) # type: ignore | ||
| except StopIteration: |
There was a problem hiding this comment.
Lets use contextlib.suppress instead except StopIteration: pass, looks a bit nicer
There was a problem hiding this comment.
Oh yes, How could I have overlooked the suppress for this case :)
Add clarification comment Add missing typing
No description provided.