[9front] PATCH walk(1) to (optionally) quote name and path

[9front] PATCH walk(1) to (optionally) quote name and path

[Romano]

I'm working with files on a backup drive that have spaces and
quotation marks in their names, using walk(1).  I can pipe to sed
s/''''/''''''''''/g and do similarly for other characters requiring
escaping for rc(1), but figured patching walk(1) would be better.  I
don't like how I just copied case 'p': the quoting "works", but isn't
as clean as the output for ls(1).  For example:

Desktop/Movies/'A Bug''s Life.mp4'
'Desktop/Movies/TV/TV Library.tvlibrary'/'Library Preferences.tvdb'
Desktop/Movies/'The American President.mp4'

instead of:

'Desktop/Movies/A Bug''s Life.mp4'
'Desktop/Movies/TV/TV Library.tvlibrary/Library Preferences.tvdb'
'Desktop/Movies/The American President.mp4'

But it's better than what happens now, where there's no quoting.

From: Romano <unobe@cpan.org>
Date: Sat, 12 Aug 2023 03:18:22 +0000
Subject: [PATCH] walk: quote name and paths using N and P

---
diff deb39a43ae5ed09c7971726cedeb06a9f65ccc6d 2da52094832662c533f60629813dbd5ac805e3ad
--- a/sys/man/1/walk
+++ b/sys/man/1/walk
@@ -81,8 +81,14 @@
 .B n
 final path element (name)
 .TP
+.B N
+final path element (name), rc (1) quoted when necessary
+.TP
 .B p
 path
