[PATCH] Completion: Sort lz4 compression levels properly (+ a question)

[PATCH] Completion: Sort lz4 compression levels properly (+ a question)

[dana]

This isn't super necessary but it irritated me.

Is there a better way to do this? Off the top of my head i came up with either
this state method or calling compadd from the argument spec... but both of those
involve an awful lot of boiler-plate just to keep some numbers sorted.

If there's no generic way to do it, would it make sense to have some kind of
spec modifier that tells action forms like `(item ...)` that -V should be used,
or, if that's too weird, a helper function (_presorted maybe) that just wraps
compadd?

dana


diff --git a/Completion/Unix/Command/_lz4 b/Completion/Unix/Command/_lz4
index d69091d00..4d2721bd5 100644
--- a/Completion/Unix/Command/_lz4
+++ b/Completion/Unix/Command/_lz4
@@ -42,8 +42,8 @@ args=(
   '(b t -k --keep)--rm[remove source file]'
   '!(b t -c --stdout)--to-stdout'
   + b # Benchmark-mode options
-  "(C c d t)-b-[benchmark file using specified compression level]::compression level:(${(j< >)levels//-/})"
-  "(C c d t)-e-[specify upper compression level limit (with -b)]:compression level:(${(j< >)levels//-/})"
+  '(C c d t)-b-[benchmark file using specified compression level]:: :->levels'
+  '(C c d t)-e-[specify upper compression level limit (with -b)]: :->levels'
   '(C c d t)-i-[specifiy minimum evaluation time (with -b)]:evaluation time (seconds)'
   + c # Compress-mode options
   "(b d t ${(j< >)levels} -c0 -c1 -c2 -hc)"${^levels}
@@ -98,6 +98,11 @@ case $state in
       _message 'no more arguments' && ret=0
     fi
     ;;
+  levels)
+    _wanted -2V levels expl 'compression level' \
+      compadd - ${(@on)levels//-/} \
+    && ret=0
+    ;;
 esac
 
 return ret

Re: [PATCH] Completion: Sort lz4 compression levels properly (+ a question)

[Oliver Kiddle]

On 3 Nov, dana wrote:
>
> Is there a better way to do this? Off the top of my head i came up with either

You can do:
  _arguments '-b-:level: compadd "${(@)expl/#-J/-2V}" ${(on)levels}'
But what you have is perhaps better, especially as it's duplicated for
-b and -e.

I was thinking that you just needed to put - in where you wanted
_arguments to add the options but it's actually _sequence that does
that.

> If there's no generic way to do it, would it make sense to have some kind of
> spec modifier that tells action forms like `(item ...)` that -V should be used,
> or, if that's too weird, a helper function (_presorted maybe) that just wraps

I'm not especially enthused about adding more special syntax to
_arguments.

The compadd interface perhaps isn't ideal. If -V was a standalone option
like -1 and -2, intended to be used in combination with -J (to specify
the group name), then it would be easier to modify the sort order.
There's also the -o option which (per-match rather than per-group) says
that the display string should be used when comparing matches for the
purpose of sorting. As far as I can tell, -o has never been used.
Sorting by the description part of the display string would be useful.

Can we improve it without breaking backward compatibility. -o with an
argument could allow for more sorting options, e.g. -o numeric to do
numeric sorting. Do we need to be concerned that someone somewhere may
actually be using -o?

The help descriptions in compadd completion had things the wrong way
around for -o which this fixes. Also, the help for -V is perhaps
somewhat confusing. "unsorted" was used to mean "will not be sorted" but
is often used when you have already pre-sorted the matches.

Oliver

diff --git a/Completion/Zsh/Command/_compadd b/Completion/Zsh/Command/_compadd
index a7036d027..e709e400e 100644
--- a/Completion/Zsh/Command/_compadd
+++ b/Completion/Zsh/Command/_compadd
@@ -14,9 +14,9 @@ _arguments -C -s -S -A "-*" \
   '(-a)-k[matches are keys of specified associative arrays]' \
   '-d+[specify display strings]:array:_parameters -g "*array*"' \
   '-l[list display strings one per line, not in columns]' \
-  '-o[order matches by display string not by match string]' \
-  '(-1 -E)-J+[specify match group]:group' \
-  '-V+[specify unsorted match group]:group' \
+  '-o[order matches by match string not by display string]' \
+  '(-1 -E)-J+[specify match group which will be sorted]:group' \
+  '-V+[specify pre-ordered match group]:group' \
   '(-J -E)-1[remove only consecutive duplicates from group]' \
   '-2[preserve all duplicates]' \
   '(-x)-X[specify explanation]:explanation' \

Re: [PATCH] Completion: Sort lz4 compression levels properly (+ a question)

[dana]

On 7 Nov 2018, at 04:35, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>The compadd interface perhaps isn't ideal. If -V was a standalone option
>like -1 and -2, intended to be used in combination with -J (to specify
>the group name), then it would be easier to modify the sort order.

Yeah, i don't know the history behind it, but i've found myself thinking exactly
that several times.

On 7 Nov 2018, at 04:35, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>Can we improve it without breaking backward compatibility. -o with an
>argument could allow for more sorting options, e.g. -o numeric to do
>numeric sorting. Do we need to be concerned that someone somewhere may
>actually be using -o?

I can't find any instances of it in either the main zsh repo or the
zsh-completions one, nor in any of the 'vendor completions' i have installed on
my Mac or Ubuntu machines, so i bet you're right that it's not been used.

Are you suggesting removing the requirement to use it with -d, and giving it an
optarg to control the match-string sorting method?

dana

completion match ordering

[Oliver Kiddle]

On 7 Nov, I wrote:
> You can do:
>   _arguments '-b-:level: compadd "${(@)expl/#-J/-2V}" ${(on)levels}'
> But what you have is perhaps better, especially as it's duplicated for
> -b and -e.

The patch below is an initial experiment for what I was suggesting.
It allows, e.g:
  _arguments '-b-:level:compadd -o numeric -a levels'

Any thoughts on the interface? Is something more terse like -on
preferable? The -V option becomes superfluous (but remains for
compatibility). Instead of -V grp you can use -J grp -o nosort.
Do we need to worry about this breaking uses of the old -o?

By the way, the change in 41242 has a problem: zstrcmp() doesn't honour
the SORTIT_IGNORING_BACKSLASHES flag if it is passed. This patch doesn't
attempt to fix that.

With this the existing, -o would become -o match and becomes a property
of the group rather than the match. Being associated with the group is
more consistent with related options like -1, -2 and -V. It might be
confusing that it doesn't work if no group is specified, however.

Besides match, nosort and numeric, should any other orderings be
supported? Reverse or case-sensitive perhaps? Zsh's numeric sorting
works well for version numbers which is useful for things like git
tags. This implementation allows combinations, e.g. -o match,numeric An
alternative might be allowing multiple -o options but that affects how
we compose options passed down from other functions.

Currently, given two -o options, only the first applies, This is
consistent with -X, -J etc even though I think it would have been better
if the last of these were used. In many cases where functions use
zparseopts to extract compadd options, they should be using J+: rather
than J: as zparseopts is taking the last one only with J:

Oliver

diff --git a/Src/Zle/comp.h b/Src/Zle/comp.h
index 3e9834560..46a35aa2d 100644
--- a/Src/Zle/comp.h
+++ b/Src/Zle/comp.h
@@ -90,6 +90,8 @@ struct cmgroup {
 #define CGF_PACKED  32		/* LIST_PACKED for this group */
 #define CGF_ROWS    64		/* LIST_ROWS_FIRST for this group */
 #define CGF_FILES   128		/* contains file names */
+#define CGF_MATSORT 256         /* sort by match rather than by display string */
+#define CGF_NUMSORT 512         /* sort numerically */
 
 /* This is the struct used to hold matches. */
 
@@ -300,6 +302,8 @@ struct menuinfo {
 #define CAF_ARRAYS  32    /* compadd -a or -k: array/assoc parameter names */
 #define CAF_KEYS    64    /* compadd -k: assoc parameter names */
 #define CAF_ALL    128    /* compadd -C: _all_matches */
+#define CAF_MATSORT 256   /* compadd -o match: sort by match rather than by display string */
+#define CAF_NUMSORT 512   /* compadd -o numeric: sort numerically */
 
 /* Data for compadd and addmatches() */
 
@@ -314,6 +318,7 @@ struct cadata {
     char *pre;			/* prefix to insert (-P) */
     char *suf;			/* suffix to insert (-S) */
     char *group;		/* name of the group (-[JV]) */
+    char *order;		/* group order (-o) */
     char *rems;			/* remove suffix on chars... (-r) */
     char *remf;			/* function to remove suffix (-R) */
     char *ign;			/* ignored suffixes (-F) */
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 0a454ad5f..4163fa20b 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -2080,6 +2080,8 @@ addmatches(Cadata dat, char **argv)
 
         /* Select the group in which to store the matches. */
         gflags = (((dat->aflags & CAF_NOSORT ) ? CGF_NOSORT  : 0) |
+                  ((dat->aflags & CAF_MATSORT) ? CGF_MATSORT : 0) |
+                  ((dat->aflags & CAF_NUMSORT) ? CGF_NUMSORT : 0) |
                   ((dat->aflags & CAF_UNIQALL) ? CGF_UNIQALL : 0) |
                   ((dat->aflags & CAF_UNIQCON) ? CGF_UNIQCON : 0));
         if (dat->group) {
@@ -3034,8 +3036,8 @@ begcmgroup(char *n, int flags)
 		HEAP_ERROR(p->heap_id);
 	    }
 #endif
-	    if (p->name &&
-		flags == (p->flags & (CGF_NOSORT|CGF_UNIQALL|CGF_UNIQCON)) &&
+	    if (p->name && flags ==
+		(p->flags & (CGF_NOSORT|CGF_UNIQALL|CGF_UNIQCON|CGF_MATSORT|CGF_NUMSORT)) &&
 		!strcmp(n, p->name)) {
 		mgroup = p;
 
@@ -3118,32 +3120,33 @@ addexpl(int always)
 
 /* The comparison function for matches (used for sorting). */
 
+static int matchorder;
+
 /**/
 static int
 matchcmp(Cmatch *a, Cmatch *b)
 {
-    if ((*a)->disp && !((*a)->flags & CMF_MORDER)) {
-	if ((*b)->disp) {
-	    if ((*a)->flags & CMF_DISPLINE) {
-		if ((*b)->flags & CMF_DISPLINE)
-		    return strcmp((*a)->disp, (*b)->disp);
-		else
-		    return -1;
-	    } else {
-		if ((*b)->flags & CMF_DISPLINE)
-		    return 1;
-		else
-		    return strcmp((*a)->disp, (*b)->disp);
-	    }
-	}
-	return -1;
-    }
-    if ((*b)->disp && !((*b)->flags & CMF_MORDER))
-	return 1;
+    const char *as, *bs;
+    int cmp = !!(*b)->disp - !!(*a)->disp;
 
-    return zstrcmp((*a)->str, (*b)->str, (SORTIT_IGNORING_BACKSLASHES|
-					  (isset(NUMERICGLOBSORT) ?
-					   SORTIT_NUMERICALLY : 0)));
+    /* if match sorting selected or we have no display strings */
+    if ((matchorder & CGF_MATSORT) || (!cmp && !(*a)->disp)) {
+	as = (*a)->str;
+	bs = (*b)->str;
+    } else {
+        if (cmp) /* matches with display strings come first */
+	    return cmp;
+
+	cmp = ((*b)->flags & CMF_DISPLINE) - ((*a)->flags & CMF_DISPLINE);
+        if (cmp) /* sort one-per-line display strings first */
+	    return cmp;
+
+	as = (*a)->disp;
+	bs = (*b)->disp;
+    }
+
+    return zstrcmp(as, bs, (isset(NUMERICGLOBSORT) || matchorder & CGF_NUMSORT)
+	    ? SORTIT_NUMERICALLY : 0);
 }
 
 /* This tests whether two matches are equal (would produce the same
@@ -3205,6 +3208,7 @@ makearray(LinkList l, int type, int flags, int *np, int *nlp, int *llp)
     } else {
 	if (!(flags & CGF_NOSORT)) {
 	    /* Now sort the array (it contains matches). */
+	    matchorder = flags;
 	    qsort((void *) rp, n, sizeof(Cmatch),
 		  (int (*) _((const void *, const void *)))matchcmp);
 
diff --git a/Src/Zle/complete.c b/Src/Zle/complete.c
index 1dc2b01c2..316e51844 100644
--- a/Src/Zle/complete.c
+++ b/Src/Zle/complete.c
@@ -558,6 +558,12 @@ parse_class(Cpattern p, char *iptr)
     return iptr;
 }
 
+static struct { char *name; int oflag; } orderopts[] = {
+    { "nosort", CAF_NOSORT },
+    { "match", CAF_MATSORT },
+    { "numeric", CAF_NUMSORT }
+};
+
 /**/
 static int
 bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
@@ -572,8 +578,8 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	return 1;
     }
     dat.ipre = dat.isuf = dat.ppre = dat.psuf = dat.prpre = dat.mesg =
-	dat.pre = dat.suf = dat.group = dat.rems = dat.remf = dat.disp = 
-	dat.ign = dat.exp = dat.apar = dat.opar = dat.dpar = NULL;
+	dat.pre = dat.suf = dat.group = dat.order = dat.rems = dat.remf =
+	dat.disp = dat.ign = dat.exp = dat.apar = dat.opar = dat.dpar = NULL;
     dat.match = NULL;
     dat.flags = 0;
     dat.aflags = CAF_MATCH;
@@ -710,7 +716,8 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 		dat.flags |= CMF_DISPLINE;
 		break;
 	    case 'o':
-		dat.flags |= CMF_MORDER;
+		sp = &(dat.order);
+		e = "string expected after -%c";
 		break;
 	    case 'E':
                 if (p[1]) {
@@ -775,6 +782,23 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
     }
     zsfree(mstr);
 
+    if (dat.order) {
+	int o;
+	const char *next, *opt = dat.order;
+	do {
+	    next = strchr(opt, ',');
+	    if (!next)
+		next = opt + strlen(opt);
+
+	    for (o = sizeof(orderopts)/sizeof(*orderopts) - 1; o >= 0 &&
+		strncmp(orderopts[o].name, opt, next - opt); o--) {}
+	    if (o < 0) {
+		zwarnnam(name, "unknown match ordering: %s\n", dat.order);
+		return 1;
+	    }
+	    dat.aflags |= orderopts[o].oflag;
+	} while (*next && ((opt = next + 1)));
+    }
     if (!*argv && !dat.group && !dat.mesg &&
 	!(dat.aflags & (CAF_NOSORT|CAF_UNIQALL|CAF_UNIQCON|CAF_ALL)))
 	return 1;
diff --git a/Completion/Base/Core/_all_labels b/Completion/Base/Core/_all_labels
index e607d639d..54d163895 100644
--- a/Completion/Base/Core/_all_labels
+++ b/Completion/Base/Core/_all_labels
@@ -8,7 +8,7 @@ if [[ "$1" = - ]]; then
 fi
 
 __gopt=()
-zparseopts -D -a __gopt 1 2 V J x
+zparseopts -D -a __gopt 1 2 V J x o:
 
 __tmp=${argv[(ib:4:)-]}
 __len=$#
diff --git a/Completion/Base/Core/_description b/Completion/Base/Core/_description
index 304c747a6..f51f4a18c 100644
--- a/Completion/Base/Core/_description
+++ b/Completion/Base/Core/_description
@@ -1,13 +1,15 @@
 #autoload
 
 local name gropt nopt xopt format gname hidden hide match opts tag sort
+local -a sortopt
 
 opts=()
 
 gropt=(-J)
 xopt=(-X)
 nopt=()
-zparseopts -K -D -a nopt 1 2 V=gropt J=gropt x=xopt
+zparseopts -K -D -a nopt 1 2 V=gropt J=gropt x=xopt o:=sortopt
+[[ "$gropt" = -V ]] && sortopt=( -o nosort )
 
 3="${${3##[[:blank:]]#}%%[[:blank:]]#}"
 [[ -n "$3" ]] && _lastdescr=( "$_lastdescr[@]" "$3" )
@@ -32,15 +34,15 @@ zstyle -s ":completion:${curcontext}:$1" matcher match &&
 [[ -n "$_matcher" ]] && opts=($opts -M "$_matcher")
 
 # Use sort style, but ignore `menu' value to help _expand.
-# Also don't override explicit use of -V.
-if { zstyle -s ":completion:${curcontext}:$1" sort sort ||
-     zstyle -s ":completion:${curcontext}:" sort sort; } &&
-    [[ "$gropt" = -J && $sort != menu ]]; then
-    if [[ "$sort" = (yes|true|1|on) ]]; then
-	gropt=(-J)
-    else
-	gropt=(-V)
-    fi
+# Also don't override explicit use of -V or -o.
+if (( ! $+sortopt )) &&
+  { zstyle -s ":completion:${curcontext}:$1" sort sort ||
+     zstyle -s ":completion:${curcontext}:" sort sort; }; then
+  if [[ "$sort" = (match|numeric) ]]; then
+    sortopt=( -o $sort )
+  elif [[ "$sort" != (yes|true|1|on|menu) ]]; then
+    sortopt=( -o nosort )
+  fi
 fi
 
 if [[ -z "$_comp_no_ignore" ]]; then
@@ -79,15 +81,15 @@ fi
 
 if [[ -n "$gname" ]]; then
   if [[ -n "$format" ]]; then
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" "$gname" "$xopt" "$format"
+    set -A "$name" "$opts[@]" "$nopt[@]" "$sortopt[@]" -J "$gname" "$xopt" "$format"
   else
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" "$gname"
+    set -A "$name" "$opts[@]" "$nopt[@]" "$sortopt" -J "$gname"
   fi
 else
   if [[ -n "$format" ]]; then
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" -default- "$xopt" "$format"
+    set -A "$name" "$opts[@]" "$nopt[@]" "$sortopt" -J -default- "$xopt" "$format"
   else
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" -default-
+    set -A "$name" "$opts[@]" "$nopt[@]" "$sortopt" -J -default-
   fi
 fi
 
diff --git a/Completion/Base/Core/_message b/Completion/Base/Core/_message
index 4d5645eaf..e4e764f64 100644
--- a/Completion/Base/Core/_message
+++ b/Completion/Base/Core/_message
@@ -25,7 +25,7 @@ if [[ "$1" = -e ]]; then
 fi
 
 gopt=()
-zparseopts -D -a gopt 1 2 V J
+zparseopts -D -a gopt 1 2 V J o:
 
 _tags messages || return 1
 
diff --git a/Completion/Base/Core/_next_label b/Completion/Base/Core/_next_label
index 64506d05a..acc4c04a7 100644
--- a/Completion/Base/Core/_next_label
+++ b/Completion/Base/Core/_next_label
@@ -3,7 +3,7 @@
 local __gopt __descr __spec
 
 __gopt=()
-zparseopts -D -a __gopt 1 2 V J x
+zparseopts -D -a __gopt 1 2 V J x o:
 
 if comptags -A "$1" curtag __spec; then
   (( $#funcstack > _tags_level )) && _comp_tags="${_comp_tags% * }"
diff --git a/Completion/Base/Core/_requested b/Completion/Base/Core/_requested
index 4ba52ce7f..991158801 100644
--- a/Completion/Base/Core/_requested
+++ b/Completion/Base/Core/_requested
@@ -3,7 +3,7 @@
 local __gopt
 
 __gopt=()
-zparseopts -D -a __gopt 1 2 V J x
+zparseopts -D -a __gopt 1 2 V J x o:
 
 if comptags -R "$1"; then
   if [[ $# -gt 3 ]]; then
diff --git a/Completion/Base/Core/_wanted b/Completion/Base/Core/_wanted
index 5bba7fd63..17bc4ad18 100644
--- a/Completion/Base/Core/_wanted
+++ b/Completion/Base/Core/_wanted
@@ -2,7 +2,7 @@
 
 local -a __targs __gopt
 
-zparseopts -D -a __gopt 1 2 V J x C:=__targs
+zparseopts -D -a __gopt 1 2 V J x o: C:=__targs
 
 _tags "$__targs[@]" "$1"
 
diff --git a/Completion/Base/Utility/_describe b/Completion/Base/Utility/_describe
index 76ab1d995..c12eb0eab 100644
--- a/Completion/Base/Utility/_describe
+++ b/Completion/Base/Utility/_describe
@@ -108,10 +108,10 @@ while _tags; do
         fi
     
         if [[ -n $_mats ]]; then
-          compadd "$_opts[@]" "${(@)_expl:/-J/-2V}" -D $_strs -O $_mats - \
+          compadd "$_opts[@]" -2 -o nosort "${_expl[@]}" -D $_strs -O $_mats - \
                   "${(@)${(@M)${(@P)_mats}##([^:\\]|\\?)##}//\\(#b)(?)/$match[1]}"
         else
-          compadd "$_opts[@]" "${(@)_expl:/-J/-2V}" -D $_strs - \
+          compadd "$_opts[@]" -2 -o nosort "${_expl[@]}" -D $_strs - \
                   "${(@)${(@M)${(@P)_strs}##([^:\\]|\\?)##}//\\(#b)(?)/$match[1]}"
         fi
       done
diff --git a/Completion/Base/Utility/_guard b/Completion/Base/Utility/_guard
index ff62981ce..ab9bf6128 100644
--- a/Completion/Base/Utility/_guard
+++ b/Completion/Base/Utility/_guard
@@ -2,7 +2,7 @@
 
 local garbage
 
-zparseopts -K -D -a garbage M: J: V: 1 2 n F: X:
+zparseopts -K -D -a garbage M: J: V: 1 2 o: n F: X:
 
 [[ "$PREFIX$SUFFIX" != $~1 ]] && return 1
 
diff --git a/Completion/Base/Utility/_multi_parts b/Completion/Base/Utility/_multi_parts
index 3e2f36c9c..ce2f1a978 100644
--- a/Completion/Base/Utility/_multi_parts
+++ b/Completion/Base/Utility/_multi_parts
@@ -14,8 +14,8 @@ typeset -U tmp1 matches
 # Get the options.
 
 zparseopts -D -a sopts \
-    'J+:=group' 'V+:=group' 'X+:=expl' 'P:=opts' 'F:=opts' \
-    S: r: R: q 1 2 n f 'M+:=matcher' 'i=imm'
+    'J+:=group' 'V+:=group' 'x+:=expl' 'X+:=expl' 'P:=opts' 'F:=opts' \
+    S: r: R: q 1 2 o+: n f 'M+:=matcher' 'i=imm'
 
 sopts=( "$sopts[@]" "$opts[@]" )
 if (( $#matcher )); then
diff --git a/Completion/Base/Utility/_sep_parts b/Completion/Base/Utility/_sep_parts
index de836a696..6fcf54ec4 100644
--- a/Completion/Base/Utility/_sep_parts
+++ b/Completion/Base/Utility/_sep_parts
@@ -22,8 +22,8 @@ local matchflags opt group expl nm=$compstate[nmatches] opre osuf opts matcher
 
 # Get the options.
 
-zparseopts -D -a opts \
-    'J+:=group' 'V+:=group' P: F: S: r: R: q 1 2 n 'X+:=expl' 'M+:=matcher'
+zparseopts -D -a opts 'J+:=group' 'V+:=group' P: F: S: r: R: q 1 2 o+: n \
+    'x+:=expl' 'X+:=expl' 'M+:=matcher'
 
 # Get the string from the line.
 
diff --git a/Completion/Base/Utility/_sequence b/Completion/Base/Utility/_sequence
index f52955f46..101934490 100644
--- a/Completion/Base/Utility/_sequence
+++ b/Completion/Base/Utility/_sequence
@@ -10,7 +10,8 @@
 local curcontext="$curcontext" nm="$compstate[nmatches]" pre qsep nosep minus
 local -a sep num pref suf end uniq dedup
 
-zparseopts -D -a opts s:=sep n:=num p:=pref i:=pref P:=pref I:=suf S:=suf q=suf r:=suf R:=suf C:=cont d=uniq M: J: X: x:
+zparseopts -D -a opts s:=sep n:=num p:=pref i:=pref P:=pref I:=suf S:=suf \
+    q=suf r:=suf R:=suf C:=cont d=uniq M+: J+: V+: 1 2 o+: X+: x+:
 (( $#cont )) && curcontext="${curcontext%:*}:$cont[2]"
 (( $#sep )) || sep[2]=,
 
diff --git a/Completion/Base/Widget/_next_tags b/Completion/Base/Widget/_next_tags
index 8522d7c9a..83fc55a85 100644
--- a/Completion/Base/Widget/_next_tags
+++ b/Completion/Base/Widget/_next_tags
@@ -18,7 +18,7 @@ _next_tags() {
     fi
 
     __gopt=()
-    zparseopts -D -a __gopt 1 2 V J x
+    zparseopts -D -a __gopt 1 2 V J x o:
 
     __tmp=${argv[(ib:4:)-]}
     __len=$#
@@ -58,7 +58,7 @@ _next_tags() {
     local __gopt __descr __spec
 
     __gopt=()
-    zparseopts -D -a __gopt 1 2 V J x
+    zparseopts -D -a __gopt 1 2 V J x o:
 
     if comptags -A "$1" curtag __spec; then
       (( $#funcstack > _tags_level )) && _comp_tags="${_comp_tags% * }"
diff --git a/Completion/Darwin/Type/_mac_files_for_application b/Completion/Darwin/Type/_mac_files_for_application
index 299d8ff5c..44516b4db 100644
--- a/Completion/Darwin/Type/_mac_files_for_application
+++ b/Completion/Darwin/Type/_mac_files_for_application
@@ -35,7 +35,7 @@ _mac_parse_info_plist() {
 # Try to complete files for the specified application.
 _mac_files_for_application() {
   local -a opts
-  zparseopts -D -a opts q n 1 2 P: S: r: R: W: X+: M+: F: J+: V+:
+  zparseopts -D -a opts q n 1 2 o+: P: S: r: R: W: x+: X+: M+: F: J+: V+:
 
   local app_path
   _retrieve_mac_apps
diff --git a/Completion/Redhat/Command/_yum b/Completion/Redhat/Command/_yum
index 34a337109..8abd23ebb 100644
--- a/Completion/Redhat/Command/_yum
+++ b/Completion/Redhat/Command/_yum
@@ -212,7 +212,7 @@ _yum_ids() {
   # `${(@)@[...]}' selects a subrange from $@
   # `${(@)@[1,-2]}' are all except the last argument
   # `$@[$#]' is the last argument, e.g. the first suggestable ID
-  compadd "${(@)@[1,-2]:/-J/-V}" -M "B:0=" {$@[$#]..$maxid}
+  compadd "${(@)@[1,-2]}" -o numeric -M "B:0=" {$@[$#]..$maxid}
 }
 
 _yum_ranges() {
diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 093464625..19698f0b1 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5696,7 +5696,7 @@ __git_ignore_line () {
 __git_ignore_line_inside_arguments () {
   declare -a compadd_opts
 
-  zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
+  zparseopts -D -E -a compadd_opts V+: J+: 1 2 o+: n f x+: X+: M+: P: S: r: R: q F:
 
   __git_ignore_line $* $compadd_opts
 }
@@ -6168,7 +6168,7 @@ __git_ref_fields () {
   local match mbegin mend
   local -a cfields fields append opts all
 
-  zparseopts -D -E -a opts x: X: J: V: a=all
+  zparseopts -D -E -a opts M+: x+: X+: J+: V+: o+: 1 2 a=all
 
   if compset -P 1 '(#b)(*):'; then
     case $match[1] in
@@ -6737,7 +6737,7 @@ __git_tags_of_type () {
   tags=(${${(M)${(f)"$(_call_program ${(q)type}-tag-refs "git for-each-ref --format='%(*objecttype)%(objecttype) %(refname)' refs/tags 2>/dev/null")"}:#$type(tag|) *}#$type(tag|) refs/tags/})
   __git_command_successful $pipestatus || return 1
 
-  _wanted $type-tags expl "$type tag" compadd -M 'r:|/=* r:|=*' "$@" -a - tags
+  _wanted $type-tags expl "$type tag" compadd -M 'r:|/=* r:|=*' "$@" -o numeric -a - tags
 }
 
 # Reference Argument Types
@@ -6832,7 +6832,7 @@ __git_files_relative () {
 __git_files () {
   local compadd_opts opts tag description gitcdup gitprefix files expl
 
-  zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
+  zparseopts -D -E -a compadd_opts V+: J+: 1 2 o+: n f x+: X+: M+: P: S: r: R: q F:
   zparseopts -D -E -a opts -- -cached -deleted -modified -others -ignored -unmerged -killed x+: --exclude+:
   tag=$1 description=$2; shift 2
 
@@ -6963,7 +6963,7 @@ __git_tree_files () {
     shift
   fi
 
-  zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
+  zparseopts -D -E -a compadd_opts V+: J+: 1 2 o+: n f x+: X+: M+: P: S: r: R: q F:
 
   [[ "$1" == */ ]] && Path="$1" || Path="${1:h}/"
   shift
@@ -7043,7 +7043,7 @@ __git_any_repositories_or_references () {
 __git_guard () {
   declare -A opts
 
-  zparseopts -K -D -A opts M: J: V: 1 2 n F: X:
+  zparseopts -K -D -A opts M+: J+: V+: 1 2 o+: n F: x+: X+:
 
   [[ "$PREFIX$SUFFIX" != $~1 ]] && return 1
 
@@ -7081,7 +7081,7 @@ __git_guard_diff-stat-width () {
 __git_guard_number () {
   declare -A opts
 
-  zparseopts -K -D -A opts M: J: V: 1 2 n F: X:
+  zparseopts -K -D -A opts M+: J+: V+: 1 2 o+: n F: x+: X+:
 
   _guard '[[:digit:]]#' ${1:-number}
 }
diff --git a/Completion/Unix/Type/_baudrates b/Completion/Unix/Type/_baudrates
index add359d13..6e9ba4d97 100644
--- a/Completion/Unix/Type/_baudrates
+++ b/Completion/Unix/Type/_baudrates
@@ -72,7 +72,6 @@ if (( ${+opts[-f]} )); then
   done
 fi
 
-# -1V removes dupes (which there shouldn't be) and otherwise leaves the
-# order in the $rates array intact.
-_description -1V baud-rates expl 'baud rate'
-compadd "${(@)argv/#-J/-V}" "$expl[@]" -- "${rates[@]}"
+# -1 removes dupes (which there shouldn't be)
+_description -1 -o numeric baud-rates expl 'baud rate'
+compadd "${argv[@]}" "$expl[@]" -- "${rates[@]}"
diff --git a/Completion/Unix/Type/_canonical_paths b/Completion/Unix/Type/_canonical_paths
index 6eab7b677..cddc3b405 100644
--- a/Completion/Unix/Type/_canonical_paths
+++ b/Completion/Unix/Type/_canonical_paths
@@ -5,13 +5,14 @@
 # (relative path when an absolute path is given, and vice versa; when ..'s are
 # present in the word to be completed, and some paths got from symlinks).
 
-# Usage: _canonical_paths [-A var] [-N] [-MJV12nfX] tag desc [paths...]
+# Usage: _canonical_paths [-A var] [-N] [-MJV12onfX] tag desc [paths...]
 
 # -A, if specified, takes the paths from the array variable specified. Paths
 # can also be specified on the command line as shown above. -N, if specified,
 # prevents canonicalizing the paths given before using them for completion, in
 # case they are already so. `tag' and `desc' arguments are well, obvious :) In
-# addition, the options -M, -J, -V, -1, -2, -n, -F, -X are passed to compadd.
+# addition, the options -M, -J, -V, -1, -2, -o, -n, -F, -x, -X are passed to
+# compadd.
 
 _canonical_paths_add_paths () {
   # origpref = original prefix
@@ -59,7 +60,7 @@ _canonical_paths() {
   local __index
   typeset -a __gopts __opts
 
-  zparseopts -D -a __gopts M: J: V: 1 2 n F: X: A:=__opts N=__opts
+  zparseopts -D -a __gopts M+: J+: V+: o+: 1 2 n F: x+: X+: A:=__opts N=__opts
 
   : ${1:=canonical-paths} ${2:=path}
 
diff --git a/Completion/Unix/Type/_files b/Completion/Unix/Type/_files
index 2b0c5580a..6276ff451 100644
--- a/Completion/Unix/Type/_files
+++ b/Completion/Unix/Type/_files
@@ -23,7 +23,7 @@ local opts tmp glob pat pats expl tag i def descr end ign tried
 local type sdef ignvars ignvar prepath oprefix rfiles rfile
 
 zparseopts -a opts \
-    '/=tmp' 'f=tmp' 'g+:-=tmp' q n 1 2 P: S: r: R: W: X+: M+: F: J+: V+:
+    '/=tmp' 'f=tmp' 'g+:-=tmp' q n 1 2 P: S: r: R: W: x+: X+: M+: F: J+: V+: o+:
 
 type="${(@j::M)${(@)tmp#-}#?}"
 if (( $tmp[(I)-g*] )); then
diff --git a/Completion/Unix/Type/_list_files b/Completion/Unix/Type/_list_files
index 6c52bc1f4..0ea02a55a 100644
--- a/Completion/Unix/Type/_list_files
+++ b/Completion/Unix/Type/_list_files
@@ -64,6 +64,6 @@ for f in ${(PQ)1}; do
  ${(r:8:)stat[gid]} ${(l:8:)stat[size]} $stat[mtime] $f")
 done
 
-(( ${#listfiles} )) && listopts=(-d listfiles -l -o)
+(( ${#listfiles} )) && listopts=(-d listfiles -l -o match)
 
 return 0
diff --git a/Completion/Unix/Type/_path_files b/Completion/Unix/Type/_path_files
index 9fa6ae9fc..d8af018a0 100644
--- a/Completion/Unix/Type/_path_files
+++ b/Completion/Unix/Type/_path_files
@@ -54,7 +54,7 @@ exppaths=()
 zparseopts -a mopts \
     'P:=pfx' 'S:=pfxsfx' 'q=pfxsfx' 'r:=pfxsfx' 'R:=pfxsfx' \
     'W:=prepaths' 'F:=ignore' 'M+:=matcher' \
-    J+: V+: X+: 1 2 n 'f=tmp1' '/=tmp1' 'g+:-=tmp1'
+    J+: V+: x+: X+: 1 2 o+: n 'f=tmp1' '/=tmp1' 'g+:-=tmp1'
 
 sopt="-${(@j::M)${(@)tmp1#-}#?}"
 (( $tmp1[(I)-[/g]*] )) && haspats=yes
@@ -163,7 +163,7 @@ if zstyle -s ":completion:${curcontext}:" file-sort tmp1; then
   if [[ "$sort" = on ]]; then
     sort=
   else
-    mopts=( "${(@)mopts/#-J/-V}" )
+    mopts=( -o nosort "${mopts[@]}" )
 
     tmp2=()
     for tmp1 in "$pats[@]"; do
diff --git a/Completion/Zsh/Type/_file_descriptors b/Completion/Zsh/Type/_file_descriptors
index 3e9f4968b..be96dd0e4 100644
--- a/Completion/Zsh/Type/_file_descriptors
+++ b/Completion/Zsh/Type/_file_descriptors
@@ -56,4 +56,4 @@ fi
 fds=( 0 1 2 $fds )
 
 _description -V file-descriptors expl 'file descriptor'
-compadd $disp "${@/-J/-V}" "$expl[@]" -a fds
+compadd $disp -o nosort "$@" "$expl[@]" -a fds

Re: completion match ordering

[Daniel Shahaf]

Oliver Kiddle wrote on Mon, 26 Nov 2018 02:25 +0100:
> On 7 Nov, I wrote:
> > You can do:
> >   _arguments '-b-:level: compadd "${(@)expl/#-J/-2V}" ${(on)levels}'
> > But what you have is perhaps better, especially as it's duplicated for
> > -b and -e.
> 
> The patch below is an initial experiment for what I was suggesting.
> It allows, e.g:
>   _arguments '-b-:level:compadd -o numeric -a levels'
> 
> Any thoughts on the interface? Is something more terse like -on
> preferable? The -V option becomes superfluous (but remains for
> compatibility). Instead of -V grp you can use -J grp -o nosort.
> Do we need to worry about this breaking uses of the old -o?

In general, I would default to assuming we should worry about
compatibility, unless there's a specific reason not to.  (As a user,
I assume that my zsh code is forward compatible with new zsh releases
unless something is specifically documented to be an exception.)

Is there a reason for this case to be an exception?  If people use
«compadd -od foo -a bar» in 5.6.2, what would the failure mode be?

> With this the existing, -o would become -o match and becomes a property
> of the group rather than the match. Being associated with the group is
> more consistent with related options like -1, -2 and -V. It might be
> confusing that it doesn't work if no group is specified, however.

Sounds like bin_compadd() could detect this situation and warn.

> Currently, given two -o options, only the first applies, This is
> consistent with -X, -J etc even though I think it would have been better
> if the last of these were used. In many cases where functions use
> zparseopts to extract compadd options, they should be using J+: rather
> than J: as zparseopts is taking the last one only with J:

No, what they *should* be doing is passing arguments transparently,
without parsing and reserializing them, so when we add "-$letter"
options to compadd we can do so by changing just one place (namely, the
C struct declaring the builtin).  Probably by putting the compadd
options in an array and passing its name, or by putting them in ${argv}
after ${argv[(i)--]}, etc..

Cheers,

Daniel

Re: completion match ordering

[dana]

On 25 Nov 2018, at 21:09, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>In general, I would default to assuming we should worry about
>compatibility, unless there's a specific reason not to.  (As a user,
>I assume that my zsh code is forward compatible with new zsh releases
>unless something is specifically documented to be an exception.)

Just in case you missed the background, Oliver found that -o was added
specifically to be used in _list_files, and i couldn't find anywhere else in the
main repo nor in the zsh-completions one that uses it. It is documented, though,
so i guess that's bothersome.

On 25 Nov 2018, at 21:09, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>No, what they *should* be doing is passing arguments transparently,
>without parsing and reserializing them, so when we add "-$letter"
>options to compadd we can do so by changing just one place (namely, the
>C struct declaring the builtin).  Probably by putting the compadd
>options in an array and passing its name, or by putting them in ${argv}
>after ${argv[(i)--]}, etc..

I like the idea of putting them in an array, that would alleviate some anxiety
i've had about this very issue. compadd has so many options, it becomes really
irritating to work around them in helper functions.

On 25 Nov 2018, at 19:25, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>The patch below is an initial experiment for what I was suggesting.

I haven't really looked at this yet but i will soon. Thank you for working on
it!

dana

Re: completion match ordering

[Peter Stephenson]

On Mon, 2018-11-26 at 02:25 +0100, Oliver Kiddle wrote:
> On 7 Nov, I wrote:
> > 
> > You can do:
> >   _arguments '-b-:level: compadd "${(@)expl/#-J/-2V}" ${(on)levels}'
> > But what you have is perhaps better, especially as it's duplicated for
> > -b and -e.
> The patch below is an initial experiment for what I was suggesting.
> It allows, e.g:
>   _arguments '-b-:level:compadd -o numeric -a levels'
> 
> Any thoughts on the interface? Is something more terse like -on
> preferable? The -V option becomes superfluous (but remains for
> compatibility). Instead of -V grp you can use -J grp -o nosort.

Thanks, this certainly looks useful.

Terseness has it's uses --- some of the completion syntax would be very
wordy otherwise --- but I think spelling it out is reasonable here.

> Do we need to worry about this breaking uses of the old -o?

Ideally, I suppose, yes, even if it's not widely used.  The more obscure
the use, the harder to track down if it does break something...

pws

Re: completion match ordering

[dana]

On 25 Nov 2018, at 19:25, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>Any thoughts on the interface? Is something more terse like -on
>preferable?

Experimenting with it in (what would be) 'real-world' applications, i do find it
kind of wordy, and if/when additional ordering options (like reverse) are added
it'll be even more so. Maybe a compromise would be possible, where the ordering
options can be abbreviated like -onum,rev? I could be over-complicating it,
though.

On 25 Nov 2018, at 19:25, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>Besides match, nosort and numeric, should any other orderings be
>supported? Reverse or case-sensitive perhaps?

I like the idea of reverse and case modifiers as i mentioned in our other
conversation. But i suppose in a lot of cases it'll be simple enough to just do
`compadd -o nosort - ${(Oi)foo}` or whatever, so... it'd be cool, but not
necessarily critical.

Other than that, i didn't test *super* extensively, but it seems to work well
for the sort of thing that inspired my original question (keeping numbers in the
right order, as in _lz4).

dana

PATCH: completion match ordering

[Oliver Kiddle]

I finally got around to finishing off the changes to the options for completion
match ordering via compadd -o. Thanks to dana for reminding me about this.

The argument to -o is now optional and can be abbreviated, e.g. -omat,rev
The sort style also accepts values like "numeric" and documentation and
tests have been added.

_description, _all_labels etc all take -V, -1, -2 options and in the previous
patch, I had added -o to that. However, as far as I can tell that only has any
use in the case of -V because the functions need to decide whether to add -J
group or -V group. So it was only needed because the interface conflates the
use of -V - specifying the group name and making it unsorted. I guess -1 and -2
were handled because it is common to combine them as -1V, -2J etc. Given that
it'd be useless, I decided not to add it.

Back when sort was added to _description in 18859, it was decided to have it
explicitly ignore the style if -V was passed. While, I would tend towards the
view that it is better to give users control anyway, it remains that way for
now. You don't need to pass -V to _wanted as such. To allow the style to enable
sorting, we would need to add a compadd -o value corresponding to normal sort
order: perhaps `sort' or `lexical' (i.e. not numeric). We could then rely on
compadd honouring only the first -o option passed to it.

The combination nosort,reverse could perhaps preserve (but reverse) the
original order.

We could perhaps also add the backslash ignoring that dana recently restored as
an option to -o. In my testing, that patch looks good by the way - thanks dana.

Oliver

diff --git a/Completion/Base/Core/_description b/Completion/Base/Core/_description
index 304c747a6..c2a0e080b 100644
--- a/Completion/Base/Core/_description
+++ b/Completion/Base/Core/_description
@@ -1,13 +1,13 @@
 #autoload
 
-local name gropt nopt xopt format gname hidden hide match opts tag sort
+local name nopt xopt format gname hidden hide match opts tag
+local -a gropt sort
 
 opts=()
 
-gropt=(-J)
 xopt=(-X)
 nopt=()
-zparseopts -K -D -a nopt 1 2 V=gropt J=gropt x=xopt
+zparseopts -K -D -a nopt 1 2 V=gropt J x=xopt
 
 3="${${3##[[:blank:]]#}%%[[:blank:]]#}"
 [[ -n "$3" ]] && _lastdescr=( "$_lastdescr[@]" "$3" )
@@ -33,14 +33,18 @@ zstyle -s ":completion:${curcontext}:$1" matcher match &&
 
 # Use sort style, but ignore `menu' value to help _expand.
 # Also don't override explicit use of -V.
-if { zstyle -s ":completion:${curcontext}:$1" sort sort ||
-     zstyle -s ":completion:${curcontext}:" sort sort; } &&
-    [[ "$gropt" = -J && $sort != menu ]]; then
-    if [[ "$sort" = (yes|true|1|on) ]]; then
-	gropt=(-J)
-    else
-	gropt=(-V)
+if [[ -z "$gropt" ]]; then
+  if zstyle -a ":completion:${curcontext}:$1" sort sort ||
+     zstyle -a ":completion:${curcontext}:" sort sort
+  then
+    if [[ -z "${(@)sort:#(match|numeric|reverse)}" ]]; then
+      gropt=( -o ${(j.,.)sort} )
+    elif [[ "$sort" != (yes|true|1|on|menu) ]]; then
+      gropt=( -o nosort )
     fi
+  fi
+else
+  gropt=( -o nosort )
 fi
 
 if [[ -z "$_comp_no_ignore" ]]; then
@@ -79,15 +83,15 @@ fi
 
 if [[ -n "$gname" ]]; then
   if [[ -n "$format" ]]; then
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" "$gname" "$xopt" "$format"
+    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt[@]" -J "$gname" "$xopt" "$format"
   else
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" "$gname"
+    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt[@]" -J "$gname"
   fi
 else
   if [[ -n "$format" ]]; then
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" -default- "$xopt" "$format"
+    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt[@]" -J -default- "$xopt" "$format"
   else
-    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt" -default-
+    set -A "$name" "$opts[@]" "$nopt[@]" "$gropt[@]" -J -default-
   fi
 fi
 
diff --git a/Completion/Base/Utility/_describe b/Completion/Base/Utility/_describe
index 76ab1d995..c12eb0eab 100644
--- a/Completion/Base/Utility/_describe
+++ b/Completion/Base/Utility/_describe
@@ -108,10 +108,10 @@ while _tags; do
         fi
     
         if [[ -n $_mats ]]; then
-          compadd "$_opts[@]" "${(@)_expl:/-J/-2V}" -D $_strs -O $_mats - \
+          compadd "$_opts[@]" -2 -o nosort "${_expl[@]}" -D $_strs -O $_mats - \
                   "${(@)${(@M)${(@P)_mats}##([^:\\]|\\?)##}//\\(#b)(?)/$match[1]}"
         else
-          compadd "$_opts[@]" "${(@)_expl:/-J/-2V}" -D $_strs - \
+          compadd "$_opts[@]" -2 -o nosort "${_expl[@]}" -D $_strs - \
                   "${(@)${(@M)${(@P)_strs}##([^:\\]|\\?)##}//\\(#b)(?)/$match[1]}"
         fi
       done
diff --git a/Completion/Base/Utility/_guard b/Completion/Base/Utility/_guard
index ff62981ce..1cbd4f39b 100644
--- a/Completion/Base/Utility/_guard
+++ b/Completion/Base/Utility/_guard
@@ -2,7 +2,7 @@
 
 local garbage
 
-zparseopts -K -D -a garbage M: J: V: 1 2 n F: X:
+zparseopts -K -D -a garbage M+: J+: V+: 1 2 o+: n F: X+:
 
 [[ "$PREFIX$SUFFIX" != $~1 ]] && return 1
 
diff --git a/Completion/Base/Utility/_multi_parts b/Completion/Base/Utility/_multi_parts
index 12ff965ed..00e3e26cc 100644
--- a/Completion/Base/Utility/_multi_parts
+++ b/Completion/Base/Utility/_multi_parts
@@ -14,8 +14,8 @@ typeset -U tmp1 matches
 # Get the options.
 
 zparseopts -D -a sopts \
-    'J+:=group' 'V+:=group' 'X+:=expl' 'P:=opts' 'F:=opts' \
-    S: r: R: q 1 2 n f 'M+:=matcher' 'i=imm'
+    'J+:=group' 'V+:=group' 'x+:=expl' 'X+:=expl' 'P:=opts' 'F:=opts' \
+    S: r: R: q 1 2 o+: n f 'M+:=matcher' 'i=imm'
 
 sopts=( "$sopts[@]" "$opts[@]" )
 if (( $#matcher )); then
diff --git a/Completion/Base/Utility/_sep_parts b/Completion/Base/Utility/_sep_parts
index de836a696..6fcf54ec4 100644
--- a/Completion/Base/Utility/_sep_parts
+++ b/Completion/Base/Utility/_sep_parts
@@ -22,8 +22,8 @@ local matchflags opt group expl nm=$compstate[nmatches] opre osuf opts matcher
 
 # Get the options.
 
-zparseopts -D -a opts \
-    'J+:=group' 'V+:=group' P: F: S: r: R: q 1 2 n 'X+:=expl' 'M+:=matcher'
+zparseopts -D -a opts 'J+:=group' 'V+:=group' P: F: S: r: R: q 1 2 o+: n \
+    'x+:=expl' 'X+:=expl' 'M+:=matcher'
 
 # Get the string from the line.
 
diff --git a/Completion/Base/Utility/_sequence b/Completion/Base/Utility/_sequence
index f52955f46..101934490 100644
--- a/Completion/Base/Utility/_sequence
+++ b/Completion/Base/Utility/_sequence
@@ -10,7 +10,8 @@
 local curcontext="$curcontext" nm="$compstate[nmatches]" pre qsep nosep minus
 local -a sep num pref suf end uniq dedup
 
-zparseopts -D -a opts s:=sep n:=num p:=pref i:=pref P:=pref I:=suf S:=suf q=suf r:=suf R:=suf C:=cont d=uniq M: J: X: x:
+zparseopts -D -a opts s:=sep n:=num p:=pref i:=pref P:=pref I:=suf S:=suf \
+    q=suf r:=suf R:=suf C:=cont d=uniq M+: J+: V+: 1 2 o+: X+: x+:
 (( $#cont )) && curcontext="${curcontext%:*}:$cont[2]"
 (( $#sep )) || sep[2]=,
 
diff --git a/Completion/Darwin/Type/_mac_files_for_application b/Completion/Darwin/Type/_mac_files_for_application
index 299d8ff5c..44516b4db 100644
--- a/Completion/Darwin/Type/_mac_files_for_application
+++ b/Completion/Darwin/Type/_mac_files_for_application
@@ -35,7 +35,7 @@ _mac_parse_info_plist() {
 # Try to complete files for the specified application.
 _mac_files_for_application() {
   local -a opts
-  zparseopts -D -a opts q n 1 2 P: S: r: R: W: X+: M+: F: J+: V+:
+  zparseopts -D -a opts q n 1 2 o+: P: S: r: R: W: x+: X+: M+: F: J+: V+:
 
   local app_path
   _retrieve_mac_apps
diff --git a/Completion/Redhat/Command/_yum b/Completion/Redhat/Command/_yum
index 34a337109..8abd23ebb 100644
--- a/Completion/Redhat/Command/_yum
+++ b/Completion/Redhat/Command/_yum
@@ -212,7 +212,7 @@ _yum_ids() {
   # `${(@)@[...]}' selects a subrange from $@
   # `${(@)@[1,-2]}' are all except the last argument
   # `$@[$#]' is the last argument, e.g. the first suggestable ID
-  compadd "${(@)@[1,-2]:/-J/-V}" -M "B:0=" {$@[$#]..$maxid}
+  compadd "${(@)@[1,-2]}" -o numeric -M "B:0=" {$@[$#]..$maxid}
 }
 
 _yum_ranges() {
diff --git a/Completion/Unix/Command/_git b/Completion/Unix/Command/_git
index 4eb07e921..112098d3a 100644
--- a/Completion/Unix/Command/_git
+++ b/Completion/Unix/Command/_git
@@ -5688,7 +5688,7 @@ __git_ignore_line () {
 __git_ignore_line_inside_arguments () {
   declare -a compadd_opts
 
-  zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
+  zparseopts -D -E -a compadd_opts V+: J+: 1 2 o+: n f x+: X+: M+: P: S: r: R: q F:
 
   __git_ignore_line $* $compadd_opts
 }
@@ -6160,7 +6160,7 @@ __git_ref_fields () {
   local match mbegin mend
   local -a cfields fields append opts all
 
-  zparseopts -D -E -a opts x: X: J: V: a=all
+  zparseopts -D -E -a opts M+: x+: X+: J+: V+: o+: 1 2 a=all
 
   if compset -P 1 '(#b)(*):'; then
     case $match[1] in
@@ -6731,7 +6731,7 @@ __git_tags_of_type () {
   tags=(${${(M)${(f)"$(_call_program ${(q)type}-tag-refs "git for-each-ref --format='%(*objecttype)%(objecttype) %(refname)' refs/tags 2>/dev/null")"}:#$type(tag|) *}#$type(tag|) refs/tags/})
   __git_command_successful $pipestatus || return 1
 
-  _wanted $type-tags expl "$type tag" compadd -M 'r:|/=* r:|=*' "$@" -a - tags
+  _wanted $type-tags expl "$type tag" compadd -M 'r:|/=* r:|=*' "$@" -o numeric -a - tags
 }
 
 # Reference Argument Types
@@ -6826,7 +6826,7 @@ __git_files_relative () {
 __git_files () {
   local compadd_opts opts tag description gitcdup gitprefix files expl
 
-  zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
+  zparseopts -D -E -a compadd_opts V+: J+: 1 2 o+: n f x+: X+: M+: P: S: r: R: q F:
   zparseopts -D -E -a opts -- -cached -deleted -modified -others -ignored -unmerged -killed x+: --exclude+:
   tag=$1 description=$2; shift 2
 
@@ -6957,7 +6957,7 @@ __git_tree_files () {
     shift
   fi
 
-  zparseopts -D -E -a compadd_opts V: J: 1 2 n f X: M: P: S: r: R: q F:
+  zparseopts -D -E -a compadd_opts V+: J+: 1 2 o+: n f x+: X+: M+: P: S: r: R: q F:
 
   [[ "$1" == */ ]] && Path="$1" || Path="${1:h}/"
   shift
@@ -7037,7 +7037,7 @@ __git_any_repositories_or_references () {
 __git_guard () {
   declare -A opts
 
-  zparseopts -K -D -A opts M: J: V: 1 2 n F: X:
+  zparseopts -K -D -A opts M+: J+: V+: 1 2 o+: n F: x+: X+:
 
   [[ "$PREFIX$SUFFIX" != $~1 ]] && return 1
 
@@ -7075,7 +7075,7 @@ __git_guard_diff-stat-width () {
 __git_guard_number () {
   declare -A opts
 
-  zparseopts -K -D -A opts M: J: V: 1 2 n F: X:
+  zparseopts -K -D -A opts M+: J+: V+: 1 2 o+: n F: x+: X+:
 
   _guard '[[:digit:]]#' ${1:-number}
 }
diff --git a/Completion/Unix/Type/_baudrates b/Completion/Unix/Type/_baudrates
index add359d13..6e9ba4d97 100644
--- a/Completion/Unix/Type/_baudrates
+++ b/Completion/Unix/Type/_baudrates
@@ -72,7 +72,6 @@ if (( ${+opts[-f]} )); then
   done
 fi
 
-# -1V removes dupes (which there shouldn't be) and otherwise leaves the
-# order in the $rates array intact.
-_description -1V baud-rates expl 'baud rate'
-compadd "${(@)argv/#-J/-V}" "$expl[@]" -- "${rates[@]}"
+# -1 removes dupes (which there shouldn't be)
+_description -1 -o numeric baud-rates expl 'baud rate'
+compadd "${argv[@]}" "$expl[@]" -- "${rates[@]}"
diff --git a/Completion/Unix/Type/_canonical_paths b/Completion/Unix/Type/_canonical_paths
index 6eab7b677..cddc3b405 100644
--- a/Completion/Unix/Type/_canonical_paths
+++ b/Completion/Unix/Type/_canonical_paths
@@ -5,13 +5,14 @@
 # (relative path when an absolute path is given, and vice versa; when ..'s are
 # present in the word to be completed, and some paths got from symlinks).
 
-# Usage: _canonical_paths [-A var] [-N] [-MJV12nfX] tag desc [paths...]
+# Usage: _canonical_paths [-A var] [-N] [-MJV12onfX] tag desc [paths...]
 
 # -A, if specified, takes the paths from the array variable specified. Paths
 # can also be specified on the command line as shown above. -N, if specified,
 # prevents canonicalizing the paths given before using them for completion, in
 # case they are already so. `tag' and `desc' arguments are well, obvious :) In
-# addition, the options -M, -J, -V, -1, -2, -n, -F, -X are passed to compadd.
+# addition, the options -M, -J, -V, -1, -2, -o, -n, -F, -x, -X are passed to
+# compadd.
 
 _canonical_paths_add_paths () {
   # origpref = original prefix
@@ -59,7 +60,7 @@ _canonical_paths() {
   local __index
   typeset -a __gopts __opts
 
-  zparseopts -D -a __gopts M: J: V: 1 2 n F: X: A:=__opts N=__opts
+  zparseopts -D -a __gopts M+: J+: V+: o+: 1 2 n F: x+: X+: A:=__opts N=__opts
 
   : ${1:=canonical-paths} ${2:=path}
 
diff --git a/Completion/Unix/Type/_files b/Completion/Unix/Type/_files
index 467ed747c..6adaa8154 100644
--- a/Completion/Unix/Type/_files
+++ b/Completion/Unix/Type/_files
@@ -27,7 +27,7 @@ local opts tmp glob pat pats expl tag i def descr end ign tried
 local type sdef ignvars ignvar prepath oprefix rfiles rfile
 
 zparseopts -a opts \
-    '/=tmp' 'f=tmp' 'g+:-=tmp' q n 1 2 P: S: r: R: W: X+: M+: F: J+: V+:
+    '/=tmp' 'f=tmp' 'g+:-=tmp' q n 1 2 P: S: r: R: W: x+: X+: M+: F: J+: V+: o+:
 
 type="${(@j::M)${(@)tmp#-}#?}"
 if (( $tmp[(I)-g*] )); then
diff --git a/Completion/Unix/Type/_list_files b/Completion/Unix/Type/_list_files
index 6c52bc1f4..0ea02a55a 100644
--- a/Completion/Unix/Type/_list_files
+++ b/Completion/Unix/Type/_list_files
@@ -64,6 +64,6 @@ for f in ${(PQ)1}; do
  ${(r:8:)stat[gid]} ${(l:8:)stat[size]} $stat[mtime] $f")
 done
 
-(( ${#listfiles} )) && listopts=(-d listfiles -l -o)
+(( ${#listfiles} )) && listopts=(-d listfiles -l -o match)
 
 return 0
diff --git a/Completion/Unix/Type/_path_files b/Completion/Unix/Type/_path_files
index 1021c34e6..19ae59629 100644
--- a/Completion/Unix/Type/_path_files
+++ b/Completion/Unix/Type/_path_files
@@ -59,7 +59,7 @@ exppaths=()
 zparseopts -a mopts \
     'P:=pfx' 'S:=pfxsfx' 'q=pfxsfx' 'r:=pfxsfx' 'R:=pfxsfx' \
     'W:=prepaths' 'F:=ignore' 'M+:=matcher' \
-    J+: V+: X+: 1 2 n 'f=tmp1' '/=tmp1' 'g+:-=tmp1'
+    J+: V+: x+: X+: 1 2 o+: n 'f=tmp1' '/=tmp1' 'g+:-=tmp1'
 
 sopt="-${(@j::M)${(@)tmp1#-}#?}"
 (( $tmp1[(I)-[/g]*] )) && haspats=yes
@@ -168,7 +168,7 @@ if zstyle -s ":completion:${curcontext}:" file-sort tmp1; then
   if [[ "$sort" = on ]]; then
     sort=
   else
-    mopts=( "${(@)mopts/#-J/-V}" )
+    mopts=( -o nosort "${mopts[@]}" )
 
     tmp2=()
     for tmp1 in "$pats[@]"; do
diff --git a/Completion/Zsh/Command/_compadd b/Completion/Zsh/Command/_compadd
index e709e400e..781fa2af8 100644
--- a/Completion/Zsh/Command/_compadd
+++ b/Completion/Zsh/Command/_compadd
@@ -14,9 +14,13 @@ _arguments -C -s -S -A "-*" \
   '(-a)-k[matches are keys of specified associative arrays]' \
   '-d+[specify display strings]:array:_parameters -g "*array*"' \
   '-l[list display strings one per line, not in columns]' \
-  '-o[order matches by match string not by display string]' \
-  '(-1 -E)-J+[specify match group which will be sorted]:group' \
-  '-V+[specify pre-ordered match group]:group' \
+  '-o[specify order for matches by match string not by display string]:: : _values -s , order
+    "match[order by match not by display string]"
+    "nosort[matches are pre-ordered]"
+    "numeric[order numerically]"
+    "reverse[order backwards]"' \
+  '(-1 -E)-J+[specify match group]:group' \
+  '!-V+:group' \
   '(-J -E)-1[remove only consecutive duplicates from group]' \
   '-2[preserve all duplicates]' \
   '(-x)-X[specify explanation]:explanation' \
@@ -45,7 +49,7 @@ if [[ -n $state ]]; then
   elif (( $+opt_args[-k] )); then
     _parameters -g "*assoc*" && ret=0
   else
-    _message -e candidate candidates
+    _message -e candidates candidate
   fi
 fi
 
diff --git a/Completion/Zsh/Type/_file_descriptors b/Completion/Zsh/Type/_file_descriptors
index 3e9f4968b..be96dd0e4 100644
--- a/Completion/Zsh/Type/_file_descriptors
+++ b/Completion/Zsh/Type/_file_descriptors
@@ -56,4 +56,4 @@ fi
 fds=( 0 1 2 $fds )
 
 _description -V file-descriptors expl 'file descriptor'
-compadd $disp "${@/-J/-V}" "$expl[@]" -a fds
+compadd $disp -o nosort "$@" "$expl[@]" -a fds
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index 8b00517b7..66c476fab 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -2534,20 +2534,20 @@ is started, making it easy to select either of them.
 )
 kindex(sort, completion style)
 item(tt(sort))(
-Many completion widgets call tt(_description) at some point which
-decides whether the matches are added sorted or unsorted (often
-indirectly via tt(_wanted) or tt(_requested)).  This style can be set
-explicitly to one of the usual `true' or `false' values as an override.
-If it is not set for the context, the standard behaviour of the
-calling widget is used.
+This allows the standard ordering of matches to be overridden.
+
+If its value is `tt(true)' or `tt(false)', sorting is enabled or disabled.
+Additionally the values associated with the `tt(-o)' option to tt(compadd) can
+also be listed: tt(match), tt(nosort), tt(numeric), tt(reverse).  If it is not
+set for the context, the standard behaviour of the calling widget is used.
 
 The style is tested first against the full context including the tag, and
 if that fails to produce a value against the context without the tag.
 
-If the calling widget explicitly requests unsorted matches, this is usually
-honoured.  However, the default (unsorted) behaviour of completion
-for the command history may be overridden by setting the style to
-`true'.
+In many cases where a calling widget explicitly selects a particular ordering
+in lieu of the default, a value of `tt(true)' is not honoured.  An example of
+where this is not the case is for command history where the default of sorting
+matches chronologically may be overridden by setting the style to `true'.
 
 In the tt(_expand) completer, if it is set to
 `true', the expansions generated will always be sorted.  If it is set
@@ -4404,11 +4404,11 @@ convention is not enforced).  The description for the corresponding set
 of matches is passed to the function in var(descr).
 
 The styles tested are: tt(format), tt(hidden), tt(matcher),
-tt(ignored-patterns) and tt(group-name).  The tt(format) style is first
-tested for the given var(tag) and then for the tt(descriptions) tag if
-no value was found, while the remainder are only tested for the tag
-given as the first argument.  The function also calls tt(_setup)
-which tests some more styles.
+tt(ignore-line), tt(ignored-patterns), tt(group-name) and tt(sort).
+The tt(format) style is first tested for the given var(tag) and then for
+the tt(descriptions) tag if no value was found, while the remainder are
+only tested for the tag given as the first argument.  The function also
+calls tt(_setup) which tests some more styles.
 
 The string returned by the tt(format) style (if any) will be modified so
 that the sequence `tt(%d)' is replaced by the var(descr) given as the third
diff --git a/Doc/Zsh/compwid.yo b/Doc/Zsh/compwid.yo
index 1cc94bf95..0d8d4cc40 100644
--- a/Doc/Zsh/compwid.yo
+++ b/Doc/Zsh/compwid.yo
@@ -446,12 +446,13 @@ startitem()
 findex(compadd)
 cindex(completion widgets, adding specified matches)
 redef(SPACES)(0)(tt(ifztexi(NOTRANS(@ @ @ @ @ @ @ @ ))ifnztexi(        )))
-xitem(tt(compadd )[ tt(-akqQfenUlo12C) ] [ tt(-F) var(array) ])
+xitem(tt(compadd )[ tt(-akqQfenUl12C) ] [ tt(-F) var(array) ])
 xitem(SPACES()[tt(-P) var(prefix) ] [ tt(-S) var(suffix) ])
 xitem(SPACES()[tt(-p) var(hidden-prefix) ] [ tt(-s) var(hidden-suffix) ])
 xitem(SPACES()[tt(-i) var(ignored-prefix) ] [ tt(-I) var(ignored-suffix) ])
 xitem(SPACES()[tt(-W) var(file-prefix) ] [ tt(-d) var(array) ])
-xitem(SPACES()[tt(-J) var(name) ] [ tt(-V) var(name) ] [ tt(-X) var(explanation) ] [ tt(-x) var(message) ])
+xitem(SPACES()[tt(-J) var(group-name) ] [ tt(-X) var(explanation) ] [ tt(-x) var(message) ])
+xitem(SPACES()[tt(-V) var(group-name) ] [ tt(-o) [ var(order) ] ])
 xitem(SPACES()[tt(-r) var(remove-chars) ] [ tt(-R) var(remove-func) ])
 xitem(SPACES()[tt(-D) var(array) ] [ tt(-O) var(array) ] [ tt(-A) var(array) ])
 xitem(SPACES()[tt(-E) var(number) ])
@@ -540,18 +541,40 @@ This option only has an effect if used together with the tt(-d)
 option. If it is given, the display strings are listed one per line,
 not arrayed in columns.
 )
-item(tt(-o))(
-This option only has an effect if used together with the tt(-d)
-option.  If it is given, the order of the output is determined by the
-match strings;  otherwise it is determined by the display strings
-(i.e. the strings given by the tt(-d) option).
+item(tt(-o) [ var(order) ])(
+This controls the order in which matches are sorted. var(order) is a
+comma-separated list comprising the following possible values.  These values
+can be abbreviated to their initial two or three characters.  Note that the
+order forms part of the group name space so matches with different orderings
+will not be in the same group.
+
+startitem()
+item(tt(match))(
+If given, the order of the output is determined by the match strings;
+otherwise it is determined by the display strings (i.e. the strings given
+by the tt(-d) option). This is the default if `tt(-o)' is specified but
+the var(order) argument is omitted.
 )
-item(tt(-J) var(name))(
+item(tt(nosort))(
+This specifies that the matches are pre-sorted and their order should be
+preserved.  This value only makes sense alone and cannot be combined with any
+others.
+)
+item(tt(numeric))(
+If the matches include numbers, sort them numerically rather than
+lexicographically.
+)
+item(tt(reverse))(
+Arrange the matches backwards by reversing the sort ordering.
+)
+enditem()
+)
+item(tt(-J) var(group-name))(
 Gives the name of the group of matches the words should be stored in.
 )
-item(tt(-V) var(name))(
-Like tt(-J) but naming an unsorted group. These are in a different name
-space than groups created with the tt(-J) flag.
+item(tt(-V) var(group-name))(
+Like tt(-J) but naming an unsorted group. This option is identical to
+the combination of tt(-J) and tt(-o nosort).
 )
 item(tt(-1))(
 If given together with the tt(-V) option, makes
diff --git a/NEWS b/NEWS
index 7e9ec05ff..ec20b4982 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,11 @@ functions may wish to familiarise themselves with `_normal -p` and
 The option CD_SILENT was added to suppress all output from cd (whether
 explicit or implicit with AUTO_CD). It is disabled by default.
 
+The compadd builtin's -o option now takes an optional argument to
+specify the order of completion matches. This affects the display
+of candidate matches and the order in which they are selected when
+cycling between them using menu completion.
+
 Changes from 5.6.2 to 5.7.1
 ---------------------------
 
diff --git a/Src/Zle/comp.h b/Src/Zle/comp.h
index 3e9834560..743a2e3ac 100644
--- a/Src/Zle/comp.h
+++ b/Src/Zle/comp.h
@@ -90,6 +90,9 @@ struct cmgroup {
 #define CGF_PACKED  32		/* LIST_PACKED for this group */
 #define CGF_ROWS    64		/* LIST_ROWS_FIRST for this group */
 #define CGF_FILES   128		/* contains file names */
+#define CGF_MATSORT 256		/* sort by match rather than by display string */
+#define CGF_NUMSORT 512		/* sort numerically */
+#define CGF_REVSORT 1024	/* sort in reverse */
 
 /* This is the struct used to hold matches. */
 
@@ -300,6 +303,9 @@ struct menuinfo {
 #define CAF_ARRAYS  32    /* compadd -a or -k: array/assoc parameter names */
 #define CAF_KEYS    64    /* compadd -k: assoc parameter names */
 #define CAF_ALL    128    /* compadd -C: _all_matches */
+#define CAF_MATSORT 256   /* compadd -o match: sort by match rather than by display string */
+#define CAF_NUMSORT 512   /* compadd -o numeric: sort numerically */
+#define CAF_REVSORT 1024  /* compadd -o numeric: sort in reverse */
 
 /* Data for compadd and addmatches() */
 
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 0a454ad5f..12c70e46e 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -2080,6 +2080,9 @@ addmatches(Cadata dat, char **argv)
 
         /* Select the group in which to store the matches. */
         gflags = (((dat->aflags & CAF_NOSORT ) ? CGF_NOSORT  : 0) |
+                  ((dat->aflags & CAF_MATSORT) ? CGF_MATSORT : 0) |
+                  ((dat->aflags & CAF_NUMSORT) ? CGF_NUMSORT : 0) |
+                  ((dat->aflags & CAF_REVSORT) ? CGF_REVSORT : 0) |
                   ((dat->aflags & CAF_UNIQALL) ? CGF_UNIQALL : 0) |
                   ((dat->aflags & CAF_UNIQCON) ? CGF_UNIQCON : 0));
         if (dat->group) {
@@ -3034,8 +3037,9 @@ begcmgroup(char *n, int flags)
 		HEAP_ERROR(p->heap_id);
 	    }
 #endif
-	    if (p->name &&
-		flags == (p->flags & (CGF_NOSORT|CGF_UNIQALL|CGF_UNIQCON)) &&
+	    if (p->name && flags ==
+		(p->flags & (CGF_NOSORT|CGF_UNIQALL|CGF_UNIQCON|
+			     CGF_MATSORT|CGF_NUMSORT|CGF_REVSORT)) &&
 		!strcmp(n, p->name)) {
 		mgroup = p;
 
@@ -3118,32 +3122,34 @@ addexpl(int always)
 
 /* The comparison function for matches (used for sorting). */
 
+static int matchorder;
+
 /**/
 static int
 matchcmp(Cmatch *a, Cmatch *b)
 {
-    if ((*a)->disp && !((*a)->flags & CMF_MORDER)) {
-	if ((*b)->disp) {
-	    if ((*a)->flags & CMF_DISPLINE) {
-		if ((*b)->flags & CMF_DISPLINE)
-		    return strcmp((*a)->disp, (*b)->disp);
-		else
-		    return -1;
-	    } else {
-		if ((*b)->flags & CMF_DISPLINE)
-		    return 1;
-		else
-		    return strcmp((*a)->disp, (*b)->disp);
-	    }
-	}
-	return -1;
-    }
-    if ((*b)->disp && !((*b)->flags & CMF_MORDER))
-	return 1;
+    const char *as, *bs;
+    int cmp = !!(*b)->disp - !!(*a)->disp;
+    int sortdir = (matchorder & CGF_REVSORT) ? -1 : 1;
 
-    return zstrcmp((*a)->str, (*b)->str, (SORTIT_IGNORING_BACKSLASHES|
-					  (isset(NUMERICGLOBSORT) ?
-					   SORTIT_NUMERICALLY : 0)));
+    /* if match sorting selected or we have no display strings */
+    if ((matchorder & CGF_MATSORT) || (!cmp && !(*a)->disp)) {
+	as = (*a)->str;
+	bs = (*b)->str;
+    } else {
+        if (cmp) /* matches with display strings come first */
+	    return cmp;
+
+	cmp = ((*b)->flags & CMF_DISPLINE) - ((*a)->flags & CMF_DISPLINE);
+        if (cmp) /* sort one-per-line display strings first */
+	    return cmp;
+
+	as = (*a)->disp;
+	bs = (*b)->disp;
+    }
+
+    return sortdir * zstrcmp(as, bs, (isset(NUMERICGLOBSORT) ||
+	    matchorder & CGF_NUMSORT) ? SORTIT_NUMERICALLY : 0);
 }
 
 /* This tests whether two matches are equal (would produce the same
@@ -3205,6 +3211,7 @@ makearray(LinkList l, int type, int flags, int *np, int *nlp, int *llp)
     } else {
 	if (!(flags & CGF_NOSORT)) {
 	    /* Now sort the array (it contains matches). */
+	    matchorder = flags;
 	    qsort((void *) rp, n, sizeof(Cmatch),
 		  (int (*) _((const void *, const void *)))matchcmp);
 
diff --git a/Src/Zle/complete.c b/Src/Zle/complete.c
index 1dc2b01c2..c2f46c7f5 100644
--- a/Src/Zle/complete.c
+++ b/Src/Zle/complete.c
@@ -558,12 +558,53 @@ parse_class(Cpattern p, char *iptr)
     return iptr;
 }
 
+static struct { char *name; int abbrev; int oflag; } orderopts[] = {
+    { "nosort", 2, CAF_NOSORT },
+    { "match", 3, CAF_MATSORT },
+    { "numeric", 3, CAF_NUMSORT },
+    { "reverse", 3, CAF_REVSORT }
+};
+
+/* Parse the option to compadd -o, if flags is non-NULL set it
+ * returns -1 if the argument isn't a valid ordering, 0 otherwise */
+
+/**/
+static int
+parse_ordering(const char *arg, int *flags)
+{
+    int o, fl = 0;
+    const char *next, *opt = arg;
+    do {
+	int found = 0;
+	next = strchr(opt, ',');
+	if (!next)
+	    next = opt + strlen(opt);
+
+	for (o = sizeof(orderopts)/sizeof(*orderopts) - 1; o >= 0 &&
+		!found; --o)
+	{
+	    if ((found = next - opt >= orderopts[o].abbrev &&
+	            !strncmp(orderopts[o].name, opt, next - opt)))
+		fl |= orderopts[o].oflag;
+	}
+	if (!found) {
+	    if (flags) /* default to "match" */
+		*flags = CAF_MATSORT;
+	    return -1;
+	}
+    } while (*next && ((opt = next + 1)));
+    if (flags)
+	*flags |= fl;
+    return 0;
+}
+
 /**/
 static int
 bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 {
     struct cadata dat;
     char *mstr = NULL; /* argument of -M options, accumulated */
+    char *oarg = NULL; /* argument of -o option */
     int added; /* return value */
     Cmatcher match = NULL;
 
@@ -572,7 +613,7 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	return 1;
     }
     dat.ipre = dat.isuf = dat.ppre = dat.psuf = dat.prpre = dat.mesg =
-	dat.pre = dat.suf = dat.group = dat.rems = dat.remf = dat.disp = 
+	dat.pre = dat.suf = dat.group = dat.rems = dat.remf = dat.disp =
 	dat.ign = dat.exp = dat.apar = dat.opar = dat.dpar = NULL;
     dat.match = NULL;
     dat.flags = 0;
@@ -587,6 +628,7 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	}
 	for (p = *argv + 1; *p; p++) {
 	    char *m = NULL; /* argument of -M option (this one only) */
+	    int order = 0;  /* if -o found (argument to which is optional) */
 	    char **sp = NULL; /* the argument to an option should be copied
 				 to *sp. */
 	    const char *e; /* error message */
@@ -710,7 +752,11 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 		dat.flags |= CMF_DISPLINE;
 		break;
 	    case 'o':
-		dat.flags |= CMF_MORDER;
+		/* we honour just the first -o option but need to skip
+		 * over a valid argument to subsequent -o options */
+		order = oarg ? -1 : 1;
+		sp = &oarg;
+		/* no error string because argument is optional */
 		break;
 	    case 'E':
                 if (p[1]) {
@@ -741,15 +787,18 @@ bin_compadd(char *name, char **argv, UNUSED(Options ops), UNUSED(int func))
 	    if (sp) {
 		if (p[1]) {
 		    /* Pasted argument: -Xfoo. */
-		    if (!*sp)
+		    if (!*sp) /* take first option only */
 			*sp = p + 1;
-		    p += strlen(p+1);
+		    if (!order || !parse_ordering(oarg, order == 1 ? &dat.aflags : NULL))
+			p += strlen(p+1);
 		} else if (argv[1]) {
 		    /* Argument in a separate word: -X foo. */
 		    argv++;
 		    if (!*sp)
 			*sp = *argv;
-		} else {
+		    if (order && parse_ordering(oarg, order == 1 ? &dat.aflags : NULL))
+			--argv;
+		} else if (!order) {
 		    /* Missing argument: argv[N] == "-X", argv[N+1] == NULL. */
 		    zwarnnam(name, e, *p);
 		    zsfree(mstr);
diff --git a/Test/Y01completion.ztst b/Test/Y01completion.ztst
index b1c0e40e5..901556167 100644
--- a/Test/Y01completion.ztst
+++ b/Test/Y01completion.ztst
@@ -116,6 +116,46 @@ F:regression test workers/31611
 0:allow for suffixes when moving cursor to end of match (without ignored suffix)
 >line: {tst word:/}{}
 
+  comptesteval "_tst() { compadd -onum,rev -J versions r1.10 r1.1 r1.2 r2.3 r2.34 }"
+  comptest $'tst r\t'
+0:reverse numeric sorting of matches
+>line: {tst r}{}
+>NO:{r2.34}
+>NO:{r2.3}
+>NO:{r1.10}
+>NO:{r1.2}
+>NO:{r1.1}
+
+  comptesteval "_tst() { local expl; _wanted times expl time compadd -o match r1.10 r1.2 r2.3 r2.34 }"
+  comptesteval "zstyle ':completion:*:tst:*' sort reverse numeric"
+  comptest $'tst r\t'
+0:reverse numeric sorting of matches via a style
+>line: {tst r}{}
+>DESCRIPTION:{time}
+>NO:{r2.34}
+>NO:{r2.3}
+>NO:{r1.10}
+>NO:{r1.2}
+
+  comptesteval "_tst() { local disp=(a b c); compadd -o -J letters -d disp 3 2 1 }"
+  comptest $'tst \t'
+0:sort in match rather than display name order
+>line: {tst }{}
+>NO:{c}
+>NO:{b}
+>NO:{a}
+
+  comptesteval "_tst() { local expl; _wanted times expl time compadd 3am 12pm 3pm 10pm }"
+  comptesteval "zstyle ':completion:*:tst:*' sort false"
+  comptest $'tst \t'
+0:sorting disabled via the sort style
+>line: {tst }{}
+>DESCRIPTION:{time}
+>NO:{3am}
+>NO:{12pm}
+>NO:{3pm}
+>NO:{10pm}
+
 %clean
 
   zmodload -ui zsh/zpty

Re: PATCH: completion match ordering

[dana]

On 6 May 2019, at 16:16, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
> I finally got around to finishing off the changes to the options for completion
> match ordering via compadd -o. Thanks to dana for reminding me about this.

Thank you!

Your matchcmp() update removes SORTIT_IGNORING_BACKSLASHES entirely — is that
deliberate in light of my changes? It should always make sense to skip
back-slashes in completion matches unless `compadd -Q` is used to bypass
escaping, shouldn't it? Or do i misunderstand?

(If it is deliberate, the test i added — which causes a conflict with this
patch, btw — will fail until some replacement is devised)

dana

Re: PATCH: completion match ordering

[Oliver Kiddle]

You wrote:
> Your matchcmp() update removes SORTIT_IGNORING_BACKSLASHES entirely ??? is that
> deliberate in light of my changes? It should always make sense to skip

No, not deliberate.
I picked up my old git branch for this change and hadn't really tested
together. I didn't think the patch would even apply cleanly. I'll add in
the missing bitwise OR against SORTIT_IGNORING_BACKSLASHES in the call
to zstrcmp() in matchcmp(), when committing the rest of it.

> back-slashes in completion matches unless `compadd -Q` is used to bypass
> escaping, shouldn't it? Or do i misunderstand?

That makes sense as you describe it. 

Oliver

Re: PATCH: completion match ordering

[Daniel Hahler]

Hi Oliver,

unfortunately this patch (Git commit cd6fd2b0a) breaks completion for
"git commit --fixup" for me, which then looks like this:

% git commit --fixup=
--fixup
 -- option --
















--fixup

-- stage all modified and deleted paths
-- allow recording an empty commit
...

I hope you can reproduce this, otherwise I'd have to investigate.


Best,
Daniel.

On 06.05.19 23:16, Oliver Kiddle wrote:

> I finally got around to finishing off the changes to the options for completion
> match ordering via compadd -o. Thanks to dana for reminding me about this.
>
> The argument to -o is now optional and can be abbreviated, e.g. -omat,rev
> The sort style also accepts values like "numeric" and documentation and
> tests have been added.
>
> _description, _all_labels etc all take -V, -1, -2 options and in the previous
> patch, I had added -o to that. However, as far as I can tell that only has any
> use in the case of -V because the functions need to decide whether to add -J
> group or -V group. So it was only needed because the interface conflates the
> use of -V - specifying the group name and making it unsorted. I guess -1 and -2
> were handled because it is common to combine them as -1V, -2J etc. Given that
> it'd be useless, I decided not to add it.
>
> Back when sort was added to _description in 18859, it was decided to have it
> explicitly ignore the style if -V was passed. While, I would tend towards the
> view that it is better to give users control anyway, it remains that way for
> now. You don't need to pass -V to _wanted as such. To allow the style to enable
> sorting, we would need to add a compadd -o value corresponding to normal sort
> order: perhaps `sort' or `lexical' (i.e. not numeric). We could then rely on
> compadd honouring only the first -o option passed to it.
>
> The combination nosort,reverse could perhaps preserve (but reverse) the
> original order.
>
> We could perhaps also add the backslash ignoring that dana recently restored as
> an option to -o. In my testing, that patch looks good by the way - thanks dana.
>
> Oliver
>
> diff --git a/Completion/Base/Core/_description b/Completion/Base/Core/_description
> index 304c747a6..c2a0e080b 100644
> --- a/Completion/Base/Core/_description
> +++ b/Completion/Base/Core/_description
> @@ -1,13 +1,13 @@
>  #autoload
>
…

Re: PATCH: completion match ordering

[Oliver Kiddle]

Daniel Hahler wrote:
> unfortunately this patch (Git commit cd6fd2b0a) breaks completion for
> "git commit --fixup" for me, which then looks like this:

> ..

> I hope you can reproduce this, otherwise I'd have to investigate.

I'm afraid I can't reproduce it.

Output from _complete_debug might help. Do other uses of _describe or,
more expecially _describe -V have problems? Are you certain you're using
functions matching the code and not, e.g using a new _git with an older
zsh. Perhaps try tweaking __git_recent_commits: remove the _wanted
calls, remove the -V, dump the descr array somewhere with typeset -p.
And then try completing the same array from a minimal function.

Oliver

Re: PATCH: completion match ordering

[Daniel Hahler]

[-- Attachment #1.1: Type: text/plain, Size: 537 bytes --]

Oliver Kiddle wrote:
> Daniel Hahler wrote:
>> unfortunately this patch (Git commit cd6fd2b0a) breaks completion for
>> "git commit --fixup" for me, which then looks like this:
> 
>> ..
> 
>> I hope you can reproduce this, otherwise I'd have to investigate.
> 
> I'm afraid I can't reproduce it.

> Are you certain you're using functions matching the code and not,
> e.g using a new _git with an older zsh.

That was the reason.  I was using the newer _git with an older Zsh
version.


Sorry for the noise,
Daniel.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]