Allow custom timestamp formats.

[mjkillough]

Past experience has taught me that you'll never have one logging time
format that pleases everyone, so perhaps we should just allow it to be
configurable. :)

We can use StrftimeItems to parse the format string once when we build
the Logger. We limit the lifetime of the format string to 'static as
it keeps things simple. (If we wanted to accept arbitrary lifetimes,
I think we'd either need to introduce a lifetime parameter on Logger,
or have a self-referential struct somewhere).

This commit also removes the nanoseconds, as suggested in #34. It does
not alias +00:00 to +Z, as there doesn't seem to be a way to do this
with chrono::format::strftime. (It is possible to do it by
constructing our own iterator of formatting Items, but I think
chrono should learn to allow this via StftimeItems instead).

[mjkillough]

r? @KodrAus

[mjkillough]

Er, whoops. This doesn't actually do what I intended it to do, as it does parse the format string each time a log message is formatted.

I think I meant to drain the StrftimeItems iterator into a Vec when building the logger, then pass a copy of that Vec to each Formatter. It would mean an extra allocation when each thread first logs, but shouldn't involve any allocations for subsequent log messages.

[KodrAus]

Thanks @mjkillough!

I think we'll want to think about the question of supporting multiple formats eventually and it might look a lot like this, but I'm a bit nervous about leaking chrono as a dependency in this way on the outset.

We can get the default more compact RFC3339 format by manually constructing a slice of items, something like:

use chrono::format::Item;

const ITEMS: &'static [Item<'static>] = {
    use chrono::format::Item::*;
    use chrono::format::Numeric::*;
    use chrono::format::Fixed::*;
    use chrono::format::Pad::*;

    &[
        Numeric(Year, Zero),
        Literal("-"),
        Numeric(Month, Zero),
        Literal("-"),
        Numeric(Day, Zero),
        Literal("T"),
        Numeric(Hour, Zero),
        Literal(":"),
        Numeric(Minute, Zero),
        Literal(":"),
        Numeric(Second, Zero),
        Fixed(TimezoneOffsetZ),
    ]
}

It's not exactly pretty, but it's simple and means we can keep all the formatting localised to the fmt module for now, and think about an API for configuring the default format without having to redefine it later. Things like colours might also factor into that.

What do you think?

Thanks again for tackling this!

[KodrAus]

It turns out the gymnastics I went through in the last PR for making a custom iterator were also not necessary. We can just use ITEMS.iter().cloned() when ITEMS: &'static [Item<'static>] :)

[mjkillough]

Thanks a lot for the quick response!

I'm a bit nervous about leaking chrono as a dependency in this way on the outset.

Yeah, that's fair. I sat down to do this because I was curious about how difficult it would be, but I agree it might be worth deferring this type of customization until later.

I had hoped we could expose it as "strftime/strptime style formatting" and hide the fact that chrono is doing it. However, it seems there are a few quirks in how chrono implements this, which may mean we're really leaking it as a dependency anyway.

Things like colours might also factor into that.

That's a great point - I hadn't considered that.

We can get the default more compact RFC3339 format by manually constructing a slice of items, something like:

Yeah, I think that is best for now. It's certainly concise as a slice, and it does allow us to use TimezoneOffsetZ (which didn't seem possible with StrftimeItems).

I'll close this. :)