+.TP
+.B P
+path, rc (1) quoted when necessary
 .TP
 .B q
 qid path.version.type (see
--- a/sys/src/cmd/walk.c
+++ b/sys/src/cmd/walk.c
@@ -43,7 +43,7 @@
 void
 dofile(char *path, Dir *f, int pathonly)
 {
-	char *p;
+	char *p, *quoted;
 
 	if(
 		(f == dotdir)
@@ -60,6 +60,10 @@
 		case 'a': Bprint(bout, "%uld", f->atime); break;
 		case 'm': Bprint(bout, "%uld", f->mtime); break;
 		case 'n': Bwrite(bout, f->name, strlen(f->name)); break;
+		case 'N':
+			quoted = quotestrdup(f->name);
+			Bwrite(bout, quoted, strlen(quoted));
+			break;
 		case 'p':
 			if(path != dotpath)
 				Bwrite(bout, path, strlen(path));
@@ -69,6 +73,18 @@
 				Bwrite(bout, f->name, strlen(f->name));
 			}
 			break;
+		case 'P':
+			if(path != dotpath) {
+				quoted = quotestrdup(path);
+				Bwrite(bout, quoted, strlen(quoted));
+			}
+			if(! (f->qid.type & QTDIR) && !pathonly){
+				if(path != dotpath)
+					Bputc(bout, '/');
+				quoted = quotestrdup(f->name);
+				Bwrite(bout, quoted, strlen(quoted));
+			}
+			break;
 		case 'q': Bprint(bout, "%ullx.%uld.%.2uhhx", f->qid.path, f->qid.vers, f->qid.type); break;
 		case 's': Bprint(bout, "%lld", f->length); break;
 		case 'x': Bprint(bout, "%M", f->mode); break;
@@ -243,7 +259,7 @@
 		if((stfmt = s_reset(stfmt)) == nil)
 			sysfatal("s_reset: %r");
 		s_append(stfmt, EARGF(usage()));
-		i = strspn(s_to_c(stfmt), "UGMamnpqsxDT");
+		i = strspn(s_to_c(stfmt), "UGMamnNpPqsxDT");
 		if(i != s_len(stfmt))
 			sysfatal("bad stfmt: %s", s_to_c(stfmt));
 		break;

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[ori]

Quoth Romano <unobe@cpan.org>:
> I'm working with files on a backup drive that have spaces and
> quotation marks in their names, using walk(1).  I can pipe to sed
> s/''''/''''''''''/g and do similarly for other characters requiring
> escaping for rc(1), but figured patching walk(1) would be better.

generally no objections, though it's usually relatively easy to
structure scripts such that there's relatively little that is
interpreted by rc; is this for interactive use?

> I don't like how I just copied case 'p': the quoting "works", but isn't
> as clean as the output for ls(1).

I'll need to look at the code to see if there's a way to
improve this; regardless, a few comments inline:

> But it's better than what happens now, where there's no quoting.
> 
> From: Romano <unobe@cpan.org>
> Date: Sat, 12 Aug 2023 03:18:22 +0000
> Subject: [PATCH] walk: quote name and paths using N and P
> 
> ---
> diff deb39a43ae5ed09c7971726cedeb06a9f65ccc6d 2da52094832662c533f60629813dbd5ac805e3ad
> --- a/sys/man/1/walk
> +++ b/sys/man/1/walk
> @@ -81,8 +81,14 @@
>  .B n
>  final path element (name)
>  .TP
> +.B N
> +final path element (name), rc (1) quoted when necessary
> +.TP
>  .B p
>  path
> +.TP
> +.B P
> +path, rc (1) quoted when necessary
>  .TP
>  .B q
>  qid path.version.type (see
> --- a/sys/src/cmd/walk.c
> +++ b/sys/src/cmd/walk.c
> @@ -43,7 +43,7 @@
>  void
>  dofile(char *path, Dir *f, int pathonly)
>  {
> -	char *p;
> +	char *p, *quoted;

nitpicking on style; perhaps '*q' instead of '*quoted'

>  
>  	if(
>  		(f == dotdir)
> @@ -60,6 +60,10 @@
>  		case 'a': Bprint(bout, "%uld", f->atime); break;
>  		case 'm': Bprint(bout, "%uld", f->mtime); break;
>  		case 'n': Bwrite(bout, f->name, strlen(f->name)); break;
> +		case 'N':
> +			quoted = quotestrdup(f->name);
> +			Bwrite(bout, quoted, strlen(quoted));

looks like this leaks 'quoted'; same with all other quotestrdup calls.

> +			break;
>  		case 'p':
>  			if(path != dotpath)
>  				Bwrite(bout, path, strlen(path));
> @@ -69,6 +73,18 @@
>  				Bwrite(bout, f->name, strlen(f->name));
>  			}
>  			break;
> +		case 'P':

this quoting could be done by concating the path and the
dotpath, possibly even with libstring. Not sure if it'd
be cleaner to dedup the 'p' and 'P' cases or not yet.

> +			if(path != dotpath) {
> +				quoted = quotestrdup(path);
> +				Bwrite(bout, quoted, strlen(quoted));
> +			}
> +			if(! (f->qid.type & QTDIR) && !pathonly){
> +				if(path != dotpath)
> +					Bputc(bout, '/');
> +				quoted = quotestrdup(f->name);
> +				Bwrite(bout, quoted, strlen(quoted));
> +			}
> +			break;
>  		case 'q': Bprint(bout, "%ullx.%uld.%.2uhhx", f->qid.path, f->qid.vers, f->qid.type); break;
>  		case 's': Bprint(bout, "%lld", f->length); break;
>  		case 'x': Bprint(bout, "%M", f->mode); break;
> @@ -243,7 +259,7 @@
>  		if((stfmt = s_reset(stfmt)) == nil)
>  			sysfatal("s_reset: %r");
>  		s_append(stfmt, EARGF(usage()));
> -		i = strspn(s_to_c(stfmt), "UGMamnpqsxDT");
> +		i = strspn(s_to_c(stfmt), "UGMamnNpPqsxDT");
>  		if(i != s_len(stfmt))
>  			sysfatal("bad stfmt: %s", s_to_c(stfmt));
>  		break;
> 

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[unobe]

Thanks for the feedback, Ori.  I'm not a C programmer, and it shows.
See my replies below.

Quoth ori@eigenstate.org:
> Quoth Romano <unobe@cpan.org>:
> > I'm working with files on a backup drive that have spaces and
> > quotation marks in their names, using walk(1).  I can pipe to sed
> > s/''''/''''''''''/g and do similarly for other characters requiring
> > escaping for rc(1), but figured patching walk(1) would be better.
> 
> generally no objections, though it's usually relatively easy to
> structure scripts such that there's relatively little that is
> interpreted by rc; is this for interactive use?
> 
> > I don't like how I just copied case 'p': the quoting "works", but isn't
> > as clean as the output for ls(1).
> 
> I'll need to look at the code to see if there's a way to
> improve this; regardless, a few comments inline:
> 
> > But it's better than what happens now, where there's no quoting.
> > 
> > From: Romano <unobe@cpan.org>
> > Date: Sat, 12 Aug 2023 03:18:22 +0000
> > Subject: [PATCH] walk: quote name and paths using N and P
> > 
> > ---
> > diff deb39a43ae5ed09c7971726cedeb06a9f65ccc6d 2da52094832662c533f60629813dbd5ac805e3ad
> > --- a/sys/man/1/walk
> > +++ b/sys/man/1/walk
> > @@ -81,8 +81,14 @@
> >  .B n
> >  final path element (name)
> >  .TP
> > +.B N
> > +final path element (name), rc (1) quoted when necessary
> > +.TP
> >  .B p
> >  path
> > +.TP
> > +.B P
> > +path, rc (1) quoted when necessary
> >  .TP
> >  .B q
> >  qid path.version.type (see
> > --- a/sys/src/cmd/walk.c
> > +++ b/sys/src/cmd/walk.c
> > @@ -43,7 +43,7 @@
> >  void
> >  dofile(char *path, Dir *f, int pathonly)
> >  {
> > -	char *p;
> > +	char *p, *quoted;
> 
> nitpicking on style; perhaps '*q' instead of '*quoted'

Got it--be terse.

> 
> >  
> >  	if(
> >  		(f == dotdir)
> > @@ -60,6 +60,10 @@
> >  		case 'a': Bprint(bout, "%uld", f->atime); break;
> >  		case 'm': Bprint(bout, "%uld", f->mtime); break;
> >  		case 'n': Bwrite(bout, f->name, strlen(f->name)); break;
> > +		case 'N':
> > +			quoted = quotestrdup(f->name);
> > +			Bwrite(bout, quoted, strlen(quoted));
> 
> looks like this leaks 'quoted'; same with all other quotestrdup calls.

Okay, so if I'm dup'ing a string, there's memory allocated that needs
to be free()'d.

> 
> > +			break;
> >  		case 'p':
> >  			if(path != dotpath)
> >  				Bwrite(bout, path, strlen(path));
> > @@ -69,6 +73,18 @@
> >  				Bwrite(bout, f->name, strlen(f->name));
> >  			}
> >  			break;
> > +		case 'P':
> 
> this quoting could be done by concating the path and the
> dotpath, possibly even with libstring. Not sure if it'd
> be cleaner to dedup the 'p' and 'P' cases or not yet.

Yeah, I was thinking about consolidating the code for 'p' and 'P', but
figured the path of least resistance is to first just copy-and-modify.
Why not use sprint() instead of concat, since I need to interpose '/'
for the path? Anyway, see below for a patch to my patch: does it look
better?

> 
> > +			if(path != dotpath) {
> > +				quoted = quotestrdup(path);
> > +				Bwrite(bout, quoted, strlen(quoted));
> > +			}
> > +			if(! (f->qid.type & QTDIR) && !pathonly){
> > +				if(path != dotpath)
> > +					Bputc(bout, '/');
> > +				quoted = quotestrdup(f->name);
> > +				Bwrite(bout, quoted, strlen(quoted));
> > +			}
> > +			break;
> >  		case 'q': Bprint(bout, "%ullx.%uld.%.2uhhx", f->qid.path, f->qid.vers, f->qid.type); break;
> >  		case 's': Bprint(bout, "%lld", f->length); break;
> >  		case 'x': Bprint(bout, "%M", f->mode); break;
> > @@ -243,7 +259,7 @@
> >  		if((stfmt = s_reset(stfmt)) == nil)
> >  			sysfatal("s_reset: %r");
> >  		s_append(stfmt, EARGF(usage()));
> > -		i = strspn(s_to_c(stfmt), "UGMamnpqsxDT");
> > +		i = strspn(s_to_c(stfmt), "UGMamnNpPqsxDT");
> >  		if(i != s_len(stfmt))
> >  			sysfatal("bad stfmt: %s", s_to_c(stfmt));
> >  		break;
> > 
> 

From: Romano <unobe@cpan.org>
Date: Sat, 12 Aug 2023 06:32:08 +0000
Subject: [PATCH] walk: use quotefmt and free()

---
diff 2da52094832662c533f60629813dbd5ac805e3ad c371afbc8bcf6882fb1e77b5ddfefb198e331a8f
--- a/sys/src/cmd/walk.c
+++ b/sys/src/cmd/walk.c
@@ -43,7 +43,7 @@
 void
 dofile(char *path, Dir *f, int pathonly)
 {
-	char *p, *quoted;
+	char *p, *q;
 
 	if(
 		(f == dotdir)
@@ -61,8 +61,7 @@
 		case 'm': Bprint(bout, "%uld", f->mtime); break;
 		case 'n': Bwrite(bout, f->name, strlen(f->name)); break;
 		case 'N':
-			quoted = quotestrdup(f->name);
-			Bwrite(bout, quoted, strlen(quoted));
+			Bprint(bout, "%q", f->name);
 			break;
 		case 'p':
 			if(path != dotpath)
@@ -74,16 +73,13 @@
 			}
 			break;
 		case 'P':
-			if(path != dotpath) {
-				quoted = quotestrdup(path);
-				Bwrite(bout, quoted, strlen(quoted));
-			}
-			if(! (f->qid.type & QTDIR) && !pathonly){
-				if(path != dotpath)
-					Bputc(bout, '/');
-				quoted = quotestrdup(f->name);
-				Bwrite(bout, quoted, strlen(quoted));
-			}
+			if(!pathonly && path != dotpath && ! (f->qid.type & QTDIR)) {
+				q = smprint("%s/%s", path, f->name);
+				Bprint(bout, "%q", q);
+				free(q);
+}
+			else if (path != dotpath)
+				Bprint(bout, "%q", path);
 			break;
 		case 'q': Bprint(bout, "%ullx.%uld.%.2uhhx", f->qid.path, f->qid.vers, f->qid.type); break;
 		case 's': Bprint(bout, "%lld", f->length); break;
@@ -268,6 +264,7 @@
 	}ARGEND;
 
 	fmtinstall('M', dirmodefmt);
+	quotefmtinstall();
 
 	if((bout = Bfdopen(1, OWRITE)) == nil)
 		sysfatal("Bfdopen: %r");

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[unobe]

Quoth ori@eigenstate.org:
> Quoth Romano <unobe@cpan.org>:
> > I'm working with files on a backup drive that have spaces and
> > quotation marks in their names, using walk(1).  I can pipe to sed
> > s/''''/''''''''''/g and do similarly for other characters requiring
> > escaping for rc(1), but figured patching walk(1) would be better.
> 
> generally no objections, though it's usually relatively easy to
> structure scripts such that there's relatively little that is
> interpreted by rc; is this for interactive use?

I realized I didn't reply to your question, sorry.  No, not directly.
I am trying to consolidate some files spread across different paths to
one directory, so first doing:

	walk -f -eP | grep 'pattern' > /tmp/files_of_interest.txt

Then after reviewing and deleting further, I was going to do something like:

	clone `{cat /tmp/files_of_interest.txt} /path/

Are you thinking I'd do something like set the ifs manually?

	ifs='
'
	clone `{cat /tmp/files_of_interest.txt} /path

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[hiro]

> 'Desktop/Movies/TV/TV Library.tvlibrary'/'Library Preferences.tvdb'

> 'Desktop/Movies/TV/TV Library.tvlibrary/Library Preferences.tvdb'

what about

Desktop/Movies/TV/'TV Library.tvlibrary'/'Library Preferences.tvdb'

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[ori]

Quoth unobe@cpan.org:
> I realized I didn't reply to your question, sorry.  No, not directly.
> I am trying to consolidate some files spread across different paths to
> one directory, so first doing:
> 
> 	walk -f -eP | grep 'pattern' > /tmp/files_of_interest.txt
> 
> Then after reviewing and deleting further, I was going to do something like:
> 
> 	clone `{cat /tmp/files_of_interest.txt} /path/

in this case, rc doesn't evaluate the quotes, it simply splits
on spaces (or whatever ifs is).

> Are you thinking I'd do something like set the ifs manually?
> 
> 	ifs='
> '
> 	clone `{cat /tmp/files_of_interest.txt} /path
> 

yes; though I tend to use the inline version, since
it has fewer warts around nested shell functions.

	nl='
	'
	clone `$nl{cat /tmp/files} /path

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[unobe]

Quoth ori@eigenstate.org:
> Quoth unobe@cpan.org:
> > I realized I didn't reply to your question, sorry.  No, not directly.
> > I am trying to consolidate some files spread across different paths to
> > one directory, so first doing:
> > 
> > 	walk -f -eP | grep 'pattern' > /tmp/files_of_interest.txt
> > 
> > Then after reviewing and deleting further, I was going to do something like:
> > 
> > 	clone `{cat /tmp/files_of_interest.txt} /path/
> 
> in this case, rc doesn't evaluate the quotes, it simply splits
> on spaces (or whatever ifs is).

Oy, that's right.

> > Are you thinking I'd do something like set the ifs manually?
> > 
> > 	ifs='
> > '
> > 	clone `{cat /tmp/files_of_interest.txt} /path
> > 
> 
> yes; though I tend to use the inline version, since
> it has fewer warts around nested shell functions.
> 
> 	nl='
> 	'
> 	clone `$nl{cat /tmp/files} /path
> 

+1

Anyway, I'll let this simmer for another few days before consolidating
the two commits and merging as one commit (including fixing the
indentation for the end curly bracket on my last commit).

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[unobe]

Quoth hiro <23hiro@gmail.com>:
> > 'Desktop/Movies/TV/TV Library.tvlibrary'/'Library Preferences.tvdb'
> 
> > 'Desktop/Movies/TV/TV Library.tvlibrary/Library Preferences.tvdb'
> 
> what about
> 
> Desktop/Movies/TV/'TV Library.tvlibrary'/'Library Preferences.tvdb'

The directory is processed at one time, so without a more significant refactoring, I don't see how that would be possible. For reference, ls(1) does this:

cpu% ls Desktop/Movies/TV/'TV Library.tvlibrary'/'Library Preferences.tvdb'
'Desktop/Movies/TV/TV Library.tvlibrary/Library Preferences.tvdb'
cpu%

So the latest commit I did matches that output.

Re: [9front] PATCH walk(1) to (optionally) quote name and path

[hiro]

indeed. and thus preferable from my pov.

On Sat, Aug 12, 2023 at 10:13 PM <unobe@cpan.org> wrote:
>
> Quoth hiro <23hiro@gmail.com>:
> > > 'Desktop/Movies/TV/TV Library.tvlibrary'/'Library Preferences.tvdb'
> >
> > > 'Desktop/Movies/TV/TV Library.tvlibrary/Library Preferences.tvdb'
> >
> > what about
> >
> > Desktop/Movies/TV/'TV Library.tvlibrary'/'Library Preferences.tvdb'
>
> The directory is processed at one time, so without a more significant refactoring, I don't see how that would be possible. For reference, ls(1) does this:
>
> cpu% ls Desktop/Movies/TV/'TV Library.tvlibrary'/'Library Preferences.tvdb'
> 'Desktop/Movies/TV/TV Library.tvlibrary/Library Preferences.tvdb'
> cpu%
>
> So the latest commit I did matches that output.
>