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

Support non-stdin pty/pts devices: Assign the correct IO to the stty calls #158

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

byteit101
Copy link

@byteit101 byteit101 commented Mar 21, 2019

The current stty size calls implicitly assume the device to read from is stdin, which is not overridden when @rl_instream is manually set to a pts device that isn't mapped to $stdin. As such, when $stdin is a pipe or other device where $stdin.tty? == false, the prompt and history editing is not visible. Further, if $stdin is sized differently than @rl_instream, the size will be erroneously for $stdin and not @rl_instream.

This PR:

  • Adds a new private method io_exec that correctly calls IO.popen with @rl_instream as stdin
  • Replaces all calls to backtick-launched processes with io_exec
  • Uses @rl_instream in the search-exit logic instead of hardcoding $stdin
  • Adds a new test to test that pty/pts devices correctly receive the prompt and visual editing. Fails on current master, passes with the other changes enclosed.

I have successfully tested this on JRuby 9.2, MRI 2.2.0, MRI 2.5.1, MRI 2.6.2 on RHEL 7.6 & Debian 9.

I have not tested this on Windows or macOS, and would appreciate anyone with such a machine testing this change, as I have limited access to such boxes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant