From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from yow.a-b.xyz ([45.32.152.219]) by ewsd; Wed May 13 08:19:17 EDT 2020
Received: 
	by yow.a-b.xyz (OpenSMTPD) with ESMTPSA id d88d53d7 (TLSv1.2:ECDHE-RSA-AES256-SHA:256:NO)
	for <9front@9front.org>;
	Wed, 13 May 2020 14:19:12 +0200 (CEST)
Message-ID: <5908F322508015DD8DD7AF2C02532108@a-b.xyz>
To: 9front@9front.org
Subject: [PATCH] rio: add -label wctl param; improve window(1)
Date: Wed, 13 May 2020 12:20:36 +0200
From: kvik@a-b.xyz
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
List-ID: <9front.9front.org>
List-Help: <http://lists.9front.org>
X-Glyph: ➈
X-Bullshit: virtualized AJAX metadata-oriented wrapper blockchain map/reduce backend 

This set of changes started out with wanting to fix some unrelated
annoyances with the window(1) command:
* window -m and window (no -m) act differently in an unexpected way,
  the latter accepting a compound command to be passed on through rio's
  wctl to rc -c, whereas the former runs a command directly with
  exec(1).
* window -m always exits with a non-empty status due to a normally-false
  if-match whose status return spills off the end of the script.
  As the call to window(1) is often the last thing a script may do this
  leak has a tendency to propagate and cause hair pulling down the line.

While fixing the above issues it became obvious window(1) is needlesly
complicated, going as far as calling itself recursively and special
casing this call to accomplish a single goal of setting the new window's
label, something that it could only do from inside its namespace.

To remedy such silliness:
* rio(4) is taught to accept a -label option in its 'new' wctl command.
* window(1) is simplified using this new facility and has the same
  option added to its arguments.

I'm running with these changes and things seem to work fine, but I'd
appreciate more eyes on this and reports of additional testing before
commiting.

Be careful applying this patch, though.  The changes to window(1) may
confuse unpatched rio which fails to recognize '-label' as a command
parameter and tries to execute it as a command.

As these three changes are somewhat unrelated would we prefer to have
them separately commited?

diff -r	4be989798860 rc/bin/window
--- a/rc/bin/window	Wed May	13 00:17:07 2020 +0200
+++ b/rc/bin/window	Wed May	13 10:47:42 2020 +0200
@@ -5,9	+5,9 @@
 cmd=()
 spec=()
 wdir=()
+label=()
 wpid=()
 mflag=()
