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

Catches Errno::ENODEV and Errno::EBADF in get_screen_size. Closes #690 #702

Merged
merged 2 commits into from
May 22, 2024

Conversation

vtamara
Copy link
Contributor

@vtamara vtamara commented May 10, 2024

No description provided.

@tompng
Copy link
Member

tompng commented May 10, 2024

Additional information how to reproduce these Errno

Errno::ENOTTY

echo | ruby -rio/console -e "STDIN.winsize"

Errno::ENODEV

ruby -rio/console -e "STDIN.winsize" < /dev/null

Errno::EBADF

ruby -rio/console -e "io=IO.popen('cat','r+'); fd=io.inspect[/\d+/].to_i; io2=IO.for_fd(fd); io.close; io2.winsize"

(Haven't found a way to raise EBADF from STDIN.winsize)

@@ -235,7 +235,7 @@ def self.get_screen_size
s = [ENV["LINES"].to_i, ENV["COLUMNS"].to_i]
return s if s[0] > 0 && s[1] > 0
[24, 80]
rescue Errno::ENOTTY
rescue Errno::ENOTTY, Errno::ENODEV, Errno::EBADF
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding Errno::ENODEV is enough.

The main problem of #690 is that just requiring 'reline' was calling IO#winsize. #703 will fix it.

We need to rescue Errno::ENOTTY and Errno::ENODEV to make reline work in these two scenario.

# For Errno::ENODEV
ruby -rreline -e "Reline.readline" < /dev/null
# For Errno::ENOTTY
ruby -rreline -e "Reline.readline" < file

But for Errno::EBADF, I think we don't need to rescue it. Just like we don't rescue IOError here.

STDIN.close
STDIN.winsize #=> closed stream (IOError)
Reline.readline #=> `tty?': closed stream (IOError)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, just catching Errno::ENOTTY and Errno::ENODEV

Copy link
Member

@tompng tompng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@tompng tompng merged commit a5b5298 into ruby:master May 22, 2024
40 checks passed
matzbot pushed a commit to ruby/ruby that referenced this pull request May 22, 2024
get_screen_size. Closes ruby/reline#690
(ruby/reline#702)

* Catches exceptions Errno::ENODEV and Errno::EBADF in get_screen_size. Closes ruby/reline#690

* Just catching Errno::ENOTTY and Errno::ENODEV

ruby/reline@a5b5298e4a
@vtamara vtamara deleted the fix690 branch May 22, 2024 19:17
@vtamara
Copy link
Contributor Author

vtamara commented May 22, 2024

Thank you 👍

Welcome. Thank you.

@ima1zumi ima1zumi added the bug Something isn't working label May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

3 participants