[PATCH] rio: add -label wctl param; improve window(1)

[PATCH] rio: add -label wctl param; improve window(1)

[kvik]

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;

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[kvik]

> Good in principle, I think, but just to note: if window(1) no longer calls
> itself, users will probably have to change quoting in scripts which call
> window.

I may be missing something but I don't see how this would be the case.

The arguments constituting a command are evaluated once by the calling
shell and are passed into window(1), ending up in the $cmd list.  Before
or after this patch this list stays the same as it is passed along;
specifically it does not get reevaluated until the final rc -c in either
rio itself or in (new) window(1).  Consider the example:

	fn nargs {
		echo $#* : $*
	}
	fn once {
		nargs $*
		twice $*
	}
	fn twice {
		nargs $*
		echo 'nargs '$"*
		rc -c 'nargs '$"*
	}

	; once 1 '2 3' '''4 5'''
	3 : 1 2 3 '4 5'
	3 : 1 2 3 '4 5'
	nargs 1 2 3 '4 5'
	4 : 1 2 3 4 5

You see that the initial argument list stays the same as it is being
passed along, that is until another round of evaluation inside the
rc -c subshell where the next layer of quotes is stripped.

In the argument parsing sense removing the recursive call to window -x
is the same as skipping the call to 'once' in the above example.

> window -r 268 0 868 400 -scroll rc -c \
> 	'''label -f kprint; echo kprint; tail -f /tmp/kprint'''
> window -r 0 0 128 128 colorclock_l
> window -r 0 124 128 252 rc -c \
> 	'''font=/lib/font/bit/dejavusans/unicode.12.font; stats -Ime'''

These commands run as expected with the patch.

> Now I think of it, I don't really agree with window(1) setting the label
> at all, but it's a minor point.

I'm not a fan of automatic labeling either.  Ideally the programs would
set the label themselves if the user didn't request a name, but I don't
think this is easily accomplished.

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[Ethan Gardener]

Good in principle, I think, but just to note: if window(1) no longer calls itself, users will probably have to change quoting in scripts which call window. Examples from my riostart:

window -r 268 0 868 400 -scroll rc -c \
	'''label -f kprint; echo kprint; tail -f /tmp/kprint'''
window -r 0 0 128 128 colorclock_l
window -r 0 124 128 252 rc -c \
	'''font=/lib/font/bit/dejavusans/unicode.12.font; stats -Ime'''

Granted, reducing the necessary quotes would be a benefit for new users. I'm not even sure how I figured out 2 levels of quoting were necessary.

Now I think of it, I don't really agree with window(1) setting the label at all, but it's a minor point.

(label -f is my own extension.)

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[kvik]

> It looks like it's one of those things which ends up a little untidy
> whichever option is chosen.

It's a choice triangle where the point on simplicity is disallowed.

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[Ethan Gardener]

On Wed, May 13, 2020, at 2:14 PM, kvik@a-b.xyz wrote:
> 
> You see that the initial argument list stays the same as it is being
> passed along, that is until another round of evaluation inside the
> rc -c subshell where the next layer of quotes is stripped.
> 
> In the argument parsing sense removing the recursive call to window -x
> is the same as skipping the call to 'once' in the above example.
> 
> > window -r 268 0 868 400 -scroll rc -c \
> > 	'''label -f kprint; echo kprint; tail -f /tmp/kprint'''
> > window -r 0 0 128 128 colorclock_l
> > window -r 0 124 128 252 rc -c \
> > 	'''font=/lib/font/bit/dejavusans/unicode.12.font; stats -Ime'''
> 
> These commands run as expected with the patch.

Ah, you're quite right. Thanks for the examples and testing.

> 
> > Now I think of it, I don't really agree with window(1) setting the label
> > at all, but it's a minor point.
> 
> I'm not a fan of automatic labeling either.  Ideally the programs would
> set the label themselves if the user didn't request a name, but I don't
> think this is easily accomplished.

It looks like it's one of those things which ends up a little untidy whichever option is chosen.

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[kvik]

> that is horrible. this definitely needs to be fixed. the caller
> should never have to know implementation details like that.

Yes, this is fixed with the patch for both window -m and no-m.

rc -c is never needed anymore, and only a single level of surrounding
quotes was ever needed to guard against caller shell evaluation in case
a compound command was passed.

This works as one'd hope:

	window echo good night, linuks
	window 'echo good morning, plan 9 | tr a-z A-Z; rc'

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[cinap_lenrek]

that is horrible. this definitely needs to be fixed. the caller
should never have to know implementation details like that.

> window -r 268 0 868 400 -scroll rc -c \
>	'''label -f kprint; echo kprint; tail -f /tmp/kprint'''
> window -r 0 0 128 128 colorclock_l
> window -r 0 124 128 252 rc -c \
>	'''font=/lib/font/bit/dejavusans/unicode.12.font; stats -Ime'''

--
cinap

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[cinap_lenrek]

you want:

#pragma varargck argpos wsetlabel 2

and change:

+	if(label)
+		wsetlabel(w, label);

to:

+	if(label)
+		wsetlabel(w, "%s", label);

otherwise you got a classical format string attack.

about the -label flag in wctl message. i'm not so sure it is a good
idea. adding new flags there means a breaking change and updating rio
needs to be synchronized.

