From 98449fec513fc7200eeda6de544a09a10dee01b9 Mon Sep 17 00:00:00 2001
From: Kurtis Rader <krader@skepticism.us>
Date: Fri, 14 Jul 2017 16:03:31 -0700
Subject: [PATCH] fix `math` regression

The previous change to use `argparse` for parity with every other
builtin and function introduced a regression. Invocations that start
with a negative number can fail because the negative value looks like an
invalid flag.
---
 doc_src/math.txt          |  6 ++++--
 share/functions/math.fish | 18 +++++++++++++++---
 tests/math.in             |  8 ++++++--
 tests/math.out            |  5 +++++
 4 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/doc_src/math.txt b/doc_src/math.txt
index a22b0e6ea..32535d7cb 100644
--- a/doc_src/math.txt
+++ b/doc_src/math.txt
@@ -2,7 +2,7 @@
 
 \subsection math-synopsis Synopsis
 \fish{synopsis}
-math [-sN] EXPRESSION
+math [-sN | --scale=N] [--] EXPRESSION
 \endfish
 
 \subsection math-description Description
@@ -13,7 +13,7 @@ For a description of the syntax supported by math, see the manual for the bc pro
 
 The following options are available:
 
-- `-sN` Sets the scale of the result. `N` must be an integer and defaults to zero. This simply sets bc's `scale` variable to the provided value. Note that you cannot put a space between `-s` and `N`.
+- `-sN` or `--scale=N` sets the scale of the result. `N` must be an integer and defaults to zero. This simply sets bc's `scale` variable to the provided value.
 
 \subsection return-values Return Values
 
@@ -33,4 +33,6 @@ If invalid options or no expression is provided the return `status` is two. If t
 
 \subsection math-cautions Cautions
 
+You should always place a `--` flag separator before the expression. 99.99% of the time you'll get the desired result without the separator. Something like `math -10.0 / 2` will fail because the negative floating point value gets treated as an invalid flag. But `math -10 / 2` will work because negative integers are special-cased.
+
 Note that the modulo operator (`x % y`) is not well defined for floating point arithmetic. The `bc` command produces a nonsensical result rather than emit an error and fail in that case. It doesn't matter if the arguments are integers; e.g., `10 % 4`. You'll still get an incorrect result. Do not use the `-sN` flag with N greater than zero if you want sensible answers when using the modulo operator.
diff --git a/share/functions/math.fish b/share/functions/math.fish
index 8f1ecc78b..efcdf9757 100644
--- a/share/functions/math.fish
+++ b/share/functions/math.fish
@@ -1,6 +1,11 @@
 function math --description "Perform math calculations in bc"
-    set -l options 'h/help' 's/scale='
-    argparse -n math --min-args=1 $options -- $argv
+    if not set -q argv[2]
+        # Make sure an invocation like `math "-1 + 1"` doesn't treat the string as an option.
+        set argv -- $argv
+    end
+
+    set -l options 'h/help' 's/scale=' '#-val'
+    argparse -n math --stop-nonopt --min-args=1 $options -- $argv
     or return
 
     if set -q _flag_help
@@ -8,7 +13,7 @@ function math --description "Perform math calculations in bc"
         return 0
     end
 
-    set -l scale 0  # default is integer arithmetic
+    set -l scale 0 # default is integer arithmetic
     if set -q _flag_scale
         set scale $_flag_scale
         if not string match -q -r '^\d+$' "$scale"
@@ -17,6 +22,13 @@ function math --description "Perform math calculations in bc"
         end
     end
 
+    if set -q _flag_val
+        # The expression began with a negative number. Put it back in the expression.
+        # The correct thing is for the person calling us to insert a `--` separator before the
+        # expression to stop parsing flags. But we'll work around that missing token here.
+        set argv -$_flag_val $argv
+    end
+
     # Set BC_LINE_LENGTH to a ridiculously high number so it only uses one line for most results.
     # We can't use 0 since some systems (including macOS) use an ancient bc that doesn't support it.
     # We also can't count on this being recognized since some BSD systems don't recognize this env
diff --git a/tests/math.in b/tests/math.in
index 40ad59f5b..3809561ee 100644
--- a/tests/math.in
+++ b/tests/math.in
@@ -5,6 +5,10 @@ math -s3 10/6
 math '10 % 6'
 math -s0 '10 % 6'
 math '23 % 7'
-math -s6 '5 / 3 * 0.3'
-true
+math --scale=6 '5 / 3 * 0.3'
 math "1 + 1233242342353453463458972349873489273984873289472914712894791824712941"
+math -1 + 1
+math '-2 * -2'
+math 5 \* -2
+math -- -4 / 2
+math -- '-4 * 2'
diff --git a/tests/math.out b/tests/math.out
index a49461da2..d1720d67d 100644
--- a/tests/math.out
+++ b/tests/math.out
@@ -7,3 +7,8 @@
 2
 .499999
 1233242342353453463458972349873489273984873289472914712894791824712942
+0
+4
+-10
+-2
+-8