-
Notifications
You must be signed in to change notification settings - Fork 37
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
Default console width when api is unavailable #38
Default console width when api is unavailable #38
Conversation
58372c6
to
0e1313c
Compare
@@ -70,8 +71,7 @@ def initialize(opts = {}) | |||
# updated below if we got a "Rows" option | |||
self.rows = [] | |||
|
|||
# TODO: Discuss a cleaner way to handle this information | |||
self.width = opts['Width'] || ::IO.console.winsize[1] | |||
self.width = opts['Width'] || ::IO.console&.winsize&.[](1) || ::BigDecimal::INFINITY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Safe navigation of arrays has quirky syntax IMO. I believe if console is defined, winsize should be a valid array:
self.width = opts['Width'] || ::IO.console&.winsize&.[](1) || ::BigDecimal::INFINITY | |
self.width = opts['Width'] || ::IO.console&.winsize[1] || ::BigDecimal::INFINITY |
However - I'll err on the side of caution though, in case there's cross-platform differences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checking the code it doesn't seem like this scenario should arise, but i'm not strongly opinionated
Confirmed that this fixes my original issue. Thanks! |
0e1313c
to
b6c799e
Compare
b6c799e
to
17f6728
Compare
@@ -42,31 +42,32 @@ def with_whitespace_highlighted(string) | |||
|
|||
describe Rex::Text::Table do | |||
let(:formatter) do | |||
Formatter = Class.new do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just fixing these whilst I'm here, this was creating constant warnings
Alternative implementation to #34