Add new `functions` flag -V/--inherit-variable

--inherit-variable takes a variable name and snapshots its current
value. When the function is executed, it will have a local variable with
this value already defined. Printing the function source will include
synthesized `set -l` lines for the values.

This is primarily useful for functions that are created on the fly, such
as in `psub`.

10 files changed, 152 insertions, 7 deletions

diff --git a/builtin.cpp b/builtin.cpp
index 97a5c932..a01c2519 100644
--- a/builtin.cpp
+++ b/builtin.cpp
@@ -1391,6 +1391,26 @@ static void functions_def(const wcstring &name, wcstring &out)
         out.append(escape_string(name, true));
     }
 
+    /* Output any inherited variables as `set -l` lines */
+    std::map<wcstring,env_var_t> inherit_vars = function_get_inherit_vars(name);
+    for (std::map<wcstring,env_var_t>::const_iterator it = inherit_vars.begin(), end = inherit_vars.end(); it != end; ++it)
+    {
+        wcstring_list_t lst;
+        if (!it->second.missing())
+        {
+            tokenize_variable_array(it->second, lst);
+        }
+
+        /* This forced tab is crummy, but we don't know what indentation style the function uses */
+        append_format(out, L"\n\tset -l %ls", it->first.c_str());
+        for (wcstring_list_t::const_iterator arg_it = lst.begin(), arg_end = lst.end(); arg_it != arg_end; ++arg_it)
+        {
+            wcstring earg = escape_string(*arg_it, ESCAPE_ALL);
+            out.push_back(L' ');
+            out.append(earg);
+        }
+    }
+
     /* This forced tab is sort of crummy - not all functions start with a tab */
     append_format(out, L"\n\t%ls", def.c_str());
 
@@ -1976,6 +1996,7 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr
     wchar_t *desc=0;
     std::vector<event_t> events;
     std::auto_ptr<wcstring_list_t> named_arguments(NULL);
+    wcstring_list_t inherit_vars;
 
     wchar_t *name = 0;
     bool shadows = true;
@@ -1996,6 +2017,7 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr
         { L"help", no_argument, 0, 'h' },
         { L"argument-names", no_argument, 0, 'a' },
         { L"no-scope-shadowing", no_argument, 0, 'S' },
+        { L"inherit-variable", required_argument, 0, 'V' },
         { 0, 0, 0, 0 }
     };
 
@@ -2005,7 +2027,7 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr
 
         int opt = wgetopt_long(argc,
                                argv,
-                               L"d:s:j:p:v:e:haS",
+                               L"d:s:j:p:v:e:haSV:",
                                long_options,
                                &opt_index);
         if (opt == -1)
@@ -2154,11 +2176,24 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr
             case 'S':
                 shadows = 0;
                 break;
-                
+
             case 'w':
                 wrap_targets.push_back(woptarg);
                 break;
 
+            case 'V':
+            {
+                if (wcsvarname(woptarg))
+                {
+                    append_format(*out_err, _(L"%ls: Invalid variable name '%ls'\n"), argv[0], woptarg);
+                    res = STATUS_BUILTIN_ERROR;
+                    break;
+                }
+
+                inherit_vars.push_back(woptarg);
+                break;
+            }
+
             case 'h':
                 builtin_print_help(parser, argv[0], stdout_buffer);
                 return STATUS_BUILTIN_OK;
@@ -2255,6 +2290,7 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr
         d.shadows = shadows;
         if (named_arguments.get())
             d.named_arguments.swap(*named_arguments);
