Make Pry work with tee #1372
Merged
Make Pry work with tee #1372
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Unbuffered output allows using the tee command.
If the first "input" is a StringIO, so the old approach never sets the realine output, so we have to check every time.
kyrylo
added a commit
to kyrylo/foreman
that referenced
this pull request
Mar 3, 2015
The motivation behind this commit is to make Foreman respect GNU Readline [1]. As Foreman is often used with Pry (or at least many people want to), and Pry is dependent on Readline, using these tools together is very problematic [2][3]. The problem is that when you start Pry by means of Foreman, Foreman supresses output from Pry, so you don't see what you type. The output can only be seen after pressing carriage return. Demonstration (a bit hard to follow, but you may want to try it yourself; clone the Pry repo and execute the same command): http://showterm.io/3eb716461d6602ac90b09 Although this problem was reported by Pry users, any Readline application is affected. This commit uses IRB for tests, because it also depends on Readline, hence it suits for testing perfectly. Other Readline dependent applications that I tested were GHCi, the Lua REPL, Eshell (erlang shell). Previously, Pry was using a trick, which allowed us to bypass this problem [4]. The fix was featured in Pry v0.9.12.6, but then it was removed by accident. It's been readded on HEAD recently (not released yet). However, the fix we currently have is still not perfect. Although you can see your input immediately, you can't see your output now :D Demo: http://showterm.io/f648de27568d96a02f1b3 The fix breaks correct piping. Now, when you pipe Pry's output, it doesn't output Readline's prompt and user's input. What's being piped is only return values of executed expressions. Demo: http://showterm.io/3651389faf1fdc0d18211 To fix the missing output and pipe everything, including the user's input, we need to set `Readline.output` correctly. However, if we do that, we break minimal support for Foreman. So when I fixed Pry [5], I figured it would be interesting to try to fix Foreman. Now, it's time to talk about the changes here. I'm not very familiar with Foreman, so take the code with a grain of salt. Basically, the whole point of the fix is to read one character at a time and print it immediately, instead of waiting for a newline character. This technique allows us to prepend so-called markers (timestamps, if you wish) to a Readline prompt and display the user's input. It works for all the Readline apps that I mentioned above and doesn't break piping (that is, tee). Note. I had to remove `$stdout.flush` from the output method, because it was causing thread deadlocks with the flush in `#handle_io`. The deadlock appears when you use Foreman with Pry, pipe the output and trigger SIGINT (control-c press). Example: % foreman start -f Procfile | tee log 15:50:05 pryhere.1 | started with pid 26881 15:50:28 pryhere.1 | [1] pry(main)> <CTRL+C><CTRL+D> 15:50:49 pryhere.1 | [2] pry(main)> foreman/engine.rb:323:in `synchronize': deadlock; recursive locking (ThreadError) This is probably because of the race condition, when on SIGINT Foreman prints stuff here [6], which flushes $stdout. [1]: http://ruby-doc.org/stdlib-2.2.0/libdoc/readline/rdoc/Readline.html [2]: pry/pry#1290 [3]: pry/pry#1275 (comment) [4]: https://github.com/pry/pry/blob/f0cbec507111743fdbc273735ea4f0f6164a5b21/lib/pry/repl.rb#L184 [5]: pry/pry#1372 [6]: https://github.com/ddollar/foreman/blob/339ff1df2347328134e471e413ad70766058faa3/lib/foreman/engine.rb#L413
mfpiccolo
pushed a commit
to mfpiccolo/foreman
that referenced
this pull request
Aug 10, 2015
The motivation behind this commit is to make Foreman respect GNU Readline [1]. As Foreman is often used with Pry (or at least many people want to), and Pry is dependent on Readline, using these tools together is very problematic [2][3]. The problem is that when you start Pry by means of Foreman, Foreman supresses output from Pry, so you don't see what you type. The output can only be seen after pressing carriage return. Demonstration (a bit hard to follow, but you may want to try it yourself; clone the Pry repo and execute the same command): http://showterm.io/3eb716461d6602ac90b09 Although this problem was reported by Pry users, any Readline application is affected. This commit uses IRB for tests, because it also depends on Readline, hence it suits for testing perfectly. Other Readline dependent applications that I tested were GHCi, the Lua REPL, Eshell (erlang shell). Previously, Pry was using a trick, which allowed us to bypass this problem [4]. The fix was featured in Pry v0.9.12.6, but then it was removed by accident. It's been readded on HEAD recently (not released yet). However, the fix we currently have is still not perfect. Although you can see your input immediately, you can't see your output now :D Demo: http://showterm.io/f648de27568d96a02f1b3 The fix breaks correct piping. Now, when you pipe Pry's output, it doesn't output Readline's prompt and user's input. What's being piped is only return values of executed expressions. Demo: http://showterm.io/3651389faf1fdc0d18211 To fix the missing output and pipe everything, including the user's input, we need to set `Readline.output` correctly. However, if we do that, we break minimal support for Foreman. So when I fixed Pry [5], I figured it would be interesting to try to fix Foreman. Now, it's time to talk about the changes here. I'm not very familiar with Foreman, so take the code with a grain of salt. Basically, the whole point of the fix is to read one character at a time and print it immediately, instead of waiting for a newline character. This technique allows us to prepend so-called markers (timestamps, if you wish) to a Readline prompt and display the user's input. It works for all the Readline apps that I mentioned above and doesn't break piping (that is, tee). Note. I had to remove `$stdout.flush` from the output method, because it was causing thread deadlocks with the flush in `#handle_io`. The deadlock appears when you use Foreman with Pry, pipe the output and trigger SIGINT (control-c press). Example: % foreman start -f Procfile | tee log 15:50:05 pryhere.1 | started with pid 26881 15:50:28 pryhere.1 | [1] pry(main)> <CTRL+C><CTRL+D> 15:50:49 pryhere.1 | [2] pry(main)> foreman/engine.rb:323:in `synchronize': deadlock; recursive locking (ThreadError) This is probably because of the race condition, when on SIGINT Foreman prints stuff here [6], which flushes $stdout. [1]: http://ruby-doc.org/stdlib-2.2.0/libdoc/readline/rdoc/Readline.html [2]: pry/pry#1290 [3]: pry/pry#1275 (comment) [4]: https://github.com/pry/pry/blob/f0cbec507111743fdbc273735ea4f0f6164a5b21/lib/pry/repl.rb#L184 [5]: pry/pry#1372 [6]: https://github.com/ddollar/foreman/blob/339ff1df2347328134e471e413ad70766058faa3/lib/foreman/engine.rb#L413
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1334 (Pry doesn't play well with tee).