Weird bug with "bindkey" in a function

Weird bug with "bindkey" in a function

[Bart Schaefer]

I was playing around with something and wrote the following little
function:

bindfoo () {
        bindkey -v
        bindkey "[A" up-line-or-history
        bindkey "[B" down-line-or-history
        bindkey "[C" forward-char
        bindkey "[D" backward-char
        bindkey -r '\e[A'
        bindkey -r '\e[B'
        bindkey -r '\e[C'
        bindkey -r '\e[D'
        bindkey
        bindkey -e
}

The weird thing is, if any of the "bindkey" commands in that function fails
(e.g. with "bindkey: in-string is not bound") then the ENTIRE FUNCTION exits.

What the devil is up with that?

zsh 3.0.5

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Re: Weird bug with "bindkey" in a function

[Andrew Main]

Bart Schaefer wrote:
>The weird thing is, if any of the "bindkey" commands in that function fails
>(e.g. with "bindkey: in-string is not bound") then the ENTIRE FUNCTION exits.

Possibly related: can anyone tell me what the global `errflag' does?

>zsh 3.0.5

That version has another (very old) bug that prevents multi-character
keybindings from working properly.  This didn't get fixed until 3.1.
(By the time I posted a patch for 3.0, it was too late to make radical
changes.)

-zefram

PATCH: 3.1 Re: Weird bug with "bindkey"

[Peter Stephenson]

> The weird thing is, if any of the "bindkey" commands in that function fails
> (e.g. with "bindkey: in-string is not bound") then the ENTIRE FUNCTION exits.
> 
> What the devil is up with that?
> 
> zsh 3.0.5

As Zefram suggested, it's to do with errflag, which is completely
global:  it will abort back to the main command loop, or exit a
script.  Originally, all zsh internal errors worked like this, until
some mug with nothing better to do (guess who?) introduced zwarnnam(),
which doesn't pass on errflag.  There was some discussion at the time
about what should do which, but with the whole shell to rewrite in
this fashion things were obviously going to get missed, so I would
guess this bug has always been there.

Here's a patch for 3.1, which simply replaces zerrnam() with
zwarnnam() everywhere in zle_keymap.c (the unidiff was very much
shorter so I've sent that, but mail me if you need a context diff).

Looks like I've got 3.0.x lying around, so I'll dig out a patch for
that, too.

--- Src/Zle/zle_keymap.c.warn	Tue Jan 13 11:41:41 1998
+++ Src/Zle/zle_keymap.c	Wed Mar 25 10:10:29 1998
@@ -604,17 +604,17 @@
     if(op->o)
 	for(opp = op; (++opp)->o; )
 	    if(ops[opp->o]) {
-		zerrnam(name, "incompatible operation selection options",
+		zwarnnam(name, "incompatible operation selection options",
 		    NULL, 0);
 		return 1;
 	    }
     n = ops['e'] + ops['v'] + ops['a'] + ops['M'];
     if(!op->selp && n) {
-	zerrnam(name, "keymap cannot be selected with -%c", NULL, op->o);
+	zwarnnam(name, "keymap cannot be selected with -%c", NULL, op->o);
 	return 1;
     }
     if(n > 1) {
-	zerrnam(name, "incompatible keymap selection options", NULL, 0);
+	zwarnnam(name, "incompatible keymap selection options", NULL, 0);
 	return 1;
     }
 
@@ -629,14 +629,14 @@
 	else if(ops['M']) {
 	    kmname = *argv++;
 	    if(!kmname) {
-		zerrnam(name, "-M option requires a keymap argument", NULL, 0);
+		zwarnnam(name, "-M option requires a keymap argument", NULL, 0);
 		return 1;
 	    }
 	} else
 	    kmname = "main";
 	km = openkeymap(kmname);
 	if(!km) {
-	    zerrnam(name, "no such keymap `%s'", kmname, 0);
+	    zwarnnam(name, "no such keymap `%s'", kmname, 0);
 	    return 1;
 	}
 	if(ops['e'] || ops['v'])
@@ -656,10 +656,10 @@
     /* check number of arguments */
     for(n = 0; argv[n]; n++) ;
     if(n < op->min) {
-	zerrnam(name, "not enough arguments for -%c", NULL, op->o);
+	zwarnnam(name, "not enough arguments for -%c", NULL, op->o);
 	return 1;
     } else if(op->max != -1 && n > op->max) {
-	zerrnam(name, "too many arguments for -%c", NULL, op->o);
+	zwarnnam(name, "too many arguments for -%c", NULL, op->o);
 	return 1;
     }
 