-xflag=()
 argv0=$0

 if(~ $1 *[0-9][' ,'][0-9]*){
@@ -17,46 +17,47 @@
	mflag=1
 }
 if not	{
-	while(~	$1 -* && ~ $#xflag 0)
+	while(~	$1 -* && ! ~ $1	--){
		switch($1){
		case -hide -scroll -noscroll
			spec=($spec $1)
-			shift
		case -dx -dy -minx -miny -maxx -maxy
			spec=($spec $1 $2)
-			shift 2
+			shift
		case -r
			spec=($spec $1 $2 $3 $4	$5)
-			shift 5
+			shift 4
		case -cd
			wdir=$2
-			shift 2
+			shift
+		case -label
+			label=$2
+			shift
		case -pid
			wpid=$2
-			shift 2
+			shift
		case -m
			mflag=1
-			shift
-		case -x
-			xflag=1
-			shift
		case *
			echo usage: $argv0 '[ -m ] [ -r	minx miny maxx maxy ]' \
			'[ -dx n ] [ -dy n ] [ -minx n ] [ -miny n ] [ -maxx n ] [ -maxy n ]' \
-			'[ -cd dir ] [ -hide ] [ -scroll ] [ -noscroll ] [ cmd arg ... ]' >[1=2]
+			'[ -cd dir ] [ -label name ] [ -hide ] [ -scroll ] [ -noscroll ] [ cmd arg ... ]' >[1=2]
			exit usage
		}
+		shift
+	}
+	if(~ $1	--) shift
 }

 if(~ $#* 0) cmd=rc
 if not	cmd=$*

-if(~ $#xflag 1){
-	echo -n	`{basename $cmd(1)} >/dev/label	>[2]/dev/null
-	rm -f /env/^(cmd spec wdir wpid	mflag xflag argv0)
-	exec $cmd
-	exit exec
+if(~ $#label 0){
+	label=`{echo $cmd(1)} #	command	might've been quoted
+	label=`{basename $label(1)}
 }
+if(~ $#label 1)
+	spec=($spec -label $label)

 if(~ $#mflag 1) {
	if(~ $wsys ''){
@@ -65,7 +66,7 @@
	}

	{
-		rfork n
+		rfork ne

		if(~ $wsys /srv/*){
			if(~ $#wpid 0)
@@ -83,7 +84,8 @@
		{unmount /mnt/acme /dev; unmount $wsys /dev} >[2]/dev/null
		if(mount $wsys /mnt/wsys 'new '$"spec){
			bind -b	/mnt/wsys /dev
-			exec $argv0 -x $cmd </dev/cons >/dev/cons >[2]/dev/cons
+			rm -f /env/^(cmd spec wdir label wpid mflag argv0)
+			exec rc	-c $"cmd </dev/cons >/dev/cons >[2]/dev/cons
		}
	}&
 }
@@ -100,5 +102,6 @@

	if(! ~ $#wdir 0)
		spec=($spec -cd	$wdir)
-	echo new $spec $argv0 -x $cmd >>$wctl
+	echo new $spec $cmd >>$wctl
 }
+exit ''
diff -r	4be989798860 sys/man/1/rio
--- a/sys/man/1/rio	Wed May	13 00:17:07 2020 +0200
+++ b/sys/man/1/rio	Wed May	13 10:47:42 2020 +0200
@@ -51,6 +51,9 @@
 .B -cd
 .I dir
 ] [
+.B -label
+.I name
+] [
 .B -hide
 ] [
 .B -scroll
@@ -145,6 +148,9 @@
 The
 .B cd
 option	sets the working directory.
+The
+.B label
+option	sets the window	label.
 The optional command and arguments
 define	which program to run in	the window.
 .PP
diff -r	4be989798860 sys/man/4/rio
--- a/sys/man/4/rio	Wed May	13 00:17:07 2020 +0200
+++ b/sys/man/4/rio	Wed May	13 10:47:42 2020 +0200
@@ -326,6 +326,10 @@
 .B -cd
 .I directory
 option.
+The window label may be set by	a
+.B -label
+.I name
+option.
 The
 .B -hide
 option	causes the window to be	created	off-screen, in the hidden state, while
diff -r	4be989798860 sys/src/cmd/rio/dat.h
--- a/sys/src/cmd/rio/dat.h	Wed May	13 00:17:07 2020 +0200
+++ b/sys/src/cmd/rio/dat.h	Wed May	13 10:47:42 2020 +0200
@@ -214,7 +214,8 @@
 void		wsetcursor(Window*, int);
 void		wsetname(Window*);
 void		wsetorigin(Window*, uint, int);
-void		wsetpid(Window*, int, int);
+void		wsetpid(Window*, int);
+void		wsetlabel(Window*, char*, ...);
 void		wsetselect(Window*, uint, uint);
 void		wshow(Window*, uint);
 void		wsnarf(Window*);
diff -r	4be989798860 sys/src/cmd/rio/fns.h
--- a/sys/src/cmd/rio/fns.h	Wed May	13 00:17:07 2020 +0200
+++ b/sys/src/cmd/rio/fns.h	Wed May	13 10:47:42 2020 +0200
@@ -1,9	+1,9 @@
 int	whide(Window*);
 int	wunhide(Window*);
 void	freescrtemps(void);
-int	parsewctl(char**, Rectangle, Rectangle*, int*, int*, int*, int*, char**, char*,	char*);
+int	parsewctl(char**, Rectangle, Rectangle*, int*, int*, int*, int*, char**, char**, char*,	char*);
 int	writewctl(Xfid*, char*);
-Window	*new(Image*, int, int, int, char*, char*, char**);
+Window	*new(Image*, int, int, int, char*, char*, char*, char**);
 void	riosetcursor(Cursor*);
 int	min(int, int);
 int	max(int, int);
diff -r	4be989798860 sys/src/cmd/rio/rio.c
--- a/sys/src/cmd/rio/rio.c	Wed May	13 00:17:07 2020 +0200
+++ b/sys/src/cmd/rio/rio.c	Wed May	13 10:47:42 2020 +0200
@@ -222,7 +222,7 @@
			r = screen->r;
			r.min.y	= r.max.y-Dy(r)/3;
			i = allocwindow(wscreen, r, Refbackup, DNofill);
-			wkeyboard = new(i, FALSE, scrolling, 0,	nil, "/bin/rc",	kbdargv);
+			wkeyboard = new(i, FALSE, scrolling, 0,	nil, "keyboard", "/bin/rc", kbdargv);
			if(wkeyboard ==	nil)
				error("can't create keyboard window");
		}
@@ -715,7 +715,7 @@
	case -1:
		break;
	case New:
-		new(sweep(), FALSE, scrolling, 0, nil, "/bin/rc", nil);
+		new(sweep(), FALSE, scrolling, 0, nil, nil, "/bin/rc", nil);
		break;
	case Reshape:
		resize();
@@ -1126,7 +1126,7 @@
 }

 Window*
-new(Image *i, int hideit, int scrollit, int pid, char *dir, char *cmd,	char **argv)
+new(Image *i, int hideit, int scrollit, int pid, char *dir, char *label, char *cmd, char **argv)
 {
	Window *w;
	Mousectl *mc;
@@ -1180,7 +1180,11 @@
		chanfree(cpid);
		return nil;
	}
-	wsetpid(w, pid,	1);
+	wsetpid(w, pid);
+	if(label)
+		wsetlabel(w, label);
+	else
+		wsetlabel(w, "rc %d", pid);
	wsetname(w);
	if(dir){
		free(w->dir);
diff -r	4be989798860 sys/src/cmd/rio/wctl.c
--- a/sys/src/cmd/rio/wctl.c	Wed May	13 00:17:07 2020 +0200
+++ b/sys/src/cmd/rio/wctl.c	Wed May	13 10:47:42 2020 +0200
@@ -51,6 +51,7 @@
 enum
 {
	Cd,
+	Label,
	Deltax,
	Deltay,
	Hidden,
@@ -67,6 +68,7 @@

 static	char *params[] = {
	[Cd]				= "-cd",
+	[Label]			= "-label",
	[Deltax]			= "-dx",
	[Deltay]			= "-dy",
	[Hidden]			= "-hide",
@@ -195,7 +197,7 @@


 int
-parsewctl(char	**argp,	Rectangle r, Rectangle *rp, int	*pidp, int *idp, int *hiddenp, int *scrollingp,	char **cdp, char *s, char *err)
+parsewctl(char	**argp,	Rectangle r, Rectangle *rp, int	*pidp, int *idp, int *hiddenp, int *scrollingp,	char **cdp, char **labelp, char	*s, char *err)
 {
	int cmd, param,	xy, sign;
	char *t;
@@ -204,6 +206,7 @@
	*hiddenp = 0;
	*scrollingp = scrolling;
	*cdp = nil;
+	*labelp	= nil;
	cmd = word(&s, cmds);
	if(cmd < 0){
		strcpy(err, "unrecognized wctl command");
@@ -252,6 +255,13 @@
			if(*s != '\0')
				*s++ = '\0';
			continue;
+		}else if(param == Label){
+			*labelp	= s;
+			while(*s && !isspace(*s))
+				s++;
+			if(*s != '\0')
+				*s++ = '\0';
+			continue;
		}
		sign = 0;
		if(*s == '-'){
@@ -313,7 +323,7 @@
 }

 int
-wctlnew(Rectangle rect, char *arg, int	pid, int hideit, int scrollit, char *dir, char *err)
+wctlnew(Rectangle rect, char *arg, int	pid, int hideit, int scrollit, char *dir, char *label, char *err)
 {
	char **argv;
	Image *i;
@@ -343,7 +353,7 @@
		return -1;
	}

-	new(i, hideit, scrollit, pid, dir, "/bin/rc", argv);
+	new(i, hideit, scrollit, pid, dir, label, "/bin/rc", argv);

	free(argv);	/* when	new() returns, argv and	args have been copied */
	return 1;
@@ -444,7 +454,7 @@
 writewctl(Xfid	*x, char *err)
 {
	int cnt, cmd, id, hideit, scrollit, pid;
-	char *arg, *dir;
+	char *arg, *dir, *label;
	Rectangle r;
	Window *w;

@@ -454,7 +464,7 @@
	id = 0;

	r = rectsubpt(w->screenr, screen->r.min);
-	cmd = parsewctl(&arg, r, &r, &pid, &id,	&hideit, &scrollit, &dir, x->data, err);
+	cmd = parsewctl(&arg, r, &r, &pid, &id,	&hideit, &scrollit, &dir, &label, x->data, err);
	if(cmd < 0)
		return -1;

@@ -468,10 +478,10 @@

	switch(cmd){
	case New:
-		return wctlnew(r, arg, pid, hideit, scrollit, dir, err);
+		return wctlnew(r, arg, pid, hideit, scrollit, dir, label, err);
	case Set:
		if(pid > 0)
-			wsetpid(w, pid,	0);
+			wsetpid(w, pid);
		return 1;
	}

@@ -485,7 +495,7 @@
 void
 wctlthread(void *v)
 {
-	char *buf, *arg, *dir;
+	char *buf, *arg, *dir, *label;
	int cmd, id, pid, hideit, scrollit;
	Rectangle rect;
	char err[ERRMAX];
@@ -497,11 +507,11 @@

	for(;;){
		buf = recvp(c);
-		cmd = parsewctl(&arg, ZR, &rect, &pid, &id, &hideit, &scrollit,	&dir, buf, err);
+		cmd = parsewctl(&arg, ZR, &rect, &pid, &id, &hideit, &scrollit,	&dir, &label, buf, err);

		switch(cmd){
		case New:
-			wctlnew(rect, arg, pid,	hideit,	scrollit, dir, err);
+			wctlnew(rect, arg, pid,	hideit,	scrollit, dir, label, err);
		}
		free(buf);
	}
diff -r	4be989798860 sys/src/cmd/rio/wind.c
--- a/sys/src/cmd/rio/wind.c	Wed May	13 00:17:07 2020 +0200
+++ b/sys/src/cmd/rio/wind.c	Wed May	13 10:47:42 2020 +0200
@@ -1414,7 +1414,7 @@
 }

 void
-wsetpid(Window	*w, int	pid, int dolabel)
+wsetpid(Window	*w, int	pid)
 {
	char buf[64];
	int ofd;
@@ -1423,11 +1423,6 @@
	if(pid <= 0)
		w->notefd = -1;
	else {
-		if(dolabel){
-			snprint(buf, sizeof(buf), "rc %d", pid);
-			free(w->label);
-			w->label = estrdup(buf);
-		}
		snprint(buf, sizeof(buf), "/proc/%d/notepg", pid);
		w->notefd = open(buf, OWRITE|OCEXEC);
	}
@@ -1436,6 +1431,21 @@
 }

 void
+wsetlabel(Window *w, char *fmt, ...)
+{
+	va_list	va;
+	char buf[128];
+
+	if(w ==	nil)
+		return;
+	va_start(va, fmt);
+	vsnprint(buf, sizeof(buf), fmt,	va);
+	va_end(va);
+	free(w->label);
+	w->label = estrdup(buf);
+}
+
+void
 winshell(void *args)
 {
	Window *w;
diff -r	4be989798860 sys/src/cmd/rio/xfid.c
--- a/sys/src/cmd/rio/xfid.c	Wed May	13 00:17:07 2020 +0200
+++ b/sys/src/cmd/rio/xfid.c	Wed May	13 10:47:42 2020 +0200
@@ -170,7 +170,7 @@
	Fcall t;
	int id,	hideit,	scrollit;
	Window *w;
-	char *err, *n, *dir, errbuf[ERRMAX];
+	char *err, *n, *dir, *label, errbuf[ERRMAX];
	int pid, newlymade;
	Rectangle r;
	Image *i;
@@ -180,6 +180,7 @@
	w = nil;
	err = Eunkid;
	dir = nil;
+	label =	nil;
	newlymade = FALSE;
	hideit = 0;
	scrollit = scrolling;
@@ -210,14 +211,14 @@
			if(i){
				if(pid == 0)
					pid = -1;	/* make	sure we	don't pop a shell! - UGH */
-				w = new(i, hideit, scrollit, pid, dir, nil, nil);
+				w = new(i, hideit, scrollit, pid, dir, label, nil, nil);
				newlymade = TRUE;
			}else
				err = Ewindow;
		}
	}else if(strncmp(x->aname, "new", 3) ==	0){	/* new -dx -dy - new syntax, as	in wctl	*/
		pid = 0;
-		if(parsewctl(nil, ZR, &r, &pid,	nil, &hideit, &scrollit, &dir, x->aname, errbuf) < 0)
+		if(parsewctl(nil, ZR, &r, &pid,	nil, &hideit, &scrollit, &dir, &label, x->aname, errbuf) < 0)
			err = errbuf;
		else
			goto Allocate;