zsh-workers
 help / color / mirror / code / Atom feed
* Re: PATCH: Re: The `zle' command and traps
@ 2000-03-14  9:54 Sven Wischnowsky
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Wischnowsky @ 2000-03-14  9:54 UTC (permalink / raw)
  To: zsh-workers


I wrote:

> ...
>
> > If we can restrict widgets to those that do "safe" things, then we can
> > allow "zle widgetname" and there's no need for the special parameters to
> > be visible directly to the trap function.  If instead we must prohibit
> > widget-calling entirely, then perhaps the parameters should be available
> > to the trap function.
> 
> No patch for that yet, either. The problem is that traps are called
> from the core (of course) and the zle module might not be there. Maybe 
> calling a hook in dotrapargs() so that zle can install its own hook
> function when it is loaded to make the parameters available read-only
> when zle is active. Or does this sound too complicated to anyone?

Well, it didn't look too complicated to me... this also documents in
zle.yo that the zle parameters are available in completion widgets and 
traps called when zle is active.


Bye
 Sven

diff -ru ../z.old/Doc/Zsh/zle.yo Doc/Zsh/zle.yo
--- ../z.old/Doc/Zsh/zle.yo	Tue Mar 14 10:39:02 2000
+++ Doc/Zsh/zle.yo	Tue Mar 14 10:42:35 2000
@@ -115,6 +115,9 @@
 when the widget function exits.  These special parameters in fact have
 local scope, like parameters created in a function using tt(local).
 
+Inside completion widgets and traps called while ZLE is active, these
+parameters are available read-only.
+
 startitem()
 vindex(BUFFER)
 item(tt(BUFFER) (scalar))(
diff -ru ../z.old/Src/Zle/zle_main.c Src/Zle/zle_main.c
--- ../z.old/Src/Zle/zle_main.c	Tue Mar 14 10:38:53 2000
+++ Src/Zle/zle_main.c	Tue Mar 14 10:41:09 2000
@@ -992,6 +992,28 @@
 	kungetct = 0;
 }
 
+/* Hook functions. Used to allow access to zle parameters if zle is
+ * active. */
+
+static int
+zlebeforetrap(Hookdef dummy, void *dat)
+{
+    if (zleactive) {
+	startparamscope();
+	makezleparams(1);
+    }
+    return 0;
+}
+
+static int
+zleaftertrap(Hookdef dummy, void *dat)
+{
+    if (zleactive)
+	endparamscope();
+
+    return 0;
+}
+
 static struct builtin bintab[] = {
     BUILTIN("bindkey", 0, bin_bindkey, 0, -1, 0, "evaMldDANmrsLR", NULL),
     BUILTIN("vared",   0, bin_vared,   1,  7, 0, NULL,             NULL),
@@ -1049,6 +1071,8 @@
 int
 boot_(Module m)
 {
+    addhookfunc("before_trap", (Hookfn) zlebeforetrap);
+    addhookfunc("after_trap", (Hookfn) zleaftertrap);
     addbuiltins(m->nam, bintab, sizeof(bintab)/sizeof(*bintab));
     addhookdefs(m->nam, zlehooks, sizeof(zlehooks)/sizeof(*zlehooks));
     return 0;
@@ -1063,6 +1087,8 @@
 	    NULL, 0);
 	return 1;
     }
+    deletehookfunc("before_trap", (Hookfn) zlebeforetrap);
+    deletehookfunc("after_trap", (Hookfn) zleaftertrap);
     deletebuiltins(m->nam, bintab, sizeof(bintab)/sizeof(*bintab));
     deletehookdefs(m->nam, zlehooks, sizeof(zlehooks)/sizeof(*zlehooks));
     return 0;
diff -ru ../z.old/Src/init.c Src/init.c
--- ../z.old/Src/init.c	Tue Mar 14 10:38:47 2000
+++ Src/init.c	Tue Mar 14 10:41:09 2000
@@ -90,6 +90,8 @@
 /**/
 mod_export struct hookdef zshhooks[] = {
     HOOKDEF("exit", NULL, HOOKF_ALL),
+    HOOKDEF("before_trap", NULL, HOOKF_ALL),
+    HOOKDEF("after_trap", NULL, HOOKF_ALL),
 };
 
 /* keep executing lists until EOF found */
diff -ru ../z.old/Src/signals.c Src/signals.c
--- ../z.old/Src/signals.c	Tue Mar 14 10:38:49 2000
+++ Src/signals.c	Tue Mar 14 10:41:09 2000
@@ -894,6 +894,7 @@
     lexsave();
     execsave();
     breaks = 0;
+    runhookdef(BEFORETRAPHOOK, NULL);
     if (*sigtr & ZSIG_FUNC) {
 	int osc = sfcontext;
 
@@ -912,6 +913,7 @@
 	zsfree(name);
     } else
 	execode(sigfn, 1, 0);
+    runhookdef(AFTERTRAPHOOK, NULL);
 
     if (trapreturn > 0)
 	trapret = trapreturn;
diff -ru ../z.old/Src/zsh.h Src/zsh.h
--- ../z.old/Src/zsh.h	Tue Mar 14 10:38:50 2000
+++ Src/zsh.h	Tue Mar 14 10:41:10 2000
@@ -1675,4 +1675,6 @@
 /* Hooks in core.                      */
 /***************************************/
 
-#define EXITHOOK (zshhooks + 0)
+#define EXITHOOK       (zshhooks + 0)
+#define BEFORETRAPHOOK (zshhooks + 1)
+#define AFTERTRAPHOOK  (zshhooks + 2)

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Re: The `zle' command and traps
  2000-03-14 10:35 Sven Wischnowsky
@ 2000-03-14 16:56 ` Bart Schaefer
  0 siblings, 0 replies; 6+ messages in thread
From: Bart Schaefer @ 2000-03-14 16:56 UTC (permalink / raw)
  To: Sven Wischnowsky, zsh-workers

On Mar 14, 11:35am, Sven Wischnowsky wrote:
} Subject: Re: PATCH: Re: The `zle' command and traps
}
} Killing loops containing only builtins should work fine, because we
} set breaks=loops (and errflag, btw) in the handler -- and it /seems/
} to work fine for me.

Hrm.  I got zsh into a state yesterday where running this loop:

    for ((i=10000; i; --i)) echo LOOP

was not killable.  I had to blow it away with 'kill -9' from another
terminal (because I'd used a much bigger number than 10000 and didn't
want to wait).  I tried sending it INTs, HUPs, QUITs and TERMs with no
effect, after I was unable to ^C it.

Today when I try the same thing in a fresh shell, I'm able to interrupt
it with ^C.  The unkillable shell hadn't been running for more that a
couple of hours, but long enough for me to have forgotten even vaguely
what I did to get it into that state.  All I know is that it was one
where I'd been experimenting with various things ... hmmm, I bet some
function I ran, used the "trap" command without "setopt localtraps".

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Re: The `zle' command and traps
@ 2000-03-14 10:35 Sven Wischnowsky
  2000-03-14 16:56 ` Bart Schaefer
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Wischnowsky @ 2000-03-14 10:35 UTC (permalink / raw)
  To: zsh-workers


Bart Schaefer wrote:

> On Mar 13,  5:49pm, Sven Wischnowsky wrote:
> } Subject: Re: PATCH: Re: The `zle' command and traps
> }
> } After a bit of checking... I think in many places where we call
> } zerrnam() we should really be calling zwarnnam() in functions
> } implementing builtins. Otherwise a error reported by the builtin will
> } exits loops and sublists and whatnot.
> 
> I think this is done because it's so difficult (still) to stop/kill a
> loop that contains nothing but builtins.  If you've got a builtin going
> wrong and that prevents the loop from ever reaching the end ...

Yes, that's why people clerer than me sometimes used zerrnam() and
sometimes zwarnnam() (and `continue' was a bad example, of course).

