Fix nan print, simplify negative number printing.
#7413
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
GNU testsuite comparison: |
sylvestre
reviewed
Mar 7, 2025
There was a problem hiding this comment.
PR Overview
This pull request fixes the formatting of NaN and simplifies negative number printing while also adding new tests for non‐finite and negative zero floats.
- Updated formatting functions in uucore to correctly handle negative numbers and force lowercase NaN
- Simplified logic for generating sign indicators and default precision handling
- Added new tests in both the printf and seq utilities to validate these formatting changes
Reviewed Changes
| File | Description |
|---|---|
| src/uucore/src/lib/features/format/num_format.rs | Refactored float formatting code and sign handling logic |
| tests/by-util/test_seq.rs | Added tests for default precision formatting of floats |
| tests/by-util/test_printf.rs | Added tests for non-finite floats and negative zero printing |
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/uucore/src/lib/features/format/num_format.rs:309
- [nitpick] Consider renaming the 'negative' parameter to 'is_negative' for improved clarity, making it more descriptive of its boolean nature.
fn get_sign_indicator(sign: PositiveSign, negative: bool) -> String {
|
This also accidentally fixes negative number scientific printing: I'll add a test for that, so it doesn't regress. |
|
GNU testsuite comparison: |
|
GNU testsuite comparison: |
Fixes formatting of `NaN` to `nan`. Fixes part 1 of uutils#7412.
Simplifies the code, but also fixes printing of negative and positive `NaN`: `cargo run printf "%f %f\n" nan -nan` Fixes part 2 of uutils#7412.
Add a few end-to-end tests for printf of unusual floats (nan, infinity, negative zero).
This was broken before the last few commits.
|
Rebased. Also, while doing further work I realized that I can now remove the sign handling logic from |
|
GNU testsuite comparison: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
test: printf: Add a test for scientific printing of negative number
This was broken before the last few commits.
test: printf: Add nan, inf, negative zero
Add a few end-to-end tests for printf of unusual floats (nan,
infinity, negative zero).
uucore: format: print absolute value of float, then add sign
Simplifies the code, but also fixes printing of negative and positive
NaN:cargo run printf "%f %f\n" nan -nanFixes part 2 of #7412.
uucore: format: force NaN back to lowercase
Fixes formatting of
NaNtonan.Fixes part 1 of #7412.