PATCH Re: let unset array element remove compatible with bash

[Bart Schaefer <schaefer@brasslantern.com>]

Revisiting this thread because I found an uncommitted diff in my local
sandbox.  I won't commit this pre-5.0 without further confirmation from
PWS.

On Feb 22,  8:19pm, Peter Stephenson wrote:
}
} On Wed, 22 Feb 2012 09:28:27 -0800
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > On Feb 22,  9:52am, Peter Stephenson wrote:
} > }
} > } So it's not simply a case of unconditionally deleting an element.  I
} > } think you need to check the element is present in the first place.
} > 
} > Is there a straightforward way to do that?  The code in subst.c that
} > handles ${+var[x]} is rather convoluted, and every combination of
} > getvalue()/fetchvalue() that I've tried always returns non-NULL for
} > subscripted expressions.
} 
} At this level, I'd have thought it was good enough to test the length of
} the array directly.  However, you can only do that after you've
} evaluated the subscript, so it probably needs some poking down into
} params.c first.
} 
} Presumably if you're trying to unset an element of the array, retrieving
} it isn't likely to be difficult.

So I stared at this a bit more.  Once I figured out that getvalue() did
in fact return enough information (in v->pm) to determine the length of
the array, it was easy to implement a test.

However, that was a bit unsatisfactory, because getvalue() would convert
the slice to be unset into its string value too, which is wasteful in
context.  Poking around in fetchvalue() reminded me of SCAPM_ARRONLY
which fetches the array without constructing a value for it.  Then it
was just a matter of cribbing the way fetchvalue() calls getindex().

So the patch below makes

    noglob unset foo[x]

work properly.  If x is an existing index (or valid slice), it is cut
from the array as if by foo[x]=().  Attempts to unset index values
outside the array, do nothing and return zero status.

As a bonus this will also unset slices of scalars.  If you try to
unset a slice of an integer or float, it returns a nonzero status but
doesn't error.  If we want an error here, then this should probably
also be an error:

torch% integer foo=123
torch% print $foo[2]
2
torch% foo[2]=4
torch% print $foo
4
torch% 

One thing I'm still scratching my head about:  In foo[(i)x] for scalar
foo, setstrvalue() doesn't properly apply the subscript range returned
from getindex().  That's why the PM_SCALAR branch below uses getvalue(),
which is somehow subtly different.  If someone can figure out how to
unify them, go for it.

Index: Src/builtin.c
--- ../zsh-forge/current/Src/builtin.c	2012-06-30 10:33:44.000000000 -0700
+++ Src/builtin.c	2012-07-01 09:33:55.000000000 -0700
@@ -3055,9 +3055,39 @@
 		    *sse = ']';
 		}
 		paramtab = tht;
+	    } else if (PM_TYPE(pm->node.flags) == PM_SCALAR) {
+		struct value vbuf;
+		Value v;
+		char *t = s;
+		*ss = '[';
+		if ((v = getvalue(&vbuf, &t, 1))) {
+		    setstrvalue(v, ztrdup(""));
+		}
+		returnval = errflag;
+		errflag = 0;
+	    } else if (PM_TYPE(pm->node.flags) == PM_ARRAY) {
+		struct value vbuf;
+		vbuf.isarr = SCANPM_ARRONLY;
+		vbuf.pm = pm;
+		vbuf.flags = 0;
+		vbuf.start = 0;
+		vbuf.end = -1;
+		vbuf.arr = 0;
+		*ss = '[';
+		if (getindex(&ss, &vbuf, SCANPM_ASSIGNING) == 0 &&
+		    vbuf.pm && !(vbuf.pm->node.flags & PM_UNSET)) {
+		    /* start is after the element for reverse index */
+		    int start = vbuf.start - !!(vbuf.flags & VALFLAG_INV);
+		    if (start < arrlen(vbuf.pm->u.arr)) {
+			char *arr[1];
+			arr[0] = 0;
+			setarrvalue(&vbuf, zarrdup(arr));
+		    }
+		}
+		returnval = errflag;
+		errflag = 0;
 	    } else {
-		zerrnam(name, "%s: invalid element for unset", s);
-		returnval = 1;
+	      returnval = 1;
 	    }
 	} else {
 	    if (unsetparam_pm(pm, 0, 1))

-- 
Barton E. Schaefer