JEG2 / highline

A higher level command-line oriented interface.
Other
1.29k stars 137 forks source link

terminal_size return nil in some environments in Linux #85

Closed boutil closed 9 years ago

boutil commented 11 years ago

Hi, In some environments on Linux, stty returns an empty string, which means that terminal_size returns nil instead of integers. This causes test_terminal_size test in test/tc_highline.rb, line 932 to fail.

Examples of such environments are the clean environment created by pbuilder and sbuild in Debian to build packages. It probably can happen on other occasions.

If you want to insist on terminal size to return Fixnums, should terminal_size return a size of 0 if stty cannot return the size of the terminal?

Cheers,

Cédric

boutil commented 11 years ago

I should mention that in these environments, stty prints the following warning: stty: standard input: Inappropriate ioctl for device

JEG2 commented 11 years ago

Is there another way for us to get the terminal size on such systems?

boutil commented 11 years ago

I am not sure it is possible on purpose, as a minimalist environment is created. The warning says that the flow passes directly through stdin, and not via a terminal. I will ask. At least, what that shows is that there are cases where terminal_size, as it is written now, does not return a Fixnum, and thus the test is not reliable.

JEG2 commented 11 years ago

Right. We just need to decide what it should do in those cases. Should it raise an exception on such platforms?

abinoam commented 9 years ago

Hi @boutil and @JEG2 ,

What about just falling back to the old and gold 80x24 terminal size when stty doesn't get the size correctly?

It fixes temporarily until interested people implement support for getting the terminal size for specific system with other methods (not stty size).

What do you think?

abinoam commented 9 years ago

I could "reproduce" the issue with a stdin redirection at command line:

rake test < anyfile.txt

It gives me

[ 48/108] TestHighLine#test_terminal_sizestty: stdin isn't a terminal
 = 0.00 s
  1) Failure:
TestHighLine#test_terminal_size [/Users/abinoam/src/highline/test/tc_highline.rb:1098]:
Expected nil to be an instance of Fixnum, not NilClass.
JEG2 commented 9 years ago

Could we use Ruby's standard io/console to get the size when stty returns nothing? Just a thought. I'm fine with the default too.

Could we use the tty?() method to detect $stdin redirects? Not sure if that's helpful.

abinoam commented 9 years ago

Testing here. IO.console.console_size is consistently getting terminal size even with redirections. Doing some more testing.

JEG2 commented 9 years ago

My fear though is that it could fail if run as a Cron job or in a similar minimized environment. In which case we probably still need a default. I can't decide if this is better or worse.

abinoam commented 9 years ago

We could sequentially try more than one method of getting terminal size and fall back into a default if fail. I'll do some experimentation later and give a report. Meanwhile, don't merge the PR. Em 02/02/2015 12:45, "James Edward Gray II" notifications@github.com escreveu:

My fear though is that it could fail if run as a Cron job or in a similar minimized environment. In which case we probably still need a default. I can't decide if this is better or worse.

— Reply to this email directly or view it on GitHub https://github.com/JEG2/highline/issues/85#issuecomment-72478877.

abinoam commented 9 years ago

I don't understand clearly. IO.console.winsize is working well (I misspelled to console_size at my last comment). IO.console_size not. I'll try some code now.

abinoam commented 9 years ago

Hi @JEG2

Using IO.console.winsize is really great. I also think we could refactor (later) most of the code to rely on it. (trimming all that tests for solaris, etc...)

What do you think of how the PR is now with this new commit?

JEG2 commented 9 years ago

Looks awesome. You went above an beyond the call. Thank you.