> We really need to figure out how to handle interrupts during builtins
> before we start fooling with clearing errflag in additional cases.

The patch below looks monstrous, but it actually makes the use of
zwarnnam() more consistent. Most calls to zerrnam() changed come from
me, because I wasn't aware of *this* effect of zerrnam(), so I hope
that's ok for you. And if we don't change it, the return value of many
builtins is completely useless.

Killing loops containing only builtins should work fine, because we
set breaks=loops (and errflag, btw) in the handler -- and it /seems/
to work fine for me. Stopping, on the other hand...

Bye
 Sven

diff -ru ../z.old/Src/Modules/stat.c Src/Modules/stat.c
--- ../z.old/Src/Modules/stat.c	Tue Mar 14 11:19:56 2000
+++ Src/Modules/stat.c	Tue Mar 14 11:24:16 2000
@@ -347,10 +347,10 @@
 		    iwhich = aptr - statelts;
 		}
 	    if (found > 1) {
-		zerrnam(name, "%s: ambiguous stat element", arg, 0);
+		zwarnnam(name, "%s: ambiguous stat element", arg, 0);
 		return 1;
 	    } else if (found == 0) {
-		zerrnam(name, "%s: no such stat element", arg, 0);
+		zwarnnam(name, "%s: no such stat element", arg, 0);
 		return 1;
 	    }
 	    /* if name of link requested, turn on lstat */
@@ -365,7 +365,7 @@
 		    if (arg[1]) {
 			arrnam = arg+1;
 		    } else if (!(arrnam = *++args)) {
-			zerrnam(name, "missing parameter name\n",
+			zwarnnam(name, "missing parameter name",
 				NULL, 0);
 			return 1;
 		    }
@@ -375,7 +375,7 @@
 		    if (arg[1]) {
 			hashnam = arg+1;
 		    } else if (!(hashnam = *++args)) {
-			zerrnam(name, "missing parameter name\n",
+			zwarnnam(name, "missing parameter name",
 				NULL, 0);
 			return 1;
 		    }
@@ -387,12 +387,12 @@
 		    if (arg[1]) {
 			sfd = arg+1;
 		    } else if (!(sfd = *++args)) {
-			zerrnam(name, "missing file descriptor\n", NULL, 0);
+			zwarnnam(name, "missing file descriptor", NULL, 0);
 			return 1;
 		    }
 		    fd = zstrtol(sfd, &sfd, 10);
 		    if (*sfd) {
-			zerrnam(name, "bad file descriptor\n", NULL, 0);
+			zwarnnam(name, "bad file descriptor", NULL, 0);
 			return 1;
 		    }
 		    break;
@@ -400,14 +400,14 @@
 		    if (arg[1]) {
 			timefmt = arg+1;
 		    } else if (!(timefmt = *++args)) {
-			zerrnam(name, "missing time format\n", NULL, 0);
+			zwarnnam(name, "missing time format", NULL, 0);
 			return 1;
 		    }
 		    /* force string format in order to use time format */
 		    ops['s'] = 1;
 		    break;
 		} else {
-		    zerrnam(name, "bad option: -%c", NULL, *arg);
+		    zwarnnam(name, "bad option: -%c", NULL, *arg);
 		    return 1;
 		}
 	    }
diff -ru ../z.old/Src/Modules/zpty.c Src/Modules/zpty.c
--- ../z.old/Src/Modules/zpty.c	Tue Mar 14 11:19:56 2000
+++ Src/Modules/zpty.c	Tue Mar 14 11:24:17 2000
@@ -261,17 +261,17 @@
     char *cmd;
 
     if (!(cmd = findcmd(*args, 1))) {
-	zerrnam(nam, "unknown command: %s", *args, 0);
+	zwarnnam(nam, "unknown command: %s", *args, 0);
 	return 1;
     }
     if (get_pty(&master, &slave)) {
-	zerrnam(nam, "can't open pseudo terminal", NULL, 0);
+	zwarnnam(nam, "can't open pseudo terminal", NULL, 0);
 	return 1;
     }
     if ((pid = fork()) == -1) {
 	close(master);
 	close(slave);
-	zerrnam(nam, "couldn't create pty command: %s", pname, 0);
+	zwarnnam(nam, "couldn't create pty command: %s", pname, 0);
 	return 1;
     } else if (!pid) {
 	if (!echo) {
@@ -404,14 +404,14 @@
 	char *p;
 
 	if (args[2]) {
-	    zerrnam(nam, "too many arguments", NULL, 0);
+	    zwarnnam(nam, "too many arguments", NULL, 0);
 	    return 1;
 	}
 	p = dupstring(args[1]);
 	tokenize(p);
 	remnulargs(p);
 	if (!(prog = patcompile(p, PAT_STATIC, NULL))) {
-	    zerrnam(nam, "bad pattern: %s", args[1], 0);
+	    zwarnnam(nam, "bad pattern: %s", args[1], 0);
 	    return 1;
 	}
     }
@@ -526,17 +526,17 @@
 		      ops['d'] || ops['L'])) ||
 	(ops['d'] && (ops['b'] || ops['e'] || ops['L'])) ||
 	(ops['L'] && (ops['b'] || ops['e']))) {
-	zerrnam(nam, "illegal option combination", NULL, 0);
+	zwarnnam(nam, "illegal option combination", NULL, 0);
 	return 1;
     }
     if (ops['r'] || ops['w']) {
 	Ptycmd p;
 
 	if (!*args) {
-	    zerrnam(nam, "missing pty command name", NULL, 0);
+	    zwarnnam(nam, "missing pty command name", NULL, 0);
 	    return 1;
 	} else if (!(p = getptycmd(*args))) {
-	    zerrnam(nam, "no such pty command: %s", *args, 0);
+	    zwarnnam(nam, "no such pty command: %s", *args, 0);
 	    return 1;
 	}
 	checkptycmd(p);
@@ -563,11 +563,11 @@
 	return ret;
     } else if (*args) {
 	if (!args[1]) {
-	    zerrnam(nam, "missing command", NULL, 0);
+	    zwarnnam(nam, "missing command", NULL, 0);
 	    return 1;
 	}
 	if (getptycmd(*args)) {
-	    zerrnam(nam, "pty command name already used: %s", *args, 0);
+	    zwarnnam(nam, "pty command name already used: %s", *args, 0);
 	    return 1;
 	}
 	return newptycmd(nam, *args, args + 1, ops['e'], ops['b']);
diff -ru ../z.old/Src/Modules/zutil.c Src/Modules/zutil.c
--- ../z.old/Src/Modules/zutil.c	Tue Mar 14 11:19:56 2000
+++ Src/Modules/zutil.c	Tue Mar 14 11:24:17 2000
@@ -213,7 +213,7 @@
 
 	if ((oc = args[0][1]) && oc != '-') {
 	    if (args[0][2]) {
-		zerrnam(nam, "invalid argument: %s", args[0], 0);
+		zwarnnam(nam, "invalid argument: %s", args[0], 0);
 		return 1;
 	    }
 	    if (oc == 'L')
@@ -231,14 +231,14 @@
 	char *pat;
 
 	if (arrlen(args) < 2) {
-	    zerrnam(nam, "not enough arguments", NULL, 0);
+	    zwarnnam(nam, "not enough arguments", NULL, 0);
 	    return 1;
 	}
 	pat = dupstring(args[0]);
 	tokenize(pat);
 
 	if (!(prog = patcompile(pat, PAT_ZDUP, NULL))) {
-	    zerrnam(nam, "invalid pattern: %s", args[0], 0);
+	    zwarnnam(nam, "invalid pattern: %s", args[0], 0);
 	    return 1;
 	}
 	if (!(s = getstyle(args[1])))
@@ -285,15 +285,15 @@
     case 'm': min = 3; max =  3; break;
     case 'g': min = 1; max =  3; break;
     default:
-	zerrnam(nam, "invalid option: %s", args[0], 0);
+	zwarnnam(nam, "invalid option: %s", args[0], 0);
 	return 1;
     }
     n = arrlen(args) - 1;
     if (n < min) {
-	zerrnam(nam, "not enough arguments", NULL, 0);
+	zwarnnam(nam, "not enough arguments", NULL, 0);
 	return 1;
     } else if (max >= 0 && n > max) {
-	zerrnam(nam, "too many arguments", NULL, 0);
+	zwarnnam(nam, "too many arguments", NULL, 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -511,7 +511,7 @@
     char opt;
 
     if (args[0][0] != '-' || !(opt = args[0][1]) || args[0][2]) {
-	zerrnam(nam, "invalid argument: %s", args[0], 0);
+	zwarnnam(nam, "invalid argument: %s", args[0], 0);
 	return 1;
     }
     args++;
@@ -528,7 +528,7 @@
 		if (!ap[0][0] || ap[0][0] == '-' || ap[0][0] == '.' ||
 		    (ap[0][0] >= '0' && ap[0][0] <= '9') ||
 		    ap[0][1] != ':') {
-		    zerrnam(nam, "invalid argument: %s", *ap, 0);
+		    zwarnnam(nam, "invalid argument: %s", *ap, 0);
 		    return 1;
 		}
 		specs[STOUC(ap[0][0])] = ap[0] + 2;
@@ -670,7 +670,7 @@
 	}
 	break;
     }
-    zerrnam(nam, "invalid option: -%c", 0, opt);
+    zwarnnam(nam, "invalid option: -%c", 0, opt);
     return 1;
 }
 