also we have the problem with quoting. (-cd also suffers from this).

i think running window recursively is not really the issue. the real
issue is that we have to properly preserve the arguments. all text
that comes after the last parsed word in wctl "new" word is passed
as a single argument to rc -c

however, when window receives multiple cmd arguments, we need to quote
the argument list proper as we'r essentially generating a rc script
that reproduces the argument list. the bug is when we do $"cmd
which destroys list tokenization.

a solution can be accomplished with rc's whatis, which lets rc
serialize a variable as a string that we can tunnel thru the wctl
text interface.

something like the diff at the bottom (this is just
for illustration, you can still simplify this further)?

note that we still handle one argument as a rc script for backwards
compatibility. so you can still do:

window 'echo hello; echo world; sleep 1'

but if you pass multile arguments, it will preserve the argument
list and not fail on strings that have spaces in them.

diff -r 4be989798860 rc/bin/window
--- a/rc/bin/window	Wed May 13 00:17:07 2020 +0200
+++ b/rc/bin/window	Sat May 16 15:52:47 2020 +0200
@@ -48,8 +48,11 @@
 		}
 }
 
-if(~ $#* 0) cmd=rc
-if not cmd=$*
+switch($#*){
+case 0; cmd=(rc -i)
+case 1; cmd=(rc -c $1)
+case *; cmd=$*
+}
 
 if(~ $#xflag 1){
 	echo -n `{basename $cmd(1)} >/dev/label >[2]/dev/null
@@ -100,5 +103,5 @@
 
 	if(! ~ $#wdir 0)
 		spec=($spec -cd $wdir)
-	echo new $spec $argv0 -x $cmd >>$wctl
+	echo new $spec `{whatis cmd} $argv0 -x '$cmd' >>$wctl
 }

--
cinap

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[kvik]

I appreciate you taking the time to review.

> otherwise you got a classical format string attack.

Ouch!

> about the -label flag in wctl message. i'm not so sure it is a good
> idea. adding new flags there means a breaking change and updating rio
> needs to be synchronized.

> i think running window recursively is not really the issue.

It's not an issue per se, but it seems we are only doing it the
roundabout way because of wanting to label the new window while missing
the -label parameter to do so.  We also do the environment cleanup, but
this only affects the -m case so it can be moved there.

I'd argue that the label is a window property just like the others we
are already setting through the 'new' wctl.  I see no reason why it
should be handled specially from the rest, or alternatively why other
parameters shouldn't be set in a similar fashion by writing to /dev/wctl
in new window's namespace.

Introducing breakage is quite unfortunate, though I'm not entirely
convinced it should be a single show stopper.

> also we have the problem with quoting. (-cd also suffers from this).

Yes, the field parser is janky and could be helped with a rewrite in
terms of getfields(2).  I can do it independently of this other stuff.

> the real issue is that we have to properly preserve the arguments. 

> the bug is when we do $"cmd which destroys list tokenization.

> a solution can be accomplished with rc's whatis, which lets rc
> serialize a variable as a string that we can tunnel thru the wctl
> text interface.

Oh, wow, whatis is gold and I somehow missed the list serialization
feature completely!  I'll make use of it in a followup patch.

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[cinap_lenrek]

>	window echo good night, linuks
>	window 'echo good morning, plan 9 | tr a-z A-Z; rc'

what about:

window rc -c 'echo $#* $*; sleep 10' 4 '5 6' 7

that neeeds to print: 3 4 5 6 7

just passing rc code to window is a shitty interface. you
need proper argument passing as well. basically, it has to
work like cpu/rcpu and ssh where you can just prefix any
valid command with window and it will run that command
inside the window and thats it and pass the arguments
without any mangeling. $"cmd is just wrong.

the manpage also says [cmd args] and not [rccode]

--
cinap

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[kvik]

> maybe, but parsewctlmsg() in this form is not up to the task. you can't
> have labels with spaces in them for example.

Hm, if we do it in steps maybe that's an opportunity to avoid the
breakage as well!  Something like:
* rewrite the wctl parser to properly handle argument lists,
	* make it *ignore* the -label NAME parameter,
* wait for a while and shame people into sysupdate,
* land the label-related window(1) changes.

This way when the new window(1) is eventually pulled it will break
label-setting until a new rio is compiled and started, but everything
will otherwise continue working, which is much less of a breakage than
refusing to spawn windows entirely.

Re: [9front] [PATCH] rio: add -label wctl param; improve window(1)

[cinap_lenrek]

> It's not an issue per se, but it seems we are only doing it the
> roundabout way because of wanting to label the new window while missing
> the -label parameter to do so.  We also do the environment cleanup, but
> this only affects the -m case so it can be moved there.

yeah. sorry. i agree it can be simplified.

> I'd argue that the label is a window property just like the others we
> are already setting through the 'new' wctl.  I see no reason why it
> should be handled specially from the rest, or alternatively why other
> parameters shouldn't be set in a similar fashion by writing to /dev/wctl
> in new window's namespace.

maybe, but parsewctlmsg() in this form is not up to the task. you can't
have labels with spaces in them for example.

i said -cd has the same issue. if you have a directory that has a space
in it you'r fucked.

> Yes, the field parser is janky and could be helped with a rewrite in
> terms of getfields(2).  I can do it independently of this other stuff.

ok.

--
cinap