+        d.inherit_vars.swap(inherit_vars);
 
         for (size_t i=0; i<d.events.size(); i++)
         {
@@ -2265,7 +2301,7 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr
         d.definition = contents.c_str();
 
         function_add(d, parser, definition_line_offset);
-        
+
         // Handle wrap targets
         for (size_t w=0; w < wrap_targets.size(); w++)
         {
diff --git a/doc_src/function.txt b/doc_src/function.txt
index c98d4f65..1f9fb47d 100644
--- a/doc_src/function.txt
+++ b/doc_src/function.txt
@@ -21,7 +21,9 @@ The following options are available:
 
 - `-e` or `--on-event EVENT_NAME` tells fish to run this function when the specified named event is emitted. Fish internally generates named events e.g. when showing the prompt.
 
-- `-j PID` or `--on-job-exit PID` tells fish to run this function when the job with group ID PID exits. Instead of PID, the string 'caller' can be specified. This is only legal when in a command substitution, and will result in the handler being triggered by the exit of the job which created this command substitution.
+- `-v` or `--on-variable VARIABLE_NAME` tells fish to run this function when the variable VARIABLE_NAME changes value.
+
+- `-j PGID` or `--on-job-exit PGID` tells fish to run this function when the job with group ID PGID exits. Instead of PGID, the string 'caller' can be specified. This is only legal when in a command substitution, and will result in the handler being triggered by the exit of the job which created this command substitution.
 
 - `-p PID` or `--on-process-exit PID` tells fish to run this function when the fish child process with process ID PID exits.
 
@@ -29,7 +31,7 @@ The following options are available:
 
 - `-S` or `--no-scope-shadowing` allows the function to access the variables of calling functions. Normally, any variables inside the function that have the same name as variables from the calling function are "shadowed", and their contents is independent of the calling function.
 
-- `-v` or `--on-variable VARIABLE_NAME` tells fish to run this function when the variable VARIABLE_NAME changes value.
+- `-V` or `--inherit-variable NAME` snapshots the value of the variable `NAME` and defines a local variable with that same name and value when the function is executed.
 
 If the user enters any additional arguments after the function, they are inserted into the environment <a href="index.html#variables-arrays">variable array</a> `$argv`. If the `--argument-names` option is provided, the arguments are also assigned to names specified in that option.
 
@@ -75,3 +77,17 @@ end
 
 This will run the `mkdir` command, and if it is successful, change the current working directory to the one just created.
 
+\fish
+function notify
+    set -l job (jobs -l -g)
+    or begin; echo "There are no jobs" >&2; return 1; end
+
+    function _notify_job_$job --on-job-exit $job --inherit-variable job
+        echo -n \a # beep
+        functions -e _notify_job_$job
+    end
+end
+\endfish
+
+This will beep when the most recent job completes.
+
diff --git a/env.h b/env.h
index 8d4a41a9..e69b7fc9 100644
--- a/env.h
+++ b/env.h
@@ -166,7 +166,7 @@ public:
 };
 
 /**
-   Gets the variable with the specified name, or env_var_t::missing_var if it does not exist.
+   Gets the variable with the specified name, or env_var_t::missing_var if it does not exist or is an empty array.
 
    \param key The name of the variable to get
    \param mode An optional scope to search in. All scopes are searched if unset
diff --git a/exec.cpp b/exec.cpp
index 10baa76b..10e746ff 100644
--- a/exec.cpp
+++ b/exec.cpp
@@ -856,6 +856,7 @@ void exec_job(parser_t &parser, job_t *j)
 
                 wcstring_list_t named_arguments = function_get_named_arguments(p->argv0());
                 bool shadows = function_get_shadows(p->argv0());
+                std::map<wcstring,env_var_t> inherit_vars = function_get_inherit_vars(p->argv0());
 
                 signal_block();
 
@@ -868,12 +869,16 @@ void exec_job(parser_t &parser, job_t *j)
                 parser.push_block(newv);
 
                 /*
-                  set_argv might trigger an event
+                  setting variables might trigger an event
                   handler, hence we need to unblock
                   signals.
                 */
                 signal_unblock();
                 parse_util_set_argv(p->get_argv()+1, named_arguments);
+                for (std::map<wcstring,env_var_t>::const_iterator it = inherit_vars.begin(), end = inherit_vars.end(); it != end; ++it)
+                {
+                    env_set(it->first, it->second.missing() ? NULL : it->second.c_str(), ENV_LOCAL | ENV_USER);
+                }
                 signal_block();
 
                 parser.forbid_function(p->argv0());
diff --git a/function.cpp b/function.cpp
index f4ae2442..11c23114 100644
--- a/function.cpp
+++ b/function.cpp
@@ -150,12 +150,23 @@ void function_init()
     VOMIT_ON_FAILURE(pthread_mutexattr_destroy(&a));
 }
 
+static std::map<wcstring,env_var_t> snapshot_vars(const wcstring_list_t &vars)
+{
+    std::map<wcstring,env_var_t> result;
+    for (wcstring_list_t::const_iterator it = vars.begin(), end = vars.end(); it != end; ++it)
+    {
+        result.insert(std::make_pair(*it, env_get_string(*it)));
+    }
+    return result;
+}
+
 function_info_t::function_info_t(const function_data_t &data, const wchar_t *filename, int def_offset, bool autoload) :
     definition(data.definition),
     description(data.description),
     definition_file(intern(filename)),
     definition_offset(def_offset),
     named_arguments(data.named_arguments),
