[PATCH 1/2] internal: Add a second parameter to zlinklist2array(), analogously to hlinklist2array().

[PATCH 1/2] internal: Add a second parameter to zlinklist2array(), analogously to hlinklist2array().

[Daniel Shahaf]

Will be used in the next commit.
---
 Src/Modules/curses.c |  4 ++--
 Src/Zle/compcore.c   |  4 ++--
 Src/Zle/computil.c   |  4 ++--
 Src/builtin.c        | 10 +++++-----
 Src/linklist.c       | 13 +++++++++----
 5 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/Src/Modules/curses.c b/Src/Modules/curses.c
index 19f285e34..e46903916 100644
--- a/Src/Modules/curses.c
+++ b/Src/Modules/curses.c
@@ -1212,7 +1212,7 @@ zccmd_input(const char *nam, char **args)
 		    addlinknode(margs, "CTRL");
 		if (mevent.bstate & BUTTON_ALT)
 		    addlinknode(margs, "ALT");
-		if (!setaparam(args[3], zlinklist2array(margs)))
+		if (!setaparam(args[3], zlinklist2array(margs, 1)))
 		    return 1;
 	    } else {
 #endif
@@ -1464,7 +1464,7 @@ zccmd_querychar(const char *nam, char **args)
     }
 
     /* Turn this into an array and store it. */
-    return !setaparam(args[1] ? args[1] : "reply", zlinklist2array(clist));
+    return !setaparam(args[1] ? args[1] : "reply", zlinklist2array(clist, 1));
 }
 
 
diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 7e3badc57..958fef8e7 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -648,7 +648,7 @@ callcompfunc(char *s, char *fn)
 	if (compredirs)
 	    freearray(compredirs);
         if (rdstrs)
-            compredirs = zlinklist2array(rdstrs);
+            compredirs = zlinklist2array(rdstrs, 1);
         else
             compredirs = (char **) zshcalloc(sizeof(char *));
 
@@ -1922,7 +1922,7 @@ set_comp_sep(void)
 mod_export void
 set_list_array(char *name, LinkList l)
 {
-    setaparam(name, zlinklist2array(l));
+    setaparam(name, zlinklist2array(l, 1));
 }
 
 /* Get the words from a variable or a (list of words). */
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index 90db8b4b8..ddfa70077 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -3591,7 +3591,7 @@ bin_compvalues(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	if (cv_laststate.vals) {
 	    char **ret;
 
-	    ret = zlinklist2array(cv_laststate.vals);
+	    ret = zlinklist2array(cv_laststate.vals, 1);
 	    sethparam(args[1], ret);
 
 	    return 0;
@@ -4016,7 +4016,7 @@ bin_comptry(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 
 		    set = (Ctset) zalloc(sizeof(*set));
 
-		    set->tags = zlinklist2array(list);
+		    set->tags = zlinklist2array(list, 1);
 		    set->next = NULL;
 		    set->ptr = NULL;
 		    set->tag = NULL;
diff --git a/Src/builtin.c b/Src/builtin.c
index 3dab3f9b4..d5a874a95 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -2280,7 +2280,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	} else if (asg->flags & ASG_ARRAY) {
 	    int flags = (asg->flags & ASG_KEY_VALUE) ? ASSPM_KEY_VALUE : 0;
 	    if (!(pm = assignaparam(pname, asg->value.array ?
-				 zlinklist2array(asg->value.array) :
+				 zlinklist2array(asg->value.array, 1) :
 				 mkarray(NULL), flags)))
 		return NULL;
 	}
@@ -2442,7 +2442,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 	} else if (PM_TYPE(on) == PM_ARRAY && ASG_ARRAYP(asg)) {
 	    int flags = (asg->flags & ASG_KEY_VALUE) ? ASSPM_KEY_VALUE : 0;
 	    if (!(pm = assignaparam(pname, asg->value.array ?
-				    zlinklist2array(asg->value.array) :
+				    zlinklist2array(asg->value.array, 1) :
 				    mkarray(NULL), flags)))
 		return NULL;
 	    dont_set = 1;
@@ -2536,7 +2536,7 @@ typeset_single(char *cname, char *pname, Param pm, UNUSED(int func),
 		    arrayval = mkarray(NULL);
 		}
 	    } else if (asg->value.array)
-		arrayval = zlinklist2array(asg->value.array);
+		arrayval = zlinklist2array(asg->value.array, 1);
 	    else
 		arrayval = mkarray(NULL);
 	    if (!(pm=assignaparam(pname, arrayval, flags)))
@@ -2923,7 +2923,7 @@ bin_typeset(char *name, char **argv, LinkList assigns, Options ops, int func)
 	apm->ename = ztrdup(asg0.name);
 	if (asg->value.array) {
 	    int flags = (asg->flags & ASG_KEY_VALUE) ? ASSPM_KEY_VALUE : 0;
-	    assignaparam(asg->name, zlinklist2array(asg->value.array), flags);
+	    assignaparam(asg->name, zlinklist2array(asg->value.array, 1), flags);
 	} else if (oldval)
 	    assignsparam(asg0.name, oldval, 0);
 	unqueue_signals();
@@ -3901,7 +3901,7 @@ bin_whence(char *nam, char **argv, Options ops, int func)
 	}
 	unqueue_signals();
 	if (all) {
-	    allmatched = argv = zlinklist2array(matchednodes);
+	    allmatched = argv = zlinklist2array(matchednodes, 1);
 	    matchednodes = NULL;
 	    popheap();
 	} else
diff --git a/Src/linklist.c b/Src/linklist.c
index 85d9bb367..f64685d9e 100644
--- a/Src/linklist.c
+++ b/Src/linklist.c
@@ -438,22 +438,27 @@ hlinklist2array(LinkList list, int copy)
 
 /*
  * Convert a linked list whose data elements are strings to
- * an array.  The result is a permanently allocated, freearrayable
- * array.
+ * a permanently-allocated array.  The elements of the array are the same
+ * elements as the linked list data if copy is 0, else they are duplicated
+ * into permanent memory so the result is a permanently allocated,
+ * freearrayable array that's a deep copy of the linked list.
  */
 
 /**/
 mod_export char **
-zlinklist2array(LinkList list)
+zlinklist2array(LinkList list, int copy)
 {
     int l = countlinknodes(list);
     char **ret = (char **) zalloc((l + 1) * sizeof(char *)), **p;
     LinkNode n;
 
     for (n = firstnode(list), p = ret; n; incnode(n), p++) {
-	*p = ztrdup((char *) getdata(n));
+	*p = (char *) getdata(n);
+	if (copy)
+	    *p = ztrdup(*p);
     }
     *p = NULL;
 
     return ret;
 }
+

[PATCH 2/2] _arguments: Add the -0 flag, which makes $opt_args be populated sanely.

[Daniel Shahaf]

Also, write/extend docstrings for sepjoin() and zjoin().
---
With this, «local -a values=( ${(0)opt_args[--foo]} )» would get the
value or values of the --foo option, as typed on the command line.
Without this, the completion function would have to reverse the "escape
colons and backslashes and join with colons" operation, and I don't know
of an easy way to do that.

Cheers,

Daniel


 Completion/Base/Utility/_arguments |  8 +++---
 Doc/Zsh/compsys.yo                 | 25 ++++++++++++++++---
 Src/Zle/computil.c                 | 40 ++++++++++++++++++++++++++----
 Src/utils.c                        | 15 ++++++++++-
 Test/Y03arguments.ztst             |  8 +++++-
 5 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/Completion/Base/Utility/_arguments b/Completion/Base/Utility/_arguments
index 136dd5826..3f1b39304 100644
--- a/Completion/Base/Utility/_arguments
+++ b/Completion/Base/Utility/_arguments
@@ -7,11 +7,13 @@ local long cmd="$words[1]" descr odescr mesg subopts opt opt2 usecc autod
 local oldcontext="$curcontext" hasopts rawret optarg singopt alwopt
 local setnormarg start rest
 local -a match mbegin mend
+integer opt_args_use_NUL_separators=0
 
 subopts=()
 singopt=()
-while [[ "$1" = -([AMO]*|[CRSWnsw]) ]]; do
+while [[ "$1" = -([AMO]*|[0CRSWnsw]) ]]; do
   case "$1" in
+  -0) opt_args_use_NUL_separators=1; shift ;;
   -C)  usecc=yes; shift ;;
   -O)  subopts=( "${(@P)2}" ); shift 2 ;;
   -O*) subopts=( "${(@P)${1[3,-1]}}" ); shift ;;
@@ -388,7 +390,7 @@ if (( $# )) && comparguments -i "$autod" "$singopt[@]" "$@"; then
             if [[ "$action" = -\>* ]]; then
 	      action="${${action[3,-1]##[ 	]#}%%[ 	]#}"
 	      if (( ! $state[(I)$action] )); then
-                comparguments -W line opt_args
+                comparguments -W line opt_args $opt_args_use_NUL_separators
                 state+=( "$action" )
                 state_descr+=( "$descr" )
 	        if [[ -n "$usecc" ]]; then
@@ -406,7 +408,7 @@ if (( $# )) && comparguments -i "$autod" "$singopt[@]" "$@"; then
                 local=yes
               fi
 
-              comparguments -W line opt_args
+              comparguments -W line opt_args $opt_args_use_NUL_separators
 
               if [[ "$action" = \ # ]]; then
 
diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
index a1a1da935..6ceb4d59d 100644
--- a/Doc/Zsh/compsys.yo
+++ b/Doc/Zsh/compsys.yo
@@ -3734,6 +3734,12 @@ The default var(matchspec) allows partial word completion after `tt(_)' and
 var(matchspec) is:
 example(tt(r:|[_-]=* r:|=*))
 )
+item(tt(-0))(
+When populating values of the `tt(opt_args)' associative array, don't
+backslash-escape colons and backslashes and use NUL rather than colon for
+joining multiple values. This option is described in more detail below, under
+the heading em(var(spec)s: actions).
+)
 enditem()
 
 em(var(spec)s: overview)
@@ -3906,6 +3912,7 @@ specific contexts: on the first call `tt(_arguments $global_options)' is
 used, and on subsequent calls `tt(_arguments !$^global_options)'.
 
 em(var(spec)s: actions)
+COMMENT(If you change this section title, change the references to it in running text.)
 
 In each of the forms above the var(action) determines how
 completions should be generated.  Except for the `tt(->)var(string)'
@@ -4027,9 +4034,21 @@ the normal arguments from the command line, i.e. the words from the
 command line after the command name excluding all options and their
 arguments.  Options are stored in the associative array
 `tt(opt_args)' with option names as keys and their arguments as
-the values.  For options that have more than one argument these are
-given as one string, separated by colons.  All colons and backslashes
-in the original arguments are preceded with backslashes.
+the values.  By default, all colons and backslashes in the value are escaped
+with backslashes, and if an option has multiple arguments (for example, when
+using an var(optspec) of the form `tt(*)var(optspec)'), they are joined with
+(unescaped) colons.  However, if the tt(-0) option was passed, no backslash
+escaping is performed, and multiple values are joined with NUL bytes.  For
+example, after `tt(zsh -o foo:foo -o bar:bar -o <TAB>)', the contents of
+`tt(opt_args)' would be
+
+example(typeset -A opt_args=( [-o]='foo\:foo:bar\:bar:' ))
+
+by default, and
+
+example(typeset -A opt_args=( [-o]=$'foo:foo\x00bar:bar\x00' ))
+
+if tt(_arguments) had been called with the tt(-0) option.
 
 The parameter `tt(context)' is set when returning to the calling function
 to perform an action of the form `tt(->)var(string)'.  It is set to an
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index ddfa70077..e08788e89 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -2375,6 +2375,23 @@ ca_parse_line(Cadef d, Cadef all, int multi, int first)
     return 0;
 }
 
+/* Build a NUL-separated from a list.
+ *
+ * This is only used to populate values of $opt_args.
+ */
+
+static char *
+ca_nullist(LinkList l)
+{
+    if (l) {
+	char **array = zlinklist2array(l, 0 /* don't dup elements */);
+	char *ret = zjoin(array, '\0', 0 /* permanent allocation */);
+	free(array); /* the elements are owned by the list */
+	return ret;
+    } else
+	return ztrdup("");
+}
+
 /* Build a colon-list from a list.
  *
  * This is only used to populate values of $opt_args.
@@ -2563,7 +2580,7 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
     case 's': min = 1; max =  1; break;
     case 'M': min = 1; max =  1; break;
     case 'a': min = 0; max =  0; break;
-    case 'W': min = 2; max =  2; break;
+    case 'W': min = 3; max =  3; break;
     case 'n': min = 1; max =  1; break;
     default:
 	zwarnnam(nam, "invalid option: %s", args[0]);
@@ -2795,11 +2812,20 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 		return 0;
 	return 1;
     case 'W':
-        /* This gets two parameter names as arguments.  The first is set to
-         * the current word sans any option prefixes handled by comparguments.
+        /* This gets two parameter names and one integer as arguments.
+         *
+         * The first parameter is set to the current word sans any option
+         * prefixes handled by comparguments.
+         *
          * The second parameter is set to an array containing the options on
          * the line and their arguments.  I.e. the stuff _arguments returns
-         * to its caller in the `line' and `opt_args' parameters. */
+         * to its caller in the `line' and `opt_args' parameters.
+         *
+         * The integer is one if the second parameter (which is just $opt_args,
+         * you know) should encode multiple values by joining them with NULs
+         * and zero if it should encode multiple values by joining them with
+         * colons after backslash-escaping colons and backslashes.
+         */
 	{
 	    Castate s;
 	    char **ret, **p;
@@ -2807,6 +2833,7 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	    LinkList *a;
 	    Caopt o;
 	    int num;
+	    int opt_args_use_NUL_separators = (args[3][0] != '0');
 
 	    for (num = 0, s = lstate; s; s = s->snext)
 		num += countlinknodes(s->args);
@@ -2832,7 +2859,10 @@ bin_comparguments(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 		    if (*a) {
 			*p++ = (o->gsname ? tricat(o->gsname, o->name, "") :
 				ztrdup(o->name));
-			*p++ = ca_colonlist(*a);
+			if (opt_args_use_NUL_separators)
+			    *p++ = ca_nullist(*a);
+			else
+			    *p++ = ca_colonlist(*a);
 		    }
 	    *p = NULL;
 
diff --git a/Src/utils.c b/Src/utils.c
index e258ef836..5158a70b1 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3596,6 +3596,17 @@ strftimehandling:
     return buf - origbuf;
 }
 
+/*
+ * Return a string consisting of the elements of 'arr' joined by the character
+ * 'delim', which will be metafied if necessary.  The string will be allocated
+ * on the heap iff 'heap'.
+ *
+ * Comparable to:
+ *
+ *     char metafied_delim[] = { Meta, delim ^ 32, '\0' };
+ *     sepjoin(arr, metafied_delim, heap)
+ */
+
 /**/
 mod_export char *
 zjoin(char **arr, int delim, int heap)
@@ -3894,10 +3905,12 @@ wordcount(char *s, char *sep, int mul)
 
 /*
  * 's' is a NULL-terminated array of strings.
- * 'sep' is a string.
+ * 'sep' is a string, or NULL to split on ${IFS[1]}.
  *
  * Return a string consisting of the elements of 's' joined by 'sep',
  * allocated on the heap iff 'heap'.
+ *
+ * See also zjoin().
  */
 
 /**/
diff --git a/Test/Y03arguments.ztst b/Test/Y03arguments.ztst
index fa4589374..a815799b3 100644
--- a/Test/Y03arguments.ztst
+++ b/Test/Y03arguments.ztst
@@ -231,9 +231,15 @@
 
  tst_arguments '-a:one: :two' ':descr:{compadd -Q - $opt_args[-a]}'
  comptest $'tst -a 1:x \\2 \t'
-0:opt_args with multiple arguments and quoting of colons and backslashes
+0:opt_args with multiple arguments and quoting of colons and backslashes, part #1: default behaviour
 >line: {tst -a 1:x \2 1\:x:\\2 }{}
 
+ # Same as previous test, except with -0 and (qqqq) added
+ tst_arguments -0 : '-a:one: :two' ':descr:{compadd -Q - ${(qqqq)opt_args[-a]}}'
+ comptest $'tst -a 1:x \\2 \t'
+0:opt_args with multiple arguments and quoting of colons and backslashes, part #2: NUL escaping
+>line: {tst -a 1:x \2 $'1:x\0\\2' }{}
+
  tst_arguments -a -b
  comptest $'tst  rest -\t\C-w\eb\C-b-\t'
 0:option completion with rest arguments on the line but not in the specs

Re: [PATCH 2/2] _arguments: Add the -0 flag, which makes $opt_args be populated sanely.

[dana]

On 27 Apr 2020, at 14:30, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Without this, the completion function would have to reverse the "escape
> colons and backslashes and join with colons" operation, and I don't know
> of an easy way to do that.

I think this is a problem in a few other parts of the completion system
(though i can't remember specifically where), and with anything that uses DSV,
like PATH-style variables. I always thought it'd be cool if (j) and (s)
supported optional (un)escaping, maybe something like this:

  % arr=( 'foo' 'bar:baz' 'qux\quux' )
  % print -r ${str::=${(j<:><\>)arr}}  # Join by :, use \ as escape character
  foo:bar\:baz:qux\\quux
  % print -rl ${(s<:><\>)str}  # Split by :, use \ as escape character
  foo
  bar:baz
  qux\quux

Then you wouldn't have to do weird find/replace stuff, and it wouldn't break
if you had like an escaped back-slash in front of your delimiter

In this case `_arguments -0` would be faster/simpler and require less
'user-land' boiler-plate though. Haven't looked closely at the patch, but i'd
use it

dana

Re:[PATCH 2/2] _arguments: Add the -0 flag, which makes $opt_args be populated sanely.

[oxiedi]

> With this, «local -a values=( ${(0)opt_args[--foo]} )» would get the
> value or values of the --foo option, as typed on the command line.
> Without this, the completion function would have to reverse the "escape
> colons and backslashes and join with colons" operation, and I don't know
> of an easy way to do that.

I've used

    local -a values
    IFS=: read -A values <<<$opt_args[--foo]

once. Does it have some drawbacks?

Re: [PATCH 2/2] _arguments: Add the -0 flag, which makes $opt_args be populated sanely.

[Daniel Shahaf]

oxiedi@yandex.ru wrote on Tue, 28 Apr 2020 14:37 +0500:
> > With this, «local -a values=( ${(0)opt_args[--foo]} )» would get the
> > value or values of the --foo option, as typed on the command line.
> > Without this, the completion function would have to reverse the "escape
> > colons and backslashes and join with colons" operation, and I don't know
> > of an easy way to do that.  
> 
> I've used
> 
>     local -a values
>     IFS=: read -A values <<<$opt_args[--foo]
> 
> once. Does it have some drawbacks?

Good idea, thanks.

It would DTWT when there's a literal newline in ${opt_args[--foo]}.

Also, it requires an additional temporary variable before accessing
${opt_args[--foo]} whenever the value of --foo may contain colons, even
if the --foo option is not repeatable.

On the other hand, the NUL separators approach would DTWT when there is
a literal NUL in the arguments.  That could in theory happen when
completing the arguments to a builtin.  If someone runs into _that_,
they should implement and use dana's ${(s.:..\.)} [grep for «spsep»],
or invent another way for _arguments to communicate the values to its
caller.

Thanks again,

Daniel