Partial fix for #715 #716

wants to merge 11 commits into

2 participants


After some thought and source code, I think that it's simpler to modify the code as I've done (instead of adding another configuration value that is not obvious for beginners to set): changeset add a new interactive flag to the run command.

This fix implements the following:

  • Make a class of the read loop
  • Split the main loop to have function to handle password and try again
  • Introduce a interactive flag for run commands that actually disables the need to any capture to occur during the run:
    • This triggers a codepath where there is nothing bound to stdin and where outputs are directly sent to the output
    • Because this codepath does not do any capture, OutputLooper will read by 1024 bytes chunks
    • However the code in OutputLooper does miss the prefix adding to each line. This is a regression from master (this behaviour is due to which is pre-existing code so I didn't change anything here)...
  • All tests pass
  • My slow test case run quickly
  • The only remaining issue would be to ensure prefix is added to each line correctly.

Let me know if I headed in the right direction, I can work on fixing the line prefixing issue in this particular case.

octplane added some commits Aug 24, 2012
@octplane octplane Small refactor in
- Optimize read when there is no capture made
@octplane octplane Added interactive flag to run
- Setting interactive to False propagate the flag down to the io and ensure reads are fast
- This use a codepath that was never used before in the read loop and in which no prefix is displayed

This pull request passes (merged 19002e9 into 994a76e).

@octplane octplane closed this Sep 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment