unset "hash[key]" isn't matched with what "key" may be

unset "hash[key]" isn't matched with what "key" may be

[Sebastian Gniazdowski]

Hello,
I was testing following plugin:

https://github.com/hchbaw/opp.zsh

It creates functions with sophisticated names, like

opp+aB                               opp+aW
opp+a"                               opp+a'
opp+a(                               opp+a)
opp+a<                               opp+a>
opp+a[                               opp+a]


I had a loop in which I browsed $functions, setting:

                func[$i]=1

where "func" is a hash and "$i" is ${(k)functions}. This worked, however this:

                unset "func[$i]"

yielded:

-zplg-diff-functions:unset:36: func[opp+a\[]: invalid parameter name

Tried ${(q)i}, qq, qqq, qqqq.

Related code:

https://github.com/psprint/zplugin/commit/61a59fe0e2e367b3536fb028f4a36e9730322402

Best regards,
Sebastian Gniazdowski

Re: unset "hash[key]" isn't matched with what "key" may be

[Peter Stephenson]

On Sun, 7 Feb 2016 15:16:05 +0100
Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> I had a loop in which I browsed $functions, setting:
> 
>                 func[$i]=1
> 
> where "func" is a hash and "$i" is ${(k)functions}. This worked, however this:
> 
>                 unset "func[$i]"

Not sure what you're doing, but it's apparently not equivalent to the
following, which appears to work.  There may be some relevant option,
I suppose.

% i='${(k)functions}'
% typeset -A hash
% hash[$i]=1
% print ${(kv)hash}
${(k)functions} 1
% unset "hash[$i]"
% print ${(kv)hash}
<this line intentionally left blank>

PWS

Re: unset "hash[key]" isn't matched with what "key" may be

[Sebastian Gniazdowski]

On 7 February 2016 at 20:08, Peter Stephenson
<p.w.stephenson@ntlworld.com> wrote:
> On Sun, 7 Feb 2016 15:16:05 +0100
> Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
>> I had a loop in which I browsed $functions, setting:
>>
>>                 func[$i]=1
>>
>> where "func" is a hash and "$i" is ${(k)functions}. This worked, however this:
>>
>>                 unset "func[$i]"
>
> Not sure what you're doing, but it's apparently not equivalent to the
> following, which appears to work.  There may be some relevant option,
> I suppose.
>
> % i='${(k)functions}'

Sorry, by:

>> where "func" is a hash and "$i" is ${(k)functions}. This worked, however this:

I meant that "$i" iterates over keys of $functions

Best regards,
Sebastian Gniazdowski

Re: unset "hash[key]" isn't matched with what "key" may be

[Peter Stephenson]

On Sun, 7 Feb 2016 20:10:48 +0100
Sebastian Gniazdowski <sgniazdowski@gmail.com> wrote:
> I meant that "$i" iterates over keys of $functions

Ah, you mean

% i='opp+a[' 
% function 'opp+a[' { print Hi; }
% 'opp+a['
Hi
% unset "function[$i]"
unset: function[opp+a[]: invalid parameter name

That's because i is expanded first, so the brackets get confused.  So
you might expect

i='opp+a\['

to work to compensate, but it doesn't.  Nor does

unset 'function[$i]'

since there's no later expansion.

No, I can't see an obvious answer either, and it's not specific to
$functions, it happens with an ordinary hash.   It's not actually clear
to me what the right answer should be.  Somehow you need to make the
key stand out from the surrounding syntax.  Allowing backslash quoting,
which was the next guess, might be the asnswer, but see
http://xkcd.com/1638 for commentary on backslashes.

pws

Re: unset "hash[key]" isn't matched with what "key" may be

[Bart Schaefer]

[I see PWS has already responded while I was trying to work out what the
code is doing.  Here's what I've written so far, maybe I can come up with
a patch in a bit longer.]

On Feb 7,  3:16pm, Sebastian Gniazdowski wrote:
}
} I was testing following plugin:
} 
} https://github.com/hchbaw/opp.zsh

Which incidentally says (README.md) it is obsolete for zsh >= 5.0.8.

} It creates functions with sophisticated names, like
} 
} opp+a(                               opp+a)
} opp+a<                               opp+a>
} opp+a[                               opp+a]