@@ -731,10 +731,10 @@
 {
     km = openkeymap(argv[0]);
     if(!km) {
-	zerrnam(name, "no such keymap `%s'", argv[0], 0);
+	zwarnnam(name, "no such keymap `%s'", argv[0], 0);
 	return 1;
     } else if(linkkeymap(km, argv[1])) {
-	zerrnam(name, "keymap name `%s' is protected", argv[1], 0);
+	zwarnnam(name, "keymap name `%s' is protected", argv[1], 0);
 	return 1;
     }
     return 0;
@@ -749,13 +749,13 @@
     KeymapName kmn = (KeymapName) keymapnamtab->getnode(keymapnamtab, argv[0]);
 
     if(kmn && (kmn -> flags & KMN_IMMORTAL)) {
-	zerrnam(name, "keymap name `%s' is protected", argv[0], 0);
+	zwarnnam(name, "keymap name `%s' is protected", argv[0], 0);
 	return 1;
     }
     if(argv[1]) {
 	km = openkeymap(argv[1]);
 	if(!km) {
-	    zerrnam(name, "no such keymap `%s'", argv[0], 0);
+	    zwarnnam(name, "no such keymap `%s'", argv[0], 0);
 	    return 1;
 	}
     } else
@@ -779,7 +779,7 @@
     Thingy fn;
 
     if(km->flags & KM_IMMUTABLE) {
-	zerrnam(name, "keymap `%s' is protected", kmname, 0);
+	zwarnnam(name, "keymap `%s' is protected", kmname, 0);
 	return 1;
     }
     for(i = 128; i < 256; i++)
@@ -811,12 +811,12 @@
 
 	for(a = argv+2; *a; a++)
 	    if(!*++a) {
-		zerrnam(name, "even number of arguments required", NULL, 0);
+		zwarnnam(name, "even number of arguments required", NULL, 0);
 		return 1;
 	    }
     }
     if(km->flags & KM_IMMUTABLE) {
-	zerrnam(name, "keymap `%s' is protected", kmname, 0);
+	zwarnnam(name, "keymap `%s' is protected", kmname, 0);
 	return 1;
     }
     do {

-- 
Peter Stephenson <pws@ifh.de>       Tel: +39 50 844536
WWW:  http://www.ifh.de/~pws/
Gruppo Teorico, Dipartimento di Fisica
Piazza Torricelli 2, 56100 Pisa, Italy

PATCH: 3.0.5 Re: Weird bug with "bindkey"

[Peter Stephenson]

> Here's a patch for 3.1, which simply replaces zerrnam() with
> zwarnnam() everywhere in zle_keymap.c (the unidiff was very much
> shorter so I've sent that, but mail me if you need a context diff).
> 
> Looks like I've got 3.0.x lying around, so I'll dig out a patch for
> that, too.

This is the corresponding patch.  Key binding was rather simpler, so
it's shorter.  One of the errors was a zerr() instead of a zerrnam(),
which are not usually used in builtins.  I've just assumed that was an
inconsistency. (Goodness!  An inconsistency in zsh!  Whatever next, a
stone in the Great Wall of China??)

If you're looking around, you'll notice other zerr()'s in zle_main.c
haven't been touched, because they're part of the editor itself, not
bindkey, so they really should abort back to main command level.  I
think.  All zerr/zerrnam vs. zwarnnam in the shell are potentially
negotiable (although builtins should certainly have zwarnnam(), as
here).

--- Src/zle_main.c.warn	Fri Sep 26 03:42:19 1997
+++ Src/zle_main.c	Wed Mar 25 10:29:02 1998
@@ -888,12 +888,12 @@
     int i, *tab;
 
     if (ops['v'] && ops['e']) {
-	zerrnam(name, "incompatible options", NULL, 0);
+	zwarnnam(name, "incompatible options", NULL, 0);
 	return 1;
     }
     if (ops['v'] || ops['e'] || ops['d'] || ops['m']) {
 	if (*argv) {
-	    zerrnam(name, "too many arguments", NULL, 0);
+	    zwarnnam(name, "too many arguments", NULL, 0);
 	    return 1;
 	}
 	if (ops['d']) {
@@ -962,7 +962,7 @@
 		func = (ky = (Key) keybindtab->getnode(keybindtab, s)) ? ky->func
 		    : z_undefinedkey;
 	    if (func == z_undefinedkey) {
-		zerrnam(name, "in-string is not bound", NULL, 0);
+		zwarnnam(name, "in-string is not bound", NULL, 0);
 		zfree(s, len);
 		return 1;
 	    }
@@ -1022,7 +1022,7 @@
 		if (!strcmp(*argv, zlecmds[i].name))
 		    break;
 	    if (i == ZLECMDCOUNT) {
-		zerr("undefined function: %s", *argv, 0);
+		zwarnnam(name, "undefined function: %s", *argv, 0);
 		zfree(s, len);
 		return 1;
 	    }

-- 
Peter Stephenson <pws@ifh.de>       Tel: +39 50 844536
WWW:  http://www.ifh.de/~pws/
Gruppo Teorico, Dipartimento di Fisica
Piazza Torricelli 2, 56100 Pisa, Italy