From: Florian Obser Subject: Re: responsive watch wip To: "Ted Unangst" Cc: "Job Snijders" , tech@openbsd.org Date: Sun, 15 Jun 2025 11:01:26 +0200 On 2025-06-14 13:50 -04, "Ted Unangst" wrote: > I think I've figured out most of the interactions. Pause, update, etc. > > Also should support unicode now. This is not working correctly. consider this file: $ cat test.txt line 1 line 2 line 3 And then run obj/watch cat test.txt you get line 1 scroll down once, you get an empty screen, scroll down once more you get line 2 ... and so on. There seems to be some confusing about empty lines. > > > Index: watch.c > =================================================================== > RCS file: /home/cvs/src/usr.bin/watch/watch.c,v > diff -u -p -r1.28 watch.c > --- watch.c 31 May 2025 08:26:26 -0000 1.28 > +++ watch.c 14 Jun 2025 17:41:41 -0000 > @@ -72,6 +72,7 @@ int start_line = 0, start_column = 0; /* > > int pause_on_error = 0; > int paused = 0; > +int want_update; > int last_exitcode = 0; > time_t lastupdate; > int xflag = 0; > @@ -83,6 +84,15 @@ static char *cmdstr; > static size_t cmdlen; > static char **cmdv; > > +struct child { > + char buf[65000]; > + size_t bufsiz; > + size_t pos; > + pid_t pid; > + int fd; > + struct event evin; > +}; > + > typedef wchar_t BUFFER[MAXLINE][MAXCOLUMN + 1]; > BUFFER buf0, buf1; > BUFFER *cur_buf, *prev_buf; > @@ -93,17 +103,22 @@ struct event ev_timer; > > #define ctrl(c) ((c) & 037) > int display(BUFFER *, BUFFER *, highlight_mode_t); > -void read_result(BUFFER *); > kbd_result_t kbd_command(int); > void show_help(void); > void untabify(wchar_t *, int); > void on_signal(int, short, void *); > +void on_sigchild(int, short, void *); > void timer(int, short, void *); > void input(int, short, void *); > void quit(void); > void usage(void); > void swap_buffers(void); > > +struct child *running_child; > +void start_child(); > +void child_input(int, short, void *); > +void child_done(struct child *); > + > int > set_interval(const char *str) > { > @@ -124,6 +139,7 @@ int > main(int argc, char *argv[]) > { > struct event ev_sigint, ev_sighup, ev_sigterm, ev_sigwinch, ev_stdin; > + struct event ev_sigchild; > size_t len, rem; > int i, ch; > char *p; > @@ -216,19 +232,19 @@ main(int argc, char *argv[]) > signal_set(&ev_sighup, SIGHUP, on_signal, NULL); > signal_set(&ev_sigterm, SIGTERM, on_signal, NULL); > signal_set(&ev_sigwinch, SIGWINCH, on_signal, NULL); > + signal_set(&ev_sigchild, SIGCHLD, on_sigchild, NULL); > signal_add(&ev_sigint, NULL); > signal_add(&ev_sighup, NULL); > signal_add(&ev_sigterm, NULL); > signal_add(&ev_sigwinch, NULL); > + signal_add(&ev_sigchild, NULL); > > event_set(&ev_stdin, STDIN_FILENO, EV_READ | EV_PERSIST, input, NULL); > event_add(&ev_stdin, NULL); > evtimer_set(&ev_timer, timer, NULL); > - evtimer_add(&ev_timer, &opt_interval.tv); > > cur_buf = &buf0; prev_buf = &buf1; > - read_result(cur_buf); > - display(cur_buf, prev_buf, highlight_mode); > + start_child(); > > event_dispatch(); > > @@ -379,21 +395,21 @@ display(BUFFER * cur, BUFFER * prev, hig > } > > void > -read_result(BUFFER *buf) > +start_child() > { > - FILE *fp; > - int i, st, fds[2]; > - pid_t pipe_pid, pid; > + struct child *child; > + int fds[2]; > > - /* Clear buffer */ > - memset(buf, 0, sizeof(*buf)); > + child = calloc(1, sizeof(*child)); > + child->bufsiz = sizeof(child->buf); > > if (pipe(fds) == -1) > err(1, "pipe()"); > > - if ((pipe_pid = vfork()) == -1) > - err(1, "vfork()"); > - else if (pipe_pid == 0) { > + child->pid = vfork(); > + if (child->pid == -1) > + err(1, "vfork"); > + if (child->pid == 0) { > close(fds[0]); > if (fds[1] != STDOUT_FILENO) { > dup2(fds[1], STDOUT_FILENO); > @@ -407,21 +423,56 @@ read_result(BUFFER *buf) > /* use warn(3) + _exit(2) not to call exit(3) */ > warn("exec(%s)", cmdstr); > _exit(1); > - > /* NOTREACHED */ > } > - if ((fp = fdopen(fds[0], "r")) == NULL) > - err(1, "fdopen()"); > close(fds[1]); > + child->fd = fds[0]; > + > + event_set(&child->evin, child->fd, EV_READ | EV_PERSIST, child_input, child); > + event_add(&child->evin, NULL); > + > + running_child = child; > +} > + > +void > +update() > +{ > + if (running_child) { > + /* not yet */ > + want_update = 1; > + return; > + } > + want_update = 0; > + swap_buffers(); > + start_child(); > +} > + > +void > +child_done(struct child *child) > +{ > + event_del(&child->evin); > + close(child->fd); > + free(child); > + // assert(running_child == child); > + if (running_child == child) > + running_child = NULL; > + if (want_update) > + update(); > + else if (!paused) > + evtimer_add(&ev_timer, &opt_interval.tv); > +} > + > +void > +on_sigchild(int sig, short event, void *arg) > +{ > + pid_t pid; > + int st; > > - /* Read command output and convert tab to spaces * */ > - for (i = 0; i < MAXLINE && fgetws((*buf)[i], MAXCOLUMN, fp) != NULL; > - i++) > - untabify((*buf)[i], sizeof((*buf)[i])); > - fclose(fp); > do { > - pid = waitpid(pipe_pid, &st, 0); > + pid = waitpid(WAIT_ANY, &st, 0); > } while (pid == -1 && errno == EINTR); > + if (!running_child || running_child->pid != pid) > + return; > > /* Remember update time */ > time(&lastupdate); > @@ -430,6 +481,45 @@ read_result(BUFFER *buf) > last_exitcode = WEXITSTATUS(st); > if (pause_on_error && last_exitcode) > paused = 1; > + > + child_done(running_child); > +} > + > +void > +child_input(int sig, short event, void *arg) > +{ > + struct child *child = arg; > + ssize_t n; > + > + n = read(child->fd, child->buf + child->pos, child->bufsiz - child->pos); > + if (n == -1) > + return; > + child->pos += n; > + > + size_t l = 0, c = 0; > + BUFFER *buf = cur_buf; > + memset(*buf, 0, sizeof(*buf)); > + for (size_t i = 0; i < child->pos;/* i += len */) { > + wchar_t wc; > + int len = mbtowc(&wc, &child->buf[i], MB_CUR_MAX); > + if (len == -1) { > + wc = '?'; > + i += 1; > + } else { > + i += len; > + } > + if (wc == '\n') { > + l++; > + c = 0; > + if (l == MAXLINE) > + break; > + continue; > + } > + if (c == MAXCOLUMN) > + continue; > + (*buf)[l][c++] = wc; > + } > + display(buf, prev_buf, highlight_mode); > } > > kbd_result_t > @@ -534,12 +624,13 @@ kbd_command(int ch) > case 'p': > if (paused == 1) { > paused = 0; > - evtimer_del(&ev_timer); > - return (RSLT_REDRAW); > - } else { > - paused = 1; > evtimer_add(&ev_timer, &opt_interval.tv); > return (RSLT_UPDATE); > + } else { > + paused = 1; > + want_update = 0; > + evtimer_del(&ev_timer); > + return (RSLT_REDRAW); > } > > case 's': > @@ -687,11 +778,7 @@ on_signal(int sig, short event, void *ar > void > timer(int sig, short event, void *arg) > { > - swap_buffers(); > - read_result(cur_buf); > - display(cur_buf, prev_buf, highlight_mode); > - if (!paused) > - evtimer_add(&ev_timer, &opt_interval.tv); > + update(); > } > > void > @@ -704,11 +791,7 @@ input(int sig, short event, void *arg) > kbd_result_t result = kbd_command(ch); > switch (result) { > case RSLT_UPDATE: /* update buffer */ > - swap_buffers(); > - read_result(cur_buf); > - display(cur_buf, prev_buf, highlight_mode); > - if (!paused) > - evtimer_add(&ev_timer, &opt_interval.tv); > + update(); > break; > case RSLT_REDRAW: /* scroll with current buffer */ > display(cur_buf, prev_buf, highlight_mode); > -- In my defence, I have been left unsupervised.