[PATCH] Re: chpwd, precmd hooks have "zsh" in $0

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

Moving discussion to zsh-workers.

On Wed, Aug 11, 2021 at 9:40 PM Roman Neuhauser <neuhauser@xxxxxxxxxx> wrote:
>
> # schaefer@xxxxxxxxxxxxxxxx / 2021-08-10 10:31:00 -0700:
> >
> > Theoretically, a function might prefer to know that it was being
> > called as a consequence of being found in a given hook list, vs. being
> > called directly.  That might argue for [calling all with the hookname].
>
> yes, but there's other ways to let a function know it's being called as
> part of a particular hook array.  there's only one reasonable way
> of letting a function know the name it was called with.

Attached patch implements calling hook functions with $0 as their own
name, even in the hook arrays.  If there's a better way to
nondestructively copy the tail of a LinkList, please tell me.

In order to do this for functions that already get an argument list
(preexec and zshaddhistory, for example) I had to modify
callhookfunc(), so I did NOT change doshfunc(), which means the
behavior of zftp callbacks is unchanged and will have to be addressed
separately.

> > >     function a b { ... } > ab.log
> > >     precmd_functions=(a)
> > >     preexec_functions=(b)
> > >
> > > calls the function with $0 == "a" in precmd, "b" in preexec,
> > > and that their runtime output goes to ab.log.  hooks do neither
> > > and TRAPNAL functions only [use ...]
> [...] the correct name for a function defined with multiple names.

So I dug a little further into why this happens (that is, why
redirection does NOT) and found that it's because doshfunc() extracts
the function body and jumps directly to the first command in the body
via runshfunc(), thereby skipping the redirections, which are normally
handled by execcmd_exec() after the function object is looked up
there.

I haven't verified it, but I believe this also means that "sticky"
emulations are not honored by functions called as hooks (and various
other entry points that call doshfunc() directly).

There's a chicken-and-egg problem here because execcmd_exec() itself
eventually runs shell functions by calling execshfunc() which then
calls doshfunc(), so we can't push the redirection handling down into
doshfunc() ... the order of events for forks and subshells would
become wrong.

One obvious way to fix this -- and I vaguely recall this having been
suggested during the discussion back in 2014 -- is to dispose of the
Eprog redir from struct shfunc and instead change the parser to push
the redirections onto the Eprog funcdef -- in effect, convert at parse
time
   func() { ... } > file
into
   function func { { ... } > file }

What the equivalent is for sticky emulations, I don't know.

Another way to fix it might be to change the call signature of
doshfunc() to include a flag that says whether to process redirections
[and sticky emulations] and then copy code from execmd_exec() that
would be run when that flag is true.  I'm not sure what the
consequences are in terms of correctly unwinding state in those cases.

diff --git a/Src/utils.c b/Src/utils.c
index 5a9222919..c32741ca7 100644
--- a/Src/utils.c
+++ b/Src/utils.c
@@ -1498,7 +1498,7 @@ time_t lastwatch;
 
 /*
  * Call a function given by "name" with optional arguments
- * "lnklist".  If these are present the first argument is the function name.
+ * "lnklst".  If these are present the first argument is the function name.
  *
  * If "arrayp" is not zero, we also look through
  * the array "name"_functions and execute functions found there.
@@ -1527,6 +1527,10 @@ callhookfunc(char *name, LinkList lnklst, int arrayp, int *retval)
     incompfunc = 0;
 
     if ((shfunc = getshfunc(name))) {
+	if (!lnklst) {
+	    lnklst = newlinklist();
+	    addlinknode(lnklst, name);
+	}
 	ret = doshfunc(shfunc, lnklst, 1);
 	stat = 0;
     }
@@ -1539,10 +1543,16 @@ callhookfunc(char *name, LinkList lnklst, int arrayp, int *retval)
 	memcpy(arrnam + namlen, HOOK_SUFFIX, HOOK_SUFFIX_LEN);
 
 	if ((arrptr = getaparam(arrnam))) {
+	    char **argarr = lnklst ? hlinklist2array(lnklst, 0) : NULL;
 	    arrptr = arrdup(arrptr);
 	    for (; *arrptr; arrptr++) {
 		if ((shfunc = getshfunc(*arrptr))) {
-		    int newret = doshfunc(shfunc, lnklst, 1);
+		    int newret, i = 1;
+		    LinkList arg0 = newlinklist();
+		    addlinknode(arg0, *arrptr);
+		    while (argarr && argarr[i])
+			addlinknode(arg0, argarr[i++]);
+		    newret = doshfunc(shfunc, arg0, 1);
 		    if (!ret)
 			ret = newret;
 		    stat = 0;