-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
printf: fix formatting for zero value when precision is zero #8351
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
For clippy related issues, it may be helpful to run locally:
|
Ah, forgot to check lint. Thank you. |
GNU testsuite comparison:
|
Will look into failing tests + other lints I didn't pick up earlier |
Reran lint and addressed issues there. The failing GNU test case was because I didn't switch the precision parser to return a Should be good to run workflow again. |
GNU testsuite comparison:
|
Updated the one test case. Not sure there's much I can do about the other one that's complaining about linting issues in the |
GNU testsuite comparison:
|
GNU testsuite comparison:
|
Can you squash your commits in a single one for better readability in the main branch ? |
@RenjiSann Done! Sorry it took so long to get back on that. |
GNU testsuite comparison:
|
Just rebased on top of main. |
GNU testsuite comparison:
|
@@ -1218,7 +1269,7 @@ mod test { | |||
} | |||
|
|||
#[test] | |||
#[ignore = "Need issues #7509 and #7510 to be fixed"] | |||
#[ignore = "Needs issue #7510 to be fixed"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: noticed #7510 was closed, do we think we can remove this ignore now?
s = format!("{prefix}{s:0>precision$}"); | ||
} | ||
} else { | ||
s = format!("{prefix}{s:0>width$}", width = 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using width = 0 here seems unnecessary since you're not actually using width
padding when precision is None. Could be simplified to just s = format!("{prefix}{s}");
no ?
_ => "", | ||
}; | ||
|
||
s = format!("{prefix}{s:0>width$}", width = self.precision); | ||
if let Some(precision) = self.precision { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about something like ?
s = match (self.precision, x) {
(Some(0), 0) => match self.variant {
UnsignedIntVariant::Octal(Prefix::Yes) => String::from("0"),
_ => String::new(),
},
(Some(p), _) => format!("{prefix}{s:0>p$}"),
(None, _) => format!("{prefix}{s}"),
};
let s = if let Some(precision) = self.precision { | ||
if precision > 0 { | ||
format!("{abs:0>precision$}") | ||
} else if abs == 0 { | ||
String::from("") | ||
} else { | ||
abs.to_string() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same
let s = match (self.precision, abs) {
(Some(0), 0) => String::new(),
(Some(p), _) if p > 0 => format!("{abs:0>p$}"),
_ => abs.to_string(),
};
Fixes issue #7509
According to the C/C++ reference, "if both the value and precision are 0, the conversion results in no characters. "
This PR also includes code to handle the following exceptions:
0
should be outputted