@@ -1298,7 +1298,7 @@
 		break;
 	    case 'a':
 		if (defarr) {
-		    zerrnam(nam, "default array given more than once", NULL, 0);
+		    zwarnnam(nam, "default array given more than once", NULL, 0);
 		    return 1;
 		}
 		if (o[2])
@@ -1306,7 +1306,7 @@
 		else if (*args)
 		    n = *args++;
 		else {
-		    zerrnam(nam, "missing array name", NULL, 0);
+		    zwarnnam(nam, "missing array name", NULL, 0);
 		    return 1;
 		}
 		defarr = (Zoptarr) zhalloc(sizeof(*defarr));
@@ -1322,7 +1322,7 @@
 		else if (*args)
 		    assoc = *args++;
 		else {
-		    zerrnam(nam, "missing array name", NULL, 0);
+		    zwarnnam(nam, "missing array name", NULL, 0);
 		    return 1;
 		}
 		break;
@@ -1336,7 +1336,7 @@
     }
     while ((o = dupstring(*args++))) {
 	if (!*o) {
-	    zerrnam(nam, "invalid option description: %s", o, 0);
+	    zwarnnam(nam, "invalid option description: %s", o, 0);
 	    return 1;
 	}
 	f = 0;
@@ -1375,10 +1375,10 @@
 		opt_arrs = a;
 	    }
 	} else if (*p) {
-	    zerrnam(nam, "invalid option description: %s", args[-1], 0);
+	    zwarnnam(nam, "invalid option description: %s", args[-1], 0);
 	    return 1;
 	} else if (!(a = defarr) && !assoc) {
-	    zerrnam(nam, "no default array defined: %s", args[-1], 0);
+	    zwarnnam(nam, "no default array defined: %s", args[-1], 0);
 	    return 1;
 	}
 	for (p = n = o; *p; p++) {
@@ -1387,7 +1387,7 @@
 	    *n++ = *p;
 	}
 	if (get_opt_desc(o)) {
-	    zerrnam(nam, "option defined more than once: %s", o, 0);
+	    zwarnnam(nam, "option defined more than once: %s", o, 0);
 	    return 1;
 	}
 	d = (Zoptdesc) zhalloc(sizeof(*d));