Ooh, interesting.  Making use of the side-effect that you can give a
ZLE widget a function name that can't normally be typed unquoted on a
command line.  Kinky.

}                 unset "func[$i]"
} 
} yielded:
} 
} -zplg-diff-functions:unset:36: func[opp+a\[]: invalid parameter name

Yep, "unset" is pretty naive about parsing the subscript there.  It wants
the square brackets in balanced pairs and doesn't consider any kind of
quoting.  (The -m option also doesn't work for subscripts.)

For whatever it's worth, this doesn't work in ksh either (at least as of

$ print $KSH_VERSION
Version JM 93u 2011-02-08

) although the error isn't printed in the exact same circumstances; in
some cases it just silently fails:

$ y='['
$ ary=([$y]=1)
$ typeset -p ary
typeset -A ary=(['[']=1)
$ unset ary[$y]
$ typeset -p ary
typeset -A ary=(['[']=1)
$ ary[x]=1
$ typeset -p ary
typeset -A ary=(['[']=1 [x]=1)
$ unset ary[x]
$ typeset -p ary
typeset -A ary=(['[']=1)
$ unset "ary['[']"   
ksh: unset: ary['[']: invalid variable name
$ unset ary[\[]
$ typeset -p ary
typeset -A ary=(['[']=1)
$ 


-- 
Barton E. Schaefer

Re: unset "hash[key]" isn't matched with what "key" may be

[Bart Schaefer]

On Feb 7,  1:33pm, Bart Schaefer wrote:
}
} On Feb 7,  3:16pm, Sebastian Gniazdowski wrote:
} }                 unset "func[$i]"
} } -zplg-diff-functions:unset:36: func[opp+a\[]: invalid parameter name
} 
} Yep, "unset" is pretty naive about parsing the subscript there.  It wants
} the square brackets in balanced pairs and doesn't consider any kind of
} quoting.

Patch below for consideration.  This doesn't do any additional expansion,
but it has the effect of allowing backslash-quoting -- which therefore
requires that backslashes themselves be doubled, cf. all the caveats
about subscripts behaving as if double-quoted that appear elsewhere.

After this patch, the best way to handle unset of associative array
elements looks something like

  typeset -A ary
  ary[${key}]=value
  ...
  unset "ary[${(b)key}]"

(or use "noglob" instead of double quotes).

This does reflect a change from previous behavior, so we'll want to
consider it carefully.  If we decide to keep the old behavior, just
remove the call to remnulargs().  I think using parse_subscript() intead
of skipcomm() is still the more sensible approach; in fact, this patch
revealed an error in the E01 test script.

} (The -m option also doesn't work for subscripts.)

I haven't attempted to do anything about that, and I'm not sure that it
makes sense to.


diff --git a/Src/builtin.c b/Src/builtin.c
index 63f964d..4c8fbcd 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -3350,15 +3350,24 @@ bin_unset(char *name, char **argv, Options ops, int func)
     /* do not glob -- unset the given parameter */
     queue_signals();
     while ((s = *argv++)) {
-	char *ss = strchr(s, '[');
-	char *sse = ss;
+	char *ss = strchr(s, '['), *subscript = 0;
 	if (ss) {
-	    if (skipparens('[', ']', &sse) || *sse) {
-		zerrnam(name, "%s: invalid parameter name", s);
-		returnval = 1;
-		continue;
-	    }
+	    char *sse;
 	    *ss = 0;
+	    if ((sse = parse_subscript(ss+1, 1, ']'))) {
+		*sse = 0;
+		subscript = dupstring(ss+1);
+		*sse = ']';
+		remnulargs(subscript);
+		untokenize(subscript);
+	    }
+	}
+	if ((ss && !subscript) || !isident(s)) {
+	    if (ss)
+		*ss = '[';
+	    zerrnam(name, "%s: invalid parameter name", s);
+	    returnval = 1;
+	    continue;
 	}
 	pm = (Param) (paramtab == realparamtab ?
 		      /* getnode2() to avoid autoloading */
@@ -3376,11 +3385,8 @@ bin_unset(char *name, char **argv, Options ops, int func)
 	} else if (ss) {
 	    if (PM_TYPE(pm->node.flags) == PM_HASHED) {
 		HashTable tht = paramtab;
-		if ((paramtab = pm->gsu.h->getfn(pm))) {
-		    *--sse = 0;
-		    unsetparam(ss+1);
-		    *sse = ']';
-		}
+		if ((paramtab = pm->gsu.h->getfn(pm)))
+		    unsetparam(subscript);
 		paramtab = tht;
 	    } else if (PM_TYPE(pm->node.flags) == PM_SCALAR ||
 		       PM_TYPE(pm->node.flags) == PM_ARRAY) {
diff --git a/Test/E01options.ztst b/Test/E01options.ztst
index f270767..40e96af 100644
--- a/Test/E01options.ztst
+++ b/Test/E01options.ztst
@@ -776,7 +776,7 @@
   unsetopt pathdirs
   pathtestdir/findme
   path=($oldpath)
-  unset $oldpath
+  unset oldpath
   rm -rf pdt_topdir pathtestdir
 0:PATH_DIRS option
 >File in upper dir

Re: unset "hash[key]" isn't matched with what "key" may be

[Bart Schaefer]

On Feb 7, 11:05pm, Bart Schaefer wrote:
}
} This does reflect a change from previous behavior, so we'll want to
} consider it carefully.

Since no one commented to the contrary, I've pushed this so we can get
some additional experience with it.

Re: unset "hash[key]" isn't matched with what "key" may be

[Peter Stephenson]

On Mon, 08 Feb 2016 20:54:18 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Feb 7, 11:05pm, Bart Schaefer wrote:
> }
> } This does reflect a change from previous behavior, so we'll want to
> } consider it carefully.
> 
> Since no one commented to the contrary, I've pushed this so we can get
> some additional experience with it.

