Starting lf on 'first' instance of foot messes up lf's UI #20

Closed
opened 1 year ago by drv28 · 7 comments
drv28 commented 1 year ago

Foot version:

foot version 1.3.0-49-g8fd7c83 (May 29 2020, branch 'master')

Running on compositor:

sway version 1.4

How to reproduce:

  • Add line bindsym $mod+u exec foot lf to sway config and reload it.
  • Quit all instances of foot.
  • Press mod+u.

Consequence:

Much of lf's UI is not visible unless some key combination of j and k is pressed.

### Foot version: `foot version 1.3.0-49-g8fd7c83 (May 29 2020, branch 'master')` ### Running on compositor: `sway version 1.4` ### How to reproduce: * Add line `bindsym $mod+u exec foot lf` to `sway` config and reload it. * Quit all instances of `foot`. * Press `mod+u`. ### Consequence: Much of `lf`'s UI is not visible unless some key combination of `j` and `k` is pressed.
dnkl commented 1 year ago
Owner

I haven't heard of lf. Where can I find it?

I haven't heard of `lf`. Where can I find it?
dnkl commented 1 year ago
Owner

Found it, I think. Is this it? https://github.com/gokcehan/lf

Found it, I think. Is this it? https://github.com/gokcehan/lf
Poster

Oh, Sorry for not including the link.

Github link: https://github.com/gokcehan/lf

There is also an AUR package available: https://aur.archlinux.org/packages/lf/

Oh, Sorry for not including the link. Github link: <https://github.com/gokcehan/lf> There is also an AUR package available: <https://aur.archlinux.org/packages/lf/>
dnkl commented 1 year ago
Owner

And which version of foot is this (foot --version)?

And which version of foot is this (`foot --version`)?
Poster

foot version 1.3.0-49-g8fd7c83 (May 29 2020, branch 'master')

`foot version 1.3.0-49-g8fd7c83 (May 29 2020, branch 'master')`
dnkl commented 1 year ago
Owner

This is a race, between foot reporting the terminal window size and lf reading it.

In theory, it could happen regardless of how many foot instances you already have running, but the first instance has a much higher chance of triggering it, due to it being slower to start.

Foot is pretty aggressive about spawning the client as early as possible. What this means is the client can start reading from the tty and start doing ioctls before the window has been mapped (displayed), and thus before it has a size.

On Sway, a fast client can observe three different sizes in very quick succession: 0x0, 700x5001, and then the final size assigned by Sway.

The first one is before the window is mapped. The second one is Sway telling us to size the window ourselves, and the last one is Sway telling us what size to actually use (there's a bug on Sway to skip the second step: https://github.com/swaywm/sway/issues/2176).

It appears 0x0 is a problem for lf. And I can't blame it :)

Can you try the patch below? It seems to work for me (the issue doesn't always reproduce for me, even when I've killed all other foot instances).

If it does indeed solve your problem, I'll try to come up with a better version of it, and post a PR.

If it does not solve your problem, chances are high it's a lf bug. But let's start with this:

diff --git a/terminal.c b/terminal.c
index 4f9d1d5..5b963dd 100644
--- a/terminal.c
+++ b/terminal.c
@@ -714,6 +714,8 @@ load_fonts_from_conf(const struct terminal *term, const struct config *conf,
     return success;
 }
 
+#include <sys/ioctl.h>
+
 struct terminal *
 term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper,
           struct wayland *wayl, const char *foot_exe, const char *cwd,
@@ -888,6 +890,17 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper,
         .cwd = strdup(cwd),
     };
 
+#if 1
+    if (ioctl(term->ptmx, TIOCSWINSZ,
+              &(struct winsize){
+                  .ws_row = 24,
+                      .ws_col = 80,
+                      .ws_xpixel = 0,
+                      .ws_ypixel = 0}) < 0)
+    {
+        LOG_ERRNO("TIOCSWINSZ");
+    }
+#endif
     /* Start the slave/client */
     if ((term->slave = slave_spawn(
              term->ptmx, argc, term->cwd, argv,

  1. not really 700x500, but whatever COLUMNSxLINES it corresponds to ↩︎

This is a race, between foot reporting the terminal window size and `lf` reading it. In theory, it could happen regardless of how many foot instances you already have running, but the first instance has a much higher chance of triggering it, due to it being slower to start. Foot is pretty aggressive about spawning the client as early as possible. What this means is the client can start reading from the tty and start doing `ioctl`s before the window has been mapped (displayed), and thus before it has a size. On Sway, a fast client can observe three different sizes in very quick succession: `0x0`, `700x500`[^1], and then the final size assigned by Sway. [^1]: not really `700x500`, but whatever `COLUMNSxLINES` it corresponds to The first one is before the window is mapped. The second one is Sway telling **us** to size the window ourselves, and the last one is Sway telling us what size to actually use (there's a bug on Sway to skip the second step: https://github.com/swaywm/sway/issues/2176). It _appears_ `0x0` is a problem for `lf`. And I can't blame it :) Can you try the patch below? It seems to work for me (the issue doesn't _always_ reproduce for me, even when I've killed all other foot instances). If it does indeed solve your problem, I'll try to come up with a better version of it, and post a PR. If it does **not** solve your problem, chances are high it's a `lf` bug. But let's start with this: ```diff diff --git a/terminal.c b/terminal.c index 4f9d1d5..5b963dd 100644 --- a/terminal.c +++ b/terminal.c @@ -714,6 +714,8 @@ load_fonts_from_conf(const struct terminal *term, const struct config *conf, return success; } +#include <sys/ioctl.h> + struct terminal * term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, struct wayland *wayl, const char *foot_exe, const char *cwd, @@ -888,6 +890,17 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper, .cwd = strdup(cwd), }; +#if 1 + if (ioctl(term->ptmx, TIOCSWINSZ, + &(struct winsize){ + .ws_row = 24, + .ws_col = 80, + .ws_xpixel = 0, + .ws_ypixel = 0}) < 0) + { + LOG_ERRNO("TIOCSWINSZ"); + } +#endif /* Start the slave/client */ if ((term->slave = slave_spawn( term->ptmx, argc, term->cwd, argv, ```
dnkl self-assigned this 1 year ago
Poster

Above patch solves the issue. 👍

Above patch solves the issue. :+1:
dnkl closed this issue 1 year ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.