Default math scale to 6

This changes the behavior of builtin math to floating point by default.
If the result of a computation is an integer, then it will be printed as an
integer; otherwise it will be printed as a floating point decimal with up to
'scale' digits past the decimal point (default is 6, matching printf).
Trailing zeros are trimmed. Values are rounded following printf semantics.

Fixes #4478

CHANGELOG.md

@@ -32,7 +32,7 @@ fish 3.0 is a major release which brings with it both improvements in functional
 - `abbr` has been reimplemented to be faster. This means the old `fish_user_abbreviations` variable is ignored (#4048).
 - Setting variables is much faster (#4200, #4341).
 - Using a read-only variable in a for loop is now an error. Note that this never worked. It simply failed to set the for loop var and thus silently produced incorrect results (#4342).
-- `math` is now a builtin rather than a wrapper around `bc` (#3157).
+- `math` is now a builtin rather than a wrapper around `bc` (#3157). The default scale is now 6, so that floating point computations produce decimals (#4478).
 - `history search` supports globs for wildcard searching (#3136).
 - `bind` has a new `--silent` option to ignore bind requests for named keys not available under the current `$TERMINAL` (#4188, #4431).
 - Globs are faster (#4579).

src/builtin_math.cpp

@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <cmath>
+#include <limits>
 #include <string>
 
 #include "tinyexpr.h"
@@ -18,9 +19,17 @@
 #include "wgetopt.h"
 #include "wutil.h"  // IWYU pragma: keep
 
+// The maximum number of points after the decimal that we'll print.
+static constexpr int kDefaultScale = 6;
+
+// The end of the range such that every integer is representable as a double.
+// i.e. this is the first value such that x + 1 == x (or == x + 2, depending on rounding mode).
+static constexpr double kMaximumContiguousInteger =
+    double(1LLU << std::numeric_limits<double>::digits);
+
 struct math_cmd_opts_t {
     bool print_help = false;
-    int scale = 0;
+    int scale = kDefaultScale;
 };
 
 // This command is atypical in using the "+" (REQUIRE_ORDER) option for flag parsing.
@@ -130,6 +139,27 @@ static wcstring math_describe_error(te_error_t& error) {
     }
 }
 
+/// Return a formatted version of the value \p v respecting the given \p opts.
+static wcstring format_double(double v, const math_cmd_opts_t &opts) {
+    wcstring ret = format_string(L"%.*f", opts.scale, v);
+    // If we contain a decimal separator, trim trailing zeros after it, and then the separator
+    // itself if there's nothing after it. Detect a decimal separator as a non-digit.
+    const wchar_t *const digits = L"0123456789";
+    if (ret.find_first_not_of(digits) != wcstring::npos) {
+        while (ret.back() == L'0') {
+            ret.pop_back();
+        }
+        if (!wcschr(digits, ret.back())) {
+            ret.pop_back();
+        }
+    }
+    // If we trimmed everything it must have just been zero.
+    if (ret.empty()) {
+        ret.push_back(L'0');
+    }
+    return ret;
+}
+
 /// Evaluate math expressions.
 static int evaluate_expression(const wchar_t *cmd, parser_t &parser, io_streams_t &streams,
                                math_cmd_opts_t &opts, wcstring &expression) {
@@ -150,27 +180,21 @@ static int evaluate_expression(const wchar_t *cmd, parser_t &parser, io_streams_
         // TODO: Really, this should be done in tinyexpr
         // (e.g. infinite is the result of "x / 0"),
         // but that's much more work.
+        const char *error_message = NULL;
         if (std::isinf(v)) {
-            streams.err.append_format(L"%ls: Error: Result is infinite\n", cmd);
+            error_message = "Result is infinite";
-            streams.err.append_format(L"'%ls'\n", expression.c_str());
-            retval = STATUS_CMD_ERROR;
         } else if (std::isnan(v)) {
-            streams.err.append_format(L"%ls: Error: Result is not a number\n", cmd);
+            error_message = "Result is not a number";
+        } else if (std::abs(v) >= kMaximumContiguousInteger) {
+            error_message = "Result magnitude is too large";
+        }
+        if (error_message) {
+            streams.err.append_format(L"%ls: Error: %s\n", cmd, error_message);
             streams.err.append_format(L"'%ls'\n", expression.c_str());
             retval = STATUS_CMD_ERROR;
-        } else if (v >= LONG_MAX) {
-            streams.err.append_format(L"%ls: Error: Result is too large\n", cmd);
-            streams.err.append_format(L"'%ls'\n", expression.c_str());
-            retval = STATUS_CMD_ERROR;
-        } else if (v <= LONG_MIN) {
-            streams.err.append_format(L"%ls: Error: Result is too small\n", cmd);
-            streams.err.append_format(L"'%ls'\n", expression.c_str());
-            retval = STATUS_CMD_ERROR;
-        } else if (opts.scale == 0) {
-            // Normal results
-            streams.out.append_format(L"%ld\n", static_cast<long>(v));
         } else {
-            streams.out.append_format(L"%.*lf\n", opts.scale, v);
+            streams.out.append(format_double(v, opts));
+            streams.out.push_back(L'\n');
         }
     } else {
         streams.err.append_format(L"%ls: Error: %ls\n", cmd, math_describe_error(error).c_str());

tests/math.err

@@ -2,6 +2,9 @@
 ####################
 # Validate basic expressions
 
+####################
+# Validate some integral computations
+
 ####################
 # Validate how variables in an expression are handled
 
@@ -24,7 +27,7 @@ math: Error: Too many arguments
        ^
 math: Expected at least 1 args, got only 0
 math: Expected at least 1 args, got only 0
-math: Error: Result is too large
+math: Error: Result is infinite
-'2^650'
+'2^999999'
 math: Error: Result is infinite
 '1 / 0'

tests/math.in

@@ -2,6 +2,7 @@ logmsg Validate basic expressions
 math 3 / 2
 math 10/6
 math -s0 10 / 6
+math 'floor(10 / 6)'
 math -s3 10/6
 math '10 % 6'
 math -s0 '10 % 6'
@@ -14,6 +15,19 @@ math 5 \* -2
 math -- -4 / 2
 math -- '-4 * 2'
 
+logmsg Validate some integral computations
+math 1
+math 10
+math 100
+math 1000
+math '10^15'
+math '-10^14'
+math '-10^15'
+
+math -s0 '1.0 / 2.0'
+math -s0 '3.0 / 2.0'
+math -s0 '10^15 / 2.0'
+
 logmsg Validate how variables in an expression are handled
 math $x + 1
 set x 1
@@ -21,7 +35,7 @@ math $x + 1
 set x 3
 set y 1.5
 math "-$x * $y"
-math -s1 "-$x * $y"
+math -s0 "-$x * $y"
 
 logmsg Validate math error reporting
 not math '2 - '
@@ -31,5 +45,5 @@ not math 'sin()'
 not math '2 + 2 4'
 not math
 not math -s 12
-not math 2^650
+not math 2^999999
 not math 1 / 0

tests/math.out

@@ -1,14 +1,15 @@
 
 ####################
 # Validate basic expressions
-1
+1.5
-1
+1.666667
+2
 1
 1.667
 4
 4
 2
-0.500000
+0.5
 49
 0
 4
@@ -16,12 +17,25 @@
 -2
 -8
 
+####################
+# Validate some integral computations
+1
+10
+100
+1000
+1000000000000000
+100000000000000
+-1000000000000000
+0
+2
+500000000000000
+
 ####################
 # Validate how variables in an expression are handled
 1
 2
--4
 -4.5
+-4
 
 ####################
 # Validate math error reporting