@@ -1419,7 +1419,7 @@
 			break;
 		    } else if (!(d->flags & ZOF_OPT)) {
 			if (!pp[1]) {
-			    zerrnam(nam, "missing argument for option: %s",
+			    zwarnnam(nam, "missing argument for option: %s",
 				    d->name, 0);
 			    return 1;
 			}
@@ -1439,7 +1439,7 @@
 		    add_opt_val(d, e);
 		else if (!(d->flags & ZOF_OPT)) {
 		    if (!pp[1]) {
-			zerrnam(nam, "missing argument for option: %s",
+			zwarnnam(nam, "missing argument for option: %s",
 				d->name, 0);
 			return 1;
 		    }
diff -ru ../z.old/Src/Zle/compctl.c Src/Zle/compctl.c
--- ../z.old/Src/Zle/compctl.c	Tue Mar 14 11:19:52 2000
+++ Src/Zle/compctl.c	Tue Mar 14 11:24:17 2000
@@ -508,7 +508,7 @@
 		    char *p;
 
 		    if (cl) {
-			zerrnam(name, "illegal option -%c", NULL, **argv);
+			zwarnnam(name, "illegal option -%c", NULL, **argv);
 			return 1;
 		    }
 		    if ((*argv)[1]) {
@@ -654,7 +654,7 @@
 		break;
 	    case 'l':
 		if (cl) {
-		    zerrnam(name, "illegal option -%c", NULL, **argv);
+		    zwarnnam(name, "illegal option -%c", NULL, **argv);
 		    return 1;
 		} else if ((*argv)[1]) {
 		    cct.subcmd = (*argv) + 1;
@@ -670,7 +670,7 @@
 		break;
 	    case 'h':
 		if (cl) {
-		    zerrnam(name, "illegal option -%c", NULL, **argv);
+		    zwarnnam(name, "illegal option -%c", NULL, **argv);
 		    return 1;
 		} else if ((*argv)[1]) {
 		    cct.substr = (*argv) + 1;
@@ -783,7 +783,7 @@
 		break;
 	    case 'C':
 		if (cl) {
-		    zerrnam(name, "illegal option -%c", NULL, **argv);
+		    zwarnnam(name, "illegal option -%c", NULL, **argv);
 		    return 1;
 		}
 		if (first && !hx) {
@@ -796,7 +796,7 @@
 		break;
 	    case 'D':
 		if (cl) {
-		    zerrnam(name, "illegal option -%c", NULL, **argv);
+		    zwarnnam(name, "illegal option -%c", NULL, **argv);
 		    return 1;
 		}
 		if (first && !hx) {
@@ -810,7 +810,7 @@
 		break;
  	    case 'T':
 		if (cl) {
-		    zerrnam(name, "illegal option -%c", NULL, **argv);
+		    zwarnnam(name, "illegal option -%c", NULL, **argv);
 		    return 1;
 		}
 		if (first && !hx) {
@@ -823,7 +823,7 @@
  		break;
 	    case 'L':
 		if (cl) {
-		    zerrnam(name, "illegal option -%c", NULL, **argv);
+		    zwarnnam(name, "illegal option -%c", NULL, **argv);
 		    return 1;
 		}
 		if (!first || hx) {
@@ -834,7 +834,7 @@
 		break;
 	    case 'x':
 		if (cl) {
-		    zerrnam(name, "extended completion not allowed", NULL, 0);
+		    zwarnnam(name, "extended completion not allowed", NULL, 0);
 		    return 1;
 		}
 		if (!argv[1]) {
@@ -869,7 +869,7 @@
 	if (*++argv && (!ready || ready == 2) &&
 	    **argv == '+' && !argv[0][1]) {
 	    if (cl) {
-		zerrnam(name, "xor'ed completion illegal", NULL, 0);
+		zwarnnam(name, "xor'ed completion illegal", NULL, 0);
 		return 1;
 	    }
 	    /* There's an alternative (+) completion:  assign
@@ -1681,7 +1681,7 @@
 bin_compcall(char *name, char **argv, char *ops, int func)
 {
     if (incompfunc != 1) {
-	zerrnam(name, "can only be called from completion function", NULL, 0);
+	zwarnnam(name, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     return makecomplistctl((ops['T'] ? 0 : CFN_FIRST) |
diff -ru ../z.old/Src/Zle/complete.c Src/Zle/complete.c
--- ../z.old/Src/Zle/complete.c	Tue Mar 14 11:19:52 2000
+++ Src/Zle/complete.c	Tue Mar 14 11:24:18 2000
@@ -388,7 +388,7 @@
     Cmatcher match = NULL;
 
     if (incompfunc != 1) {
-	zerrnam(name, "can only be called from completion function", NULL, 0);
+	zwarnnam(name, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     dat.ipre = dat.isuf = dat.ppre = dat.psuf = dat.prpre =
@@ -521,7 +521,7 @@
 		argv++;
 		goto ca_args;
 	    default:
-		zerrnam(name, "bad option: -%c", NULL, *p);
+		zwarnnam(name, "bad option: -%c", NULL, *p);
 		return 1;
 	    }
 	    if (sp) {
@@ -535,7 +535,7 @@
 			*sp = *argv;
 		    p = "" - 1;
 		} else {
-		    zerrnam(name, e, NULL, *p);
+		    zwarnnam(name, e, NULL, *p);
 		    return 1;
 		}
 		if (dm) {
@@ -800,11 +800,11 @@
     char *sa = NULL, *sb = NULL;
 
     if (incompfunc != 1) {
-	zerrnam(name, "can only be called from completion function", NULL, 0);
+	zwarnnam(name, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     if (argv[0][0] != '-') {
-	zerrnam(name, "missing option", NULL, 0);
+	zwarnnam(name, "missing option", NULL, 0);
 	return 1;
     }
     switch (argv[0][1]) {
@@ -816,7 +816,7 @@
     case 'S': test = CVT_SUFPAT; break;
     case 'q': return set_comp_sep();
     default:
-	zerrnam(name, "bad option -%c", NULL, argv[0][1]);
+	zwarnnam(name, "bad option -%c", NULL, argv[0][1]);
 	return 1;
     }
     if (argv[0][2]) {
@@ -825,7 +825,7 @@
 	na = 2;
     } else {
 	if (!(sa = argv[1])) {
-	    zerrnam(name, "missing string for option -%c", NULL, argv[0][1]);
+	    zwarnnam(name, "missing string for option -%c", NULL, argv[0][1]);
 	    return 1;
 	}
 	sb = argv[2];
@@ -833,7 +833,7 @@
     }
     if (((test == CVT_PRENUM || test == CVT_SUFNUM) ? !!sb :
 	 (sb && argv[na]))) {
-	zerrnam(name, "too many arguments", NULL, 0);
+	zwarnnam(name, "too many arguments", NULL, 0);
 	return 1;
     }
     switch (test) {
diff -ru ../z.old/Src/Zle/computil.c Src/Zle/computil.c
--- ../z.old/Src/Zle/computil.c	Tue Mar 14 11:19:52 2000
+++ Src/Zle/computil.c	Tue Mar 14 11:24:18 2000
@@ -127,7 +127,7 @@
 	setp = &(set->next);
 
 	if (!(ap = get_user_var(*args))) {
-	    zerrnam(nam, "invalid argument: %s", *args, 0);
+	    zwarnnam(nam, "invalid argument: %s", *args, 0);
 	    return 1;
 	}
 	set->strs = zarrdup(ap);
@@ -137,7 +137,7 @@
 
 	if (*++args && **args != '-') {
 	    if (!(ap = get_user_var(*args))) {
-		zerrnam(nam, "invalid argument: %s", *args, 0);
+		zwarnnam(nam, "invalid argument: %s", *args, 0);
 		return 1;
 	    }
 	    set->matches = zarrdup(ap);
@@ -250,11 +250,11 @@
 bin_compdescribe(char *nam, char **args, char *ops, int func)
 {
     if (incompfunc != 1) {
-	zerrnam(nam, "can only be called from completion function", NULL, 0);
+	zwarnnam(nam, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     if (!args[0][0] || !args[0][1] || args[0][2]) {
-	zerrnam(nam, "invalid argument: %s", args[0], 0);
+	zwarnnam(nam, "invalid argument: %s", args[0], 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -269,17 +269,17 @@
 	    int n = arrlen(args);
 
 	    if (n != 6) {
-		zerrnam(nam, (n < 6 ? "not enough arguments" :
+		zwarnnam(nam, (n < 6 ? "not enough arguments" :
 			      "too many arguments"), NULL, 0);
 		return 1;
 	    }
 	    return cd_get(args + 1);
 	} else {
-	    zerrnam(nam, "no parsed state", NULL, 0);
+	    zwarnnam(nam, "no parsed state", NULL, 0);
 	    return 1;
 	}
     }
-    zerrnam(nam, "invalid option: %s", args[0], 0);
+    zwarnnam(nam, "invalid option: %s", args[0], 0);
     return 1;
 }
 
@@ -584,7 +584,7 @@
 	    /* Oops, end-of-string. */
 	    if (*p != ')') {
 		freecadef(ret);
-		zerrnam(nam, "invalid argument: %s", *args, 0);
+		zwarnnam(nam, "invalid argument: %s", *args, 0);
 		return NULL;
 	    }
 	    xor = (char **) zalloc((xnum + 2) * sizeof(char *));
@@ -627,7 +627,7 @@
 	    }
 	    if (!p[1]) {
 		freecadef(ret);
-		zerrnam(nam, "invalid argument: %s", *args, 0);
+		zwarnnam(nam, "invalid argument: %s", *args, 0);
 		return NULL;
 	    }
 
@@ -659,7 +659,7 @@
 
 		if (!*p) {
 		    freecadef(ret);
-		    zerrnam(nam, "invalid option definition: %s", *args, 0);
+		    zwarnnam(nam, "invalid option definition: %s", *args, 0);
 		    return NULL;
 		}
 		*p++ = '\0';
@@ -669,7 +669,7 @@
 
 	    if (c && c != ':') {
 		freecadef(ret);
-		zerrnam(nam, "invalid option definition: %s", *args, 0);
+		zwarnnam(nam, "invalid option definition: %s", *args, 0);
 		return NULL;
 	    }
 	    /* Add the option name to the xor list if not `*-...'. */
@@ -713,7 +713,7 @@
 			if (*p != ':') {
 			    freecadef(ret);
 			    freecaargs(oargs);
-			    zerrnam(nam, "invalid option definition: %s",
+			    zwarnnam(nam, "invalid option definition: %s",
 				    *args, 0);
 			    return NULL;
 			}
@@ -790,12 +790,12 @@
 
 	    if (*++p != ':') {
 		freecadef(ret);
-		zerrnam(nam, "invalid rest argument definition: %s", *args, 0);
+		zwarnnam(nam, "invalid rest argument definition: %s", *args, 0);
 		return NULL;
 	    }
 	    if (ret->rest) {
 		freecadef(ret);
-		zerrnam(nam, "doubled rest argument definition: %s", *args, 0);
+		zwarnnam(nam, "doubled rest argument definition: %s", *args, 0);
 		return NULL;
 	    }
 	    if (*++p == ':') {
@@ -826,7 +826,7 @@
 
 	    if (*p != ':') {
 		freecadef(ret);
-		zerrnam(nam, "invalid argument: %s", *args, 0);
+		zwarnnam(nam, "invalid argument: %s", *args, 0);
 		return NULL;
 	    }
 	    if (*++p == ':') {
@@ -845,7 +845,7 @@
 	    if (tmp && tmp->num == anum - 1) {
 		freecadef(ret);
 		freecaargs(arg);
-		zerrnam(nam, "doubled argument definition: %s", *args, 0);
+		zwarnnam(nam, "doubled argument definition: %s", *args, 0);
 		return NULL;
 	    }
 	    arg->next = tmp;
@@ -1305,15 +1305,15 @@
     int min, max, n;
 
     if (incompfunc != 1) {
-	zerrnam(nam, "can only be called from completion function", NULL, 0);
+	zwarnnam(nam, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     if (args[0][0] != '-' || !args[0][1] || args[0][2]) {
-	zerrnam(nam, "invalid argument: %s", args[0], 0);
+	zwarnnam(nam, "invalid argument: %s", args[0], 0);
 	return 1;
     }
     if (args[0][1] != 'i' && !ca_parsed) {
-	zerrnam(nam, "no parsed state", NULL, 0);
+	zwarnnam(nam, "no parsed state", NULL, 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -1327,15 +1327,15 @@
     case 'a': min = 0; max =  0; break;
     case 'W': min = 2; max =  2; break;
     default:
-	zerrnam(nam, "invalid option: %s", args[0], 0);
+	zwarnnam(nam, "invalid option: %s", args[0], 0);
 	return 1;
     }
     n = arrlen(args) - 1;
     if (n < min) {
-	zerrnam(nam, "not enough arguments", NULL, 0);
+	zwarnnam(nam, "not enough arguments", NULL, 0);
 	return 1;
     } else if (max >= 0 && n > max) {
-	zerrnam(nam, "too many arguments", NULL, 0);
+	zwarnnam(nam, "too many arguments", NULL, 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -1589,7 +1589,7 @@
 
     if (args[0][0] == '-' && args[0][1] == 's' && !args[0][2]) {
 	if (args[1][0] && args[1][1]) {
-	    zerrnam(nam, "invalid separator: %s", args[1], 0);
+	    zwarnnam(nam, "invalid separator: %s", args[1], 0);
 	    return NULL;
 	}
 	hassep = 1;
@@ -1597,7 +1597,7 @@
 	args += 2;
     }
     if (!args[0] || !args[1]) {
-	zerrnam(nam, "not enough arguments", NULL, 0);
+	zwarnnam(nam, "not enough arguments", NULL, 0);
 	return NULL;
     }
     descr = *args++;
@@ -1640,7 +1640,7 @@
 	    }
 	    if (*p != ')') {
 		freecvdef(ret);
-		zerrnam(nam, "invalid argument: %s", *args, 0);
+		zwarnnam(nam, "invalid argument: %s", *args, 0);
 		return NULL;
 	    }
 	    xor = (char **) zalloc((xnum + 2) * sizeof(char *));
@@ -1664,7 +1664,7 @@
 
 	if (hassep && !sep && name + 1 != p) {
 	    freecvdef(ret);
-	    zerrnam(nam, "no multi-letter values with empty separator allowed", NULL, 0);
+	    zwarnnam(nam, "no multi-letter values with empty separator allowed", NULL, 0);
 	    return NULL;
 	}
 	/* Optional description? */
@@ -1677,7 +1677,7 @@
 
 	    if (!*p) {
 		freecvdef(ret);
-		zerrnam(nam, "invalid value definition: %s", *args, 0);
+		zwarnnam(nam, "invalid value definition: %s", *args, 0);
 		return NULL;
 	    }
 	    *p++ = '\0';
@@ -1688,7 +1688,7 @@
 	}
 	if (c && c != ':') {
 	    freecvdef(ret);
-	    zerrnam(nam, "invalid value definition: %s", *args, 0);
+	    zwarnnam(nam, "invalid value definition: %s", *args, 0);
 	    return NULL;
 	}
 	if (!multi) {
@@ -1703,7 +1703,7 @@
 	if (c == ':') {
 	    if (hassep && !sep) {
 		freecvdef(ret);
-		zerrnam(nam, "no value with argument with empty separator allowed", NULL, 0);
+		zwarnnam(nam, "no value with argument with empty separator allowed", NULL, 0);
 		return NULL;
 	    }
 	    if (*++p == ':') {
@@ -1921,15 +1921,15 @@
     int min, max, n;
 
     if (incompfunc != 1) {
-	zerrnam(nam, "can only be called from completion function", NULL, 0);
+	zwarnnam(nam, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     if (args[0][0] != '-' || !args[0][1] || args[0][2]) {
-	zerrnam(nam, "invalid argument: %s", args[0], 0);
+	zwarnnam(nam, "invalid argument: %s", args[0], 0);
 	return 1;
     }
     if (args[0][1] != 'i' && !cv_parsed) {
-	zerrnam(nam, "no parsed state", NULL, 0);
+	zwarnnam(nam, "no parsed state", NULL, 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -1942,15 +1942,15 @@
     case 'L': min = 3; max =  4; break;
     case 'v': min = 1; max =  1; break;
     default:
-	zerrnam(nam, "invalid option: %s", args[0], 0);
+	zwarnnam(nam, "invalid option: %s", args[0], 0);
 	return 1;
     }
     n = arrlen(args) - 1;
     if (n < min) {
-	zerrnam(nam, "not enough arguments", NULL, 0);
+	zwarnnam(nam, "not enough arguments", NULL, 0);
 	return 1;
     } else if (max >= 0 && n > max) {
-	zerrnam(nam, "too many arguments", NULL, 0);
+	zwarnnam(nam, "too many arguments", NULL, 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -2227,19 +2227,19 @@
     int min, max, n;
 
     if (incompfunc != 1) {
-	zerrnam(nam, "can only be called from completion function", NULL, 0);
+	zwarnnam(nam, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     if (args[0][0] != '-' || !args[0][1] || args[0][2]) {
-	zerrnam(nam, "invalid argument: %s", args[0], 0);
+	zwarnnam(nam, "invalid argument: %s", args[0], 0);
 	return 1;
     }
     if (locallevel >= MAX_TAGS) {
-	zerrnam(nam, "nesting level too deep", NULL, 0);
+	zwarnnam(nam, "nesting level too deep", NULL, 0);
 	return 1;
     }
     if (args[0][1] != 'i' && !comptags[locallevel]) {
-	zerrnam(nam, "no tags registered", NULL, 0);
+	zwarnnam(nam, "no tags registered", NULL, 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -2250,15 +2250,15 @@
     case 'R': min = 1; max =  1; break;
     case 'S': min = 1; max =  1; break;
     default:
-	zerrnam(nam, "invalid option: %s", args[0], 0);
+	zwarnnam(nam, "invalid option: %s", args[0], 0);
 	return 1;
     }
     n = arrlen(args) - 1;
     if (n < min) {
-	zerrnam(nam, "not enough arguments", NULL, 0);
+	zwarnnam(nam, "not enough arguments", NULL, 0);
 	return 1;
     } else if (max >= 0 && n > max) {
-	zerrnam(nam, "too many arguments", NULL, 0);
+	zwarnnam(nam, "too many arguments", NULL, 0);
 	return 1;
     }
     switch (args[0][1]) {
@@ -2309,11 +2309,11 @@
 bin_comptry(char *nam, char **args, char *ops, int func)
 {
     if (incompfunc != 1) {
-	zerrnam(nam, "can only be called from completion function", NULL, 0);
+	zwarnnam(nam, "can only be called from completion function", NULL, 0);
 	return 1;
     }
     if (!lasttaglevel || !comptags[lasttaglevel]) {
-	zerrnam(nam, "no tags registered", NULL, 0);
+	zwarnnam(nam, "no tags registered", NULL, 0);
 	return 1;
     }
     if (*args) {
@@ -2387,7 +2387,7 @@
 
     for (args += 2; *args; args++) {
 	if (args[0][1] != ':') {
-	    zerrnam(nam, "invalid argument `%s'", args[0], 0);
+	    zwarnnam(nam, "invalid argument `%s'", args[0], 0);
 	    return 1;
 	}
 	str = fmtstr(str, **args, *args + 2);
diff -ru ../z.old/Src/Zle/zle_main.c Src/Zle/zle_main.c
--- ../z.old/Src/Zle/zle_main.c	Tue Mar 14 11:19:53 2000
+++ Src/Zle/zle_main.c	Tue Mar 14 11:24:18 2000
@@ -826,7 +826,7 @@
     if (SHTTY == -1) {
 	/* need to open /dev/tty specially */
 	if ((SHTTY = open("/dev/tty", O_RDWR|O_NOCTTY)) == -1) {
-	    zerrnam(name, "can't access terminal", NULL, 0);
+	    zwarnnam(name, "can't access terminal", NULL, 0);
 	    return 1;
 	}
 	oshout = shout;
diff -ru ../z.old/Src/Zle/zle_thingy.c Src/Zle/zle_thingy.c
--- ../z.old/Src/Zle/zle_thingy.c	Tue Mar 14 11:19:54 2000
+++ Src/Zle/zle_thingy.c	Tue Mar 14 11:24:19 2000
@@ -349,7 +349,7 @@
     if(op->o)
 	for(opp = op; (++opp)->o; )
 	    if(ops[STOUC(opp->o)]) {
-		zerrnam(name, "incompatible operation selection options",
+		zwarnnam(name, "incompatible operation selection options",
 		    NULL, 0);
 		return 1;
 	    }
@@ -357,10 +357,10 @@
     /* check number of arguments */
     for(n = 0; args[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;
     }
 
@@ -397,7 +397,7 @@
     int sl = statusll, ocl = clearlist;
 
     if (!zleactive) {
-	zerrnam(name, "can only be called from widget function", NULL, 0);
+	zwarnnam(name, "can only be called from widget function", NULL, 0);
 	return 1;
     }
     statusline = NULL;
@@ -441,7 +441,7 @@
 bin_zle_mesg(char *name, char **args, char *ops, char func)
 {
     if (!zleactive) {
-	zerrnam(name, "can only be called from widget function", NULL, 0);
+	zwarnnam(name, "can only be called from widget function", NULL, 0);
 	return 1;
     }
     showmsg(*args);
@@ -457,7 +457,7 @@
     char *b = *args, *p = b + strlen(b);
 
     if (!zleactive) {
-	zerrnam(name, "can only be called from widget function", NULL, 0);
+	zwarnnam(name, "can only be called from widget function", NULL, 0);
 	return 1;
     }
     while (p > b)
@@ -534,10 +534,10 @@
     Thingy t = (Thingy) thingytab->getnode(thingytab, args[0]);
 
     if(!t) {
-	zerrnam(name, "no such widget `%s'", args[0], 0);
+	zwarnnam(name, "no such widget `%s'", args[0], 0);
 	return 1;
     } else if(bindwidget(t->widget, rthingy(args[1]))) {
-	zerrnam(name, "widget name `%s' is protected", args[1], 0);
+	zwarnnam(name, "widget name `%s' is protected", args[1], 0);
 	return 1;
     }
     return 0;
@@ -556,7 +556,7 @@
     if(!bindwidget(w, rthingy(args[0])))
 	return 0;
     freewidget(w);
-    zerrnam(name, "widget name `%s' is protected", args[0], 0);
+    zwarnnam(name, "widget name `%s' is protected", args[0], 0);
     return 1;
 }
 
@@ -568,14 +568,14 @@
     Widget w, cw;
 
     if (!require_module(name, "zsh/complete", 0, 0)) {
-	zerrnam(name, "can't load complete module", NULL, 0);
+	zwarnnam(name, "can't load complete module", NULL, 0);
 	return 1;
     }
     t = rthingy((args[1][0] == '.') ? args[1] : dyncat(".", args[1]));
     cw = t->widget;
     unrefthingy(t);
     if (!cw || !(cw->flags & ZLE_ISCOMP)) {
-	zerrnam(name, "invalid widget `%s'", args[1], 0);
+	zwarnnam(name, "invalid widget `%s'", args[1], 0);
 	return 1;
     }
     w = zalloc(sizeof(*w));
@@ -586,7 +586,7 @@
     w->u.comp.func = ztrdup(args[2]);
     if (bindwidget(w, rthingy(args[0]))) {
 	freewidget(w);
-	zerrnam(name, "widget name `%s' is protected", args[0], 0);
+	zwarnnam(name, "widget name `%s' is protected", args[0], 0);
 	return 1;
     }
     return 0;
@@ -608,7 +608,7 @@
 		sfcontext != SFC_WIDGET);
     }
     if(!zleactive || incompctlfunc || incompfunc || sfcontext != SFC_WIDGET) {
-	zerrnam(name, "widgets can only be called when ZLE is active",
+	zwarnnam(name, "widgets can only be called when ZLE is active",
 	    NULL, 0);
 	return 1;
     }
diff -ru ../z.old/Src/parse.c Src/parse.c
--- ../z.old/Src/parse.c	Tue Mar 14 11:19:49 2000
+++ Src/parse.c	Tue Mar 14 11:24:19 2000
@@ -2268,12 +2268,12 @@
 	Wordcode f;
 
 	if (!*args) {
-	    zerrnam(nam, "too few arguments", NULL, 0);
+	    zwarnnam(nam, "too few arguments", NULL, 0);
 	    return 1;
 	}
 	if (!(f = load_dump_header(*args)) &&
 	    !(f = load_dump_header(dyncat(*args, FD_EXT)))) {
-	    zerrnam(nam, "invalid dump file: %s", *args, 0);
+	    zwarnnam(nam, "invalid dump file: %s", *args, 0);
 	    return 1;
 	}
 	if (args[1]) {
@@ -2292,7 +2292,7 @@
 	}
     }
     if (!*args) {
-	zerrnam(nam, "too few arguments", NULL, 0);
+	zwarnnam(nam, "too few arguments", NULL, 0);
 	return 1;
     }
     map = (ops['m'] ? 2 : (ops['r'] ? 0 : 1));
@@ -2390,7 +2390,7 @@
 	dump = dyncat(dump, FD_EXT);
 
     if ((dfd = open(dump, O_WRONLY|O_CREAT, 0600)) < 0) {
-	zerrnam(nam, "can't write dump file: %s", dump, 0);
+	zwarnnam(nam, "can't write dump file: %s", dump, 0);
 	return 1;
     }
     progs = newlinklist();
@@ -2402,7 +2402,7 @@
 	    if (fd >= 0)
 		close(fd);
 	    close(dfd);
-	    zerrnam(nam, "can't open file: %s", *files, 0);
+	    zwarnnam(nam, "can't open file: %s", *files, 0);
 	    noaliases = ona;
 	    unlink(dump);
 	    return 1;
@@ -2414,7 +2414,7 @@
 	    close(fd);
 	    close(dfd);
 	    zfree(file, flen);
-	    zerrnam(nam, "can't read file: %s", *files, 0);
+	    zwarnnam(nam, "can't read file: %s", *files, 0);
 	    noaliases = ona;
 	    unlink(dump);
 	    return 1;

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Re: The `zle' command and traps
  2000-03-13 16:49 Sven Wischnowsky
@ 2000-03-13 17:08 ` Bart Schaefer
  0 siblings, 0 replies; 6+ messages in thread
From: Bart Schaefer @ 2000-03-13 17:08 UTC (permalink / raw)
  To: Sven Wischnowsky, zsh-workers

On Mar 13,  5:49pm, Sven Wischnowsky wrote:
} Subject: Re: PATCH: Re: The `zle' command and traps
}
} After a bit of checking... I think in many places where we call
} zerrnam() we should really be calling zwarnnam() in functions
} implementing builtins. Otherwise a error reported by the builtin will
} exits loops and sublists and whatnot.

I think this is done because it's so difficult (still) to stop/kill a
loop that contains nothing but builtins.  If you've got a builtin going
wrong and that prevents the loop from ever reaching the end ...

We really need to figure out how to handle interrupts during builtins
before we start fooling with clearing errflag in additional cases.

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PATCH: Re: The `zle' command and traps
@ 2000-03-13 16:49 Sven Wischnowsky
  2000-03-13 17:08 ` Bart Schaefer
  0 siblings, 1 reply; 6+ messages in thread
From: Sven Wischnowsky @ 2000-03-13 16:49 UTC (permalink / raw)
  To: zsh-workers


I wrote:

> Bart Schaefer wrote:
> 
> ...
> > That isn't strictly necessary as one can redirect stdout to hide the
> > warning.  However, it'd be nice if zle returned a non-zero status when
> > it fails; try typing
> > 
> > 	zle -R "not echoed" || echo zle failed
> > 
> > and you'll get
> > 
> > 	zle: can only be called from widget function
> > 
> > but not "zle failed".
> 
> Argh! That's a much more general problem: try `continue || echo foo'. And
> 3.0.8 does the same, too.
> 
> And it's connected to my patch for `zcompile' (10106). I have to check 
> that again.

Ok, the real problem was that execsimple() didn't check errflag. And
then there were two functions in loop.c that didn't always leave
state->pc behind the body of the loop.

> Hm. Some builtins sometimes reset errflag and sometimes not. execbuiltin()
> resets errflag if it was set before the call to the function
> implementing the builtin and after that only uses zwarnnam(), not
> zerrnam(). Is it correct to reset errflag in execbuiltin() after the
> call to the function? Or do we have to find all the places where we
> have to reset errflag? Or just replace some calls to zerrnam() with
> calls to zwarnnam(). No patch for that yet.

After a bit of checking... I think in many places where we call
zerrnam() we should really be calling zwarnnam() in functions
implementing builtins. Otherwise a error reported by the builtin will
exits loops and sublists and whatnot. And in some places we may even
have to check if something called from the function for the builtin
might set errflag and the builtin-function has to reset it -- as was
the case for zcompile.


The patch also makes zcompile remove the wordcode file if an error
occured and the file was already created. An oversight.

Bye
 Sven

diff -ru ../z.old/Src/exec.c Src/exec.c
--- ../z.old/Src/exec.c	Mon Mar 13 14:14:41 2000
+++ Src/exec.c	Mon Mar 13 17:38:24 2000
@@ -735,6 +735,9 @@
 {
     wordcode code = *state->pc++;
 
+    if (errflag)
+	return (lastval = 1);
+
     if (code)
 	lineno = code - 1;
 
diff -ru ../z.old/Src/loop.c Src/loop.c
--- ../z.old/Src/loop.c	Mon Mar 13 14:14:44 2000
+++ Src/loop.c	Mon Mar 13 17:10:30 2000
@@ -171,6 +171,7 @@
     popheap();
     cmdpop();
     loops--;
+    state->pc = end;
     return lastval;
 }
 
@@ -394,6 +395,7 @@
     cmdpop();
     popheap();
     loops--;
+    state->pc = end;
     return lastval;
 }
 
diff -ru ../z.old/Src/parse.c Src/parse.c
--- ../z.old/Src/parse.c	Mon Mar 13 14:14:46 2000
+++ Src/parse.c	Mon Mar 13 17:45:00 2000
@@ -2404,6 +2404,7 @@
 	    close(dfd);
 	    zerrnam(nam, "can't open file: %s", *files, 0);
 	    noaliases = ona;
+	    unlink(dump);
 	    return 1;
 	}
 	file = (char *) zalloc(flen + 1);
@@ -2415,17 +2416,19 @@
 	    zfree(file, flen);
 	    zerrnam(nam, "can't read file: %s", *files, 0);
 	    noaliases = ona;
+	    unlink(dump);
 	    return 1;
 	}
 	close(fd);
 	file = metafy(file, flen, META_REALLOC);
 
 	if (!(prog = parse_string(file, 1)) || errflag) {
+	    errflag = 0;
 	    close(dfd);
 	    zfree(file, flen);
-	    zerrnam(nam, "can't read file: %s", *files, 0);
+	    zwarnnam(nam, "can't read file: %s", *files, 0);
 	    noaliases = ona;
-	    errflag  = 0;
+	    unlink(dump);
 	    return 1;
 	}
 	zfree(file, flen);

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de


^ permalink raw reply	[flat|nested] 6+ messages in thread

* PATCH: Re: The `zle' command and traps
@ 2000-03-13 15:07 Sven Wischnowsky
  0 siblings, 0 replies; 6+ messages in thread
From: Sven Wischnowsky @ 2000-03-13 15:07 UTC (permalink / raw)
  To: zsh-workers


Bart Schaefer wrote:

> On Mar 9,  1:21pm, Sven Wischnowsky wrote:
> } Subject: Re: The `zle' command and traps
> }
> } Bart Schaefer wrote:
> } 
> } > 	TMOUT=2 ; TRAPALRM() { zle -R "Hi there, I'm a trap." }
> } > 
> } > Of course if the signal arrives when ZLE is not active, this generates an
> } > ugly warning message.
> } 
> } I was about to add a/some test(s) for sfcontext in bin_zle()
> } but... this is quite cool, isn't it? ;-)
> 
> Yes, I confess I thought so too ... but some things don't work so well.
> For example, try using "zle -M ..." instead of -R in that trap.
> 
> Or try
> 
> 	TRAPINT() { zle push-input }
> 
> and then type ctrl-C.  Note that the line gets pushed onto the buffer
> stack, but the display is not refreshed so you can't tell.  You need an
> explicit refresh:
> 
> 	TRAPINT() { zle push-input ; zle -R }
> 
> (An explicit refresh makes the "zle -M" trap work as well.)
> 
> I'm also concerned about what happens if a zle widget is invoked from a
> trap in the middle of some interactive thing like menu-selection.  That
> could get messy quickly, especially if it's something that alters the
> buffer.
>  
> } So, should we disallow it completely?
> 
> I can't decide.  The message modes and refresh seem harmless enough, but
> perhaps we should refuse to execute widgets from within traps?

The patch does that. And it adds the automatic re-display for -M.

> } Or make the zle builtin be quiet if zle isn't active?
> 
> That isn't strictly necessary as one can redirect stdout to hide the
> warning.  However, it'd be nice if zle returned a non-zero status when
> it fails; try typing
> 
> 	zle -R "not echoed" || echo zle failed
> 
> and you'll get
> 
> 	zle: can only be called from widget function
> 
> but not "zle failed".

Argh! That's a much more general problem: try `continue || echo foo'. And
3.0.8 does the same, too.

And it's connected to my patch for `zcompile' (10106). I have to check 
that again.

Hm. Some builtins sometimes reset errflag and sometimes not. execbuiltin()
resets errflag if it was set before the call to the function
implementing the builtin and after that only uses zwarnnam(), not
zerrnam(). Is it correct to reset errflag in execbuiltin() after the
call to the function? Or do we have to find all the places where we
have to reset errflag? Or just replace some calls to zerrnam() with
calls to zwarnnam(). No patch for that yet.

> } Make the zle special parameters accessible (read-only) in traps if zle
> } is active?
> 
> If we allow widgets to be called, you can cause the special parameters
> to become available by using "zle widgetname".  So instead of writing
> 
> 	TRAPINT() { .... something with $BUFFER .... }
> 
> One writes
> 
> 	widgetname() { .... something with $BUFFER .... }
> 	zle -N widgetname
> 	TRAPINT() { zle widgetname }
> 
> Can you extend read-only-ness of the parameters to the point that widgets
> are restricted as well?  I.e., if you call from a trap a builtin widget
> that moves the cursor or inserts/deletes characters, it would fail?

Well, there are the functions in zle_utils.c that modify the line and
most zle functions use them, but they are still free to do with the
line whatever they want. And many functions like spaceinline() don't
return anything yet (we could return a boolean saying if we are
allowed to modify the line). And many functions happily alter cs...

So, I think it would be quite a bit of work to implement that.

> If we can restrict widgets to those that do "safe" things, then we can
> allow "zle widgetname" and there's no need for the special parameters to
> be visible directly to the trap function.  If instead we must prohibit
> widget-calling entirely, then perhaps the parameters should be available
> to the trap function.

No patch for that yet, either. The problem is that traps are called
from the core (of course) and the zle module might not be there. Maybe 
calling a hook in dotrapargs() so that zle can install its own hook
function when it is loaded to make the parameters available read-only
when zle is active. Or does this sound too complicated to anyone?

> } Add a way to detect if zle is active?
> 
> That would be nice.  "zle" with no arguments silently returning 0 or 1
> would be sufficient (it now says "zle: widgets can only be called ...").

Good idea. The patch does that. But... currently this is done in a way 
that allows to find out if one can call widgets. It does not say
anything about the possibility to call `zle -R'. Hm.

> While we're on the subject ... CURSOR is almost equivalent to emacs'
> (point).  Why is there no MARK variable? [...]

Yes, why not. The patch adds it.

> } Add a way to detect if (the shell thinks that) there is a
> } completion list displayed below the prompt? Etc, etc?
> 
> Hrm.  That may be getting too esoteric.  Hard to say.
> 
> While we're on the subject of zle behavior:
> 
> 	zle -U frob
> 
> This results in "brof" being inserted onto the command line.  The docs
> refer to the "input queue" not the "input stack" -- why is it LIFO?

Oops. Fixed.

Bye
 Sven

diff -ru ../z.old/Doc/Zsh/mod_zle.yo Doc/Zsh/mod_zle.yo
--- ../z.old/Doc/Zsh/mod_zle.yo	Mon Mar 13 14:15:04 2000
+++ Doc/Zsh/mod_zle.yo	Mon Mar 13 15:21:01 2000
@@ -184,7 +184,8 @@
 xitem(tt(zle) tt(-R) [ tt(-c) ] [ var(display-string) ] [ var(string) ... ])
 xitem(tt(zle) tt(-M) var(string))
 xitem(tt(zle) tt(-U) var(string))
-item(tt(zle) var(widget) tt([ -n) var(num) tt(]) tt([ -N ]) var(args) ...)(
+xitem(tt(zle) var(widget) tt([ -n) var(num) tt(]) tt([ -N ]) var(args) ...)
+item(tt(zle))(
 The tt(zle) builtin performs a number of different actions concerning
 ZLE.  Which operation it performs depends on its options:
 
@@ -289,5 +290,9 @@
 it should call the tt(beep) widget directly.
 )
 enditem()
+
+With no options and no arguments, only the returns status will be
+set. It is zero if ZLE is currently active and widgets could be
+invoked using this builtin command and non-zero if ZLE is not active.
 )
 enditem()
diff -ru ../z.old/Src/Zle/zle_params.c Src/Zle/zle_params.c
--- ../z.old/Src/Zle/zle_params.c	Mon Mar 13 14:14:54 2000
+++ Src/Zle/zle_params.c	Mon Mar 13 15:12:26 2000
@@ -57,6 +57,8 @@
 	zleunsetfn, NULL },
     { "CURSOR",  PM_INTEGER, FN(set_cursor),  FN(get_cursor),
 	zleunsetfn, NULL },
+    { "MARK",  PM_INTEGER, FN(set_mark),  FN(get_mark),
+	zleunsetfn, NULL },
     { "LBUFFER", PM_SCALAR,  FN(set_lbuffer), FN(get_lbuffer),
 	zleunsetfn, NULL },
     { "RBUFFER", PM_SCALAR,  FN(set_rbuffer), FN(get_rbuffer),
@@ -169,6 +171,25 @@
 get_cursor(Param pm)
 {
     return cs;
+}
+
+/**/
+static void
+set_mark(Param pm, zlong x)
+{
+    if (x < 0)
+	mark = 0;
+    else if (x > ll)
+	mark = ll;
+    else
+	mark = x;
+}
+
+/**/
+static zlong
+get_mark(Param pm)
+{
+    return mark;
 }
 
 /**/
diff -ru ../z.old/Src/Zle/zle_thingy.c Src/Zle/zle_thingy.c
--- ../z.old/Src/Zle/zle_thingy.c	Mon Mar 13 14:14:54 2000
+++ Src/Zle/zle_thingy.c	Mon Mar 13 15:35:02 2000
@@ -434,7 +434,13 @@
 static int
 bin_zle_mesg(char *name, char **args, char *ops, char func)
 {
+    if (!zleactive) {
+	zerrnam(name, "can only be called from widget function", NULL, 0);
+	return 1;
+    }
     showmsg(*args);
+    if (sfcontext != SFC_WIDGET)
+	zrefresh();
     return 0;
 }
 
@@ -442,14 +448,14 @@
 static int
 bin_zle_unget(char *name, char **args, char *ops, char func)
 {
-    char *p = *args;
+    char *b = *args, *p = b + strlen(b);
 
     if (!zleactive) {
 	zerrnam(name, "can only be called from widget function", NULL, 0);
 	return 1;
     }
-    while (*p)
-	ungetkey((int) *p++);
+    while (p > b)
+	ungetkey((int) *--p);
     return 0;
 }
 
@@ -589,18 +595,18 @@
     int ret, saveflag = 0;
     char *wname = *args++;
 
-    if(!zleactive || incompctlfunc || incompfunc) {
-	zerrnam(name, "widgets can only be called when ZLE is active",
-	    NULL, 0);
-	return 1;
-    }
-
     if (!wname) {
-	zwarnnam(name, "wrong number of arguments", NULL, 0);
 	if (saveflag)
 	    zmod = modsave;
+	return (!zleactive || incompctlfunc || incompfunc ||
+		sfcontext != SFC_WIDGET);
+    }
+    if(!zleactive || incompctlfunc || incompfunc || sfcontext != SFC_WIDGET) {
+	zerrnam(name, "widgets can only be called when ZLE is active",
+	    NULL, 0);
 	return 1;
     }
+
     while (*args && **args == '-') {
 	char *num;
 	if (!args[0][1] || args[0][1] == '-') {

--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2000-03-14 16:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2000-03-14  9:54 PATCH: Re: The `zle' command and traps Sven Wischnowsky
  -- strict thread matches above, loose matches on Subject: below --
2000-03-14 10:35 Sven Wischnowsky
2000-03-14 16:56 ` Bart Schaefer
2000-03-13 16:49 Sven Wischnowsky
2000-03-13 17:08 ` Bart Schaefer
2000-03-13 15:07 Sven Wischnowsky

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).