From c95b9f06e1d81e34a91c47381e114d2e7d095345 Mon Sep 17 00:00:00 2001
From: Kurtis Rader <krader@skepticism.us>
Date: Wed, 23 Aug 2017 20:38:40 -0700
Subject: [PATCH] Implement support for bare vars by `math`

This change allows you to type `math x + 3` rather than `math $x + 3`.

Another step to resolving issue #3157.
---
 src/builtin_math.cpp | 53 ++++++++++++++++++++++++++++++++++++++++++--
 tests/math.err       |  6 +++++
 tests/math.in        | 10 +++++++++
 tests/math.out       | 10 +++++++++
 4 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/src/builtin_math.cpp b/src/builtin_math.cpp
index cae45ba58..70429c19d 100644
--- a/src/builtin_math.cpp
+++ b/src/builtin_math.cpp
@@ -108,22 +108,72 @@ static const wchar_t *math_get_arg_argv(int *argidx, wchar_t **argv) {
 /// Get the arguments from argv or stdin based on the execution context. This mimics how builtin
 /// `string` does it.
 static const wchar_t *math_get_arg(int *argidx, wchar_t **argv, wcstring *storage,
-                                     const io_streams_t &streams) {
+                                   const io_streams_t &streams) {
     if (math_args_from_stdin(streams)) {
         return math_get_arg_stdin(storage, streams);
     }
     return math_get_arg_argv(argidx, argv);
 }
 
+// The MuParser mechanism for dynamic lookup of variables requires that we return a unique address
+// for each variable. The following limit is arbitrary but anyone writing a math expression in fish
+// that references more than one hundred unique variables is abusing fish.
+#define MAX_RESULTS 100
+static double double_results[MAX_RESULTS];
+static int next_result = 0;
+
+/// Return a fish var converted to a double. This allows the user to use a bar var name in the
+/// expression. That is `math a + 1` rather than `math $a + 1`.
+static double *retrieve_var(const wchar_t *var_name, void *user_data) {
+    UNUSED(user_data);
+    static double zero_result = 0.0;
+
+    env_var_t var = env_get(var_name, ENV_DEFAULT);
+    if (var.missing()) {
+        // We could report an error but we normally don't treat missing vars as a fatal error.
+        // throw mu::ParserError(L"Var '%ls' does not exist.");
+        return &zero_result;
+    }
+    if (var.empty()) {
+        return &zero_result;
+    }
+
+    const wchar_t *first_val = var.as_const_list()[0].c_str();
+    wchar_t *endptr;
+    errno = 0;
+    double result = wcstod(first_val, &endptr);
+    if (*endptr != L'\0' || errno) {
+        wchar_t errmsg[500];
+        swprintf(errmsg, sizeof(errmsg) / sizeof(wchar_t),
+                 _(L"Var '%ls' not a valid floating point number: '%ls'."), var_name, first_val);
+        throw mu::ParserError(errmsg);
+    }
+
+    // We need to return a unique address for the var. If we used a `static double` var and returned
+    // it's address then multiple vars in the expression would all refer to the same value.
+    if (next_result == MAX_RESULTS - 1) {
+        wchar_t errmsg[500];
+        swprintf(errmsg, sizeof(errmsg) / sizeof(wchar_t),
+                 _(L"More than %d var names in math expression."), MAX_RESULTS);
+        throw mu::ParserError(errmsg);
+    }
+    double_results[next_result++] = result;
+    return double_results + next_result - 1;
+}
+
 /// Implement integer modulo math operator.
 static double moduloOperator(double v, double w) { return (int)v % std::max(1, (int)w); };
 
+/// Evaluate math expressions.
 static int evaluate_expression(wchar_t *cmd, parser_t &parser, io_streams_t &streams,
                                math_cmd_opts_t &opts, wcstring &expression) {
     UNUSED(parser);
+    next_result = 0;
 
     try {
         mu::Parser p;
+        // Setup callback so variables can be retrieved dynamically.
+        p.SetVarFactory(retrieve_var, nullptr);
         // MuParser doesn't implement the modulo operator so we add it ourselves since there are
         // likely users of our old math wrapper around bc that expect it to be available.
         p.DefineOprtChars(L"%");
@@ -139,7 +189,6 @@ static int evaluate_expression(wchar_t *cmd, parser_t &parser, io_streams_t &str
                 streams.out.append_format(L"%.*lf\n", opts.scale, v[i]);
             }
         }
-
         return STATUS_CMD_OK;
     } catch (mu::Parser::exception_type &e) {
         streams.err.append_format(_(L"%ls: Invalid expression: %ls\n"), cmd, e.GetMsg().c_str());
diff --git a/tests/math.err b/tests/math.err
index e69de29bb..240044f45 100644
--- a/tests/math.err
+++ b/tests/math.err
@@ -0,0 +1,6 @@
+
+####################
+# Validate basic expressions
+
+####################
+# Validate how bare variables in an epxression are handled
diff --git a/tests/math.in b/tests/math.in
index a5f111058..8fee37df4 100644
--- a/tests/math.in
+++ b/tests/math.in
@@ -1,3 +1,4 @@
+logmsg Validate basic expressions
 math 3 / 2
 math 10/6
 math -s0 10 / 6
@@ -12,3 +13,12 @@ math '-2 * -2'
 math 5 \* -2
 math -- -4 / 2
 math -- '-4 * 2'
+
+logmsg Validate how bare variables in an epxression are handled
+math x + 1
+set x 1
+math x + 1
+set x 3
+set y 1.5
+math '-x * y'
+math -s1 '-x * y'
diff --git a/tests/math.out b/tests/math.out
index 97a2cdbe3..f0a91e8ce 100644
--- a/tests/math.out
+++ b/tests/math.out
@@ -1,3 +1,6 @@
+
+####################
+# Validate basic expressions
 1
 1
 1
@@ -12,3 +15,10 @@
 -10
 -2
 -8
+
+####################
+# Validate how bare variables in an epxression are handled
+1
+2
+-4
+-4.5