Hooks

Hooks

[Peter Stephenson]

Stephane Chazelas <Stephane_Chazelas@yahoo.fr> wrote:
> functions[preexec]+='
>   get_saved_dirstack'
> functions[chpwd]+='
>   save_dirstack'

This is even further away from the original subject, but I've often felt
that functions like preexec and chpwd are getting a bit overloaded and
are unnecessarily hard to maintain automatically.  It would be easy to
add an array which could contain names of functions to be executed,
similar to Emacs hooks.  Of course you can implement such an array
within the existing functions, but the advantage of having a fixed
mechanism is that it's easy to add to and be sure it's working without
getting in anyone else's way.  You still have namespace problems, of
course, but if you have functions called get_saved_dirstack and
save_dirstack that clash your real problem is almost certainly more
basic than naming.

However, I'm not sure if it's really needed.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php

Re: Hooks

[Nikolai Weibull]

On 11/7/06, Peter Stephenson <pws@csr.com> wrote:
> Stephane Chazelas <Stephane_Chazelas@yahoo.fr> wrote:
> > functions[preexec]+='
> >   get_saved_dirstack'
> > functions[chpwd]+='
> >   save_dirstack'
>
> This is even further away from the original subject, but I've often felt
> that functions like preexec and chpwd are getting a bit overloaded and
> are unnecessarily hard to maintain automatically.  It would be easy to
> add an array which could contain names of functions to be executed,
> similar to Emacs hooks.  Of course you can implement such an array
> within the existing functions, but the advantage of having a fixed
> mechanism is that it's easy to add to and be sure it's working without
> getting in anyone else's way.  You still have namespace problems, of
> course, but if you have functions called get_saved_dirstack and
> save_dirstack that clash your real problem is almost certainly more
> basic than naming.
>
> However, I'm not sure if it's really needed.

Yes, please do add.  It's a real, em...lady of the night, to try to
chain all these functions properly (i.e., checking if preexec is
defined and then try to get its definition or however I did it when I
did it last time (I did this for a prompt that was depended on the
value of $WIDTH, so I read it on each preexec, but I also had another
function running at preexec, so I had to get that other functions body
and eval it from the prompt preexec instead.).

Thanks!

  nikolai

Re: Hooks

[Bart Schaefer]

On Nov 7,  7:12am, Nikolai Weibull wrote:
}
} > This is even further away from the original subject, but I've often felt
} > that functions like preexec and chpwd are getting a bit overloaded and
} > are unnecessarily hard to maintain automatically.  It would be easy to
} > add an array which could contain names of functions to be executed,
} > similar to Emacs hooks.
} [...]
} > However, I'm not sure if it's really needed.
} 
} Yes, please do add.

Having previously implemented a shell-like language in which there was
such a set of hooks, I'll point out that there really ought to be a
well-defined semantics rather than simply executing every one of an
array of functions.  (I suppose that can *be* the defined semantics,
but it's not all that useful a one.)

For example, if one of the functions causes an error condition (the
sort of thing that would jump to the final clause of an "always" block)
is the whole chain aborted, or only the one function?

Related but more subtle, if a function returns a non-zero status,
should that affect the rest of the functions in the array in any way?
Put another way, are there rules for cascading the hooks?

And what is the value of "$@" when these functions are called?  Does
is differ depending on which hook array they're in?  (The signatures
of preexec, precmd, and chpwd are quite different.)

I'd suggest that, at least as a first pass, rather than coded up in C,
this be implmented with a few functions distributed in Functions/Misc
and designed to be installed as preexec, precmd, and/or chpwd.

Re: Hooks

[Peter Stephenson]

"Nikolai Weibull" <now@bitwi.se> wrote:
> > This is even further away from the original subject, but I've often felt
> > that functions like preexec and chpwd are getting a bit overloaded and
> > are unnecessarily hard to maintain automatically.  It would be easy to
> > add an array which could contain names of functions to be executed,
> > similar to Emacs hooks.
> 
> Yes, please do add.

This is it; it's not a particularly big change.  I think the array names
precmd_functions etc. are obvious enough to be uncontroversial.

