Fix clock display (regression from conflict resolution for #106)

[hartwork]

Regression from merge commit 95467bf related to #106.

# git diff 4901969fcebb0518f01932e7c7a2d38961d68b99 95467bf56dc23af61013c6ccc21791aba0a97569 | tail -n6

    redraw_and_continue:
-           time(&t1);  // to animate the clock display
            redraw_screen(errstr);
        }

[edgar-bonet]

Thinking about this after a good night... it is not a good idea to depend on execution timing for a CI test to succeed. A better approach would be to remove the display of the time altogether, or replace it with a fixed, dummy string. For example, replace commit af9827c with something like this:

// In paint_plot:
if (!keep_time_fixed)
    asctime_r(lt, ls);

// In main:
keep_time_fixed = getenv("FIXED_TIME") != NULL;
if (keep_time_fixed)
    strcpy(ls, "Thu Jan 1 00:00:00 1970");

[hartwork]

it is not a good idea to depend on execution timing for a CI test to succeed

@edgar-bonet I would like to agree but:

• unfortunately even without any clock display the CI will still rely on close-enough timing, e.g. without considering timing we could end up making screenshots right in the middle of a redraw and then we end up with a half-drawn screen and a mismatch in images; also
• if we freeze or remove the clock, the CI can no longer catch when we break the clock next time, which already proved helpful once (except that it made us aware that the clock started moving again, but that's a detail). So we'd lose value.

I'm happy to sleep over this more, but right now I'm in favor of keeping a moving clock display. But maybe we can make the UI testing CI more robust some other way in a follow-up pull request? What do you think?

[edgar-bonet]

even without any clock display the CI will still rely on close-enough timing [...] we could end up making screenshots right in the middle of a redraw

I didn't think about that. Actually, I don't event know how agg works. Ideally, it should be able to sync itself with ncurses' paint events, but I guess that's not easy...

On the other hand, I am planning to break this again in the future by making the clock display more “correct”, i.e. updating on the edges of system clock seconds.

Anyway, this PR is really not disruptive. Let's get it merged, even though we may have to rethink it later.

[hartwork]

Anyway, this PR is really not disruptive. Let's get it merged, even though we may have to rethink it later.

@edgar-bonet thanks!

On the other hand, I am planning to break this again in the future by making the clock display more “correct”, i.e. updating on the edges of system clock seconds.

We can keep this for later, but if you feel like elaborating already now, I'd be curious to learn more about it.

[edgar-bonet]

My idea is to set the timeout as the time remaining until the next full second. This way pselect would timeout only at the most appropriate moment: right after the “seconds” digits of the system clock has changed. The clock will never be late, not even 0.2 seconds late, and there will be no “useless” redraws at all.

But yes, this will be for later...

[hartwork]

@edgar-bonet that is an interesting idea, but it seems to assume that the process runtime between pselect returning and the redraw of the screen would be exactly zero. Maybe it's close enough. it would allow us to sleep for up to a second, e.g. <=500ms more than currently. I saw the same idea as a bug in stresstest.c ealier but considered it good enough: The rate will be more exact the same way.