Switch to StdioInputMethod when TERM is 'dumb' #907
Conversation
This works around Reline's misbehavior on low-capability terminals, such as when IRB is launched inside Emacs (ruby/reline#616). It should also resolve ruby#68 and resolve ruby#113 which were filed out of similar need.
|
N.B. the older "inverted triangle" issue seems to have been fixed in Reline, somewhere between the versions |
| @@ -151,6 +151,10 @@ def initialize(irb, workspace = nil, input_method = nil) | |||
| @command_aliases = @user_aliases.merge(KEYWORD_ALIASES) | |||
| end | |||
|
|
|||
| private def term_interactive? | |||
| STDIN.tty? && ENV['TERM'] != 'dumb' | |||
There was a problem hiding this comment.
How about STDIN.tty? && STDOUT.tty?? Does this solves your problem?
When STDIN and STDOUT are both tty, it is executed in an interactive terminal.
Reline and IRB both have a tty check, but there is an inconsistency. IRB only checks STDIN.tty?, and on the other hand, Reline only checks $stdout.tty?. If $stdout is not a tty or TERM is dumb, Reline enters unusable test mode.
ENV['TERM'] == 'dumb' is used for testing IRB with Reline. Although using ENV['TERM'] like this is a bit weird, changing the TERM=dumb behavior right now will break the test.
There was a problem hiding this comment.
Unfortunately, both STDIN.tty? and STDOUT.tty? return true when the shell is running inside Emacs. But apparently some of the terminal features it's using don't work as expected in that environment. So treating "dumb" terminals differently makes sense to me (that's the definition - terminal with limited capabilities).
Do you know any other real-world "dumb" terminals? It could be useful to test IRB in one of them as well.
There was a problem hiding this comment.
The tests are indeed currently broken by this change, any recommendations for finding a fix would be welcome.
There was a problem hiding this comment.
I see. Thanks for the explanation. I agree IRB should treat dumb terminals differently 👍
For test, could you add a way to test IRB by forcing term_interactive? to be true?
Let it return true if ENV["TEST_IRB_FORCE_INTERACTIVE"] is set, and in IRB's test which uses PTY.spawn, specify both TEST_IRB_FORCE_INTERACTIVE and TERM=dumb env.
See discussion in ruby#907.
See discussion in ruby#907.
dgutov
force-pushed
the
no_multiline_or_singleline_on_dumb_term
branch
from
8b8ad45
to
1edfbf1
Compare
|
The "latest reline" breakage is the same on master. |
|
Looks good! thank you 👍 |
This reverts commit 241e061.
This works around Reline's misbehavior on low-capability terminals, such as when IRB is launched inside Emacs (which can look like ruby/reline#616, even though that issue is about non-tty usage). It should also resolve #68 and resolve #113 which were filed out of similar need.
The existing solution we use in Emacs with inf-ruby is to detect the current version of the tool and launch it with both
--nomultilineand--nosinglelinewhen we think it supports those options. But that only covers one entry point -- having REPL launched from one of the few preset commands ininf-ruby, whereas all the other cases (custom console script by a user; orbinding.irbinvocation inside a test suite) remain problematic, requiring case-by-case solutions.The provided simple change looks like a significant improvement in my testing.