-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Make Duration
respect width
when formatting using Debug
#88999
Conversation
Duration's Debug formatting previously ignored the width parameter. This commit fixes that. Fixes issue rust-lang#88059.
r? @dtolnay (rust-highfive has picked a reviewer for you, use r? to override) |
Duration
handle width
when formatting using Debug
Duration
respect width
when formatting using Debug
Considering the fact that booleans are also default-left-aligned, that seems like a safe choice. We can wait for someone with strong opinions on this to give us a reason to change it. @bors r+ Thanks! I just ran into such oddities earlier this week, and you're already fixing this one, great! |
📌 Commit 77ceb2b has been approved by |
r? @oli-obk |
Make `Duration` respect `width` when formatting using `Debug` When printing or writing a `std::time::Duration` using `Debug` formatting, it previously completely ignored any specified `width`. This is unlike types like integers and floats, which do pad to `width`, for both `Display` and `Debug`, though not all types consider `width` in their `Debug` output (see e.g. rust-lang#30164). Curiously, `Duration`'s `Debug` formatting *did* consider `precision`. This PR makes `Duration` pad to `width` just like integers and floats, so that ```rust format!("|{:8?}|", Duration::from_millis(1234)) ``` returns ``` |1.234s | ``` Before you ask "who formats `Debug` output?", note that `Duration` doesn't actually implement `Display`, so `Debug` is currently the only way to format `Duration`s. I think that's wrong, and `Duration` should get a `Display` implementation, but in the meantime there's no harm in making the `Debug` formatting respect `width` rather than ignore it. I chose the default alignment to be left-aligned. The general rule Rust uses is: numeric types are right-aligned by default, non-numeric types left-aligned. It wasn't clear to me whether `Duration` is a numeric type or not. The fact that a formatted `Duration` can end with suffixes of variable length (`"s"`, `"ms"`, `"µs"`, etc.) made me lean towards left-alignment, but it would be trivial to change it. Fixes issue rust-lang#88059.
⌛ Testing commit 77ceb2b with merge 94ebf7000eb7eb645ced41792cdc1f83c0d7b7af... |
💔 Test failed - checks-actions |
I don't understand what went wrong. Looks like the build was cancelled? |
@bors retry sometimes bors has hickups |
☀️ Test successful - checks-actions |
Finished benchmarking commit (f06f9bb): comparison url. Summary: This change led to moderate relevant improvements 🎉 in compiler performance.
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
When printing or writing a
std::time::Duration
usingDebug
formatting, it previously completely ignored any specifiedwidth
. This is unlike types like integers and floats, which do pad towidth
, for bothDisplay
andDebug
, though not all types considerwidth
in theirDebug
output (see e.g. #30164). Curiously,Duration
'sDebug
formatting did considerprecision
.This PR makes
Duration
pad towidth
just like integers and floats, so thatreturns
Before you ask "who formats
Debug
output?", note thatDuration
doesn't actually implementDisplay
, soDebug
is currently the only way to formatDuration
s. I think that's wrong, andDuration
should get aDisplay
implementation, but in the meantime there's no harm in making theDebug
formatting respectwidth
rather than ignore it.I chose the default alignment to be left-aligned. The general rule Rust uses is: numeric types are right-aligned by default, non-numeric types left-aligned. It wasn't clear to me whether
Duration
is a numeric type or not. The fact that a formattedDuration
can end with suffixes of variable length ("s"
,"ms"
,"µs"
, etc.) made me lean towards left-alignment, but it would be trivial to change it.Fixes issue #88059.