Pyscript fix #653
Pyscript fix #653
Conversation
…so we can capture it
… up redirection parsing of non-multiline commands.
Codecov Report
@@ Coverage Diff @@
## master #653 +/- ##
==========================================
+ Coverage 94.34% 94.48% +0.13%
==========================================
Files 11 11
Lines 3079 3062 -17
==========================================
- Hits 2905 2893 -12
+ Misses 174 169 -5
Continue to review full report at Codecov.
|
…command was piping
tleonhardt
left a comment
tleonhardt
left a comment
There was a problem hiding this comment.
This appears to be a very nice ruggedization improvement.
|
|
||
| # Used to restore state after redirection ends | ||
| # redirecting and piping are used to know what needs to be restored | ||
| RedirectionSavedState = utils.namedtuple_with_defaults('RedirectionSavedState', |
There was a problem hiding this comment.
Perhaps this should be declared in utils and imported from there?
| @@ -1879,53 +1871,59 @@ def _complete_statement(self, line: str) -> Statement: | |||
| newline = '\n' | |||
There was a problem hiding this comment.
A unit test for the case inside this if would be nice
| newline = '\n' | ||
| self.poutput(newline) | ||
| line = '{}\n{}'.format(statement.raw, newline) | ||
| except KeyboardInterrupt as ex: |
There was a problem hiding this comment.
A unit test for anything inside tis except would be great (but maybe not easy)
| self._cmdfinalization_hooks.append(func) | ||
|
|
||
|
|
||
| class Statekeeper(object): |
There was a problem hiding this comment.
Nice job finally getting rid of this
| except KeyboardInterrupt: | ||
| try: | ||
| line = self.pseudo_raw_input(self.prompt) | ||
| except KeyboardInterrupt as ex: |
There was a problem hiding this comment.
I like how you cleaned up handling this case
|
I'm going to merge this in because I want to test it in combination with other changes. We can always modify it later. But so far it seems like a great improvement. |
Fixes #613
The root cause of #613 was
onecmd_plus_hooks()calling_restore_output()ifself.redirectingwas True. While pyscript was redirecting its output to a file, thePyscriptBridgewould callonecmd_plus_hooks()to handleapp('help'). After the help command ended,_restore_output()would be called becauseself.redirectingwas True and therefore the file handle pyscript was using to redirect its output would be closed.The new code keeps track of redirection for each command. Therefore
_restore_output()only restores stuff if the current command set up redirection.This also enhances our pyscript functionality quite a bit. We can now independently redirect each command running within the script. Therefore the outer pyscript call may be writing its output to a file while commands being run within the script can write anywhere they want, including piping to another process.
This PR also made a change to the piped process in redirection. We now set its stdout to be
self.stdout.Lastly, I no longer append a
\nto commands in PyscriptBridge. This was originally done to assist in running multiline commands in a pyscript file. However, it had the unintended consequence of breaking redirection with non-multiline commands. From now on users are expected to include a terminator in multiline commands being run in a pyscript. This fits the expected syntax anyway since commands passed toapp()should be the same as they would be typed on the command line.