swaywm / sway

i3-compatible Wayland compositor
https://swaywm.org
MIT License
14.57k stars 1.11k forks source link

status_command doesn’t handle partial lines #3857

Open alyssais opened 5 years ago

alyssais commented 5 years ago

Consider the following as a status_command:

#include <stdio.h>
#include <unistd.h>

int main() {
    for (;;) {
        printf("hello ");
        fflush(stdout);
        sleep(1);
        printf("world\n");
        fflush(stdout);
        sleep(10);
    }
}

The status bar will only show world, because AFAICT, status_handle_readable reads into a buffer, but then just reads into that same buffer again without displaying it, unless the bar happens to repaint for some other reason.

No documentation I can see for getline explains what it does to the buffer it’s given if it’s reading from a non-blocking fd — it doesn’t seem to be designed for that use case. So I think something else might need to be used instead.

ddevault commented 5 years ago

This is by design.

alyssais commented 5 years ago

sway-bar(5) says

Each line of text printed to stdout from this command will be displayed in the status area of the bar.

But that’s not what this is at all. This is “The first write to stdout from this command, up to the first newline, will be displayed in the status area of the bar”.

Should the manual be updated?

I’m also told that this is not what i3bar does.

alyssais commented 5 years ago

Sorry, I mean “the first write that ends in a newline”. Any write that doesn’t end in a newline will be ignored.

ddevault commented 5 years ago

Oh, I misunderstood your bug report - apologies.

ianyfan commented 5 years ago

I compiled your program and used it as the status_command, and it displayed "hello" followed by "world" a second later as expected, so I am not sure what is going on.

alyssais commented 5 years ago

I compiled your program and used it as the status_command, and it displayed "hello" followed by "world" a second later as expected, so I am not sure what is going on.

Just to confirm: it showed "hello ", and then "hello world" all at once, but never "world"?

ianyfan commented 5 years ago

No, it showed "hello" then "world", separately, never "hello world".

emersion commented 5 years ago

Okay, so that's a bug. If we receive one message without \n and one message with \n, we should concat both. It should be possible to check the last character returned by getline, and if it's not \n, append instead of replace the status.

ianyfan commented 5 years ago

I'm not sure we should, because the behaviour I'm getting matches the behaviour I'm getting with i3

alyssais commented 5 years ago

I'm not sure we should, because the behaviour I'm getting matches the behaviour I'm getting with i3

I think this warrants fixing even if it doesn't match i3's behaviour, because it's such a patently bad idea. How many writes a program uses to write a line is, surely, an implementation detail of that program, but if only the first write is displayed, then as a user I suddenly care how many writes a program is doing. As is, the only way to be sure that the output you want will actually be displayed correctly by swaybar is to wrap the whole command in echo "$(...)". Surely that can't be desirable behaviour?

ianyfan commented 5 years ago

You make a good point; I'm happy for this change to be made. And perhaps you should bring this up with the i3 team too if it hasn't already.

alyssais commented 5 years ago

And perhaps you should bring this up with the i3 team too if it hasn't already.

Are you aware of any documentation for i3bar that says the output will be shown line at a time? I couldn't find any (or really anything about the non-JSON format at all).

ianyfan commented 5 years ago

This is the only reference I can find: https://i3wm.org/docs/userguide.html#status_command