numfmt: align error messages for suffixes #9887
Conversation
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
sbentmar
changed the title
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
please add tests in tests/by-util/test_numfmt.rs too |
src/uu/numfmt/src/format.rs
Outdated
| } | ||
| return Err(translate!("numfmt-error-invalid-suffix", "input" => s.quote())); | ||
|
|
||
| return Err(match parse_suffix(number_part) { |
There was a problem hiding this comment.
Recursive call to parse_suffix() in error path - consider adding a comment explaining this won't cause stack overflow due to guaranteed termination
sbentmar
force-pushed
the
fix-suf-errors
branch
from
1d894e7 to
aab5323
Compare
| } | ||
|
|
||
| fn parse_suffix(s: &str) -> Result<(f64, Option<Suffix>)> { | ||
| fn find_numeric_beginning(s: &str) -> Option<&str> { |
There was a problem hiding this comment.
This could be replaced with a regex, but I'm torn on whether it's worth pulling in an extra dependency for a fairly trivial task.
|
GNU testsuite comparison: |
CodSpeed Performance ReportMerging #9887 will not alter performanceComparing Summary
Footnotes
|
sbentmar
force-pushed
the
fix-suf-errors
branch
2 times, most recently
from
923bb5d to
d307046
Compare
|
I guess parsing the numbers by hand regress the performances. I think we need to do better here |
I'll experiment a bit and see if I can improve it :) |
|
GNU testsuite comparison: |
sbentmar
force-pushed
the
fix-suf-errors
branch
from
a071231 to
04855e3
Compare
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
It's bound to take longer since we need to look at the string, previously we only looked at the last character. I moved it out of the main flow though, so now it's only called when we want to generate an error message. Parsing manually is O(n), but since we need to know where the boundary between the valid and the invalid input is to generate compliant error messages, it's hard to avoid some performance regression. |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
2 participants
In #9752 several failures are caused by the error messages not matching GNU.
Some examples:
This aligns the error message by adding a localization parameter for the incorrect suffix. It also checks whether the number would be valid if we were to remove the suffix, and returns a suffix error rather than a number error in those cases.
This lets the user know which specific suffix is invalid. If they for instance enter "5MS" it will tell them that the "S" is the invalid suffix, not the "M".