From 25f47729e70c1de11f03bd38f4b5ff0634f1fcb0 Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 7 Nov 2021 16:41:47 -0800 Subject: [PATCH] math: Correct printing of negative and large values in octal and hex This fixes printing octal and hex values that are negative or larger than UINT_MAX. Negative values get a leading -, like: > math --base hex -10 -0xa Fixes #8417. --- CHANGELOG.rst | 1 + src/builtin_math.cpp | 16 ++++++++++------ tests/checks/math.fish | 16 ++++++++++++++++ 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 5b1c405a8..3726fa77f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -71,6 +71,7 @@ Scripting improvements - On MacOS terminals that are not granted permissions to access a folder, ``cd`` would print a spurious "rotten symlink" error, which has been corrected to "permission denied" (:issue:`8264`). - Performance improvements to globbing, especially on systems using glibc (by avoiding its slow iswalnum() function). In some cases (large directories with files with many numbers in the names) this almost doubles performance. - Since fish 3.0, for-loops would trigger a variable handler an additional time before the loop was entered. This has been corrected (:issue:`8384`). +- ``math`` now correctly prints negative values and values larger than ``2**31`` when in hex or octal bases (:issue:`8417`). Interactive improvements ------------------------ diff --git a/src/builtin_math.cpp b/src/builtin_math.cpp index 97a9c1840..59b49f413 100644 --- a/src/builtin_math.cpp +++ b/src/builtin_math.cpp @@ -57,8 +57,8 @@ static int parse_cmd_opts(math_cmd_opts_t &opts, int *optind, //!OCLINT(high nc } else { opts.scale = fish_wcstoi(w.woptarg); if (errno || opts.scale < 0 || opts.scale > 15) { - streams.err.append_format(_(L"%ls: %ls: invalid scale value\n"), - cmd, w.woptarg); + streams.err.append_format(_(L"%ls: %ls: invalid scale value\n"), cmd, + w.woptarg); return STATUS_INVALID_ARGS; } } @@ -99,7 +99,8 @@ static int parse_cmd_opts(math_cmd_opts_t &opts, int *optind, //!OCLINT(high nc } } if (opts.have_scale && opts.scale != 0 && opts.base != 10) { - streams.err.append_format(BUILTIN_ERR_COMBO2, cmd, L"non-zero scale value only valid for base 10"); + streams.err.append_format(BUILTIN_ERR_COMBO2, cmd, + L"non-zero scale value only valid for base 10"); return STATUS_INVALID_ARGS; } @@ -188,10 +189,13 @@ static const wchar_t *math_describe_error(const te_error_t &error) { static wcstring format_double(double v, const math_cmd_opts_t &opts) { if (opts.base == 16) { v = trunc(v); - return format_string(L"0x%x", (unsigned int)v); + const char *mneg = (v < 0.0 ? "-" : ""); + return format_string(L"%s0x%llx", mneg, (long long)std::fabs(v)); } else if (opts.base == 8) { v = trunc(v); - return format_string(L"0%o", (unsigned int)v); + if (v == 0.0) return L"0"; // not 00 + const char *mneg = (v < 0.0 ? "-" : ""); + return format_string(L"%s0%llo", mneg, (long long)std::fabs(v)); } // As a special-case, a scale of 0 means to truncate to an integer @@ -239,7 +243,7 @@ static int evaluate_expression(const wchar_t *cmd, const parser_t &parser, io_st error_message = L"Result is infinite"; } else if (std::isnan(v)) { error_message = L"Result is not a number"; - } else if (std::abs(v) >= kMaximumContiguousInteger) { + } else if (std::fabs(v) >= kMaximumContiguousInteger) { error_message = L"Result magnitude is too large"; } if (error_message) { diff --git a/tests/checks/math.fish b/tests/checks/math.fish index 4cc71b9f3..9575c83c7 100644 --- a/tests/checks/math.fish +++ b/tests/checks/math.fish @@ -188,6 +188,22 @@ math -bhex 16 x 2 # CHECK: 0x20 math --base hex 12 + 0x50 # CHECK: 0x5c +math --base hex 0 +# CHECK: 0x0 +math --base hex -1 +# CHECK: -0x1 +math --base hex -15 +# CHECK: -0xf +math --base hex 'pow(2,40)' +# CHECK: 0x10000000000 +math --base octal 0 +# CHECK: 0 +math --base octal -1 +# CHECK: -01 +math --base octal -15 +# CHECK: -017 +math --base octal 'pow(2,40)' +# CHECK: 020000000000000 math --base octal --scale=0 55 # CHECK: 067 math --base notabase