RelativeDateTime AddAssign/SubAssign are identical: "Seek 10 mins forward" behaves like backward and repeated seeks don't accumulate

[krishna3554]

Summary

RelativeDateTime's AddAssign<TimeDelta> and SubAssign<TimeDelta> implementations are identical - both overwrite the stored offset with the right-hand side instead of accumulating it, so dt += x and dt -= x do exactly the same thing. This breaks chdig's time-frame seeking: the T ("Seek 10 mins backward") and t ("Seek 10 mins forward") key bindings behave identically (both move the window backward), and repeated presses never compound beyond a single interval.

Static-analysis finding against current master; not executed here.

Location

• File: src/common/relative_date_time.rs, lines ~165-175:

impl AddAssign<TimeDelta> for RelativeDateTime {
    fn add_assign(&mut self, rhs: TimeDelta) {
        self.offset = Some(rhs);   // overwrites; no accumulation
    }
}

impl SubAssign<TimeDelta> for RelativeDateTime {
    fn sub_assign(&mut self, rhs: TimeDelta) {
        self.offset = Some(rhs);   // identical to add_assign
    }
}

• Consumers:
  • src/interpreter/context.rs, shift_time_interval() (~lines 357-377): *new_start -= Duration::try_minutes(minutes)... / *new_end += ...
  • src/tui/navigation.rs lines 541-542: global actions "Seek 10 mins backward" (T) and "Seek 10 mins forward" (t) call seek_time_frame(is_sub) -> context.shift_time_interval(...).

Problem

Three distinct defects in one place:

1. Direction is lost. Both operators store Some(rhs). Since the field's documented invariant is that the offset is always subtracted (comment on line ~45, enforced by impl From<RelativeDateTime> for DateTime<Local> at ~lines 148-151: date_time -= offset), even the += path moves the datetime backward. "Seek forward" (t) can never move the window forward.
2. No accumulation. Assigning replaces any previous offset. Pressing T three times shifts by 10 minutes total, not 30 - each press discards the prior offset.
3. is_sub distinction is erased in shift_time_interval(): the two branches produce byte-for-byte identical state transitions.

Additionally, when both date_time and offset are set, From<...> subtracts the offset from an absolute timestamp - but nothing in the operators prevents nonsensical combinations like replacing a previously absolute-only value's semantics.

Trigger / Reproduction

Based on static analysis; not run here:

• Open any view with a time frame (e.g. query/log views), press t ("Seek 10 mins forward"): the window start/end move backward by 10 minutes, same as T.
• Press T repeatedly: window moves 10 minutes once; further presses are no-ops with respect to accumulated offset.

Equivalent unit-level repro implied by the code:

let mut dt = RelativeDateTime::new(None);
dt -= TimeDelta::try_minutes(10).unwrap();
let after_sub = dt.offset;
dt += TimeDelta::try_minutes(10).unwrap();
// dt.offset == after_sub == Some(10min); "+=" indistinguishable from "-="

Expected Behavior

• -= n should extend/subtract the offset by n (e.g. accumulate: self.offset = match self.offset { Some(o) => Some(o + rhs), None => Some(rhs) }).
• += n should move in the opposite direction - which likely needs a signed representation or applying the delta to date_time directly, since a single unsigned "always subtracted" offset cannot express both directions.

Actual Behavior

Both operators write the raw RHS into self.offset; direction information is destroyed.

Impact

The atop-inspired time navigation advertised in the UI help ("Seek 10 mins backward"/"Seek 10 mins forward") is functionally broken: half of its functionality (forward seek) does not exist, and compounding seeks silently saturate after the first press. Users inspecting recent logs cannot step the window forward to narrow the range.

Suggested Direction

Make the operators accumulate relative to their semantic (self.offset += rhs for AddAssign, and either keep a separate sign or apply subtraction consistently), or replace the pair with an explicit API like shift(delta: TimeDelta) used by shift_time_interval(), storing date_time-anchored values as computed datetimes so both directions compose. Also consider making to_editable_string() round-trip offsets (currently the (Some(dt), Some(offset)) case drops the offset).

Evidence

• src/common/relative_date_time.rs: both impl blocks assign self.offset = Some(rhs).
• src/interpreter/context.rs lines 362-372: +=/-= branches intended to differ.
• src/tui/navigation.rs lines 541-542: user-facing key bindings rely on that difference.
• Field comment line ~45 (// Always subtracted) plus the From conversion showing offsets only ever push timestamps backward.

[azat]

Thanks for the report, will be fixed in #297