Conversation
| } | ||
|
|
||
| /// GNU rejects an integer or fraction part with more than this many digits. | ||
| const MAX_ACCEPTABLE_DIGITS: usize = 33; |
There was a problem hiding this comment.
this is the GNU identifier name, and the PR description quotes GNU internals too
please don't look at the GNU source, just the behavior. could you rename it, e.g. MAX_DIGITS?
There was a problem hiding this comment.
You're right. I looked at GNU's source before I wrote the first version, and I shouldn't have done that.
I've thrown that version out and rewritten it from scratch. This time I only ran GNU numfmt and checked how it behaves, without opening its code. The new version is force-pushed.
I've also fixed your other two points:
- The constant is now MAX_DIGITS.
- The integer and fraction parts are now two separate variables, whole and fraction, so neither name is wrong anymore.
- Every error test now checks stdout as well.
Let me know if anything still looks off.
| let dec_sep = locale_decimal_separator(); | ||
|
|
||
| // Like GNU, limit the integer and fraction digit runs separately. | ||
| let int_part = s.strip_prefix('-').unwrap_or(s); |
There was a problem hiding this comment.
int_part also holds the fraction here, maybe unsigned?
There was a problem hiding this comment.
The rewrite doesn't have int_part anymore. It counts the integer part and the fraction part in two separate variables, whole and fraction, so I didn't need a single name like unsigned.
| .stdout_only("112Q\n"); | ||
| } | ||
| for input in [format!("0.{max_digits}"), format!("0.{zeros}{max_digits}")] { | ||
| new_ucmd!().args(&["--to=si", &input]).succeeds(); |
There was a problem hiding this comment.
please check stdout here too, otherwise we don't know what we print
There was a problem hiding this comment.
Every error test checks stdout now too. It's either empty, or, in the --invalid=warn/fail tests, exactly what gets printed before the error.
A number whose whole part or fraction has more than 33 digits, not counting leading zeros, is now refused with "value too large to be converted" instead of being silently rounded, matching GNU numfmt. Closes uutils#12855
3c7b038 to
e93e0b3
Compare
|
GNU testsuite comparison: |
This branch has not been deployed
2 participants
numfmt would take a number with a huge number of digits, round it without saying anything, and print something that didn't match what you typed.
Now it rejects any number with more than 33 digits on either side of the decimal point. It prints
value too large to be converted: '<input>'and exits with 2, same as GNU.I matched GNU's behaviour by running it, not by reading its code:
KorKiafter the number doesn't change the error.--invalid=warn/fail/ignoretreat it like any other bad input.One small extra: when the terminal error display is on, the long number gets underlined, and the suffix hint (which was misleading here) is no longer shown.
There are 8 new tests: whole part, fraction, leading zeros, negatives, numbers before a suffix, the
--invalidmodes, and the input from the issue. Each error test checks stderr, stdout and the exit code.Closes #12855