terminal:handle zero dimensions from tcgetwinsize

[alexs-sh]

Hello,

This PR adds default (ANSI terminal) values in case when tcgetwinsize returns Ok with columns or rows equal to 0. In practice, Ok((0,0)) often results in crashes (frequently) or rendering issues (more rarely) when running applications over a serial line.

Meanwhile, applications like vi and vim work well in the same environment. The reason is they rely on multiple sources to determine the terminal size: tcgetwinsize, environment variables, terminfo, fallbacks to defaults, and more. They use an enhanced and layered mechanism for detecting the terminal dimensions.

Since implementing such a mechanism could take quite some time, I'd prefer to provide only default values for now:

• they can serve as a starting point for discussing the problem and solutions
• they can help prevent crashes and issues in existing applications
• they are simple and predictable

Some links on issues / PRs caused by Ok with zero columns/rows:

• 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

Some links on vi / vim:

• getting terminal size in vim: https://github.com/vim/vim/blob/b88f9e4a04ce9fb70abb7cdae17688aa4f49c8c9/src/os_unix.c#L4299
• using defaults in vi: https://github.com/mirror/busybox/blob/371fe9f71d445d18be28c82a2a6d82115c8af19d/editors/vi.c#L4814

Note

This PR is mostly to demonstrate the problem, provide a quick (though likely not perfect) workaround, and help us find a correct and acceptable solution for everyone. It seems we should be careful here, as this kind of change could affect existing applications.

Example

A small proof. Helix before the change:

Helix with the change:

Please let me know if any additional info, test, information are needed.

Thank you

[mikaelmello]

I think crossterm has historically returned Err in this scenario: #680, and it worked for my use-case when we were on 0.25.0, but it must have regressed during a refactor

[alexs-sh]

Agree. Returning Err for zero dimensions is a good approach, if Ok(0,0) was implemented by mistake and doesn’t have some hidden meaning

[alexs-sh]

It seems we lose the zero-dimensions checks here: #790

@benjajaja Hi! I understand it might be difficult to remember changes from two years ago 😄 But could you please remind me why the zero-value checks were dropped?

I see some discussions here. But I didn't find the answer

@markus-bauer Hi! Feel free to join the conversation 😄

[benjajaja]

@alexs-sh mhh, hard to reconstruct, but I think the thought probably was: the new window_size() returns a WindowSize with pixel size on top of col-row-counts, and pixel size could be zero when col-row-counts is not zero, in some terminals. So the zero-check would perhaps be out of place in window_size(). It should not return Err when just the pixels are zero for obvious reasons, and then checking only cols/rows would seem surprising.
But I don't see any reason why it shouldn't simply be in size()! It was probably a mistake.

On another note, I am not using window_size() anymore. TIOCGWINSZ (not via crossterm but termios directly) is just a fallback in my case. Perhaps window_size() should be removed or marked deprecated, IIRC only a few terminals actually return pixel sizes.

[alexs-sh]

@benjajaja Thanks

1. Let’s ignore the pixels. Only column/row values in characters matter for us.

2. Unfortunately, it’s possible to call ioctl that returns success but fills the column and row values with zeroes. This is a place where problems start. And it happens quite often when running apps over a serial line. In this situation, it’s preferable to signal an error (since at this point we know the values from ioctl are unreliable) and let the upper-level code decide what to do. Using defaults is also possible. It isn’t ideal, but it preserves the Err/Ok behavior, at least as it was in recent history.

[alexs-sh]

Seems the hardest part is that three levels may be responsible for handling it.

• the lower level with ioctl. But it seems we can’t do anything there.
• Crossterm itself. It could provide errors, default values, or extended logic to handle such situations
• the upper-level code. We can just delegate the problem (Ok(0,0)) there and preserve Crossterm as is. However, it seems the upper-level code expects more advanced or predictable behavior from the library.

[benjajaja]

@alexs-sh I think it's straightforward: just return Err in the size() method if rows and cols are zero. That would reinstate previous behaviour, and yet window_size() would be consistent in returning Ok(actual reported values).

Would also be worth looking into what last_os_error() returns when rows and cols are zero - I bet it's not really related, and should have a custom error instead.

[alexs-sh]

@benjajaja OK, thanks. For me, Err also looks fine, so I can prepare another PR. However, here are several notes:

1. Checking for incorrect values should be done at the end: ioctl TIOCGWINSZ → tput_size → check. Otherwise, an error may be thrown when cols/lines are provided via environment variables.

2. last_os_error for TIOCGWINSZ will not work well, as ioctl could return success with empty cols/rows values. This is a very frequent case when ioctl runs in sessions over a serial line.

Here is a brief example in C. Running the app in a "regular" terminal session will provide correct cols/rows values. However, running the example over a serial connection often results in (0,0) without an explicit error.

#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <pty.h>

int main(int argv, const char *argc[]) {

  const char *file_name = (argv > 1) ? argc[1] : "/dev/tty";
  const int file = open(file_name, O_RDWR);

  printf("Open '%s' with result:%d\n", file_name, file);

  struct winsize window_size;
  const int result = ioctl(file, TIOCGWINSZ, &window_size);

  printf("Read terminal size with result:%d\n", result);
  printf("Errno:%d\n", errno);
  printf("Cols:%d\n", window_size.ws_col);
  printf("Rows:%d\n", window_size.ws_row);

  return 0;
}

When I run the example in a "regular" session (SSH, desktop), I see the valid sizes:

Running over serial (Qemu, RPI, Orange,...) provides zeroes without an explicit error.

But

[alexs-sh]

Closing in favor of #1011