From 367ab6ca452f0cea27b56e60aa22ffd68d767fc7 Mon Sep 17 00:00:00 2001 From: Aaron Allen Date: Sat, 29 Aug 2026 00:38:34 -0500 Subject: [PATCH] fix(time): resolve both ends of a clock range against each other `parse_range` sent each end through `chronify` on its own. A clock-only time never resolves into the future, so the two ends could land on different days, the start came out later than the end, and the swap turned a short window into the span between those days. It broke the query exactly when someone would type it: at 2pm, `--from "9am to 5pm"` returned yesterday evening through this morning. A range over midnight failed twice over, reaching back a day it was not asked for and dropping the entries that really did fall inside it. Each one exited zero and printed output that read as if it had worked. Resolve the start as before, then take the end as the first occurrence of its clock time at or after it. Skip the end-of-day extension on that path, since it exists for a date with no time and would stretch `9pm to midnight` by a day. Add `parser::clock_time` to tell a bare clock time from a timestamp that already names a day. Ranges written with dates are untouched. Closes #439 --- crates/time/src/parser.rs | 9 +++ crates/time/src/range.rs | 79 ++++++++++++++++++- .../done/when_from_flag_is_provided.rs | 21 +++-- .../finish/when_from_flag_is_provided.rs | 19 ++--- .../commands/grep/when_from_flag.rs | 13 ++- .../now/when_from_flag_is_provided.rs | 16 ++-- .../integration/commands/on/when_from_flag.rs | 8 +- 7 files changed, 122 insertions(+), 43 deletions(-) diff --git a/crates/time/src/parser.rs b/crates/time/src/parser.rs index 01bf4e5..57bd64d 100644 --- a/crates/time/src/parser.rs +++ b/crates/time/src/parser.rs @@ -63,6 +63,15 @@ pub fn chronify(input: &str) -> Result> { } /// Apply a `NaiveTime` to a date, returning a `DateTime` or `None` for DST gaps. +/// Parse a clock-only expression into a `NaiveTime`. +/// +/// Recognizes `noon`, `midnight`, `3pm`, `3:30pm` and `15:00`, and returns +/// `None` for anything carrying a date. That lets a caller tell a bare clock +/// time, whose day has to be inferred, from a timestamp that already names one. +pub fn clock_time(input: &str) -> Option { + resolve_time_expression(&input.trim().to_lowercase()) +} + fn apply_time_to_date(dt: DateTime, time: NaiveTime) -> Option> { Local.from_local_datetime(&dt.date_naive().and_time(time)).earliest() } diff --git a/crates/time/src/range.rs b/crates/time/src/range.rs index f35b3c6..ac62f36 100644 --- a/crates/time/src/range.rs +++ b/crates/time/src/range.rs @@ -4,7 +4,7 @@ use chrono::{DateTime, Duration, Local, NaiveTime, TimeZone}; use doing_error::{Error, Result}; use regex::Regex; -use crate::parser::chronify; +use crate::parser::{chronify, clock_time}; pub static RANGE_SEPARATOR_RE: LazyLock = LazyLock::new(|| Regex::new(r"(?i)\s+(?:to|through|thru|until|til)\s+|\s+--+\s+|\s+-\s+").unwrap()); @@ -35,10 +35,20 @@ pub fn parse_range(input: &str) -> Result<(DateTime, DateTime)> { let parts: Vec<&str> = RANGE_SEPARATOR_RE.splitn(input, 2).collect(); if parts.len() == 2 { - let a = chronify(parts[0])?; - let b = chronify(parts[1])?; + let start = chronify(parts[0])?; + + // A clock-only end names a time but not a day, so resolve it against the + // start rather than against now. Resolved on its own it can land on the + // other side of now from the start, which inverts the range and silently + // widens it to the span between the two days. + if let Some(time) = clock_time(parts[1]) { + let end = first_occurrence_from(start, time)?; + return Ok((start, end)); + } + + let end = chronify(parts[1])?; // Normalize reversed ranges before applying end-of-day extension - let (start, end) = if a > b { (b, a) } else { (a, b) }; + let (start, end) = if start > end { (end, start) } else { (start, end) }; // When end is at midnight (date-only expression), extend to end-of-day to make inclusive let end = if end.time() == NaiveTime::MIN { end + Duration::days(1) @@ -60,6 +70,27 @@ pub fn parse_range(input: &str) -> Result<(DateTime, DateTime)> { Ok((start, end)) } +/// The first occurrence of a wall-clock time at or after `start`. +/// +/// Rolls on to the next day when the time has already passed on the start's +/// day, so a range like `11pm to 1am` spans midnight instead of doubling back. +fn first_occurrence_from(start: DateTime, time: NaiveTime) -> Result> { + let on = |date: chrono::NaiveDate| { + Local + .from_local_datetime(&date.and_time(time)) + .earliest() + .ok_or_else(|| Error::InvalidTimeExpression(format!("ambiguous local time for: {time}"))) + }; + + let same_day = on(start.date_naive())?; + + if same_day >= start { + Ok(same_day) + } else { + on((start + Duration::days(1)).date_naive()) + } +} + #[cfg(test)] mod test { use super::*; @@ -130,6 +161,46 @@ mod test { assert_eq!(end.date_naive(), NaiveDate::from_ymd_opt(2024, 2, 1).unwrap()); } + #[test] + fn it_keeps_a_clock_range_on_one_day_whatever_the_time() { + let (start, end) = parse_range("8am to 12pm").unwrap(); + + assert_eq!(start.date_naive(), end.date_naive()); + assert_eq!(start.time(), NaiveTime::from_hms_opt(8, 0, 0).unwrap()); + assert_eq!(end.time(), NaiveTime::from_hms_opt(12, 0, 0).unwrap()); + assert_eq!(end - start, Duration::hours(4)); + } + + #[test] + fn it_does_not_extend_a_clock_range_ending_at_midnight() { + let (start, end) = parse_range("9pm to midnight").unwrap(); + + assert_eq!(end - start, Duration::hours(3)); + } + + #[test] + fn it_spans_midnight_for_a_clock_range_that_crosses_it() { + let (start, end) = parse_range("11pm to 1am").unwrap(); + + assert_eq!(start.time(), NaiveTime::from_hms_opt(23, 0, 0).unwrap()); + assert_eq!(end.time(), NaiveTime::from_hms_opt(1, 0, 0).unwrap()); + assert_eq!(end - start, Duration::hours(2)); + assert_eq!(end.date_naive(), start.date_naive() + Duration::days(1)); + } + + #[test] + fn it_never_inverts_a_clock_range() { + // Every hour pair, so one end has passed today and the other has not + // whatever time the suite runs at. + for hour in 0..23 { + let input = format!("{hour}:00 to {}:00", hour + 1); + let (start, end) = parse_range(&input).unwrap(); + + assert!(start < end, "{input} produced {start} to {end}"); + assert_eq!(end - start, Duration::hours(1), "{input} spanned the wrong length"); + } + } + #[test] fn it_rejects_empty_input() { let err = parse_range("").unwrap_err(); diff --git a/tests/integration/commands/done/when_from_flag_is_provided.rs b/tests/integration/commands/done/when_from_flag_is_provided.rs index dad40a3..84a4766 100644 --- a/tests/integration/commands/done/when_from_flag_is_provided.rs +++ b/tests/integration/commands/done/when_from_flag_is_provided.rs @@ -1,20 +1,19 @@ +use chrono::Duration; + use crate::support::helpers::{ - DoingCmd, assert_times_within_tolerance, extract_done_timestamp, extract_entry_timestamp, + DoingCmd, assert_times_within_tolerance, extract_done_timestamp, extract_entry_timestamp, fmt_naive, + most_recent_clock, }; #[test] fn it_sets_start_and_done_from_range() { let doing = DoingCmd::new(); - let expected_start = "2026-03-22 13:00"; - let expected_done = "2026-03-22 15:00"; + let start = most_recent_clock(13, 0); + let expected_start = fmt_naive(start); + let expected_done = fmt_naive(start + Duration::hours(2)); doing - .run([ - "done", - "--from", - "2026-03-22 13:00 to 2026-03-22 15:00", - "From range entry", - ]) + .run(["done", "--from", "1pm to 3pm", "From range entry"]) .assert() .success(); @@ -22,6 +21,6 @@ fn it_sets_start_and_done_from_range() { let start_time = extract_entry_timestamp(&contents); let done_time = extract_done_timestamp(&contents); - assert_times_within_tolerance(&start_time, expected_start, 1, "start should be 1pm"); - assert_times_within_tolerance(&done_time, expected_done, 1, "@done should be 3pm"); + assert_times_within_tolerance(&start_time, &expected_start, 1, "start should be 1pm"); + assert_times_within_tolerance(&done_time, &expected_done, 1, "@done should be 3pm"); } diff --git a/tests/integration/commands/finish/when_from_flag_is_provided.rs b/tests/integration/commands/finish/when_from_flag_is_provided.rs index 6ffe769..cfb7152 100644 --- a/tests/integration/commands/finish/when_from_flag_is_provided.rs +++ b/tests/integration/commands/finish/when_from_flag_is_provided.rs @@ -1,14 +1,18 @@ use std::fs; +use chrono::Duration; + use crate::support::helpers::{ - DoingCmd, assert_times_within_tolerance, extract_done_timestamp, extract_entry_timestamp, + DoingCmd, assert_times_within_tolerance, extract_done_timestamp, extract_entry_timestamp, fmt_naive, + most_recent_clock, }; #[test] fn it_sets_start_and_done_from_range() { let doing = DoingCmd::new(); - let expected_start = "2026-03-22 13:00"; - let expected_done = "2026-03-22 15:00"; + let start = most_recent_clock(13, 0); + let expected_start = fmt_naive(start); + let expected_done = fmt_naive(start + Duration::hours(2)); fs::write( doing.doing_file_path(), @@ -16,15 +20,12 @@ fn it_sets_start_and_done_from_range() { ) .expect("failed to write doing file"); - doing - .run(["finish", "--from", "2026-03-22 13:00 to 2026-03-22 15:00"]) - .assert() - .success(); + doing.run(["finish", "--from", "1pm to 3pm"]).assert().success(); let contents = doing.read_doing_file(); let start_time = extract_entry_timestamp(&contents); let done_time = extract_done_timestamp(&contents); - assert_times_within_tolerance(&start_time, expected_start, 1, "start should be 1pm"); - assert_times_within_tolerance(&done_time, expected_done, 1, "@done should be 3pm"); + assert_times_within_tolerance(&start_time, &expected_start, 1, "start should be 1pm"); + assert_times_within_tolerance(&done_time, &expected_done, 1, "@done should be 3pm"); } diff --git a/tests/integration/commands/grep/when_from_flag.rs b/tests/integration/commands/grep/when_from_flag.rs index cf8a958..0b4465d 100644 --- a/tests/integration/commands/grep/when_from_flag.rs +++ b/tests/integration/commands/grep/when_from_flag.rs @@ -1,10 +1,12 @@ -use crate::support::helpers::DoingCmd; +use crate::support::helpers::{DoingCmd, most_recent_clock}; #[test] fn it_filters_by_date_range() { let doing = DoingCmd::new(); - let today = chrono::Local::now().format("%Y-%m-%d").to_string(); + // "8am to 12pm" resolves onto whichever day 8am last fell on, so the + // entries have to be written on that same day. + let today = most_recent_clock(8, 0).format("%Y-%m-%d").to_string(); doing .run(["now", "--back", &format!("{today} 09:00"), "Morning search entry"]) @@ -16,12 +18,7 @@ fn it_filters_by_date_range() { .success(); let output = doing - .run([ - "grep", - "search entry", - "--from", - &format!("{today} 08:00 to {today} 12:00"), - ]) + .run(["grep", "search entry", "--from", "8am to 12pm"]) .output() .expect("failed to run"); diff --git a/tests/integration/commands/now/when_from_flag_is_provided.rs b/tests/integration/commands/now/when_from_flag_is_provided.rs index ba126c6..39b3b54 100644 --- a/tests/integration/commands/now/when_from_flag_is_provided.rs +++ b/tests/integration/commands/now/when_from_flag_is_provided.rs @@ -1,3 +1,5 @@ +use chrono::Duration; + use crate::support::helpers::{ DoingCmd, assert_times_within_tolerance, extract_done_timestamp, extract_entry_timestamp, fmt_naive, most_recent_clock, @@ -26,14 +28,12 @@ fn it_sets_done_to_end_of_day_when_single_time_given() { #[test] fn it_sets_start_and_done_from_range() { let doing = DoingCmd::new(); + let start = most_recent_clock(13, 0); + let expected_start = fmt_naive(start); + let expected_done = fmt_naive(start + Duration::hours(2)); doing - .run([ - "now", - "--from", - "2026-03-22 13:00 to 2026-03-22 15:00", - "From range entry", - ]) + .run(["now", "--from", "1pm to 3pm", "From range entry"]) .assert() .success(); @@ -41,6 +41,6 @@ fn it_sets_start_and_done_from_range() { let start_time = extract_entry_timestamp(&contents); let done_time = extract_done_timestamp(&contents); - assert_times_within_tolerance(&start_time, "2026-03-22 13:00", 1, "start should be 1pm"); - assert_times_within_tolerance(&done_time, "2026-03-22 15:00", 1, "done should be 3pm"); + assert_times_within_tolerance(&start_time, &expected_start, 1, "start should be 1pm"); + assert_times_within_tolerance(&done_time, &expected_done, 1, "done should be 3pm"); } diff --git a/tests/integration/commands/on/when_from_flag.rs b/tests/integration/commands/on/when_from_flag.rs index e9576d8..ab8c6ad 100644 --- a/tests/integration/commands/on/when_from_flag.rs +++ b/tests/integration/commands/on/when_from_flag.rs @@ -1,10 +1,12 @@ -use crate::support::helpers::DoingCmd; +use crate::support::helpers::{DoingCmd, most_recent_clock}; #[test] fn it_limits_to_time_range() { let doing = DoingCmd::new(); - let today = chrono::Local::now().format("%Y-%m-%d").to_string(); + // "8am to 12pm" resolves onto whichever day 8am last fell on, so the + // entries have to be written on that same day. + let today = most_recent_clock(8, 0).format("%Y-%m-%d").to_string(); // Create entries at specific times today doing @@ -17,7 +19,7 @@ fn it_limits_to_time_range() { .success(); let output = doing - .run(["on", &today, "--from", &format!("{today} 08:00 to {today} 12:00")]) + .run(["on", &today, "--from", "8am to 12pm"]) .output() .expect("failed to run"); -- knotmirror 0.0.1