inconsistency of invalid identifier error messages

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

The following seems somewhat inconsistent. Compare the error messages
and note the difference in whether the error is printed before or after
the edits.

vared -c a-b
vared -c ''
vared -- -a
vared -- -
read a-b
unset a-b
unset 'a[b'

I think unset should be producing an error in both cases.  It's fair
that it quitely succeeds if you unset an already unset parameter but an
invalid parameter name is different.

Any preference on what error message to standardise on. We currently
have:
not an identifier: -
not an identifier: `-a'
invalid parameter name: a-b
a[b: invalid parameter name

Also, any views on whether errors should be printed before or after or
whether it is correct that read consumes the input regardless while vared
immediately prints the error message? Bash and Ksh have the opposite
behaviour in this regard.

The patch below makes the first two (vared) cases consistent with the
third one. Any thoughts on how to best fix the last vared case (editing
$-) and what to do about unset? With this patch, I'm also assuming that
the first error condition should have been calling unqueue_signals().

Oliver

Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.120
diff -u -r1.120 zle_main.c
--- Src/Zle/zle_main.c	12 Nov 2008 12:59:07 -0000	1.120
+++ Src/Zle/zle_main.c	12 Nov 2008 13:17:55 -0000
@@ -1492,11 +1492,11 @@
 	unqueue_signals();
 	zwarnnam(name, "no such variable: %s", args[0]);
 	return 1;
+    } else if (*s || s == args[0]) {
+	unqueue_signals();
+	zwarnnam(name, "not an identifier: %s", args[0]);
+	return 1;
     } else if (v) {
-	if (*s) {
-	    zwarnnam(name, "not an identifier: `%s'", args[0]);
-	    return 1;
-	}
 	if (v->isarr) {
 	    /* Array: check for separators and quote them. */
 	    char **arr = getarrvalue(v), **aptr, **tmparr, **tptr;
@@ -1549,10 +1549,6 @@
 	    s = ztrdup(getstrvalue(v));
 	}
 	unqueue_signals();
-    } else if (*s) {
-	unqueue_signals();
-	zwarnnam(name, "invalid parameter name: %s", args[0]);
-	return 1;
     } else {
 	unqueue_signals();
 	s = ztrdup(s);