-
Notifications
You must be signed in to change notification settings - Fork 326
terminal:size:handle window size with zero rows or columns #1011
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
base: master
Are you sure you want to change the base?
terminal:size:handle window size with zero rows or columns #1011
Conversation
|
@mikaelmello @benjajaja Hello. Could you please take a look at this? We discussed it earlier in #1007 and agreed to add an error. This version implements that change |
This commit handles the case where window_size returns success, but the number of rows and/or columns is 0. Instead of returning Ok((0,0)), the size function will use tput as the second source of sizes. In practice, window_size returning zero rows or columns is rare when the terminal is running over SSH, desktop, etc. However, it occurs frequently when the terminal is running over serial lines, especially on embedded devices with minimal environments. (And this is how the problem was detected)
9470b87 to
cdd840c
Compare
| // In case of an error, it’s fine to return the error immediately. If simply | ||
| // reading the sizes fails, most likely something went completely wrong. | ||
| // We can signal that immediately. |
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.
This is something that's difficult to validate against all the use cases. Right now if this fails, we fallback to tput, which has set an expectation that this is how things work. I don't know enough about the failure conditions that peopel might be hitting where a window_size call would fail, but tput succeeds, but if there are any, then this change breaks those use cases.
This is a little bit of a Chesterton's fence problem, if you can't confirm the reason for some behavior accurately then you shouldn't change it. I'm against approving this without that understanding, but also I recognize that it might be impossible to answer why and when this might happen without a lot of digging. (Do you have time to find out if there's any real world problems here / actual cases - this seems like it would be difficult to find proof either way).
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.
@joshka Hello. Sure. Let me provide some more context.
I've been working on adding Helix and Aichat into Buildroot - a tool that helps prepare and deliver system images (bootloader, kernel, userspace) for embedded devices.
One of the common validation steps in this process is to generate small images for QEMU, Raspberry Pi, or other popular devices with the new applications and a minimal environment, and then run them.
During this work, I encountered crashes and/or rendering issues in applications that rely on Crossterm. For example,
- Helix - helix-term:fix crash on unwrap graphemes helix-editor/helix#14050
- Helix - Render issues during work over serial line helix-editor/helix#14101
- aichat - fix: add minimal columns bound to prevent divide-by-zero sigoden/aichat#1366
In all cases, the problems occurred when running the tools over serial lines. The main problem is ioctl can return zero sizes, but without triggering an error. For example,

(The code is here as we discussed it during #1007). So, having Ok((0,0)) often breaks the application logic, as the apps expect an error in that case.
Furthermore, it’s possible to get Ok(0, 0) from ioctl, but correct values from tput. That’s why it’s desirable to keep searching when we get Ok((0, 0)) from ioctl.
However, if we get an Err from ioctl (and window_size as result), it seems fine to return immediately. This is because if we can’t even read basic parameters from the console device, it’s very likely that we can’t operate with it at all. There are at least two scenarios where this can happen: /dev/console is not present, or the user doesn’t have permission to access it.
It’s not a problem to prepare a version where ioctl issues don’t cause an immediate return, and the logic continues to the tput branch. So, if it could help to understand or collect statistics, I can do that.
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.
"to find out if there's any real world problems " - for now, I’m aware of issues in Helix and Aichat.
It's possible to download QEMU/RPI/... images from the GitLab (CI artifacts), run and see the issues.
Here as an short example of how to download & run QEMU images
wget https://gitlab.com/alexs-sh/helix-buildroot/-/jobs/11946779371/artifacts/download -O archive.zip
unzip archive.zip
tar xzf images.tar.gz
qemu-system-aarch64 -M virt -cpu cortex-a53 -nographic -smp 1 \
-kernel images/Image \
-append "rootwait root=/dev/vda console=ttyAMA0" \
-netdev user,id=eth0 \
-device virtio-net-device,netdev=eth0 \
-drive file=images/rootfs.ext2,if=none,format=raw,id=hd0 \
-device virtio-blk-device,drive=hd0
download-and-run-helix-image-with-qemu.webm
Here, 11946779371 is the job ID. After several days, the images will expire and be deleted. However, the overall idea remains the same. And of course, it’s possible to download the images manually via the Web UI, or build them locally if you’re familiar with Buildroot.
I also can try running some of these apps to collect more information / statistics
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.
I tested broot. It also has rendering issues when working over the serial line. This is also caused by Ok(0,0) width/height returned from crossterm.
read_size -> terminal_size -> Ok(0,0) from crossterm, so termimad can't apply default sizes as there is no error.
pub fn terminal_size() -> (u16, u16) {
let size = terminal::size();
size.unwrap_or((DEFAULT_TERMINAL_WIDTH, DEFAULT_TERMINAL_HEIGHT))
}
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.
I don't know enough about the failure conditions that peopel might be hitting where a window_size call would fail, but tput succeeds, but if there are any, then this change breaks those use cases.
I think this is not formulated correctly. This change breaks the case of window_size succeeding, but with 0,0. With the change, it's unknown what will happen. In practice, @alexs-sh says:
Furthermore, it’s possible to get Ok(0, 0) from ioctl, but correct values from tput.
So the outcome after ioctl 0,0 now may be:
- Ok(real sizes) from tput
- Ok(0,0) from tput (as we don't check again for 0,0)
- Err(io)
So my questions are:
- What does tput produce when ioctl is 0,0 (in the real world)? (mighty hard to give a good answer)
- Should we return an error if tput also succeeds with 0,0?
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.
So my questions are:
1. What does tput produce when ioctl is 0,0 (in the real world)? (mighty hard to give a good answer)
@benjajaja Sure, let’s review several possible outcomes.
-
tputis missing. Sincetputis part ofncursesand there’s no hard dependency betweencrosstermandncurses, it’s possible that tput isn’t available on the system. In this case, we’ll encounter an error. -
tputis present. Here, there are two possible scenarios:
-
Likely: it will return a valid result, as it can find the terminal information in the database.
-
Unlikely: it will produce an error if the terminal information is missing.
2. Should we return an error if tput also succeeds with 0,0?
From my experience with ncurses and tput, it doesn’t normally return successful (0,0) values. When it can’t detect the correct values, it reports an error (a non-zero return value). But I can review the tput code to be more confident. Tthere’s a chance I was just lucky.
benjajaja
left a comment
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.
I think it's important to clearly have this context:
Up until two years ago, 0,0 would make an Err: https://github.com/crossterm-rs/crossterm/pull/790/files#diff-6b068659b49668e11f0448da4de645624b5dd77648196949cd20cc4c11e82f57L41
I believe that that change (made by me) was a mistake, an attempt to make something else consistent, which should not have taken priority. Sadly, now it has been there for two years, which makes it now much harder to take a decision.
| // reading the sizes fails, most likely something went completely wrong. | ||
| // We can signal that immediately. | ||
| match window_size()? { | ||
| WindowSize { rows: 0, .. } | WindowSize { columns: 0, .. } => (), |
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.
| WindowSize { rows: 0, .. } | WindowSize { columns: 0, .. } => (), | |
| WindowSize { rows: 0, columns: 0 } => (), |
I don't think "only one of them is zero" is relevant. That sounds more like a real case of the terminal window being resized, and to be left up to the application to deal with.
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.
I’m fine with handling only specific Ok(0,0)
| // In case of an error, it’s fine to return the error immediately. If simply | ||
| // reading the sizes fails, most likely something went completely wrong. | ||
| // We can signal that immediately. |
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.
I don't know enough about the failure conditions that peopel might be hitting where a window_size call would fail, but tput succeeds, but if there are any, then this change breaks those use cases.
I think this is not formulated correctly. This change breaks the case of window_size succeeding, but with 0,0. With the change, it's unknown what will happen. In practice, @alexs-sh says:
Furthermore, it’s possible to get Ok(0, 0) from ioctl, but correct values from tput.
So the outcome after ioctl 0,0 now may be:
- Ok(real sizes) from tput
- Ok(0,0) from tput (as we don't check again for 0,0)
- Err(io)
So my questions are:
- What does tput produce when ioctl is 0,0 (in the real world)? (mighty hard to give a good answer)
- Should we return an error if tput also succeeds with 0,0?



About
This commit handles the case where
window_sizereturns success, but the number of rows and/or columns is 0. Instead of returningOk((0,0)), thesizefunction will usetputas the second source of sizes. Iftputfails (e.g., if it is not available), the function will return an error.In practice,
window_sizereturning zeros is rare for terminals running over SSH, desktop, etc. However, it occurs frequently when the terminal is running over serial lines. (And this is how the problem was detected)This is a continuation of work started earlier #1007
Examples
A simple app for test
Before
After
tput is not available

tput is available

Thank you