4 ms·

Looks like it should be supported: https://docs.rs/jiff/latest/jiff/struct.Span.html#impl-Add%3CSpan%3E-for-%26Zoned https://docs.rs/jiff/latest/jiff/struct.Spa

Looks like it should be supported: https://docs.rs/jiff/latest/jiff/struct.Span.html#impl-Add%3CSpan%3E-for-%26Zoned https://docs.rs/jiff/latest/jiff/struct.Span.html#impl-Add%3...

wging 2y ago
That isn't an implementation of addition between Spans and other Spans. It looks like there isn't one in the library right now. `impl<'a> Add<Span> for &'a Zoned` means a borrow of Zoned is on the left hand side, and a Span on the right. So it says that if z is a Zoned (not a Span) and s is a Span, you can do `&z + s` to add a span to a Zoned. There are a bunch of implementations there, DateTime + Span, Date + Span, Time + Span, Offset + Span. All with Span on the right, but none for Span + Span (nor Span + &Span, or &Span + &Span, ...).
- burntsushi 2y ago
  This is correct. You can't do a `span1 + span2`. You'd have to use `span1.checked_add(span2)`. The main problem I had with overloading `+` for span addition is that, in order to add two spans with non-uniform units (like years and months), you need a relative datetime. So `+` would effectively have to panic if you did `1.year() + 2.months()`, which seems like a horrific footgun. It would be plausible to make `+` for spans do _only_ component wise addition, but this would be an extremely subtle distinction between `+` and `Span::checked_add`. To the point where sometimes `+` and `checked_add` would agree on the results and sometimes they wouldn't. I think that would also be bad. So I started conservative for the time being: no `+` for adding spans together.
  - stouset 2y ago
    What's the issue with having a span represent "one year and two months"? Both involve a variable number of days (365-366 and 28-31), but I would hope you could store the components separately so it can be unambiguously applied once given a specific moment in time. I'm thinking something along the lines of how ActiveSupport::Duration works: >> 4.years + 5.months + 3.weeks + 2.days + 1.hour + 4.minutes + 10.seconds => 4 years, 5 months, 3 weeks, 2 days, 1 hour, 4 minutes, and 10 seconds Of course the downside being that it would need to be sized large enough to contain each component, even though they may be rarely used.
    - burntsushi 2y ago
      The components are stored separately. I think what you are advocating for is component wise addition, which I mentioned in my previous comment. It can totally "work," but as I said, it will produce different results than `span1.checked_add(span2)`. For example: use jiff::{ToSpan, Unit, Zoned}; fn main() -> anyhow::Result<()> { let span1 = 1.year().months(3); let span2 = 11.months(); let now = Zoned::now().round(Unit::Minute)?; let added = span1.checked_add((span2, &now))?; println!("{added}"); Ok(()) } Has this output: $ cargo -q r P2y2m Notice how the months overflow automatically into years. Days will do the same into months. You need a reference point to do this correctly. For example, just `span1.checked_add(span2)?` would produce an error. In contrast, component wise addition would lead to a span of `1 year 14 months`. Which is a valid `Span`. Jiff is fine with it. But it's different than what `checked_add` does. Having both operations seems too subtle. Also, I don't really think using the `+` operator just to construct spans is that much of a win over what Jiff already has. So that needs to be taken into account as well.
      - Tobu 2y ago
        The Into<SpanArithmetic> argument feels a little too magical. Since you are not constrained by a trait, I would prefer something like Span::add_with_reference(&self, other: &Span, reference: impl Into<SpanRelativeTo>)

Looks like it should be supported: https://docs.rs/jiff/latest/jiff/struct.Span.html#impl-Add%3CSpan%3E-for-%26Zoned https://docs.rs/jiff/latest/jiff/struct.Span.html#impl-Add%3...

wging 2y ago

That isn't an implementation of addition between Spans and other Spans. It looks like there isn't one in the library right now. `impl<'a> Add<Span> for &'a Zoned` means a borrow of Zoned is on the left hand side, and a Span on the right. So it says that if z is a Zoned (not a Span) and s is a Span, you can do `&z + s` to add a span to a Zoned. There are a bunch of implementations there, DateTime + Span, Date + Span, Time + Span, Offset + Span. All with Span on the right, but none for Span + Span (nor Span + &Span, or &Span + &Span, ...).

burntsushi 2y ago
This is correct. You can't do a `span1 + span2`. You'd have to use `span1.checked_add(span2)`. The main problem I had with overloading `+` for span addition is that, in order to add two spans with non-uniform units (like years and months), you need a relative datetime. So `+` would effectively have to panic if you did `1.year() + 2.months()`, which seems like a horrific footgun. It would be plausible to make `+` for spans do _only_ component wise addition, but this would be an extremely subtle distinction between `+` and `Span::checked_add`. To the point where sometimes `+` and `checked_add` would agree on the results and sometimes they wouldn't. I think that would also be bad. So I started conservative for the time being: no `+` for adding spans together.
- stouset 2y ago
  What's the issue with having a span represent "one year and two months"? Both involve a variable number of days (365-366 and 28-31), but I would hope you could store the components separately so it can be unambiguously applied once given a specific moment in time. I'm thinking something along the lines of how ActiveSupport::Duration works: >> 4.years + 5.months + 3.weeks + 2.days + 1.hour + 4.minutes + 10.seconds => 4 years, 5 months, 3 weeks, 2 days, 1 hour, 4 minutes, and 10 seconds Of course the downside being that it would need to be sized large enough to contain each component, even though they may be rarely used.
  - burntsushi 2y ago
    The components are stored separately. I think what you are advocating for is component wise addition, which I mentioned in my previous comment. It can totally "work," but as I said, it will produce different results than `span1.checked_add(span2)`. For example: use jiff::{ToSpan, Unit, Zoned}; fn main() -> anyhow::Result<()> { let span1 = 1.year().months(3); let span2 = 11.months(); let now = Zoned::now().round(Unit::Minute)?; let added = span1.checked_add((span2, &now))?; println!("{added}"); Ok(()) } Has this output: $ cargo -q r P2y2m Notice how the months overflow automatically into years. Days will do the same into months. You need a reference point to do this correctly. For example, just `span1.checked_add(span2)?` would produce an error. In contrast, component wise addition would lead to a span of `1 year 14 months`. Which is a valid `Span`. Jiff is fine with it. But it's different than what `checked_add` does. Having both operations seems too subtle. Also, I don't really think using the `+` operator just to construct spans is that much of a win over what Jiff already has. So that needs to be taken into account as well.
    - Tobu 2y ago
      The Into<SpanArithmetic> argument feels a little too magical. Since you are not constrained by a trait, I would prefer something like Span::add_with_reference(&self, other: &Span, reference: impl Into<SpanRelativeTo>)