From de554589a8dce5b898cfe68e8402ef280782c824 Mon Sep 17 00:00:00 2001 From: Agus Zubiaga Date: Tue, 6 May 2025 07:18:48 -0300 Subject: [PATCH] agent: Add date separators to Thread History (#29961) Adds time-bucket separators to the thread history list: https://github.com/user-attachments/assets/c9ac3ec4-b632-4ea5-8234-382b48de2bd6 Note: I'm simulating that Today is next Thursday so that I can show the "This Week" bucket. Release Notes: - agent: Add date separators to Thread History --- crates/agent/src/assistant_panel.rs | 12 +- crates/agent/src/thread_history.rs | 572 +++++++++++++++++++------- crates/time_format/src/time_format.rs | 357 ++++++++++++++-- 3 files changed, 766 insertions(+), 175 deletions(-) diff --git a/crates/agent/src/assistant_panel.rs b/crates/agent/src/assistant_panel.rs index 1a030cabcd..c14fd3ea26 100644 --- a/crates/agent/src/assistant_panel.rs +++ b/crates/agent/src/assistant_panel.rs @@ -55,13 +55,13 @@ use crate::assistant_configuration::{AssistantConfiguration, AssistantConfigurat use crate::history_store::{HistoryEntry, HistoryStore, RecentEntry}; use crate::message_editor::{MessageEditor, MessageEditorEvent}; use crate::thread::{Thread, ThreadError, ThreadId, TokenUsageRatio}; -use crate::thread_history::{PastContext, PastThread, ThreadHistory}; -use crate::thread_store::{TextThreadStore, ThreadStore}; +use crate::thread_history::{EntryTimeFormat, PastContext, PastThread, ThreadHistory}; +use crate::thread_store::ThreadStore; use crate::{ AddContextServer, AgentDiffPane, ContextStore, DeleteRecentlyOpenThread, ExpandMessageEditor, Follow, InlineAssistant, NewTextThread, NewThread, OpenActiveThreadAsMarkdown, OpenAgentDiff, - OpenHistory, ResetTrialUpsell, ThreadEvent, ToggleContextPicker, ToggleNavigationMenu, - ToggleOptionsMenu, + OpenHistory, ResetTrialUpsell, TextThreadStore, ThreadEvent, ToggleContextPicker, + ToggleNavigationMenu, ToggleOptionsMenu, }; const AGENT_PANEL_KEY: &str = "agent_panel"; @@ -2229,11 +2229,11 @@ impl AssistantPanel { // TODO: Add keyboard navigation. match entry { HistoryEntry::Thread(thread) => { - PastThread::new(thread, cx.entity().downgrade(), false, vec![]) + PastThread::new(thread, cx.entity().downgrade(), false, vec![], EntryTimeFormat::DateAndTime) .into_any_element() } HistoryEntry::Context(context) => { - PastContext::new(context, cx.entity().downgrade(), false, vec![]) + PastContext::new(context, cx.entity().downgrade(), false, vec![], EntryTimeFormat::DateAndTime) .into_any_element() } } diff --git a/crates/agent/src/thread_history.rs b/crates/agent/src/thread_history.rs index cfd50a2dd5..8a348a5b71 100644 --- a/crates/agent/src/thread_history.rs +++ b/crates/agent/src/thread_history.rs @@ -1,11 +1,14 @@ +use std::fmt::Display; +use std::ops::Range; use std::sync::Arc; use assistant_context_editor::SavedContextMetadata; +use chrono::{Datelike as _, NaiveDate, TimeDelta, Utc}; use editor::{Editor, EditorEvent}; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ - App, Entity, FocusHandle, Focusable, ScrollStrategy, Stateful, Task, UniformListScrollHandle, - WeakEntity, Window, uniform_list, + App, Empty, Entity, FocusHandle, Focusable, ScrollStrategy, Stateful, Task, + UniformListScrollHandle, WeakEntity, Window, uniform_list, }; use time::{OffsetDateTime, UtcOffset}; use ui::{ @@ -23,14 +26,45 @@ pub struct ThreadHistory { history_store: Entity, scroll_handle: UniformListScrollHandle, selected_index: usize, - search_query: SharedString, search_editor: Entity, all_entries: Arc>, - matches: Vec, - _subscriptions: Vec, - _search_task: Option>, + // When the search is empty, we display date separators between history entries + // This vector contains an enum of either a separator or an actual entry + separated_items: Vec, + _separated_items_task: Option>, + search_state: SearchState, scrollbar_visibility: bool, scrollbar_state: ScrollbarState, + _subscriptions: Vec, +} + +enum SearchState { + Empty, + Searching { + query: SharedString, + _task: Task<()>, + }, + Searched { + query: SharedString, + matches: Vec, + }, +} + +enum HistoryListItem { + BucketSeparator(TimeBucket), + Entry { + index: usize, + format: EntryTimeFormat, + }, +} + +impl HistoryListItem { + fn entry_index(&self) -> Option { + match self { + HistoryListItem::BucketSeparator(_) => None, + HistoryListItem::Entry { index, .. } => Some(*index), + } + } } impl ThreadHistory { @@ -50,15 +84,10 @@ impl ThreadHistory { cx.subscribe(&search_editor, |this, search_editor, event, cx| { if let EditorEvent::BufferEdited = event { let query = search_editor.read(cx).text(cx); - this.search_query = query.into(); - this.update_search(cx); + this.search(query.into(), cx); } }); - let entries: Arc> = history_store - .update(cx, |store, cx| store.entries(cx)) - .into(); - let history_store_subscription = cx.observe(&history_store, |this, _, cx| { this.update_all_entries(cx); }); @@ -66,20 +95,22 @@ impl ThreadHistory { let scroll_handle = UniformListScrollHandle::default(); let scrollbar_state = ScrollbarState::new(scroll_handle.clone()); - Self { + let mut this = Self { assistant_panel, history_store, scroll_handle, selected_index: 0, - search_query: SharedString::new_static(""), - all_entries: entries, - matches: Vec::new(), + search_state: SearchState::Empty, + all_entries: Default::default(), + separated_items: Default::default(), search_editor, - _subscriptions: vec![search_editor_subscription, history_store_subscription], - _search_task: None, scrollbar_visibility: true, scrollbar_state, - } + _subscriptions: vec![search_editor_subscription, history_store_subscription], + _separated_items_task: None, + }; + this.update_all_entries(cx); + this } fn update_all_entries(&mut self, cx: &mut Context) { @@ -87,88 +118,155 @@ impl ThreadHistory { .history_store .update(cx, |store, cx| store.entries(cx)) .into(); - self.matches.clear(); - self.update_search(cx); - } - fn update_search(&mut self, cx: &mut Context) { - self._search_task.take(); + self.set_selected_index(0, cx); + self.update_separated_items(cx); - if self.has_search_query() { - self.perform_search(cx); - } else { - self.matches.clear(); - self.set_selected_index(0, cx); - cx.notify(); + match &self.search_state { + SearchState::Empty => {} + SearchState::Searching { query, .. } | SearchState::Searched { query, .. } => { + self.search(query.clone(), cx); + } } + cx.notify(); } - fn perform_search(&mut self, cx: &mut Context) { - let query = self.search_query.clone(); + fn update_separated_items(&mut self, cx: &mut Context) { + self._separated_items_task.take(); + + let mut separated_items = std::mem::take(&mut self.separated_items); + separated_items.clear(); let all_entries = self.all_entries.clone(); + let bg_task = cx.background_spawn(async move { + let mut bucket = None; + let today = Utc::now().naive_local().date(); + + for (index, entry) in all_entries.iter().enumerate() { + let entry_date = entry.updated_at().naive_local().date(); + let entry_bucket = TimeBucket::from_dates(today, entry_date); + + if Some(entry_bucket) != bucket { + bucket = Some(entry_bucket); + separated_items.push(HistoryListItem::BucketSeparator(entry_bucket)); + } + separated_items.push(HistoryListItem::Entry { + index, + format: entry_bucket.into(), + }); + } + separated_items + }); + let task = cx.spawn(async move |this, cx| { - let executor = cx.background_executor().clone(); - - let matches = cx - .background_spawn(async move { - let mut candidates = Vec::with_capacity(all_entries.len()); - - for (idx, entry) in all_entries.iter().enumerate() { - match entry { - HistoryEntry::Thread(thread) => { - candidates.push(StringMatchCandidate::new(idx, &thread.summary)); - } - HistoryEntry::Context(context) => { - candidates.push(StringMatchCandidate::new(idx, &context.title)); - } - } - } - - const MAX_MATCHES: usize = 100; - - fuzzy::match_strings( - &candidates, - &query, - false, - MAX_MATCHES, - &Default::default(), - executor, - ) - .await - }) - .await; - + let separated_items = bg_task.await; this.update(cx, |this, cx| { - this.matches = matches; - this.set_selected_index(0, cx); + this.separated_items = separated_items; cx.notify(); }) .log_err(); }); - - self._search_task = Some(task); + self._separated_items_task = Some(task); } - fn has_search_query(&self) -> bool { - !self.search_query.is_empty() + fn search(&mut self, query: SharedString, cx: &mut Context) { + if query.is_empty() { + self.search_state = SearchState::Empty; + cx.notify(); + return; + } + + let all_entries = self.all_entries.clone(); + + let fuzzy_search_task = cx.background_spawn({ + let query = query.clone(); + let executor = cx.background_executor().clone(); + async move { + let mut candidates = Vec::with_capacity(all_entries.len()); + + for (idx, entry) in all_entries.iter().enumerate() { + match entry { + HistoryEntry::Thread(thread) => { + candidates.push(StringMatchCandidate::new(idx, &thread.summary)); + } + HistoryEntry::Context(context) => { + candidates.push(StringMatchCandidate::new(idx, &context.title)); + } + } + } + + const MAX_MATCHES: usize = 100; + + fuzzy::match_strings( + &candidates, + &query, + false, + MAX_MATCHES, + &Default::default(), + executor, + ) + .await + } + }); + + let task = cx.spawn({ + let query = query.clone(); + async move |this, cx| { + let matches = fuzzy_search_task.await; + + this.update(cx, |this, cx| { + let SearchState::Searching { + query: current_query, + _task, + } = &this.search_state + else { + return; + }; + + if &query == current_query { + this.search_state = SearchState::Searched { + query: query.clone(), + matches, + }; + + this.set_selected_index(0, cx); + cx.notify(); + }; + }) + .log_err(); + } + }); + + self.search_state = SearchState::Searching { + query: query.clone(), + _task: task, + }; + cx.notify(); } fn matched_count(&self) -> usize { - if self.has_search_query() { - self.matches.len() - } else { - self.all_entries.len() + match &self.search_state { + SearchState::Empty => self.all_entries.len(), + SearchState::Searching { .. } => 0, + SearchState::Searched { matches, .. } => matches.len(), + } + } + + fn search_produced_no_matches(&self) -> bool { + match &self.search_state { + SearchState::Empty => false, + SearchState::Searching { .. } => false, + SearchState::Searched { matches, .. } => matches.is_empty(), } } fn get_match(&self, ix: usize) -> Option<&HistoryEntry> { - if self.has_search_query() { - self.matches + match &self.search_state { + SearchState::Empty => self.all_entries.get(ix), + SearchState::Searching { .. } => None, + SearchState::Searched { matches, .. } => matches .get(ix) - .and_then(|m| self.all_entries.get(m.candidate_id)) - } else { - self.all_entries.get(ix) + .and_then(|m| self.all_entries.get(m.candidate_id)), } } @@ -311,6 +409,107 @@ impl ThreadHistory { cx.notify(); } } + + fn list_items( + &mut self, + range: Range, + _window: &mut Window, + cx: &mut Context, + ) -> Vec { + let range_start = range.start; + + match &self.search_state { + SearchState::Empty => self + .separated_items + .get(range) + .iter() + .flat_map(|items| { + items + .iter() + .map(|item| self.render_list_item(item.entry_index(), item, vec![], cx)) + }) + .collect(), + SearchState::Searched { matches, .. } => matches[range] + .iter() + .enumerate() + .map(|(ix, m)| { + self.render_list_item( + Some(range_start + ix), + &HistoryListItem::Entry { + index: m.candidate_id, + format: EntryTimeFormat::DateAndTime, + }, + m.positions.clone(), + cx, + ) + }) + .collect(), + SearchState::Searching { .. } => { + vec![] + } + } + } + + fn render_list_item( + &self, + list_entry_ix: Option, + item: &HistoryListItem, + highlight_positions: Vec, + cx: &App, + ) -> AnyElement { + match item { + HistoryListItem::Entry { index, format } => match self.all_entries.get(*index) { + Some(entry) => h_flex() + .w_full() + .pb_1() + .child(self.render_history_entry( + entry, + list_entry_ix == Some(self.selected_index), + highlight_positions, + *format, + )) + .into_any(), + None => Empty.into_any_element(), + }, + HistoryListItem::BucketSeparator(bucket) => div() + .px(DynamicSpacing::Base06.rems(cx)) + .pt_2() + .pb_1() + .child( + Label::new(bucket.to_string()) + .size(LabelSize::XSmall) + .color(Color::Muted), + ) + .into_any_element(), + } + } + + fn render_history_entry( + &self, + entry: &HistoryEntry, + is_active: bool, + highlight_positions: Vec, + format: EntryTimeFormat, + ) -> AnyElement { + match entry { + HistoryEntry::Thread(thread) => PastThread::new( + thread.clone(), + self.assistant_panel.clone(), + is_active, + highlight_positions, + format, + ) + .into_any_element(), + HistoryEntry::Context(context) => PastContext::new( + context.clone(), + self.assistant_panel.clone(), + is_active, + highlight_positions, + format, + ) + .into_any_element(), + } + } } impl Focusable for ThreadHistory { @@ -321,8 +520,6 @@ impl Focusable for ThreadHistory { impl Render for ThreadHistory { fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { - let selected_index = self.selected_index; - v_flex() .key_context("ThreadHistory") .size_full() @@ -366,7 +563,7 @@ impl Render for ThreadHistory { .size(LabelSize::Small), ), ) - } else if self.has_search_query() && self.matches.is_empty() { + } else if self.search_produced_no_matches() { view.justify_center().child( h_flex().w_full().justify_center().child( Label::new("No threads match your search.").size(LabelSize::Small), @@ -379,56 +576,7 @@ impl Render for ThreadHistory { cx.entity().clone(), "thread-history", self.matched_count(), - move |history, range, _window, _cx| { - let range_start = range.start; - let assistant_panel = history.assistant_panel.clone(); - - let render_item = |index: usize, - entry: &HistoryEntry, - highlight_positions: Vec| - -> Div { - h_flex().w_full().pb_1().child(match entry { - HistoryEntry::Thread(thread) => PastThread::new( - thread.clone(), - assistant_panel.clone(), - selected_index == index + range_start, - highlight_positions, - ) - .into_any_element(), - HistoryEntry::Context(context) => PastContext::new( - context.clone(), - assistant_panel.clone(), - selected_index == index + range_start, - highlight_positions, - ) - .into_any_element(), - }) - }; - - if history.has_search_query() { - history.matches[range] - .iter() - .enumerate() - .filter_map(|(index, m)| { - history.all_entries.get(m.candidate_id).map( - |entry| { - render_item( - index, - entry, - m.positions.clone(), - ) - }, - ) - }) - .collect() - } else { - history.all_entries[range] - .iter() - .enumerate() - .map(|(index, entry)| render_item(index, entry, vec![])) - .collect() - } - }, + Self::list_items, ) .p_1() .track_scroll(self.scroll_handle.clone()) @@ -448,6 +596,7 @@ pub struct PastThread { assistant_panel: WeakEntity, selected: bool, highlight_positions: Vec, + timestamp_format: EntryTimeFormat, } impl PastThread { @@ -456,12 +605,14 @@ impl PastThread { assistant_panel: WeakEntity, selected: bool, highlight_positions: Vec, + timestamp_format: EntryTimeFormat, ) -> Self { Self { thread, assistant_panel, selected, highlight_positions, + timestamp_format, } } } @@ -470,13 +621,10 @@ impl RenderOnce for PastThread { fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { let summary = self.thread.summary; - let thread_timestamp = time_format::format_localized_timestamp( - OffsetDateTime::from_unix_timestamp(self.thread.updated_at.timestamp()).unwrap(), - OffsetDateTime::now_utc(), - self.assistant_panel - .update(cx, |this, _cx| this.local_timezone()) - .unwrap_or(UtcOffset::UTC), - time_format::TimestampFormat::EnhancedAbsolute, + let thread_timestamp = self.timestamp_format.format_timestamp( + &self.assistant_panel, + self.thread.updated_at.timestamp(), + cx, ); ListItem::new(SharedString::from(self.thread.id.to_string())) @@ -540,6 +688,7 @@ pub struct PastContext { assistant_panel: WeakEntity, selected: bool, highlight_positions: Vec, + timestamp_format: EntryTimeFormat, } impl PastContext { @@ -548,12 +697,14 @@ impl PastContext { assistant_panel: WeakEntity, selected: bool, highlight_positions: Vec, + timestamp_format: EntryTimeFormat, ) -> Self { Self { context, assistant_panel, selected, highlight_positions, + timestamp_format, } } } @@ -561,13 +712,10 @@ impl PastContext { impl RenderOnce for PastContext { fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement { let summary = self.context.title; - let context_timestamp = time_format::format_localized_timestamp( - OffsetDateTime::from_unix_timestamp(self.context.mtime.timestamp()).unwrap(), - OffsetDateTime::now_utc(), - self.assistant_panel - .update(cx, |this, _cx| this.local_timezone()) - .unwrap_or(UtcOffset::UTC), - time_format::TimestampFormat::EnhancedAbsolute, + let context_timestamp = self.timestamp_format.format_timestamp( + &self.assistant_panel, + self.context.mtime.timestamp(), + cx, ); ListItem::new(SharedString::from( @@ -627,3 +775,137 @@ impl RenderOnce for PastContext { }) } } + +#[derive(Clone, Copy)] +pub enum EntryTimeFormat { + DateAndTime, + TimeOnly, +} + +impl EntryTimeFormat { + fn format_timestamp( + &self, + assistant_panel: &WeakEntity, + timestamp: i64, + cx: &App, + ) -> String { + let timestamp = OffsetDateTime::from_unix_timestamp(timestamp).unwrap(); + let timezone = assistant_panel + .read_with(cx, |this, _cx| this.local_timezone()) + .unwrap_or(UtcOffset::UTC); + + match &self { + EntryTimeFormat::DateAndTime => time_format::format_localized_timestamp( + timestamp, + OffsetDateTime::now_utc(), + timezone, + time_format::TimestampFormat::EnhancedAbsolute, + ), + EntryTimeFormat::TimeOnly => time_format::format_time(timestamp), + } + } +} + +impl From for EntryTimeFormat { + fn from(bucket: TimeBucket) -> Self { + match bucket { + TimeBucket::Today => EntryTimeFormat::TimeOnly, + TimeBucket::Yesterday => EntryTimeFormat::TimeOnly, + TimeBucket::ThisWeek => EntryTimeFormat::DateAndTime, + TimeBucket::PastWeek => EntryTimeFormat::DateAndTime, + TimeBucket::All => EntryTimeFormat::DateAndTime, + } + } +} + +#[derive(PartialEq, Eq, Clone, Copy, Debug)] +enum TimeBucket { + Today, + Yesterday, + ThisWeek, + PastWeek, + All, +} + +impl TimeBucket { + fn from_dates(reference: NaiveDate, date: NaiveDate) -> Self { + if date == reference { + return TimeBucket::Today; + } + + if date == reference - TimeDelta::days(1) { + return TimeBucket::Yesterday; + } + + let week = date.iso_week(); + + if reference.iso_week() == week { + return TimeBucket::ThisWeek; + } + + let last_week = (reference - TimeDelta::days(7)).iso_week(); + + if week == last_week { + return TimeBucket::PastWeek; + } + + TimeBucket::All + } +} + +impl Display for TimeBucket { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + TimeBucket::Today => write!(f, "Today"), + TimeBucket::Yesterday => write!(f, "Yesterday"), + TimeBucket::ThisWeek => write!(f, "This Week"), + TimeBucket::PastWeek => write!(f, "Past Week"), + TimeBucket::All => write!(f, "All"), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use chrono::NaiveDate; + + #[test] + fn test_time_bucket_from_dates() { + let today = NaiveDate::from_ymd_opt(2023, 1, 15).unwrap(); + + let date = today; + assert_eq!(TimeBucket::from_dates(today, date), TimeBucket::Today); + + let date = NaiveDate::from_ymd_opt(2023, 1, 14).unwrap(); + assert_eq!(TimeBucket::from_dates(today, date), TimeBucket::Yesterday); + + let date = NaiveDate::from_ymd_opt(2023, 1, 13).unwrap(); + assert_eq!(TimeBucket::from_dates(today, date), TimeBucket::ThisWeek); + + let date = NaiveDate::from_ymd_opt(2023, 1, 11).unwrap(); + assert_eq!(TimeBucket::from_dates(today, date), TimeBucket::ThisWeek); + + let date = NaiveDate::from_ymd_opt(2023, 1, 8).unwrap(); + assert_eq!(TimeBucket::from_dates(today, date), TimeBucket::PastWeek); + + let date = NaiveDate::from_ymd_opt(2023, 1, 5).unwrap(); + assert_eq!(TimeBucket::from_dates(today, date), TimeBucket::PastWeek); + + // All: not in this week or last week + let date = NaiveDate::from_ymd_opt(2023, 1, 1).unwrap(); + assert_eq!(TimeBucket::from_dates(today, date), TimeBucket::All); + + // Test year boundary cases + let new_year = NaiveDate::from_ymd_opt(2023, 1, 1).unwrap(); + + let date = NaiveDate::from_ymd_opt(2022, 12, 31).unwrap(); + assert_eq!( + TimeBucket::from_dates(new_year, date), + TimeBucket::Yesterday + ); + + let date = NaiveDate::from_ymd_opt(2022, 12, 28).unwrap(); + assert_eq!(TimeBucket::from_dates(new_year, date), TimeBucket::ThisWeek); + } +} diff --git a/crates/time_format/src/time_format.rs b/crates/time_format/src/time_format.rs index 7dc0702fbf..37d4201d1b 100644 --- a/crates/time_format/src/time_format.rs +++ b/crates/time_format/src/time_format.rs @@ -42,6 +42,82 @@ pub fn format_local_timestamp( } } +/// Formats the date component of a timestamp +pub fn format_date( + timestamp: OffsetDateTime, + reference: OffsetDateTime, + enhanced_formatting: bool, +) -> String { + format_absolute_date(timestamp, reference, enhanced_formatting) +} + +/// Formats the time component of a timestamp +pub fn format_time(timestamp: OffsetDateTime) -> String { + format_absolute_time(timestamp) +} + +/// Formats the date component of a timestamp in medium style +pub fn format_date_medium( + timestamp: OffsetDateTime, + reference: OffsetDateTime, + enhanced_formatting: bool, +) -> String { + format_absolute_date_medium(timestamp, reference, enhanced_formatting) +} + +fn format_absolute_date( + timestamp: OffsetDateTime, + reference: OffsetDateTime, + #[allow(unused_variables)] enhanced_date_formatting: bool, +) -> String { + #[cfg(target_os = "macos")] + { + if !enhanced_date_formatting { + return macos::format_date(×tamp); + } + + let timestamp_date = timestamp.date(); + let reference_date = reference.date(); + if timestamp_date == reference_date { + "Today".to_string() + } else if reference_date.previous_day() == Some(timestamp_date) { + "Yesterday".to_string() + } else { + macos::format_date(×tamp) + } + } + #[cfg(not(target_os = "macos"))] + { + // todo(linux) respect user's date/time preferences + // todo(windows) respect user's date/time preferences + let current_locale = CURRENT_LOCALE + .get_or_init(|| sys_locale::get_locale().unwrap_or_else(|| String::from("en-US"))); + format_timestamp_naive_date( + timestamp, + reference, + is_12_hour_time_by_locale(current_locale.as_str()), + ) + } +} + +fn format_absolute_time(timestamp: OffsetDateTime) -> String { + #[cfg(target_os = "macos")] + { + macos::format_time(×tamp) + } + #[cfg(not(target_os = "macos"))] + { + // todo(linux) respect user's date/time preferences + // todo(windows) respect user's date/time preferences + let current_locale = CURRENT_LOCALE + .get_or_init(|| sys_locale::get_locale().unwrap_or_else(|| String::from("en-US"))); + format_timestamp_naive_time( + timestamp, + is_12_hour_time_by_locale(current_locale.as_str()), + ) + } +} + fn format_absolute_timestamp( timestamp: OffsetDateTime, reference: OffsetDateTime, @@ -52,22 +128,22 @@ fn format_absolute_timestamp( if !enhanced_date_formatting { return format!( "{} {}", - macos::format_date(×tamp), - macos::format_time(×tamp) + format_absolute_date(timestamp, reference, enhanced_date_formatting), + format_absolute_time(timestamp) ); } let timestamp_date = timestamp.date(); let reference_date = reference.date(); if timestamp_date == reference_date { - format!("Today at {}", macos::format_time(×tamp)) + format!("Today at {}", format_absolute_time(timestamp)) } else if reference_date.previous_day() == Some(timestamp_date) { - format!("Yesterday at {}", macos::format_time(×tamp)) + format!("Yesterday at {}", format_absolute_time(timestamp)) } else { format!( "{} {}", - macos::format_date(×tamp), - macos::format_time(×tamp) + format_absolute_date(timestamp, reference, enhanced_date_formatting), + format_absolute_time(timestamp) ) } } @@ -79,13 +155,62 @@ fn format_absolute_timestamp( } } -fn format_absolute_timestamp_medium( +fn format_absolute_date_medium( timestamp: OffsetDateTime, - #[allow(unused_variables)] reference: OffsetDateTime, + reference: OffsetDateTime, + enhanced_formatting: bool, ) -> String { #[cfg(target_os = "macos")] { - macos::format_date_medium(×tamp) + if !enhanced_formatting { + return macos::format_date_medium(×tamp); + } + + let timestamp_date = timestamp.date(); + let reference_date = reference.date(); + if timestamp_date == reference_date { + "Today".to_string() + } else if reference_date.previous_day() == Some(timestamp_date) { + "Yesterday".to_string() + } else { + macos::format_date_medium(×tamp) + } + } + #[cfg(not(target_os = "macos"))] + { + // todo(linux) respect user's date/time preferences + // todo(windows) respect user's date/time preferences + let current_locale = CURRENT_LOCALE + .get_or_init(|| sys_locale::get_locale().unwrap_or_else(|| String::from("en-US"))); + if !enhanced_formatting { + return format_timestamp_naive_date_medium( + timestamp, + is_12_hour_time_by_locale(current_locale.as_str()), + ); + } + + let timestamp_date = timestamp.date(); + let reference_date = reference.date(); + if timestamp_date == reference_date { + "Today".to_string() + } else if reference_date.previous_day() == Some(timestamp_date) { + "Yesterday".to_string() + } else { + format_timestamp_naive_date_medium( + timestamp, + is_12_hour_time_by_locale(current_locale.as_str()), + ) + } + } +} + +fn format_absolute_timestamp_medium( + timestamp: OffsetDateTime, + reference: OffsetDateTime, +) -> String { + #[cfg(target_os = "macos")] + { + format_absolute_date_medium(timestamp, reference, false) } #[cfg(not(target_os = "macos"))] { @@ -178,15 +303,9 @@ fn calculate_month_difference(timestamp: OffsetDateTime, reference: OffsetDateTi /// Note: /// This function does not respect the user's date and time preferences. /// This should only be used as a fallback mechanism when the OS time formatting fails. -pub fn format_timestamp_naive( - timestamp_local: OffsetDateTime, - reference_local: OffsetDateTime, - is_12_hour_time: bool, -) -> String { +fn format_timestamp_naive_time(timestamp_local: OffsetDateTime, is_12_hour_time: bool) -> String { let timestamp_local_hour = timestamp_local.hour(); let timestamp_local_minute = timestamp_local.minute(); - let reference_local_date = reference_local.date(); - let timestamp_local_date = timestamp_local.date(); let (hour, meridiem) = if is_12_hour_time { let meridiem = if timestamp_local_hour >= 12 { @@ -206,38 +325,103 @@ pub fn format_timestamp_naive( (timestamp_local_hour, None) }; - let formatted_time = match meridiem { + match meridiem { Some(meridiem) => format!("{}:{:02} {}", hour, timestamp_local_minute, meridiem), None => format!("{:02}:{:02}", hour, timestamp_local_minute), - }; + } +} - let formatted_date = match meridiem { - Some(_) => format!( +#[cfg(not(target_os = "macos"))] +fn format_timestamp_naive_date( + timestamp_local: OffsetDateTime, + reference_local: OffsetDateTime, + is_12_hour_time: bool, +) -> String { + let reference_local_date = reference_local.date(); + let timestamp_local_date = timestamp_local.date(); + + if timestamp_local_date == reference_local_date { + "Today".to_string() + } else if reference_local_date.previous_day() == Some(timestamp_local_date) { + "Yesterday".to_string() + } else { + match is_12_hour_time { + true => format!( + "{:02}/{:02}/{}", + timestamp_local_date.month() as u32, + timestamp_local_date.day(), + timestamp_local_date.year() + ), + false => format!( + "{:02}/{:02}/{}", + timestamp_local_date.day(), + timestamp_local_date.month() as u32, + timestamp_local_date.year() + ), + } + } +} + +#[cfg(not(target_os = "macos"))] +fn format_timestamp_naive_date_medium( + timestamp_local: OffsetDateTime, + is_12_hour_time: bool, +) -> String { + let timestamp_local_date = timestamp_local.date(); + + match is_12_hour_time { + true => format!( "{:02}/{:02}/{}", timestamp_local_date.month() as u32, timestamp_local_date.day(), timestamp_local_date.year() ), - None => format!( + false => format!( "{:02}/{:02}/{}", timestamp_local_date.day(), timestamp_local_date.month() as u32, timestamp_local_date.year() ), - }; + } +} + +pub fn format_timestamp_naive( + timestamp_local: OffsetDateTime, + reference_local: OffsetDateTime, + is_12_hour_time: bool, +) -> String { + let formatted_time = format_timestamp_naive_time(timestamp_local, is_12_hour_time); + let reference_local_date = reference_local.date(); + let timestamp_local_date = timestamp_local.date(); if timestamp_local_date == reference_local_date { format!("Today at {}", formatted_time) } else if reference_local_date.previous_day() == Some(timestamp_local_date) { format!("Yesterday at {}", formatted_time) } else { + let formatted_date = match is_12_hour_time { + true => format!( + "{:02}/{:02}/{}", + timestamp_local_date.month() as u32, + timestamp_local_date.day(), + timestamp_local_date.year() + ), + false => format!( + "{:02}/{:02}/{}", + timestamp_local_date.day(), + timestamp_local_date.month() as u32, + timestamp_local_date.year() + ), + }; format!("{} {}", formatted_date, formatted_time) } } +#[cfg(not(target_os = "macos"))] +static CURRENT_LOCALE: std::sync::OnceLock = std::sync::OnceLock::new(); + #[cfg(not(target_os = "macos"))] fn format_timestamp_fallback(timestamp: OffsetDateTime, reference: OffsetDateTime) -> String { - static CURRENT_LOCALE: std::sync::OnceLock = std::sync::OnceLock::new(); let current_locale = CURRENT_LOCALE .get_or_init(|| sys_locale::get_locale().unwrap_or_else(|| String::from("en-US"))); @@ -245,8 +429,8 @@ fn format_timestamp_fallback(timestamp: OffsetDateTime, reference: OffsetDateTim format_timestamp_naive(timestamp, reference, is_12_hour_time) } -#[cfg(not(target_os = "macos"))] /// Returns `true` if the locale is recognized as a 12-hour time locale. +#[cfg(not(target_os = "macos"))] fn is_12_hour_time_by_locale(locale: &str) -> bool { [ "es-MX", "es-CO", "es-SV", "es-NI", @@ -344,6 +528,131 @@ mod macos { mod tests { use super::*; + #[test] + fn test_format_date() { + let reference = create_offset_datetime(1990, 4, 12, 10, 30, 0); + + // Test with same date (today) + let timestamp_today = create_offset_datetime(1990, 4, 12, 9, 30, 0); + assert_eq!(format_date(timestamp_today, reference, true), "Today"); + + // Test with previous day (yesterday) + let timestamp_yesterday = create_offset_datetime(1990, 4, 11, 9, 30, 0); + assert_eq!( + format_date(timestamp_yesterday, reference, true), + "Yesterday" + ); + + // Test with other date + let timestamp_other = create_offset_datetime(1990, 4, 10, 9, 30, 0); + let result = format_date(timestamp_other, reference, true); + assert!(!result.is_empty()); + assert_ne!(result, "Today"); + assert_ne!(result, "Yesterday"); + } + + #[test] + fn test_format_time() { + let timestamp = create_offset_datetime(1990, 4, 12, 9, 30, 0); + + // We can't assert the exact output as it depends on the platform and locale + // But we can at least confirm it doesn't panic and returns a non-empty string + let result = format_time(timestamp); + assert!(!result.is_empty()); + } + + #[test] + fn test_format_date_medium() { + let reference = create_offset_datetime(1990, 4, 12, 10, 30, 0); + let timestamp = create_offset_datetime(1990, 4, 12, 9, 30, 0); + + // Test with enhanced formatting (today) + let result_enhanced = format_date_medium(timestamp, reference, true); + assert_eq!(result_enhanced, "Today"); + + // Test with standard formatting + let result_standard = format_date_medium(timestamp, reference, false); + assert!(!result_standard.is_empty()); + + // Test yesterday with enhanced formatting + let timestamp_yesterday = create_offset_datetime(1990, 4, 11, 9, 30, 0); + let result_yesterday = format_date_medium(timestamp_yesterday, reference, true); + assert_eq!(result_yesterday, "Yesterday"); + + // Test other date with enhanced formatting + let timestamp_other = create_offset_datetime(1990, 4, 10, 9, 30, 0); + let result_other = format_date_medium(timestamp_other, reference, true); + assert!(!result_other.is_empty()); + assert_ne!(result_other, "Today"); + assert_ne!(result_other, "Yesterday"); + } + + #[test] + fn test_format_absolute_time() { + let timestamp = create_offset_datetime(1990, 4, 12, 9, 30, 0); + + // We can't assert the exact output as it depends on the platform and locale + // But we can at least confirm it doesn't panic and returns a non-empty string + let result = format_absolute_time(timestamp); + assert!(!result.is_empty()); + } + + #[test] + fn test_format_absolute_date() { + let reference = create_offset_datetime(1990, 4, 12, 10, 30, 0); + + // Test with same date (today) + let timestamp_today = create_offset_datetime(1990, 4, 12, 9, 30, 0); + assert_eq!( + format_absolute_date(timestamp_today, reference, true), + "Today" + ); + + // Test with previous day (yesterday) + let timestamp_yesterday = create_offset_datetime(1990, 4, 11, 9, 30, 0); + assert_eq!( + format_absolute_date(timestamp_yesterday, reference, true), + "Yesterday" + ); + + // Test with other date + let timestamp_other = create_offset_datetime(1990, 4, 10, 9, 30, 0); + let result = format_absolute_date(timestamp_other, reference, true); + assert!(!result.is_empty()); + assert_ne!(result, "Today"); + assert_ne!(result, "Yesterday"); + } + + #[test] + fn test_format_absolute_date_medium() { + let reference = create_offset_datetime(1990, 4, 12, 10, 30, 0); + let timestamp = create_offset_datetime(1990, 4, 12, 9, 30, 0); + + // Test with enhanced formatting (today) + let result_enhanced = format_absolute_date_medium(timestamp, reference, true); + assert_eq!(result_enhanced, "Today"); + + // Test with standard formatting + let result_standard = format_absolute_date_medium(timestamp, reference, false); + assert!(!result_standard.is_empty()); + + // Test yesterday with enhanced formatting + let timestamp_yesterday = create_offset_datetime(1990, 4, 11, 9, 30, 0); + let result_yesterday = format_absolute_date_medium(timestamp_yesterday, reference, true); + assert_eq!(result_yesterday, "Yesterday"); + } + + #[test] + fn test_format_timestamp_naive_time() { + let timestamp = create_offset_datetime(1990, 4, 12, 9, 30, 0); + assert_eq!(format_timestamp_naive_time(timestamp, true), "9:30 AM"); + assert_eq!(format_timestamp_naive_time(timestamp, false), "09:30"); + + let timestamp_pm = create_offset_datetime(1990, 4, 12, 15, 45, 0); + assert_eq!(format_timestamp_naive_time(timestamp_pm, true), "3:45 PM"); + assert_eq!(format_timestamp_naive_time(timestamp_pm, false), "15:45"); + } + #[test] fn test_format_24_hour_time() { let reference = create_offset_datetime(1990, 4, 12, 16, 45, 0);