Re: [PATCH] Re: (Y) modifier: up to N matches?

[Daniel Shahaf <d.s@daniel.shahaf.name>]

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

Bart Schaefer wrote on Tue, Jun 03, 2014 at 23:42:29 -0700:
> On Jun 4,  2:08am, Daniel Shahaf wrote:
> } Subject: [PATCH] Re: (Y) modifier: up to N matches?
> } 
> } Bart Schaefer wrote on Mon, Jun 02, 2014 at 20:46:03 -0700:
> } > 
> } > My concern is that people are going to expect the (o)/(O) qualifiers to
> } > take effect before (Y) does, and will be confused about the "skipped"
> } > files when it takes effect after.  If (Y) can't return more than one
> } > result, there's nothing to sort.
> } 
> } The expectations about sorting are just as much of a problem with (Y) as
> } they would be with (Y42): in both cases there might be "skipped" files.
> 
> Yes, but if there's only one it's pretty obvious why.  Consider that in
> your doc update you wrote:
>   +filenames.  If more than var(n) matches exist, only the first var(n)
>   +matches in directory traversal order will be considered.
> 
> Yet those n filenames are returned in alphabetical order, which has no
> relation to directory traversal order.

By "directory traversal", I meant to refer not just to the order of
visiting directories (e.g., depth-first), but also to the order of
visiting files within those directories (the readdir() order).  Is there
a way to make the docs clearer?

"filesystem tree traversal order" might be more accurate, but I don't
really know whether it would be clearer than the current text to the
average reader.

> In fact, people might expect (Od) to cause subdirectories to be
> "traversed" before files in the current directory, which never happens
> with (Y).  It might even be preferable if (Yn) implied (odoN).
> 
> I'll repeat that this isn't a major objection.
> 

What would (odoN) do?  It produces the same output as (oN) alone, since
presence of (oN) causes any other sort qualifiers to be ignored.

+1 on defaulting to (oN).  Patch attached.  It implements "default to
(oN) if (Yn) has been specified and no sort has been specified in any
qualifiers group".

I'm also attaching a second patch that prevents GS_NONE from being
left-shifted in the code.  I couldn't produce a user-visible problem,
but I confirmed that (1<<19) gets bitwise-or'd into gf_sorts (and 1<<19
has no defined meaning in that bitfield).

> } FWIW, I'm intentionally making (Y) without argument an error; we can
> } settle on its semantics later after (Y42) has seen some "in the field"
> } use. The spelling (Y1) can be used instead.
> 
> I'm doubtful that any default semantic other than (Y1) will be useful.

I don't see any other default right now either, but I wanted to leave
the door open to assign a bare (Y) some other, currently-unanticipated
meaning in the future.

>

Thanks,

Daniel

[-- Attachment #2: 0001-Y-glob-modifier-defaults-to-no-sorting.patch --]
[-- Type: text/x-patch, Size: 2799 bytes --]

>From 03e5a2238baf8c38028e49bfe1ed9b63cfa36f18 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 4 Jun 2014 15:48:53 +0000
Subject: [PATCH 1/2] (Y) glob modifier defaults to no sorting

---
 Doc/Zsh/expn.yo   |    7 ++++++-
 Src/glob.c        |    2 +-
 Test/D02glob.ztst |    2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index 2f91fec..ff9f2b1 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -2568,10 +2568,12 @@ item(tt(Y)var(n))(
 enables short-circuit mode: the pattern will expand to at most var(n)
 filenames.  If more than var(n) matches exist, only the first var(n)
 matches in directory traversal order will be considered.
+
+Implies tt(oN) when no tt(o)var(c) qualifier is used.
 )
 item(tt(o)var(c))(
 specifies how the names of the files should be sorted. If var(c) is
-tt(n) they are sorted by name (the default); if it is tt(L) they
+tt(n) they are sorted by name; if it is tt(L) they
 are sorted depending on the size (length) of the files; if tt(l)
 they are sorted by the number of links; if tt(a), tt(m), or tt(c)
 they are sorted by the time of the last access, modification, or
@@ -2586,6 +2588,9 @@ so `tt(*(^-oL))' gives a list of all files sorted by file size in descending
 order, following any symbolic links.  Unless tt(oN) is used, multiple order
 specifiers may occur to resolve ties.
 
+The default sorting is tt(n) (by name) unless the tt(Y) glob qualifier is used,
+in which case it is tt(N) (unsorted).
+
 tt(oe) and tt(o+) are special cases; they are each followed by shell code,
 delimited as for the tt(e) glob qualifier and the tt(+) glob qualifier
 respectively (see above).  The code is executed for each matched file with
diff --git a/Src/glob.c b/Src/glob.c
index c74a560..a62cfee 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -1779,7 +1779,7 @@ zglob(LinkList list, LinkNode np, int nountok)
 	return;
     }
     if (!gf_nsorts) {
-	gf_sortlist[0].tp = gf_sorts = GS_NAME;
+	gf_sortlist[0].tp = gf_sorts = (shortcircuit ? GS_NONE : GS_NAME);
 	gf_nsorts = 1;
     }
     /* Initialise receptacle for matched files, *
diff --git a/Test/D02glob.ztst b/Test/D02glob.ztst
index c00bbe3..358c934 100644
--- a/Test/D02glob.ztst
+++ b/Test/D02glob.ztst
@@ -546,7 +546,7 @@
  (){ print $#@ } glob.tmp/dir*(Y1)
  (){ print $#@ } glob.tmp/file*(NY1)
  (){ [[ "$*" == */dir?\ */dir? ]] && print Returns matching filenames } glob.tmp/dir*(Y2)
- (){ print "Limit is upper bound:" $@:t } glob.tmp/dir*(Y5)
+ (){ print "Limit is upper bound:" ${(o)@:t} } glob.tmp/dir*(Y5)
  (){ print "Negated:" $@:t } glob.tmp/dir*(Y1^Y)
  (){ print "Sorting:" $@:t } glob.tmp/dir*(Y4On)
  (){ [[ $#@ -eq 1 ]] && print Globs before last path component } glob.tmp/dir?/subdir(NY1)
-- 
1.7.10.4


[-- Attachment #3: 0002-Fix-GS_NONE-would-be-shifted-into-undefined-value.patch --]
[-- Type: text/x-patch, Size: 885 bytes --]

>From a7abb13613cb7b4cdcd1cb2d62ff8c54b39c6634 Mon Sep 17 00:00:00 2001
From: Daniel Shahaf <d.s@daniel.shahaf.name>
Date: Wed, 4 Jun 2014 15:42:22 +0000
Subject: [PATCH 2/2] Fix GS_NONE would be shifted into undefined value

---
 Src/glob.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Src/glob.c b/Src/glob.c
index a62cfee..c6cb3d2 100644
--- a/Src/glob.c
+++ b/Src/glob.c
@@ -1619,9 +1619,10 @@ zglob(LinkList list, LinkNode np, int nountok)
 			restore_globstate(saved);
 			return;
 		    }
+		    if ((sense & 2) &&
+			(t & (GS_SIZE|GS_ATIME|GS_MTIME|GS_CTIME|GS_LINKS)))
+			t <<= GS_SHIFT; /* HERE: GS_EXEC? */
 		    if (t != GS_EXEC) {
-			if ((sense & 2) && !(t & (GS_NAME|GS_DEPTH)))
-			    t <<= GS_SHIFT; /* HERE: GS_EXEC? */
 			if (gf_sorts & t) {
 			    zerr("doubled sort specifier");
 			    restore_globstate(saved);
-- 
1.7.10.4