PATCH: function copy

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

I've had this lying around for a while, wondering if there's more to it,
but I can't think of it.

The point is that it's very easy internally to provide an interface to
tweak standard functions to add arbitrary code before and after --- we
have most of the support for this internally, and just lack the means to
add a different name for a function, which this adds.

The idiom is going to be something like

functions -c _std_fn _my_fn
_std_fn() {
  # do stuff here
  _my_fn "$@"
  # do stuff here
}

Previously to get this effect you've had to hack around with autoloads
or re-evaluating functions with kludged-in strings, or whatever, which
is inefficient and bug-prone.

pws


commit 82403cf95822a791ce6cff7ba6bb8abc74876d97
Author: Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>
Date:   Wed Apr 10 20:54:52 2019 +0100

    Copy functions using functions -c old new.
    Documentation and test.

diff --git a/Doc/Zsh/builtins.yo b/Doc/Zsh/builtins.yo
index d7b6e88fa..9eee30d46 100644
--- a/Doc/Zsh/builtins.yo
+++ b/Doc/Zsh/builtins.yo
@@ -858,10 +858,11 @@ point numbers are not permitted.
 )
 findex(functions)
 xitem(tt(functions) [ {tt(PLUS())|tt(-)}tt(UkmtTuWz) ] [ tt(-x) var(num) ] [ var(name) ... ])
