Index | Thread | Search

From:
Florian Obser <florian@openbsd.org>
Subject:
Re: responsive watch wip
To:
"Ted Unangst" <tedu@tedunangst.com>
Cc:
"Job Snijders" <job@sobornost.net>, tech@openbsd.org
Date:
Sun, 15 Jun 2025 11:01:26 +0200

Download raw body.

Thread
On 2025-06-14 13:50 -04, "Ted Unangst" <tedu@tedunangst.com> 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.