Re: [pbs-devel] [PATCH proxmox-backup 1/3] tools/systemd/time: implement some Traits for TimeSpan

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 16.03.21 12:56, Dominik Csapak wrote:
> namely
> * From<Duration> (to convert easily from duration to timespan)
> * Display (for better formatting)
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> if wanted, we can optimize the display trait a bit further, e.g.
> only showing the biggest two units instead
> 
>  src/tools/systemd/time.rs | 84 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/src/tools/systemd/time.rs b/src/tools/systemd/time.rs
> index 7cc42415..75fb0ea2 100644
> --- a/src/tools/systemd/time.rs
> +++ b/src/tools/systemd/time.rs
> @@ -141,6 +141,90 @@ impl From<TimeSpan> for f64 {
>      }
>  }
>  
> +impl From<std::time::Duration> for TimeSpan {
> +    fn from(duration: std::time::Duration) -> Self {
> +        let mut duration = duration.as_nanos();
> +        let nsec = (duration % 1000) as u64;
> +        duration /= 1000;
> +        let usec = (duration % 1000) as u64;
> +        duration /= 1000;
> +        let msec = (duration % 1000) as u64;
> +        duration /= 1000;
> +        let seconds = (duration % 60) as u64;
> +        duration /= 60;
> +        let minutes = (duration % 60) as u64;
> +        duration /= 60;
> +        let hours = (duration % 24) as u64;
> +        duration /= 24;
> +        let years = (duration as f64 / 365.25) as u64;
> +        let ydays = (duration as f64 % 365.25) as u64;
> +        let months = (ydays as f64 / 30.44) as u64;
> +        let mdays = (ydays as f64 % 30.44) as u64;
> +        let weeks = mdays / 7;
> +        let days = mdays % 7;
> +        Self {
> +            nsec,
> +            msec,
> +            usec,

nit:
nano -> milli -> micro? While it works correct, that order above seems just wrong...

> +            seconds,
> +            minutes,
> +            hours,
> +            days,
> +            weeks,
> +            months,
> +            years,
> +        }
> +    }
> +}
> +
> +impl std::fmt::Display for TimeSpan {
> +    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
> +        let mut single = true;
> +        if self.years > 0 {
> +            write!(f, "{}y", self.years)?;
> +            single = false;
> +        }
> +        let write_space = |single: bool, f: &mut std::fmt::Formatter| -> Result<bool, std::fmt::Error> {
> +            if !single {
> +                write!(f, " ")?;
> +            }
> +            Ok(false)
> +        };

No sure if something like (untested):

let mut first = true;
let mut do_write = |v: u64, unit: &str| -> Result<(), std::fmt::Error> {
    if !first {
        write!(f, " ")?;
        first = false;
    }
    write!(f, "{}{}", v, unit)
}

if self.months > 0 {
    do_write(self.months, "m")?;
}

...

would be nicer IMO, especially as the "single" handling reads a bit weird.
Maybe the seconds field makes this not really feasible though...

> +        if self.months > 0 {
> +            single = write_space(single, f)?;
> +            write!(f, "{}m", self.months)?;
> +        }
> +        if self.weeks > 0 {
> +            single = write_space(single, f)?;
> +            write!(f, "{}w", self.weeks)?;
> +        }
> +        if self.days > 0 {
> +            single = write_space(single, f)?;
> +            write!(f, "{}d", self.days)?;
> +        }
> +        if self.hours > 0 {
> +            single = write_space(single, f)?;
> +            write!(f, "{}h", self.hours)?;
> +        }
> +        if self.minutes > 0 {
> +            single = write_space(single, f)?;
> +            write!(f, "{}min", self.minutes)?;
> +        }
> +        let seconds = self.seconds as f64 + (self.msec as f64 / 1000.0);
> +        if seconds >= 0.1 {
> +            write_space(single, f)?;
> +            if seconds >= 1.0 || !single {
> +                write!(f, "{:.0}s", seconds)?;
> +            } else {
> +                write!(f, "{:.1}s", seconds)?;
> +            }
> +        } else if single {
> +            write_space(single, f)?;
> +            write!(f, "<0.1s")?;
> +        }
> +        Ok(())
> +    }
> +}
>  
>  pub fn verify_time_span(i: &str) -> Result<(), Error> {
>      parse_time_span(i)?;
>