+xitem(tt(functions -c) var(oldfn) var(newfn))
 xitem(tt(functions -M) [tt(-s)] var(mathfn) [ var(min) [ var(max) [ var(shellfn) ] ] ])
 xitem(tt(functions -M) [ tt(-m) var(pattern) ... ])
 item(tt(functions +M) [ tt(-m) ] var(mathfn) ... )(
-Equivalent to tt(typeset -f), with the exception of the tt(-x),
+Equivalent to tt(typeset -f), with the exception of the tt(-c), tt(-x),
 tt(-M) and tt(-W) options.  For tt(functions -u) and tt(functions -U),
 see tt(autoload), which provides additional options.
 
@@ -875,6 +876,14 @@ function or functions only.  The option is turned off at the start of
 nested functions (apart from anonoymous functions) unless the called
 function also has the tt(-W) attribute.
 
+The tt(-c) option causes var(oldfn) to be copied to var(newfn).  The
+copy is efficiently handled internally by reference counting.  If
+var(oldfn) was marked for autoload it is first loaded and if this
+fails the copy fails.  Either function may subsequently be redefined
+without affecting the other.  A typical idiom is that var(oldfn) is the
+name of a library shell function which is then redefined to call
+tt(newfn), thereby installing a modified version of the function.
+
 Use of the tt(-M) option may not be combined with any of the options
 handled by tt(typeset -f).
 
diff --git a/Src/builtin.c b/Src/builtin.c
index 7db36c41b..5b95cc4fd 100644
--- a/Src/builtin.c
+++ b/Src/builtin.c
@@ -74,7 +74,7 @@ static struct builtin builtins[] =
     BUILTIN("fc", 0, bin_fc, 0, -1, BIN_FC, "aAdDe:EfiIlLmnpPrRt:W", NULL),
     BUILTIN("fg", 0, bin_fg, 0, -1, BIN_FG, NULL, NULL),
     BUILTIN("float", BINF_PLUSOPTS | BINF_MAGICEQUALS | BINF_PSPECIAL | BINF_ASSIGN, (HandlerFunc)bin_typeset, 0, -1, 0, "E:%F:%HL:%R:%Z:%ghlp:%rtux", "E"),
-    BUILTIN("functions", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "kmMstTuUWx:z", NULL),
+    BUILTIN("functions", BINF_PLUSOPTS, bin_functions, 0, -1, 0, "ckmMstTuUWx:z", NULL),
     BUILTIN("getln", 0, bin_read, 0, -1, 0, "ecnAlE", "zr"),
     BUILTIN("getopts", 0, bin_getopts, 2, -1, 0, NULL, NULL),
     BUILTIN("hash", BINF_MAGICEQUALS, bin_hash, 0, -1, 0, "Ldfmrv", NULL),
@@ -3248,11 +3248,50 @@ bin_functions(char *name, char **argv, Options ops, int func)
 
     if ((off & PM_UNDEFINED) || (OPT_ISSET(ops,'k') && OPT_ISSET(ops,'z')) ||
        (OPT_ISSET(ops,'x') && !OPT_HASARG(ops,'x')) ||
-       (OPT_MINUS(ops,'X') && (OPT_ISSET(ops,'m') || !scriptname))) {
+       (OPT_MINUS(ops,'X') && (OPT_ISSET(ops,'m') || !scriptname)) ||
+       (OPT_ISSET(ops,'c') && (OPT_ISSET(ops,'x') || OPT_ISSET(ops,'X') ||
+                               OPT_ISSET(ops,'m')))) {
        zwarnnam(name, "invalid option(s)");
        return 1;
     }
 
+    if (OPT_ISSET(ops,'c')) {
+       Shfunc newsh;
+       if (!*argv || !argv[1] || argv[2]) {
+           zwarnnam(name, "-c: requires two arguments");
+           return 1;
+       }
+       shf = (Shfunc) shfunctab->getnode(shfunctab, *argv);
+       if (!shf) {
+           zwarnnam(name, "no such funciton: %s", *argv);
+           return 1;
+       }
+       if (shf->node.flags & PM_UNDEFINED) {
+           if (shf->funcdef) {
+               freeeprog(shf->funcdef);
+               shf->funcdef = &dummy_eprog;
+           }
+           shf = loadautofn(shf, 1, 0, 0);
+           if (!shf)
+               return 1;
+       }
+       newsh = zalloc(sizeof(*newsh));
+       memcpy(newsh, shf, sizeof(*newsh));
+       if (newsh->node.flags & PM_LOADDIR) {
+           /* Expand original location of autoloaded file */
+           newsh->node.flags &= ~PM_LOADDIR;
+           newsh->filename = tricat(shf->filename, "/", shf->node.nam);
+       } else
+           newsh->filename = ztrdup(shf->filename);
+       newsh->funcdef->nref++;
+       if (newsh->redir)
+           newsh->redir->nref++;
+       if (shf->sticky)
+           newsh->sticky = sticky_emulation_dup(sticky, 0);
+       shfunctab->addnode(shfunctab, ztrdup(argv[1]), &newsh->node);
+       return 0;
+    }
+
     if (OPT_ISSET(ops,'x')) {
        char *eptr;
        expand = (int)zstrtol(OPT_ARG(ops,'x'), &eptr, 10);
diff --git a/Test/C04funcdef.ztst b/Test/C04funcdef.ztst
index 3aaf7fb4a..407fc471f 100644
--- a/Test/C04funcdef.ztst
+++ b/Test/C04funcdef.ztst
@@ -503,7 +503,7 @@
   not_trashed() { print This function was not trashed; }
   autoload -Uz /foo/bar/not_trashed
   not_trashed
-0:autoload with absolute path doesn't trash loaded function
+0:autoload with absolute path does not trash loaded function
 >This function was not trashed
 
   # keep spec from getting loaded in parent shell for simplicity
@@ -542,6 +542,73 @@
 0:autoload containing dash
 >this should run automatically
 
+  tbc() {
+     print This function is called $0.
+  }
+  tbc
+  functions -c tbc newcopy
+  newcopy
+  unfunction tbc
+  newcopy
+0:functions -c
+>This function is called tbc.
+>This function is called newcopy.
+>This function is called newcopy.
+
+  (
+    fpath=(.)
+    print >tbc_auto 'print This autoloaded function is called $0.'
+    autoload -Uz tbc_auto
+    functions -c tbc_auto newcopy_auto
+    newcopy_auto
+    tbc_auto
+  )
+0:functions -c with autoload
+>This autoloaded function is called newcopy_auto.
+>This autoloaded function is called tbc_auto.
+
+  (
+    fpath=(.)
+    print >tbc_redef "print This is the core of the old function."
+    autoload -Uz tbc_redef
+    functions -c tbc_redef tbc_original
+    tbc_redef() {
+       print About to call the original.
+       tbc_original
+       print Stopped calling the original because once is enough.
+    }
+    tbc_redef
+  )
+0:function -c with redefinition
+>About to call the original.
+>This is the core of the old function.
+>Stopped calling the original because once is enough.
+
+  (
+    fpath=(.)
+    print >line_info '\nprint -P "%1x:%I is where we are."'
+    autoload -Uz line_info
+    functions -c line_info preserve_file
+    preserve_file
+  )
+0:functions -c preserves file information
+>line_info:2 is where we are.
+
+  (
+    fpath=(.)
+    print >func_info '\nprint -P "%N:%i is where we are."'
+    autoload -Uz func_info
+    functions -c func_info change_output
+    change_output
+  )
+0:functions -c updates non-file function information
+>change_output:2 is where we are.
+
+  autoload -Uz cant_autoload_for_copying
+  functions -c cant_autoload_for_copying not_copied
+1:functions -c gracefully rejects failed autoload
+?(eval):2: cant_autoload_for_copying: function definition file not found
+
 %clean
 
  rm -f file.in file.out