Index: Doc/Zsh/func.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/func.yo,v
retrieving revision 1.12
diff -u -r1.12 func.yo
--- Doc/Zsh/func.yo	20 Mar 2006 11:06:25 -0000	1.12
+++ Doc/Zsh/func.yo	7 Nov 2006 18:10:48 -0000
@@ -151,26 +151,44 @@
 example(autoload +X myfunc)
 
 sect(Special Functions)
-The following functions, if defined, have special meaning to
-the shell:
+Certain functions, if defined, have special meaning to the shell.
+
+In the case of tt(chpwd), tt(periodic), tt(precmd) and tt(preexec) it is
+possible to define an array that has the same name with `tt(_functions)'
+appended.  Any element in such an array is taken as the name of a function
+to execute; it is executed in the same context and with the same arguments
+as the basic function.  For example, if tt($chpwd_functions) is an array
+containing the values `tt(mychpwd)', `tt(chpwd_save_dirstack)', then the
+shell attempts to execute the functions `tt(chpwd)', `tt(mychpwd)' and
+`tt(chpwd_save_dirstack)', in that order.  Any function that does not exist
+is silently ignored.  A function found by this mechanism is referred to
+elsewhere as a `hook function'.
 
 startitem()
 findex(chpwd)
+vindex(chpwd_functions)
 item(tt(chpwd))(
 Executed whenever the current working directory is changed.
 )
 findex(periodic)
+vindex(periodic_functions)
 item(tt(periodic))(
 vindex(PERIOD)
 If the parameter tt(PERIOD)
 is set, this function is executed every tt($PERIOD)
-seconds, just before a prompt.
+seconds, just before a prompt.  Note that if multiple functions
+are defined using the array tt(periodic_functions) only one
+period is applied to the complete set of functions, and the
+scheduled time is not reset if the list of functions is altered.
+Hence the set of functions is always called together.
 )
 findex(precmd)
+vindex(precmd_functions)
 item(tt(precmd))(
 Executed before each prompt.
 )
 findex(preexec)
