Skip to content

Make Duration respect width when formatting using Debug #88999

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

Merged
merged 1 commit into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions library/core/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1189,7 +1189,7 @@ unsafe fn getcount(args: &[ArgumentV1<'_>], cnt: &rt::v1::Count) -> Option<usize

/// Padding after the end of something. Returned by `Formatter::padding`.
#[must_use = "don't forget to write the post padding"]
struct PostPadding {
pub(crate) struct PostPadding {
fill: char,
padding: usize,
}
Expand All @@ -1200,9 +1200,9 @@ impl PostPadding {
}

/// Write this post padding.
fn write(self, buf: &mut dyn Write) -> Result {
pub(crate) fn write(self, f: &mut Formatter<'_>) -> Result {
for _ in 0..self.padding {
buf.write_char(self.fill)?;
f.buf.write_char(self.fill)?;
}
Ok(())
}
Expand Down Expand Up @@ -1325,7 +1325,7 @@ impl<'a> Formatter<'a> {
write_prefix(self, sign, prefix)?;
let post_padding = self.padding(min - width, rt::v1::Alignment::Right)?;
self.buf.write_str(buf)?;
post_padding.write(self.buf)?;
post_padding.write(self)?;
self.fill = old_fill;
self.align = old_align;
Ok(())
Expand All @@ -1335,7 +1335,7 @@ impl<'a> Formatter<'a> {
let post_padding = self.padding(min - width, rt::v1::Alignment::Right)?;
write_prefix(self, sign, prefix)?;
self.buf.write_str(buf)?;
post_padding.write(self.buf)
post_padding.write(self)
}
}
}
Expand Down Expand Up @@ -1410,7 +1410,7 @@ impl<'a> Formatter<'a> {
let align = rt::v1::Alignment::Left;
let post_padding = self.padding(width - chars_count, align)?;
self.buf.write_str(s)?;
post_padding.write(self.buf)
post_padding.write(self)
}
}
}
Expand All @@ -1419,7 +1419,7 @@ impl<'a> Formatter<'a> {
/// Write the pre-padding and return the unwritten post-padding. Callers are
/// responsible for ensuring post-padding is written after the thing that is
/// being padded.
fn padding(
pub(crate) fn padding(
&mut self,
padding: usize,
default: rt::v1::Alignment,
Expand Down Expand Up @@ -1474,7 +1474,7 @@ impl<'a> Formatter<'a> {
} else {
let post_padding = self.padding(width - len, align)?;
self.write_formatted_parts(&formatted)?;
post_padding.write(self.buf)
post_padding.write(self)
};
self.fill = old_fill;
self.align = old_align;
Expand Down
92 changes: 70 additions & 22 deletions library/core/src/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1049,11 +1049,16 @@ impl fmt::Debug for Duration {
/// `divisor` must not be above 100_000_000. It also should be a power
/// of 10, everything else doesn't make sense. `fractional_part` has
/// to be less than `10 * divisor`!
///
/// A prefix and postfix may be added. The whole thing is padded
/// to the formatter's `width`, if specified.
fn fmt_decimal(
f: &mut fmt::Formatter<'_>,
mut integer_part: u64,
mut fractional_part: u32,
mut divisor: u32,
prefix: &str,
postfix: &str,
) -> fmt::Result {
// Encode the fractional part into a temporary buffer. The buffer
// only need to hold 9 elements, because `fractional_part` has to
Expand Down Expand Up @@ -1114,48 +1119,91 @@ impl fmt::Debug for Duration {
// set, we only use all digits up to the last non-zero one.
let end = f.precision().map(|p| crate::cmp::min(p, 9)).unwrap_or(pos);

// If we haven't emitted a single fractional digit and the precision
// wasn't set to a non-zero value, we don't print the decimal point.
if end == 0 {
write!(f, "{}", integer_part)
} else {
// SAFETY: We are only writing ASCII digits into the buffer and it was
// initialized with '0's, so it contains valid UTF8.
let s = unsafe { crate::str::from_utf8_unchecked(&buf[..end]) };
// This closure emits the formatted duration without emitting any
// padding (padding is calculated below).
let emit_without_padding = |f: &mut fmt::Formatter<'_>| {
write!(f, "{}{}", prefix, integer_part)?;

// Write the decimal point and the fractional part (if any).
if end > 0 {
// SAFETY: We are only writing ASCII digits into the buffer and
// it was initialized with '0's, so it contains valid UTF8.
let s = unsafe { crate::str::from_utf8_unchecked(&buf[..end]) };

// If the user request a precision > 9, we pad '0's at the end.
let w = f.precision().unwrap_or(pos);
write!(f, ".{:0<width$}", s, width = w)?;
}

// If the user request a precision > 9, we pad '0's at the end.
let w = f.precision().unwrap_or(pos);
write!(f, "{}.{:0<width$}", integer_part, s, width = w)
write!(f, "{}", postfix)
};

match f.width() {
None => {
// No `width` specified. There's no need to calculate the
// length of the output in this case, just emit it.
emit_without_padding(f)
}
Some(requested_w) => {
// A `width` was specified. Calculate the actual width of
// the output in order to calculate the required padding.
// It consists of 4 parts:
// 1. The prefix: is either "+" or "", so we can just use len().
// 2. The postfix: can be "µs" so we have to count UTF8 characters.
let mut actual_w = prefix.len() + postfix.chars().count();
// 3. The integer part:
if let Some(log) = integer_part.checked_log10() {
// integer_part is > 0, so has length log10(x)+1
actual_w += 1 + log as usize;
} else {
// integer_part is 0, so has length 1.
actual_w += 1;
}
// 4. The fractional part (if any):
if end > 0 {
let frac_part_w = f.precision().unwrap_or(pos);
actual_w += 1 + frac_part_w;
}

if requested_w <= actual_w {
// Output is already longer than `width`, so don't pad.
emit_without_padding(f)
} else {
// We need to add padding. Use the `Formatter::padding` helper function.
let default_align = crate::fmt::rt::v1::Alignment::Left;
let post_padding = f.padding(requested_w - actual_w, default_align)?;
emit_without_padding(f)?;
post_padding.write(f)
}
}
}
}

// Print leading '+' sign if requested
if f.sign_plus() {
write!(f, "+")?;
}
let prefix = if f.sign_plus() { "+" } else { "" };

if self.secs > 0 {
fmt_decimal(f, self.secs, self.nanos, NANOS_PER_SEC / 10)?;
f.write_str("s")
fmt_decimal(f, self.secs, self.nanos, NANOS_PER_SEC / 10, prefix, "s")
} else if self.nanos >= NANOS_PER_MILLI {
fmt_decimal(
f,
(self.nanos / NANOS_PER_MILLI) as u64,
self.nanos % NANOS_PER_MILLI,
NANOS_PER_MILLI / 10,
)?;
f.write_str("ms")
prefix,
"ms",
)
} else if self.nanos >= NANOS_PER_MICRO {
fmt_decimal(
f,
(self.nanos / NANOS_PER_MICRO) as u64,
self.nanos % NANOS_PER_MICRO,
NANOS_PER_MICRO / 10,
)?;
f.write_str("µs")
prefix,
"µs",
)
} else {
fmt_decimal(f, self.nanos as u64, 0, 1)?;
f.write_str("ns")
fmt_decimal(f, self.nanos as u64, 0, 1, prefix, "ns")
}
}
}
Expand Down
28 changes: 28 additions & 0 deletions library/core/tests/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,34 @@ fn debug_formatting_precision_two() {
assert_eq!(format!("{:.2?}", Duration::new(8, 999_999_999)), "9.00s");
}

#[test]
fn debug_formatting_padding() {
assert_eq!("0ns ", format!("{:<9?}", Duration::new(0, 0)));
assert_eq!(" 0ns", format!("{:>9?}", Duration::new(0, 0)));
assert_eq!(" 0ns ", format!("{:^9?}", Duration::new(0, 0)));
assert_eq!("123ns ", format!("{:<9.0?}", Duration::new(0, 123)));
assert_eq!(" 123ns", format!("{:>9.0?}", Duration::new(0, 123)));
assert_eq!(" 123ns ", format!("{:^9.0?}", Duration::new(0, 123)));
assert_eq!("123.0ns ", format!("{:<9.1?}", Duration::new(0, 123)));
assert_eq!(" 123.0ns", format!("{:>9.1?}", Duration::new(0, 123)));
assert_eq!(" 123.0ns ", format!("{:^9.1?}", Duration::new(0, 123)));
assert_eq!("7.1µs ", format!("{:<9?}", Duration::new(0, 7_100)));
assert_eq!(" 7.1µs", format!("{:>9?}", Duration::new(0, 7_100)));
assert_eq!(" 7.1µs ", format!("{:^9?}", Duration::new(0, 7_100)));
assert_eq!("999.123456ms", format!("{:<9?}", Duration::new(0, 999_123_456)));
assert_eq!("999.123456ms", format!("{:>9?}", Duration::new(0, 999_123_456)));
assert_eq!("999.123456ms", format!("{:^9?}", Duration::new(0, 999_123_456)));
assert_eq!("5s ", format!("{:<9?}", Duration::new(5, 0)));
assert_eq!(" 5s", format!("{:>9?}", Duration::new(5, 0)));
assert_eq!(" 5s ", format!("{:^9?}", Duration::new(5, 0)));
assert_eq!("5.000000000000s", format!("{:<9.12?}", Duration::new(5, 0)));
assert_eq!("5.000000000000s", format!("{:>9.12?}", Duration::new(5, 0)));
assert_eq!("5.000000000000s", format!("{:^9.12?}", Duration::new(5, 0)));

// default alignment is left:
assert_eq!("5s ", format!("{:9?}", Duration::new(5, 0)));
}

#[test]
fn debug_formatting_precision_high() {
assert_eq!(format!("{:.5?}", Duration::new(0, 23_678)), "23.67800µs");
Expand Down