Skip to content
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

Add pixel arguments to set window size #79

Open
queue-miscreant opened this issue Aug 16, 2024 · 3 comments
Open

Add pixel arguments to set window size #79

queue-miscreant opened this issue Aug 16, 2024 · 3 comments

Comments

@queue-miscreant
Copy link

queue-miscreant commented Aug 16, 2024

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 (pexpect/pexpect#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
Copy link
Member

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
Copy link
Member

jquast commented Aug 17, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants