PATCH: Add :^ syntax for zipping two arrays

PATCH: Add :^ syntax for zipping two arrays

[Mikael Magnusson]

Any objections to adding this? Comments on the code is also welcome
since I'm not at all sure I thought of all the gotchas in paramsubst(),
in particular some allocations might be leaking? Not adding tests until
any and all comments are incorporated.

---
 Doc/Zsh/expn.yo | 12 ++++++++++++
 Src/subst.c     | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+)

diff --git a/Doc/Zsh/expn.yo b/Doc/Zsh/expn.yo
index ff9f2b1..2fb803a 100644
--- a/Doc/Zsh/expn.yo
+++ b/Doc/Zsh/expn.yo
@@ -636,6 +636,18 @@ Similar to the preceding subsitution, but in the opposite sense,
 so that entries present in both the original substitution and as
 elements of var(arrayname) are retained and others removed.
 )
+item(tt(${)var(name)tt(:^)var(arrayname)tt(}))(
+Zips two arrays, such that the output array is twice as long as the
+longest of tt(name) and tt(arrayname), with the elements alternatingly
+being picked from them. If one of the input arrays is shorter, then the
+input is repeated until all of the other array has been used up.  Thus,
+
+example(a=(1 2 3 4); b=(a b); print ${a:^b})
+
+will output `tt(1 a 2 b 3 a 4 b)'. Either or both inputs may be a scalar,
+they will be treated as an array of length 1 with the scalar as the only
+element.
+)
 xitem(tt(${)var(name)tt(:)var(offset)tt(}))
 item(tt(${)var(name)tt(:)var(offset)tt(:)var(length)tt(}))(
 This syntax gives effects similar to parameter subscripting
diff --git a/Src/subst.c b/Src/subst.c
index d4b68e2..a66ce01 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2878,6 +2878,57 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 	    }
 	    break;
 	}
+    } else if (inbrace && (*s == '^' || *s == Hat)) {
+	char **zip, **ap, **apsrc;
+	++s;
+	if (*itype_end(s, IIDENT, 0)) {
+	    untokenize(s);
+	    zerr("not an identifier: %s", s);
+	    return NULL;
+	}
+	if (vunset) {
+	    if (unset(UNSET)) {
+		*idend = '\0';
+		zerr("%s: parameter not set", idbeg);
+		return NULL;
+	    }
+	    val = dupstring("");
+	} else {
+	    char *sval;
+	    zip = getaparam(s);
+	    if (!zip) {
+		sval = getsparam(s);
+		if (sval)
+		    zip = mkarray(sval);
+	    }
+	    if (!isarr) aval = mkarray(val);
+	    if (zip) {
+		char **out;
+		int alen, ziplen, outlen, i = 0;
+		alen = arrlen(aval);
+		ziplen = arrlen(zip);
+		outlen = alen > ziplen ? alen : ziplen;
+		out = zalloc(sizeof(char *) * (2 * outlen + 1));
+		while (i < outlen) {
+		    if (copied)
+			out[i*2] = aval[i % alen];
+		    else
+			out[i*2] = ztrdup(aval[i % alen]);
+		    out[i*2+1] = ztrdup(zip[i % ziplen]);
+		    i++;
+		}
+		out[i*2] = NULL;
+		aval = out;
+		copied = 1;
+		isarr = 1;
+	    } else {
+		if (unset(UNSET)) {
+		    zerr("%s: parameter not set", s);
+		    return NULL;
+		}
+		val = dupstring("");
+	    }
+	}
     } else if (inbrace && (*s == '|' || *s == Bar ||
 			   *s == '*' || *s == Star)) {
 	int intersect = (*s == '*' || *s == Star);
-- 
1.9.0

Re: PATCH: Add :^ syntax for zipping two arrays

[Peter Stephenson]

On Thu, 31 Jul 2014 19:45:39 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> Any objections to adding this? Comments on the code is also welcome
> since I'm not at all sure I thought of all the gotchas in paramsubst(),
> in particular some allocations might be leaking? Not adding tests until
> any and all comments are incorporated.

That looks pretty reasonable, but I think all the allocations need to be
on the heap at this point (though it's certainly potentially confusing):
so zalloc needs to be zhalloc, ztrdup needs to be dupstring, and mkarray
needs to be... er... however you do mkarray on the heap; I think you
just zhalloc(2), copy the first element, and set the second to
NULL, though I don't see why you couldn't add an hmkarray().

Generally, the command line expansion stuff is all done on the heap ---
it doesn't get turned into permanent allocation until assigned to a
variable, added to a hash, stored as a function definition, or whatever.

Then it looks like it ought basically to work.  Some tests would be
useful.

pws

Re: PATCH: Add :^ syntax for zipping two arrays

[Mikael Magnusson]

On 1 August 2014 10:46, Peter Stephenson <p.stephenson@samsung.com> wrote:
> On Thu, 31 Jul 2014 19:45:39 +0200
> Mikael Magnusson <mikachu@gmail.com> wrote:
>> Any objections to adding this? Comments on the code is also welcome
>> since I'm not at all sure I thought of all the gotchas in paramsubst(),
>> in particular some allocations might be leaking? Not adding tests until
>> any and all comments are incorporated.
>
> That looks pretty reasonable, but I think all the allocations need to be
> on the heap at this point (though it's certainly potentially confusing):
> so zalloc needs to be zhalloc, ztrdup needs to be dupstring, and mkarray
> needs to be... er... however you do mkarray on the heap; I think you
> just zhalloc(2), copy the first element, and set the second to
> NULL, though I don't see why you couldn't add an hmkarray().
>
> Generally, the command line expansion stuff is all done on the heap ---
> it doesn't get turned into permanent allocation until assigned to a
> variable, added to a hash, stored as a function definition, or whatever.

Yeah, when I started writing the code I had forgotten that arrays are
literally just arrays, and not a special zsh array type, so I was
looking for more array handling functions, then remembered. Anyway, I
modeled it loosely on the :| / :* code which is just below it, which
uses mkarray, zhalloc and dupstring, which is... confusing. (I was
probably looking at another piece of code while writing ztrdup and
zalloc, changed those now).

> Then it looks like it ought basically to work.  Some tests would be
> useful.

Got some comments on IRC that in other languages, zip() functions
usually stop at the end of the shorter array, so I made that the
behavior for :^ and added :^^ for the behavior in the patch I sent.

I noticed a problem though, and that is inside double quotes. a=(1 2);
"${a:^a}" produces two separate array elements as output when you
might expect a single space joined thing from anything inside double
quotes (and the elements it outputs are "1 2" and "1"). I'm not
entirely sure how to handle that. For the :| and :* operator, you only
ever remove elements, so once the original array is joined together,
you don't accidentally add more. I tried looking at isarr but it's 0
both inside double quotes and when the input is actually a scalar
outside double quotes. Is there some other parameter I could examine?
I don't think there's really any sensible output you can get out of
"${a:^b}" without also adding (@) but I should at least detect it so
as to not output multiple elements... although I suppose someone might
want the output of "${a:^^b}" (which is the whole array $a joined
together by spaces, shown fully between each element of b, and then
joined together by spaces again? I don't think I want to support that.
Actually the effect seems achieved already by nesting again inside
${})

-- 
Mikael Magnusson

Re: PATCH: Add :^ syntax for zipping two arrays

[Peter Stephenson]

On Fri, 01 Aug 2014 16:46:57 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> I modeled it loosely on the :| / :* code which is just below it, which
> uses mkarray, zhalloc and dupstring, which is... confusing.

That'll be one of them bug things people sometimes try to interest me
in.

pws

diff --git a/Src/subst.c b/Src/subst.c
index 4713502..d6be2f0 100644
--- a/Src/subst.c
+++ b/Src/subst.c
@@ -2935,7 +2935,7 @@ paramsubst(LinkList l, LinkNode n, char **str, int qt, int pf_flags)
 	     */
 	    if (!vunset) {
 		if (isarr) {
-		    aval = mkarray(NULL);
+		    aval = hmkarray(NULL);
 		} else {
 		    val = dupstring("");
 		}
diff --git a/Src/utils.c b/Src/utils.c
index aa978e6..998e46a 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -3372,6 +3372,17 @@ mkarray(char *s)
 }
 
 /**/
+mod_export char **
+hmkarray(char *s)
+{
+    char **t = (char **) zhalloc((s) ? (2 * sizeof s) : (sizeof s));
+
+    if ((*t = s))
+	t[1] = NULL;
+    return t;
+}
+
+/**/
 mod_export void
 zbeep(void)
 {

Re: PATCH: Add :^ syntax for zipping two arrays

[Bart Schaefer]

On Aug 1,  4:02pm, Peter Stephenson wrote:
} Subject: Re: PATCH: Add :^ syntax for zipping two arrays
}
} On Fri, 01 Aug 2014 16:46:57 +0200
} Mikael Magnusson <mikachu@gmail.com> wrote:
} > I modeled it loosely on the :| / :* code which is just below it, which
} > uses mkarray, zhalloc and dupstring, which is... confusing.
} 
} That'll be one of them bug things people sometimes try to interest me
} in.

I can't figure out the circumstances in which that mkarray() code path
is ever taken.  Or ... maybe I did figure it out, but valgrind doesn't
find any leaked memory as a result; I didn't actually try setting a
breakpoint.

Still worth fixing, I suppose.