[9front] [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window

[9front] [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window

[igor]

[-- Attachment #1: Type: text/plain, Size: 2102 bytes --]

The below patch is for review and testing.  It enables rio to open new
windows in a working directory `dirname` (i.e.  using `-cd dirname`)
where the dirname path contains spaces.  I ran into this issue while
browsing hierarchies imported via sshfs(4) in vdir⁽¹⁾, using vdir's
feature to open a window with the working directory set to the
selected path via right clicking on a path.

Please give this a try and/or provide feedback if you run into any
issues or disagree with the solution.  If there is agreement and no
one runs into problems over the next week or two I will merge this
patch.

¹…http://shithub.us/phil9/vdir/HEAD/info.html

<snip>
From: Igor Böhm <igor@9lab.org>
Date: Thu, 25 Nov 2021 01:57:04 +0000
Subject: [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window


The initial working directory of the new window may be set by a
`-cd directory` option. However, the `-cd directory` option is
not capable of handling paths with spaces.

To enable paths with spaces the function
/sys/src/cmd/rio/wctl.c:/^parsewctl is extended to handle quoted
directory paths, using a closing quote as the end of a path instead
of a space.

Before applying the patch the following will fail to open a new
window by writing to /dev/wctl:

<snip>
 % rio -i window
 % mkdir '/tmp/path with space'
 % echo new -cd '''/tmp/path with space''' window -x rc >> /dev/wctl
 % pwd
 /tmp/path with space
<snap>

After applying the patch the above sequence works as expected,
opening a window running rc and the working directory set to
'/tmp/path with space'.
---
diff f8dc73782d90c741e7886629252215c13e35af40 343aafa3a09dd808a1652670d8a7e20021cb0057
--- a/sys/src/cmd/rio/wctl.c	Mon Nov  8 00:31:11 2021
+++ b/sys/src/cmd/rio/wctl.c	Thu Nov 25 02:57:04 2021
@@ -252,8 +252,13 @@
 			s++;
 		if(param == Cd){
 			*cdp = s;
-			while(*s && !isspace(*s))
-				s++;
+			if(*s == '\''){	/* quoted directory */
+				*cdp = ++s;
+				while(*s && *s != '\'')
+					s++;
+			}else
+				while(*s && !isspace(*s))
+					s++;
 			if(*s != '\0')
 				*s++ = '\0';
 			continue;
</snap>

[-- Attachment #2: rio-working-directory-with-spaces.patch --]
[-- Type: text/plain, Size: 1437 bytes --]

From: Igor Böhm <igor@9lab.org>
Date: Thu, 25 Nov 2021 01:57:04 +0000
Subject: [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window


The initial working directory of the new window may be set by a
`-cd directory` option. However, the `-cd directory` option is
not capable of handling paths with spaces.

To enable paths with spaces the function
/sys/src/cmd/rio/wctl.c:/^parsewctl is extended to handle quoted
directory paths, using a closing quote as the end of a path instead
of a space.

Before applying the patch the following will fail to open a new
window by writing to /dev/wctl:

<snip>
 % rio -i window
 % mkdir '/tmp/path with space'
 % echo new -cd '''/tmp/path with space''' window -x rc >> /dev/wctl
 % pwd
 /tmp/path with space
<snap>

After applying the patch the above sequence works as expected,
opening a window running rc and the working directory set to
'/tmp/path with space'.
---
diff f8dc73782d90c741e7886629252215c13e35af40 343aafa3a09dd808a1652670d8a7e20021cb0057
--- a/sys/src/cmd/rio/wctl.c	Mon Nov  8 00:31:11 2021
+++ b/sys/src/cmd/rio/wctl.c	Thu Nov 25 02:57:04 2021
@@ -252,8 +252,13 @@
 			s++;
 		if(param == Cd){
 			*cdp = s;
-			while(*s && !isspace(*s))
-				s++;
+			if(*s == '\''){	/* quoted directory */
+				*cdp = ++s;
+				while(*s && *s != '\'')
+					s++;
+			}else
+				while(*s && !isspace(*s))
+					s++;
 			if(*s != '\0')
 				*s++ = '\0';
 			continue;

Re: [9front] [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window

[cinap_lenrek]

yeah, wctl is ugly.

probably is better to parse everything using gettokens().
as your patch doesnt handle quoting the quotes.

the whole window command also will probably have issues with
this kind of stuff... the quoting is definitely messed up in
there.

tho it is possible to quote from rc, like:

fn q {a=$1 whatis a | sed 's!^a=!!;q'}

or even get rid of wctl all together... it is a bit silly given
you can just fork and mount $wsys yourself... then there are not
two different code paths for creating a window.

--
cinap

Re: [9front] [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window

[igor]

Thanks for looking over it!

Quoth cinap_lenrek@felloff.net:
[…]
> probably is better to parse everything using gettokens().
> as your patch doesnt handle quoting the quotes.
[…]

Thanks for the tip. That indeed sounds like the cleaner solution.

> the whole window command also will probably have issues with
> this kind of stuff... the quoting is definitely messed up in
> there.

Yeah, that is where my journey started, thinking that window cmd
was the culprit, until I realised that the limitation goes all
the way to `/sys/src/cmd/rio/wctl.c:/^parsewctl`.

> 
> tho it is possible to quote from rc, like:
> 
> fn q {a=$1 whatis a | sed 's!^a=!!;q'}
> 
> or even get rid of wctl all together... it is a bit silly given
> you can just fork and mount $wsys yourself... then there are not
> two different code paths for creating a window.

I think I understand what you mean. The below is just checking I am
on the same page.

You are referring to the two highlighted ('^^^') code paths in
/rc/bin/window:

…
  if(~ $#mflag 1) {
  ^^^
  	…
  		if(mount $wsys /mnt/wsys 'new '$"spec){
  			bind -b /mnt/wsys /dev
  			exec $argv0 -x $cmd </dev/cons >/dev/cons >[2]/dev/cons
  		}
  	}&
  	exit ''
  }
  if not {
  ^^^  	if(~ $wctl ''){
  		if(test -f /dev/wctl) wctl=/dev/wctl
  		if not if(test -f /mnt/term/dev/wctl) wctl=/mnt/term/dev/wctl
  		if not if(test -r /mnt/term/env/wctl) wctl=/mnt/term^`{cat /mnt/term/env/wctl}
  		if not {
  			echo $argv0: '$wctl' not defined >[1=2]
  			exit bad
  		}
  	}
  
  	if(! ~ $#wdir 0)
  		spec=($spec -cd $wdir)
  	echo new $spec $argv0 -x $cmd >>$wctl
  }
…

…if I understand you correctly the suggestion is to drop the `if not  { … }`
part that writes to $wctl and modify the `if(~ $#mflag 1) { … }` in
such a way that it can handle both the `-m` and non `-m` variant.

Cheers,
Igor

Re: [9front] [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window

[igor]

[-- Attachment #1: Type: text/plain, Size: 974 bytes --]

Quoth cinap_lenrek@felloff.net:
[…]
> probably is better to parse everything using gettokens().
> as your patch doesnt handle quoting the quotes.
[…]

Done. Using gettokens(…) instead of the previous hack.

> the whole window command also will probably have issues with
> this kind of stuff... the quoting is definitely messed up in
> there.
> 
> tho it is possible to quote from rc, like:
> 
> fn q {a=$1 whatis a | sed 's!^a=!!;q'}
[…]

Done. Thanks for the tip with quoting from rc.

> or even get rid of wctl all together... it is a bit silly given
> you can just fork and mount $wsys yourself... then there are not
> two different code paths for creating a window.
[…]

The attached patch fixes the use case with spaces in paths for the
-cd option for wctl and window command.

Agree that two *different* code paths in /rc/bin/window for creating
a window is a bit silly. Will work on unifying that in the next
revision of the patch…

Thanks.

Cheers,
Igor

[-- Attachment #2: rio-working-directory-with-spaces-v2.patch --]
[-- Type: text/plain, Size: 2050 bytes --]

From: Igor Böhm <igor@9lab.org>
Date: Mon, 29 Nov 2021 00:06:45 +0000
Subject: [PATCH] rio: allow spaces in working directory path (-cd) when creating a new window


The initial working directory of the new window may be set by a
`-cd directory` option. However, the `-cd directory` option is
not capable of handling paths with spaces.

To enable paths with spaces the function
/sys/src/cmd/rio/wctl.c:/^parsewctl is extended to handle quoted
directory paths.

Before applying the patch the following will fail to open a new
window by writing to /dev/wctl:

<snip>
 % rio -i window
 % mkdir '/tmp/path with space'
 % echo new -cd '''/tmp/path with space''' window -x rc >> /dev/wctl
 % pwd
 /tmp/path with space
<snap>

The following invocation fails as well:

<snip>
 % window -cd '/tmp/path with space'
 % pwd
 /tmp/path with space
<snap>

After applying the patch the above sequences work as expected,
opening a window running rc and the working directory set to
'/tmp/path with space'.
---
diff 78c7ad88ffbfbd2b7a7269d863e5f4be7535b566 06623a45c7101085ce651a92ff8b952fe239eaac
--- a/rc/bin/window	Fri Nov 26 22:47:15 2021
+++ b/rc/bin/window	Mon Nov 29 01:06:45 2021
@@ -100,6 +100,6 @@
 	}
 
 	if(! ~ $#wdir 0)
-		spec=($spec -cd $wdir)
+		spec=($spec -cd `{a=$wdir whatis a|sed 's!^a=!!;q'})
 	echo new $spec $argv0 -x $cmd >>$wctl
 }
--- a/sys/src/cmd/rio/wctl.c	Fri Nov 26 22:47:15 2021
+++ b/sys/src/cmd/rio/wctl.c	Mon Nov 29 01:06:45 2021
@@ -203,7 +203,7 @@
 parsewctl(char **argp, Rectangle r, Rectangle *rp, int *pidp, int *idp, int *hiddenp, int *scrollingp, char **cdp, char *s, char *err)
 {
 	int cmd, param, xy, sign;
-	char *t;
+	char *f[2], *t;
 
 	*pidp = 0;
 	*hiddenp = 0;
@@ -252,10 +252,13 @@
 			s++;
 		if(param == Cd){
 			*cdp = s;
-			while(*s && !isspace(*s))
-				s++;
-			if(*s != '\0')
-				*s++ = '\0';
+			gettokens(*cdp, f, nelem(f), " \t\r\n\v\f");
+			s += strlen(*cdp);
+			if((*cdp)[0] == '\'' && s[-1] == '\''){
+				/* drop quotes */
+				*cdp += 1;
+				s[-1] = '\0';
+			}
 			continue;
 		}
 		sign = 0;