Add pixel arguments to set window size

queue-miscreant commented 2 months ago

Is there any reason the ws_xpixel and ws_ypixel fields of the TIOCSWINSZ call are always 0? Naively, shouldn't it be possible to replace

def _setwinsize(fd, rows, cols):
    # Some very old platforms have a bug that causes the value for
    # termios.TIOCSWINSZ to be truncated. There was a hack here to work
    # around this, but it caused problems with newer platforms so has been
    # removed. For details see https://github.com/pexpect/pexpect/issues/39
    TIOCSWINSZ = getattr(termios, 'TIOCSWINSZ', -2146929561)
    # Note, assume ws_xpixel and ws_ypixel are zero.
    s = struct.pack('HHHH', rows, cols, 0, 0)
    fcntl.ioctl(fd, TIOCSWINSZ, s)

with

def _setwinsize(fd, rows, cols, pixel_height=0, pixel_width=0):
    # Some very old platforms have a bug that causes the value for
    # termios.TIOCSWINSZ to be truncated. There was a hack here to work
    # around this, but it caused problems with newer platforms so has been
    # removed. For details see https://github.com/pexpect/pexpect/issues/39
    TIOCSWINSZ = getattr(termios, 'TIOCSWINSZ', -2146929561)
    # Note, assume ws_xpixel and ws_ypixel default to zero.
    s = struct.pack('HHHH', rows, cols, pixel_width, pixel_height)
    fcntl.ioctl(fd, TIOCSWINSZ, s)

I ask because of a long chain of trying to figure out why I couldn't get some things to display correctly in a Poetry shell, which uses pexpect (which uses this). On at least my kernel (Linux 6.10), the terminal is responsive to the values supplied in those fields of the struct. Ideally, it would be nice for pexpect to be able to forward size changes to its children beyond just row and column counts.

I noticed that an old issue mentions portability problems on FreeBSD (https://github.com/pexpect/pexpect/issues/39), but that involved the ioctl call having a different value. Perhaps someone better informed than me can confirm that implementing this breaks things on other systems.

takluyver commented 2 months ago

I don't know of any reason not to make them optional parameters with zero as the default. The tty_ioctl man page for Linux still says they're unused, and on my system (with Gnome terminal) they're still 0, so I guess no-one's needed them yet.

In terms of documenting them, it's also not clear to me whether they're meant to be the pixel dimensions of the window, or the size of one row & one column. Presumably if you see something actually setting them, you can see which one they're used for.

It's annoying that the struct is inconsistent in whether x or y goes first, but such is life.

jquast commented 2 months ago

I would suggest it is meant to be the full window size in pixels. This parameter was largely unused until libsixel support in terminals, search "TIOCGWINSZ + sixel",

I don't believe there is any inconsistency in regards to ws_xpixel & ws_ypixel parameters. I've been working on an update to ucs-detect detection routines for libsixel support, aspect ratio, font size estimation etc. and they have all reliably reported correct X & Y values so far.

takluyver commented 2 months ago

Thanks!

Re inconsistency, I just meant that the size in characters is rows, columns (i.e. y, x), but the size in pixels is x, y. It doesn't matter, of course - it's just funny that it's two different patterns in one struct.

pexpect / ptyprocess

Add pixel arguments to set window size #79