time icon indicating copy to clipboard operation
time copied to clipboard

Owned format descriptions

Open Roguelazer opened this issue 3 years ago • 1 comments

I'd like to have an API where clients can pass in a format string, it'll get parsed once, and the resulting Vec<FormatItem<'_>> can be reused for many formatting operations. Unfortunately, as far as I can tell, there's no "owned" equivalent of FormatItem, so this requires that I carry around a lifetime everywhere. Example:

struct SomeApplication<'a> {
    formatter: Vec<time::format_description::FormatItem<'a>>,
}

impl<'a> SomeApplication<'a> {
    fn new(format_string: &'a str) -> Result<Self, time::error::InvalidFormatDescription> {
        Ok(Self {
            formatter: time::format_description::parse(format_string)?,
        })
    }
}

The only way to avoid a lifetime dependency on the format string is to do something like Box::leak(string.into_boxed_str) to promote the string to a static lifetime and leak it, which is of course gross. One could also use ouroboros or equivalent to make a self-referential struct, but that seems like unnecessarily-deep magic for this use case.

It would be very nice if there were an owned equivalent of FormatItem. ToOwned is implemented for FormatItem, but it's just the auto-impl from Clone so it ends up being a shallow copy (and still has the same lifetime)

I don't have any strong feelings about the API; I could most easily imagine having an OwnedFormatItem which implements std::borrow::Borrow<FormatItem<'a>> (although using Borrow with structs that have a lifetime is often fraught with peril).

Note that this is basically the same as the question in https://github.com/time-rs/time/discussions/353, but since my format strings are user-provided runtime configuration, I can't use the format_description! macro.

Thoughts?

I prototyped something (poorly) and will attach a diff to this issue.

Roguelazer avatar Jan 19 '22 01:01 Roguelazer

I'll have to think about this some more, but it seems reasonable. I've dealt with lifetime hell before, so I understand the concern.

I don't think this strictly needs to implement Borrow<FormatItem<'a>>, but it's reasonable to implement it if possible. As you noted, the Borrow trait has some major limitations.

I'll take a look at your PR when I get a chance. I'm quite busy with a few things nowadays, so that'll likely be a bit.

jhpratt avatar Jan 19 '22 06:01 jhpratt

I have just implemented this on main.

jhpratt avatar Nov 03 '22 06:11 jhpratt