Skip to content
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

engine: make output Readline-friendly #536

Closed
wants to merge 1 commit into from

Commits on Mar 3, 2015

  1. engine: make output Readline-friendly

    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
    kyrylo committed Mar 3, 2015
    Configuration menu
    Copy the full SHA
    a8621bc View commit details
    Browse the repository at this point in the history