diff --git a/linux/changelog.d/3890.fixed.md b/linux/changelog.d/3890.fixed.md new file mode 100644 index 0000000000..e8d1028c92 --- /dev/null +++ b/linux/changelog.d/3890.fixed.md @@ -0,0 +1 @@ +Picking a day in Row Details reset a timestamp's time to midnight when its year or spelling was not the plain form. diff --git a/linux/crates/app/src/ui/data_grid/row_detail/calendar.rs b/linux/crates/app/src/ui/data_grid/row_detail/calendar.rs index 2298906675..f5e554e888 100644 --- a/linux/crates/app/src/ui/data_grid/row_detail/calendar.rs +++ b/linux/crates/app/src/ui/data_grid/row_detail/calendar.rs @@ -3,30 +3,46 @@ use gtk4::prelude::*; use gtk4::subclass::prelude::*; use gtk4::{self as gtk, glib}; use tablepro_core::column::{ColumnKind, TemporalZone}; +use tablepro_core::edit::{parse_timestamp_with_offset, temporal_parts}; use super::RowDetailPane; use super::value_editor::Commit; use crate::application::component_matches; -/// The day a value's text starts with, when it starts with one the -/// calendar can show. +// GtkCalendar shows the years 1 to 9999 only, so it opens on today for any other. pub fn day_of(text: &str) -> Option { - let head = text.get(..10)?; - NaiveDate::parse_from_str(head, "%Y-%m-%d") - .ok() - .filter(|day| (1..=9999).contains(&day.year())) + temporal_parts(text).date.filter(|day| (1..=9999).contains(&day.year())) } -/// The value's text with `day` picked: a date, or a timestamp in `zone`, -/// which keeps the time it had. A new zoned timestamp starts the day at -/// `offset`, as one without an offset is refused. +// Only the day changes, and the era with it, since a picked day is AD. Text with no day takes one +// before its clock, or becomes a new value; a zoned timestamp lacking an offset takes `offset`. pub fn with_day(text: &str, day: NaiveDate, zone: Option<&TemporalZone>, offset: FixedOffset) -> String { let date = day.format("%Y-%m-%d").to_string(); - match (zone, day_of(text)) { - (None, _) => date, - (Some(_), Some(_)) => format!("{date}{}", text.get(10..).unwrap_or_default()), - (Some(TemporalZone::Offset), None) => format!("{date} 00:00:00{offset}"), - (Some(_), None) => format!("{date} 00:00:00"), + let parts = temporal_parts(text); + let mut text = text.to_owned(); + if let Some(era) = parts.era { + text.replace_range(era, ""); + } + match (parts.day, parts.clock, zone) { + (Some(old), _, _) => { + text.replace_range(old, &date); + text + } + (None, Some(clock), Some(zone)) => { + text.insert_str(clock.start, &format!("{date} ")); + let (body, tail) = text.split_at(text.trim_end().len()); + let zoned = format!("{body}{offset}{tail}"); + match zone == &TemporalZone::Offset + && parse_timestamp_with_offset(&text).is_err() + && parse_timestamp_with_offset(&zoned).is_ok() + { + true => zoned, + false => text, + } + } + (None, _, None) => date, + (None, None, Some(TemporalZone::Offset)) => format!("{date} 00:00:00{offset}"), + (None, None, Some(_)) => format!("{date} 00:00:00"), } } @@ -36,9 +52,12 @@ pub fn day_start_offset(zone: &Tz, day: NaiveDate) -> FixedOffset // A wall-clock time a clock change skips takes the offset before the change. pub fn offset_at(zone: &Tz, local: NaiveDateTime) -> FixedOffset { - [local, local - TimeDelta::hours(1)] - .iter() - .find_map(|local| zone.from_local_datetime(local).earliest()) + zone.from_local_datetime(&local) + .earliest() + .or_else(|| { + let before = local.checked_sub_signed(TimeDelta::hours(1))?; + zone.from_local_datetime(&before).earliest() + }) .map_or_else(|| zone.offset_from_utc_datetime(&local).fix(), |at| at.offset().fix()) } @@ -145,7 +164,12 @@ impl RowDetailPane { ColumnKind::Timestamp { zone, .. } => Some(zone.clone()), _ => None, }); - let text = with_day(&self.typed_text(), day, zone.as_ref(), day_start_offset(&Local, day)); + let typed = self.typed_text(); + let offset = temporal_parts(&typed).time.map_or_else( + || day_start_offset(&Local, day), + |time| offset_at(&Local, day.and_time(time)), + ); + let text = with_day(&typed, day, zone.as_ref(), offset); imp.calendar_popover.popdown(); imp.value_buffer.set_text(&text); self.commit(Commit::Keep); diff --git a/linux/crates/app/src/ui/data_grid/row_detail/clock.rs b/linux/crates/app/src/ui/data_grid/row_detail/clock.rs index 02e36ac66f..b3c1b8b507 100644 --- a/linux/crates/app/src/ui/data_grid/row_detail/clock.rs +++ b/linux/crates/app/src/ui/data_grid/row_detail/clock.rs @@ -1,10 +1,9 @@ -use std::ops::Range; - use chrono::{Local, NaiveDate, NaiveTime, TimeZone, Timelike}; use gtk4::prelude::*; use gtk4::subclass::prelude::*; use gtk4::{self as gtk, glib}; use tablepro_core::column::{ColumnKind, TemporalZone, TimeRange}; +use tablepro_core::edit::temporal_parts; use super::RowDetailPane; use super::calendar::offset_at; @@ -37,72 +36,26 @@ impl ClockField { } } -// By the text's own form, as chrono reads it for the commit: SQLite keeps any date or time text in a -// DATE, TIME or DATETIME column, and chrono takes whitespace before a number and one digit for two. -fn clock_span(text: &str) -> Option> { - let after_day = day_len(text).map_or(0, |day| day + usize::from(text.as_bytes().get(day) == Some(&b'T'))); - let start = after_day + spaces(text.get(after_day..)?); - let minute = number(text, literal(text, number(text, start, 2)?, b':')?, 2)?; - let end = literal(text, minute, b':').and_then(|second| number(text, second, 2)); - Some(start..end.unwrap_or(minute)) -} - -// A year may run past 9999, as PostgreSQL writes one, and carry a sign. -fn day_len(text: &str) -> Option { - let lead = spaces(text); - let year = lead + usize::from(matches!(text.as_bytes().get(lead), Some(b'-' | b'+'))); - let month = number(text, literal(text, number(text, year, usize::MAX)?, b'-')?, 2)?; - number(text, literal(text, month, b'-')?, 2) -} - -// The end of the number at `at`, past any whitespace before it. -fn number(text: &str, at: usize, most: usize) -> Option { - let start = at + spaces(text.get(at..)?); - let run = text - .get(start..)? - .bytes() - .take(most) - .take_while(u8::is_ascii_digit) - .count(); - (run > 0).then_some(start + run) -} - -fn literal(text: &str, at: usize, byte: u8) -> Option { - (text.as_bytes().get(at) == Some(&byte)).then_some(at + 1) -} - -fn spaces(text: &str) -> usize { - text.len() - text.trim_start().len() -} - pub fn time_of(text: &str) -> Option { - let mut parts = text - .get(clock_span(text)?)? - .split(':') - .map(|part| part.trim().parse::()); - let (hour, minute) = (parts.next()?.ok()?, parts.next()?.ok()?); - let second = parts.next().transpose().ok()?.unwrap_or(0); - NaiveTime::from_hms_opt(hour, minute, second) + temporal_parts(text).time } // Only the clock changes; text with no day or clock, such as NULL or infinity, becomes a new value. pub fn with_time(text: &str, time: NaiveTime, field: ClockField, today: NaiveDate, zone: &Tz) -> String { let mut clock = time.format("%H:%M:%S").to_string(); + let parts = temporal_parts(text); let mut text = text.to_owned(); - if let Some(span) = clock_span(&text) { + if let Some(span) = parts.clock { text.replace_range(span, &clock); return text; } - let day_end = day_len(&text); - let day = day_end - .and_then(|end| NaiveDate::parse_from_str(text.get(..end)?.trim_start(), "%Y-%m-%d").ok()) - .unwrap_or(today); if field.with_offset { + let day = parts.date.unwrap_or(today); clock.push_str(&offset_at(zone, day.and_time(time)).to_string()); } - match day_end { - Some(end) => { - text.insert_str(end, &format!(" {clock}")); + match parts.day { + Some(day) => { + text.insert_str(day.end, &format!(" {clock}")); text } None if field.dated => format!("{} {clock}", today.format("%Y-%m-%d")), diff --git a/linux/crates/app/src/ui/data_grid/row_detail/tests.rs b/linux/crates/app/src/ui/data_grid/row_detail/tests.rs index 135979f0ca..1728bd4d46 100644 --- a/linux/crates/app/src/ui/data_grid/row_detail/tests.rs +++ b/linux/crates/app/src/ui/data_grid/row_detail/tests.rs @@ -589,6 +589,50 @@ fn a_picked_day_keeps_a_timestamp_time() { assert_eq!(day_of("2024-02-2é"), None, "a cut inside a character"); } +#[test] +fn a_picked_day_keeps_the_time_of_any_timestamp_the_commit_reads() { + let day = NaiveDate::from_ymd_opt(2024, 3, 1).expect("a day"); + let plus_two = chrono::FixedOffset::east_opt(2 * 3600).expect("an offset"); + let pick = |text: &str| with_day(text, day, Some(&TemporalZone::Naive), plus_two); + + assert_eq!( + pick("12024-01-05 10:00:00"), + "2024-03-01 10:00:00", + "PostgreSQL writes a far year in full" + ); + assert_eq!( + pick("2024-1-5 10:00"), + "2024-03-01 10:00", + "SQLite keeps the text as typed" + ); + assert_eq!(pick(" 2024-01-05 10:00"), " 2024-03-01 10:00"); + assert_eq!( + pick("0044-03-15 12:00:00 BC"), + "2024-03-01 12:00:00", + "an AD day kept the BC era" + ); + assert_eq!(pick("10:00"), "2024-03-01 10:00"); + let zoned = |text: &str| with_day(text, day, Some(&TemporalZone::Offset), plus_two); + assert_eq!( + zoned("10:00"), + "2024-03-01 10:00+02:00", + "a zoned timestamp took no offset" + ); + assert_eq!(zoned("10:00+05"), "2024-03-01 10:00+05"); + assert_eq!( + with_day("2024-01-05 10:00", day, None, plus_two), + "2024-03-01 10:00", + "SQLite keeps a timestamp in a DATE column" + ); + assert_eq!(day_of("2024-1-5 10:00"), NaiveDate::from_ymd_opt(2024, 1, 5)); + assert_eq!(day_of(" 2024-01-05"), NaiveDate::from_ymd_opt(2024, 1, 5)); + assert_eq!( + day_of("0044-03-15 12:00:00 BC"), + None, + "the calendar opened on an AD day" + ); +} + /// A zone whose offset moves from `before` to `after` at `change`, in UTC. #[derive(Clone)] struct OneChange { @@ -1033,6 +1077,30 @@ fn a_new_zoned_time_takes_the_offset_of_its_own_moment() { ); } +#[test] +fn a_clock_on_the_first_day_chrono_holds_takes_an_offset() { + let first = NaiveDate::MIN.format("%Y-%m-%d").to_string(); + let today = NaiveDate::from_ymd_opt(2026, 10, 29).expect("a day"); + let plus_two = chrono::FixedOffset::east_opt(2 * 3600).expect("an offset"); + + assert_eq!( + with_time(&first, a_time(0, 30, 0), ZONED, today, &plus_two), + format!("{first} 00:30:00+02:00") + ); +} + +#[test] +fn a_picker_replaces_a_part_out_of_range() { + let day = NaiveDate::from_ymd_opt(2024, 3, 1).expect("a day"); + let plus_two = chrono::FixedOffset::east_opt(2 * 3600).expect("an offset"); + + assert_eq!(set_at_nine("2024-01-01 12:34:99", NAIVE), "2024-01-01 09:05:30"); + assert_eq!( + with_day("2024-02-30 10:00", day, Some(&TemporalZone::Naive), plus_two), + "2024-03-01 10:00" + ); +} + #[test] fn the_clock_reads_the_time_a_value_shows() { assert_eq!(time_of("2024-02-29 23:59:58.125"), Some(a_time(23, 59, 58))); diff --git a/linux/crates/core/src/edit/mod.rs b/linux/crates/core/src/edit/mod.rs index 715989f0f4..bbd1ae4f37 100644 --- a/linux/crates/core/src/edit/mod.rs +++ b/linux/crates/core/src/edit/mod.rs @@ -10,6 +10,7 @@ mod is_unchanged; mod new_draft_cell; mod parse_edit_text; mod parse_literal_text; +mod temporal_parts; pub use cell_input::CellInput; pub use edit_context::EditContext; @@ -22,3 +23,4 @@ pub use parse_edit_text::parse_edit_text; pub use parse_literal_text::{ parse_filter_text, parse_literal_text, parse_time_with_offset, parse_timestamp_with_offset, }; +pub use temporal_parts::{TemporalParts, temporal_parts}; diff --git a/linux/crates/core/src/edit/parse_literal_text.rs b/linux/crates/core/src/edit/parse_literal_text.rs index 3e3ba424ea..9bae8c469f 100644 --- a/linux/crates/core/src/edit/parse_literal_text.rs +++ b/linux/crates/core/src/edit/parse_literal_text.rs @@ -343,7 +343,7 @@ fn parse_timestamp(input: &str) -> Result { /// `+10000` and `-4712`: there is no year 0 between 1 BC and AD 1, so /// 1 BC is chrono's 0. `None` for a ` BC` value with no year to count /// back from. -fn chrono_year(input: &str) -> Option> { +pub(super) fn chrono_year(input: &str) -> Option> { let (dated, before_christ) = match input.strip_suffix(" BC").or_else(|| input.strip_suffix(" bc")) { Some(dated) => (dated, true), None => (input, false), @@ -378,11 +378,7 @@ fn far_temporal( purpose: Purpose, parse: fn(&str) -> Result, ) -> Option> { - let year_digits = input.bytes().take_while(u8::is_ascii_digit).count(); - let (year, rest) = input.split_at(year_digits); - let last_year = u64::try_from(NaiveDate::MAX.year()).ok()?; - let year = year.parse::().ok().filter(|year| *year > last_year)?; - let stand_in = parse(&format!("{}{rest}", 2000 + year % 400)).ok()?; + let stand_in = parse(&far_year_stand_in(input)?).ok()?; if purpose == Purpose::Store && let Err(error) = check_declared_fraction(&stand_in, column.kind.fractional_digits()) { @@ -394,6 +390,15 @@ fn far_temporal( })))) } +// The text with a year past chrono's last moved into its range, keeping the rest as written. +pub(super) fn far_year_stand_in(input: &str) -> Option { + let year_digits = input.bytes().take_while(u8::is_ascii_digit).count(); + let (year, rest) = input.split_at(year_digits); + let last_year = u64::try_from(NaiveDate::MAX.year()).ok()?; + let year = year.parse::().ok().filter(|year| *year > last_year)?; + Some(format!("{}{rest}", 2000 + year % 400)) +} + fn infinite_temporal(input: &str) -> Option> { match input.to_ascii_lowercase().as_str() { "infinity" | "inf" => Some(Temporal::Infinity), diff --git a/linux/crates/core/src/edit/temporal_parts.rs b/linux/crates/core/src/edit/temporal_parts.rs new file mode 100644 index 0000000000..8f0588900f --- /dev/null +++ b/linux/crates/core/src/edit/temporal_parts.rs @@ -0,0 +1,162 @@ +use std::ops::Range; + +use chrono::{NaiveDate, NaiveTime}; + +use super::parse_literal_text::{chrono_year, far_year_stand_in}; + +// Byte ranges in the text, so a picker can replace one part and keep the rest as written. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct TemporalParts { + pub day: Option>, + // None for a day out of range or a year past the last chrono holds. + pub date: Option, + pub clock: Option>, + // None for a clock out of range, PostgreSQL's 24:00:00 among them. + pub time: Option, + pub era: Option>, +} + +// The day, clock and ` BC` a date or time text spells, read as `parse_literal_text` reads them: +// the same year forms, a space or `T` between day and clock, seconds optional. +pub fn temporal_parts(text: &str) -> TemporalParts { + // A 1 for every digit keeps the shape and drops the values, so a part out of range still has + // its place for a picker to replace. + let shape: String = text + .chars() + .map(|char| if char.is_ascii_digit() { '1' } else { char }) + .collect(); + let mut parts = spans(&shape); + let era = parts.era.clone().and_then(|era| text.get(era)).unwrap_or_default(); + parts.date = parts.day.clone().and_then(|day| { + let day = chrono_year(&format!("{}{era}", text.get(day)?.trim_start()))?.into_owned(); + NaiveDate::parse_from_str(&day, "%Y-%m-%d").ok() + }); + parts.time = parts.clock.clone().and_then(|clock| { + let clock = text.get(clock)?; + ["%H:%M:%S", "%H:%M"] + .into_iter() + .find_map(|format| NaiveTime::parse_from_str(clock, format).ok()) + }); + parts +} + +fn spans(text: &str) -> TemporalParts { + let start = text.len() - text.trim_start().len(); + let trimmed = text.trim(); + let written = trimmed + .strip_suffix(" BC") + .or_else(|| trimmed.strip_suffix(" bc")) + .unwrap_or(trimmed); + let Some(read) = chrono_year(trimmed) else { + return TemporalParts::default(); + }; + // `read` differs from the text only in its year and era, so a suffix of it is one of the text. + let end = start + written.len(); + let at = |rest: &str| end - rest.len(); + let mut parts = TemporalParts { + era: (written.len() < trimmed.len()).then_some(end..start + trimmed.len()), + ..TemporalParts::default() + }; + let mut clock_text = read.as_ref(); + if let Some(rest) = day_rest(&read, written) { + let after_t = rest.strip_prefix('T').unwrap_or(rest); + let joined = rest.is_empty() || rest.starts_with(char::is_whitespace) || rest.starts_with('T'); + if joined || parse_clock(after_t).is_some() { + parts.day = Some(start..at(rest)); + clock_text = after_t; + } + } + if let Some((_, rest)) = parse_clock(clock_text) { + parts.clock = Some(at(clock_text.trim_start())..at(rest)); + } + parts +} + +fn day_rest<'a>(read: &'a str, written: &str) -> Option<&'a str> { + if let Ok((_, rest)) = NaiveDate::parse_and_remainder(read, "%Y-%m-%d") { + return Some(rest); + } + let stand_in = far_year_stand_in(written)?; + let (_, rest) = NaiveDate::parse_and_remainder(&stand_in, "%Y-%m-%d").ok()?; + read.get(read.len() - rest.len()..) +} + +fn parse_clock(text: &str) -> Option<(NaiveTime, &str)> { + ["%H:%M:%S", "%H:%M"] + .into_iter() + .find_map(|format| NaiveTime::parse_and_remainder(text, format).ok()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn day(text: &str) -> Option<&str> { + text.get(temporal_parts(text).day?) + } + + fn clock(text: &str) -> Option<&str> { + text.get(temporal_parts(text).clock?) + } + + #[test] + fn the_parts_are_read_as_the_commit_reads_them() { + assert_eq!(day(" 2024-1-5T 9:05:30.25+02 "), Some("2024-1-5")); + assert_eq!(clock(" 2024-1-5T 9:05:30.25+02 "), Some("9:05:30")); + assert_eq!( + clock("2024-01-0512:00"), + Some("12:00"), + "chrono reads no space as a space" + ); + assert_eq!(day("12024-01-05 10:00:00"), Some("12024-01-05")); + assert_eq!(clock("13:45+05:30"), Some("13:45")); + assert_eq!(temporal_parts("13:45").time, NaiveTime::from_hms_opt(13, 45, 0)); + } + + #[test] + fn a_year_chrono_cannot_hold_has_a_day_and_no_date() { + let parts = temporal_parts("5874897-12-31 23:59:59"); + + assert_eq!(parts.day, Some(0..13)); + assert_eq!(parts.date, None); + assert_eq!(parts.clock, Some(14..22)); + } + + #[test] + fn a_bc_value_keeps_its_era_apart() { + let text = "0044-03-15 12:00:00 BC"; + let parts = temporal_parts(text); + + assert_eq!(day(text), Some("0044-03-15")); + assert_eq!(parts.date, NaiveDate::from_ymd_opt(-43, 3, 15)); + assert_eq!(clock(text), Some("12:00:00")); + assert_eq!(parts.era.and_then(|era| text.get(era)), Some(" BC")); + } + + #[test] + fn the_end_of_the_day_is_a_clock_with_no_time() { + let parts = temporal_parts("24:00:00+01"); + + assert_eq!(parts.clock, Some(0..8)); + assert_eq!(parts.time, None); + assert_eq!(temporal_parts("24:30:00").clock, Some(0..8)); + } + + #[test] + fn text_the_commit_refuses_has_no_parts() { + assert_eq!(temporal_parts("2024-02-2é 10:00:00").day, None); + assert_eq!(temporal_parts("infinity"), TemporalParts::default()); + assert_eq!(temporal_parts(""), TemporalParts::default()); + } + + #[test] + fn a_part_out_of_range_has_its_place_and_no_value() { + let text = "2024-02-30 12:34:99"; + let parts = temporal_parts(text); + + assert_eq!(day(text), Some("2024-02-30")); + assert_eq!(parts.date, None); + assert_eq!(clock(text), Some("12:34:99")); + assert_eq!(parts.time, None); + } +}