The old code is clearly unusable for some characters in the key, so a
change of some sort is warranted.

All that occurred to me was an additional option to provoke an extra
expansion step on the argument after locating the brackets.

  unset -? 'hash[$i]'

would then be reliable.  If the extra backslash stripping occurred early
enough it wouldn't get in the way.

pws

Re: unset "hash[key]" isn't matched with what "key" may be

[Bart Schaefer]

On Feb 9,  8:53am, Peter Stephenson wrote:
} Subject: Re: unset "hash[key]" isn't matched with what "key" may be
}
} All that occurred to me was an additional option to provoke an extra
} expansion step on the argument after locating the brackets.
} 
}   unset -? 'hash[$i]'
} 
} would then be reliable.  If the extra backslash stripping occurred early
} enough it wouldn't get in the way.

So what magic makes this work for typeset?

torch% typeset -A foo
torch% x='['
torch% typeset "foo[$x]"     
zsh: not an identifier: foo[[]
torch% typeset 'foo[$x]'		<-- note here
torch% typeset -p foo
typeset -A foo=( '[' '' )
torch% 

Or perhaps an equally interesting question is, is that behavior in fact
incorrect and typeset shouldn't do it either?

Ksh:

$ x='['
$ typeset -A foo
$ typeset "foo[$x]"
ksh: typeset: foo[[]: invalid variable name
$ typeset foo[$x]				<-- not an error?
$ typeset -p foo
typeset -A foo=(['[]']=)			<-- but weird
$ typeset 'foo[$x]'
$ typeset -p foo
typeset -A foo=(['$x']= ['[]']=)		<-- no expansion
$ 

Re: unset "hash[key]" isn't matched with what "key" may be

[Peter Stephenson]

On Tue, 09 Feb 2016 07:50:50 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> So what magic makes this work for typeset?
> 
> torch% typeset -A foo
> torch% x='['
> torch% typeset "foo[$x]"     
> zsh: not an identifier: foo[[]
> torch% typeset 'foo[$x]'		<-- note here

getindex() does parse_subcript(); later getarg() looks for tokens and if
it finds any calls singsub().  "unset" doesn't have any of this
sophistication.

pws

Re: unset "hash[key]" isn't matched with what "key" may be

[Bart Schaefer]

On Feb 9,  4:19pm, Peter Stephenson wrote:
}
} getindex() does parse_subcript(); later getarg() looks for tokens and
} if it finds any calls singsub(). "unset" doesn't have any of this
} sophistication.

Hmm.  For a normal array:

torch% x=3
torch% typeset 'foo[$x]'=1
torch% typeset -p foo
typeset -a foo=( '' '' 1 )
torch% unset 'foo[$x]'			<-- Note
torch% typeset -p foo
typeset -a foo=( '' '' '' )
torch% 

It didn't actually delete the element, but it did *something*.  (*Should*
it delete the element, as long as we're considering messing with this?)

Re: unset "hash[key]" isn't matched with what "key" may be

[Sebastian Gniazdowski]

On 7 February 2016 at 22:33, Bart Schaefer <schaefer@brasslantern.com> wrote:
> [I see PWS has already responded while I was trying to work out what the
> code is doing.  Here's what I've written so far, maybe I can come up with
> a patch in a bit longer.]
>
> On Feb 7,  3:16pm, Sebastian Gniazdowski wrote:
> }
> } I was testing following plugin:
> }
> } https://github.com/hchbaw/opp.zsh
>
> Which incidentally says (README.md) it is obsolete for zsh >= 5.0.8.

Which is rather a public suicide than the truth. I didn't seen any
functionality in Zsh >= 5.0.8 that would allow to jump between opening
and closing parentheses with "%" key.

Best regards,
Sebastian Gniazdowski

Re: unset "hash[key]" isn't matched with what "key" may be

[Nikolay Aleksandrovich Pavlov (ZyX)]

15.02.2016, 10:49, "Sebastian Gniazdowski" <sgniazdowski@gmail.com>:
> On 7 February 2016 at 22:33, Bart Schaefer <schaefer@brasslantern.com> wrote:
>>  [I see PWS has already responded while I was trying to work out what the
>>  code is doing. Here's what I've written so far, maybe I can come up with
>>  a patch in a bit longer.]
>>
>>  On Feb 7, 3:16pm, Sebastian Gniazdowski wrote:
>>  }
>>  } I was testing following plugin:
>>  }
>>  } https://github.com/hchbaw/opp.zsh
>>
>>  Which incidentally says (README.md) it is obsolete for zsh >= 5.0.8.
>
> Which is rather a public suicide than the truth. I didn't seen any
> functionality in Zsh >= 5.0.8 that would allow to jump between opening
> and closing parentheses with "%" key.

I have zsh-5.1.1 and it works with `bindkey -v`. This is vi-match-bracket widget that is bound to `%` in vicmd keymap.

>
> Best regards,
> Sebastian Gniazdowski