Provide 80x24 fallback for ansi and vt100 #465
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
2 similar comments
penguinolog
reviewed
Mar 31, 2023
urwid/raw_display.py
Outdated
| # Provide some lightweight fallbacks in case the TIOCWINSZ doesn't | ||
| # give sane answers | ||
| if x <= 0 or y <= 0: | ||
| if self.term == 'ansi' or self.term == 'vt100': |
There was a problem hiding this comment.
Suggested change
| if self.term == 'ansi' or self.term == 'vt100': | |
| if self.termin('ansi', 'vt100'): |
And will be good to join 2 if's
|
Please rebase: Travis not providing logs (technically not functional) and without logs hard to see what was wrong with current PR |
|
Rebased and made the change you suggested (I think). This was apparently open for like 2 years and I've completely forgotten it existed, so forgive me if I'm a little slow on the uptake here. |
penguinolog
requested changes
Mar 31, 2023
urwid/raw_display.py
Outdated
Comment on lines
686
to
687
| if x <= 0 or y <= 0: | ||
| if self.term in ('ansi','vt100'): |
There was a problem hiding this comment.
Suggested change
| if x <= 0 or y <= 0: | |
| if self.term in ('ansi','vt100'): | |
| if (x <= 0 or y <= 0) and self.term in ('ansi','vt100'): |
Not all terminals will necessarily respond to the TIOCGWINSZ ioctl, particularly much older ones providing vt100 or ansi terminals, which will have a fixed 80x24 size. If they respond with some kind of non-sensical value here, then at least give the ansi and vt100 terminals their default so they can draw.
penguinolog
approved these changes
Mar 31, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not all terminals will necessarily respond to the TIOCGWINSZ ioctl,
particularly much older ones providing vt100 or ansi terminals, which
will have a fixed 80x24 size. If they respond with some kind of
non-sensical value here, then at least give the ansi and vt100 terminals
their default so they can draw.
Checklist
masterorpython-dual-supportbranchtoxsuccessfully in local environment (my pypy isn't set up, but it's otherwise good)Description:
I've been working with some older machines that present a classic
ansiorvt100terminal, and whilencursesworked just fine with them, theraw_displayfromurwiddid not. I tracked this down to the use ofTIOCGWINSZ. This ioctl wasn't providing meaningful dimensions for these old terminals. It looked likeurwidwas getting 0x0, whilescreenwas getting -1x-1. These terminal types have a fixed width and height as per theirtermcapdescriptions, and they're ubiquitous, so I provided some fallback values shouldTIOCGWINSZnot give workable dimensions.I tried to see if a patch like this was rejected in the past, but forgive me if I missed it.