+vindex(preexec_functions)
 item(tt(preexec))(
 Executed just after a command has been read and is about to be
 executed.  If the history mechanism is active (and the line was not
Index: Doc/Zsh/options.yo
===================================================================
RCS file: /cvsroot/zsh/zsh/Doc/Zsh/options.yo,v
retrieving revision 1.49
diff -u -r1.49 options.yo
--- Doc/Zsh/options.yo	1 Nov 2006 12:25:22 -0000	1.49
+++ Doc/Zsh/options.yo	7 Nov 2006 18:10:49 -0000
@@ -912,8 +912,11 @@
 
 The check is omitted if the commands run from the previous command line
 included a `tt(jobs)' command, since it is assumed the user is aware that
-there are background or suspended jobs.  A `tt(jobs)' command run from the
-tt(precmd) function is not counted for this purpose.
+there are background or suspended jobs.  A `tt(jobs)' command run from one
+of the hook functions defined in
+ifnzman(the section Special Functions in noderef(Functions))\
+ifzman(the section SPECIAL FUNCTIONS in zmanref(zshmisc))
+is not counted for this purpose.
 )
 pindex(HUP)
 cindex(jobs, HUP)
Index: Src/builtin.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/builtin.c,v
retrieving revision 1.165
diff -u -r1.165 builtin.c
--- Src/builtin.c	5 Nov 2006 21:49:18 -0000	1.165
+++ Src/builtin.c	7 Nov 2006 18:10:50 -0000
@@ -1087,7 +1087,6 @@
 static void
 cd_new_pwd(int func, LinkNode dir)
 {
-    Eprog prog;
     char *new_pwd, *s;
     int dirstacksize;
 
@@ -1134,15 +1133,9 @@
     }
 
     /* execute the chpwd function */
-    if ((prog = getshfunc("chpwd")) != &dummy_eprog) {
-	int osc = sfcontext;
-
-	fflush(stdout);
-	fflush(stderr);
-	sfcontext = SFC_HOOK;
-	doshfunc("chpwd", prog, NULL, 0, 1);
-	sfcontext = osc;
-    }
+    fflush(stdout);
+    fflush(stderr);
+    callhookfunc("chpwd", NULL, 1);
 
     dirstacksize = getiparam("DIRSTACKSIZE");
     /* handle directory stack sizes out of range */
Index: Src/init.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/init.c,v
retrieving revision 1.70
diff -u -r1.70 init.c
--- Src/init.c	29 Sep 2006 20:43:33 -0000	1.70
+++ Src/init.c	7 Nov 2006 18:10:50 -0000
@@ -137,11 +137,9 @@
 	}
 	if (hend(prog)) {
 	    int toksav = tok;
-	    Eprog preprog;
 
-	    if (toplevel && (preprog = getshfunc("preexec")) != &dummy_eprog) {
+	    if (toplevel && getshfunc("preexec") != &dummy_eprog) {
 		LinkList args;
-		int osc = sfcontext;
 		char *cmdstr;
 
 		args = znewlinklist();
@@ -155,10 +153,14 @@
 		zaddlinknode(args, getjobtext(prog, NULL));
 		zaddlinknode(args, cmdstr = getpermtext(prog, NULL));
 
-		sfcontext = SFC_HOOK;
-		doshfunc("preexec", preprog, args, 0, 1);
-		sfcontext = osc;
+		callhookfunc("preexec", args, 1);
+
 		zsfree(cmdstr);
+		/*
+		 * The following frees the nodes in the list, but
+		 * doesn't free the data: the only data that
+		 * needs freeing is cmdstr, just handled.
+		 */
 		freelinklist(args, (FreeFunc) NULL);
 		errflag = 0;
 	    }
Index: Src/utils.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/utils.c,v
retrieving revision 1.144
diff -u -r1.144 utils.c
--- Src/utils.c	19 Oct 2006 08:50:00 -0000	1.144
+++ Src/utils.c	7 Nov 2006 18:10:51 -0000
@@ -1079,28 +1079,53 @@
 /**/
 time_t lastwatch;
 
+/*
+ * Call a function given by "name" with optional arguments
+ * "lnklist".  If "arrayp" is not zero, we also look through
+ * the array "name"_functions and execute functions found there.
+ */
+
 /**/
 mod_export int
-callhookfunc(char *name, LinkList lnklst)
+callhookfunc(char *name, LinkList lnklst, int arrayp)
 {
     Eprog prog;
-
-    if ((prog = getshfunc(name)) != &dummy_eprog) {
 	/*
 	 * Save stopmsg, since user doesn't get a chance to respond
 	 * to a list of jobs generated in a hook.
 	 */
-	int osc = sfcontext, osm = stopmsg;
+    int osc = sfcontext, osm = stopmsg, stat = 1;
+
+    sfcontext = SFC_HOOK;
 
-	sfcontext = SFC_HOOK;
+    if ((prog = getshfunc(name)) != &dummy_eprog) {
 	doshfunc(name, prog, lnklst, 0, 1);
-	sfcontext = osc;
-	stopmsg = osm;
+	stat = 0;
+    }
 
-	return 0;
+    if (arrayp) {
+	char **arrptr;
+	int namlen = strlen(name);
+#define HOOK_SUFFIX	"_functions"
+#define HOOK_SUFFIX_LEN	11	/* including NUL byte */
+	VARARR(char, arrnam, namlen + HOOK_SUFFIX_LEN);
+	memcpy(arrnam, name, namlen);
+	memcpy(arrnam + namlen, HOOK_SUFFIX, HOOK_SUFFIX_LEN);
+
+	if ((arrptr = getaparam(arrnam))) {
+	    for (; *arrptr; arrptr++) {
+		if ((prog = getshfunc(*arrptr)) != &dummy_eprog) {
+		    doshfunc(arrnam, prog, lnklst, 0, 1);
+		    stat = 0;
+		}
+	    }
+	}
     }
 
-    return 1;
+    sfcontext = osc;
+    stopmsg = osm;
+
+    return stat;
 }
 
 /* do pre-prompt stuff */
@@ -1136,15 +1161,15 @@
 
     /* If a shell function named "precmd" exists, *
      * then execute it.                           */
-    callhookfunc("precmd", NULL);
+    callhookfunc("precmd", NULL, 1);
     if (errflag)
 	return;
 
-    /* If 1) the parameter PERIOD exists, 2) the shell function     *
+    /* If 1) the parameter PERIOD exists, 2) a hook function for    *
      * "periodic" exists, 3) it's been greater than PERIOD since we *
-     * executed "periodic", then execute it now.                    */
+     * executed any such hook, then execute it now.                 */
     if (period && (time(NULL) > lastperiodic + period) &&
-	!callhookfunc("periodic", NULL))
+	!callhookfunc("periodic", NULL, 1))
 	lastperiodic = time(NULL);
     if (errflag)
 	return;
Index: Src/Zle/zle_main.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/Zle/zle_main.c,v
retrieving revision 1.93
diff -u -r1.93 zle_main.c
--- Src/Zle/zle_main.c	30 Oct 2006 14:13:39 -0000	1.93
+++ Src/Zle/zle_main.c	7 Nov 2006 18:10:51 -0000
@@ -716,7 +716,7 @@
 # endif
 
 
-			callhookfunc(lwatch_funcs[i], funcargs);
+			callhookfunc(lwatch_funcs[i], funcargs, 0);
 			if (errflag) {
 			    /* No sensible way of handling errors here */
 			    errflag = 0;
Index: Test/A05execution.ztst
===================================================================
RCS file: /cvsroot/zsh/zsh/Test/A05execution.ztst,v
retrieving revision 1.4
diff -u -r1.4 A05execution.ztst
--- Test/A05execution.ztst	29 Jul 2004 15:10:24 -0000	1.4
+++ Test/A05execution.ztst	7 Nov 2006 18:10:51 -0000
@@ -105,6 +105,18 @@
 0q:chpwd
 >Changed to $ZTST_testdir/command.tmp
 
+  chpwd() { print chpwd: changed to $PWD; }
+  chpwdfn1()  { print chpwdfn1: changed to $PWD; }
+  chpwdfn2()  { print chpwdfn2: changed to $PWD; }
+  chpwd_functions=(chpwdfn1 '' chpwdnonexistentfn chpwdfn2)
+  cd .
+  unfunction chpwd
+  unset chpwd_functions
+0q:chpwd_functions
+>chpwd: changed to $ZTST_testdir/command.tmp
+>chpwdfn1: changed to $ZTST_testdir/command.tmp
+>chpwdfn2: changed to $ZTST_testdir/command.tmp
+
 # Hard to test periodic, precmd and preexec non-interactively.
 
   fn() { TRAPEXIT() { print Exit; }; }


-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php

Re: Hooks

[Peter Stephenson]

Bart Schaefer wrote:
> For example, if one of the functions causes an error condition (the
> sort of thing that would jump to the final clause of an "always" block)
> is the whole chain aborted, or only the one function?

Currently we don't reset the errflag.  In some cases (precmd and
periodic) we abort all precommand processing on a failure: this is how
it has always worked, for suitable values of "always", in the case of
chained activities before a prompt.  This probably needs to be
documented, however it works.

> Related but more subtle, if a function returns a non-zero status,
> should that affect the rest of the functions in the array in any way?
> Put another way, are there rules for cascading the hooks?

I thought of this, but decided it wasn't that helpful.  The whole point
of being able to do this automatically is that you shouldn't care what
other functions are doing.  However, maybe there's a counterexample.

> And what is the value of "$@" when these functions are called?  Does
> is differ depending on which hook array they're in?  (The signatures
> of preexec, precmd, and chpwd are quite different.)

It's the same as the currently defined function for that context.
That seems the only sensible way of doing it.

> I'd suggest that, at least as a first pass, rather than coded up in C,
> this be implmented with a few functions distributed in Functions/Misc
> and designed to be installed as preexec, precmd, and/or chpwd.

Certainly I could mimic the current system in shell functions, but I'm
not sure what that gains: the whole point of doing it in C is that you
can rely on it always being there.  I will certainly consider changes to
the current system.

I made one screwup: preexec_functions aren't handled unless preexec
exists as well.  I've used a different optimisation to avoid needing the
argument handling if there's nothing to do.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
CSR PLC, Churchill House, Cambridge Business Park, Cowley Road
Cambridge, CB4 0WZ, UK                          Tel: +44 (0)1223 692070


To access the latest news from CSR copy this link into a web browser:  http://www.csr.com/email_sig.php