+    inherit_vars(snapshot_vars(data.inherit_vars)),
     is_autoload(autoload),
     shadows(data.shadows)
 {
@@ -167,6 +178,7 @@ function_info_t::function_info_t(const function_info_t &data, const wchar_t *fil
     definition_file(intern(filename)),
     definition_offset(def_offset),
     named_arguments(data.named_arguments),
+    inherit_vars(data.inherit_vars),
     is_autoload(autoload),
     shadows(data.shadows)
 {
@@ -268,6 +280,13 @@ wcstring_list_t function_get_named_arguments(const wcstring &name)
     return func ? func->named_arguments : wcstring_list_t();
 }
 
+std::map<wcstring,env_var_t> function_get_inherit_vars(const wcstring &name)
+{
+    scoped_lock lock(functions_lock);
+    const function_info_t *func = function_get(name);
+    return func ? func->inherit_vars : std::map<wcstring,env_var_t>();
+}
+
 int function_get_shadows(const wcstring &name)
 {
     scoped_lock lock(functions_lock);
diff --git a/function.h b/function.h
index efef275b..2f4fe8c5 100644
--- a/function.h
+++ b/function.h
@@ -11,10 +11,12 @@
 #define FISH_FUNCTION_H
 
 #include <wchar.h>
+#include <map>
 
 #include "util.h"
 #include "common.h"
 #include "event.h"
+#include "env.h"
 
 class parser_t;
 class env_vars_snapshot_t;
@@ -49,6 +51,11 @@ struct function_data_t
      */
     wcstring_list_t named_arguments;
     /**
+       List of all variables that are inherited from the function definition scope.
+       The variable values are snapshotted when function_add() is called.
+      */
+    wcstring_list_t inherit_vars;
+    /**
        Set to non-zero if invoking this function shadows the variables
        of the underlying function.
      */
@@ -79,6 +86,9 @@ public:
     /** List of all named arguments for this function */
     const wcstring_list_t named_arguments;
 
+    /** Mapping of all variables that were inherited from the function definition scope to their values */
+    const std::map<wcstring,env_var_t> inherit_vars;
+
     /** Flag for specifying that this function was automatically loaded */
     const bool is_autoload;
 
@@ -163,6 +173,12 @@ int function_get_definition_offset(const wcstring &name);
 wcstring_list_t function_get_named_arguments(const wcstring &name);
 
 /**
+   Returns a mapping of all variables of the specified function that were inherited
+   from the scope of the function definition to their values.
+ */
+std::map<wcstring,env_var_t> function_get_inherit_vars(const wcstring &name);
+
+/**
    Creates a new function using the same definition as the specified function.
    Returns true if copy is successful.
 */
diff --git a/tests/function.err b/tests/function.err
new file mode 100644
index 00000000..e69de29b
--- /dev/null
+++ b/tests/function.err
diff --git a/tests/function.in b/tests/function.in
new file mode 100644
index 00000000..e533b25b
--- /dev/null
+++ b/tests/function.in
@@ -0,0 +1,32 @@
+# vim: set filetype=fish:
+#
+# Test the `function` builtin
+
+# utility function
+function show_ary -a name --no-scope-shadowing
+    set -l count (count $$name)
+    echo "\$$name: ($count)"
+    if test $count -gt 0
+        for i in (seq $count)
+            echo "$i: '$$name[1][$i]'"
+        end
+    end
+end
+
+# Test the -V flag
+set -g foo 'global foo'
+set -l foo 'local foo'
+set bar one 'two    2' \t '' 3
+set baz
+function frob -V foo -V bar -V baz
+    show_ary foo
+    show_ary bar
+    show_ary baz
+end
+echo "Testing -V"
+frob
+echo "Testing -V with changed variables"
+set foo 'bad foo'
+set bar 'bad bar'
+set baz 'bad baz'
+frob
diff --git a/tests/function.out b/tests/function.out
new file mode 100644
index 00000000..3fa70990
--- /dev/null
+++ b/tests/function.out
@@ -0,0 +1,20 @@
+Testing -V
+$foo: (1)
+1: 'local foo'
+$bar: (5)
+1: 'one'
+2: 'two    2'
+3: '	'
+4: ''
+5: '3'
+$baz: (0)
+Testing -V with changed variables
+$foo: (1)
+1: 'local foo'
+$bar: (5)
+1: 'one'
+2: 'two    2'
+3: '	'
+4: ''
+5: '3'
+$baz: (0)
diff --git a/tests/function.status b/tests/function.status
new file mode 100644
index 00000000..573541ac
--- /dev/null
+++ b/tests/function.status
@@ -0,0 +1 @@
+0