Revert "agent: Add date separators to Thread History (#29961)"

Joseph T. Lyons created

This reverts commit 69fd7d57d78d07a05c7fd7d223356d8ecf520c64.

Change summary

crates/agent/src/assistant_panel.rs   |  12 
crates/agent/src/thread_history.rs    | 566 +++++++---------------------
crates/time_format/src/time_format.rs | 355 +-----------------
3 files changed, 171 insertions(+), 762 deletions(-)

Detailed changes

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::{EntryTimeFormat, PastContext, PastThread, ThreadHistory};
-use crate::thread_store::ThreadStore;
+use crate::thread_history::{PastContext, PastThread, ThreadHistory};
+use crate::thread_store::{TextThreadStore, ThreadStore};
 use crate::{
     AddContextServer, AgentDiffPane, ContextStore, DeleteRecentlyOpenThread, ExpandMessageEditor,
     Follow, InlineAssistant, NewTextThread, NewThread, OpenActiveThreadAsMarkdown, OpenAgentDiff,
-    OpenHistory, ResetTrialUpsell, TextThreadStore, ThreadEvent, ToggleContextPicker,
-    ToggleNavigationMenu, ToggleOptionsMenu,
+    OpenHistory, ResetTrialUpsell, ThreadEvent, ToggleContextPicker, ToggleNavigationMenu,
+    ToggleOptionsMenu,
 };
 
 const AGENT_PANEL_KEY: &str = "agent_panel";
@@ -2191,11 +2191,11 @@ impl AssistantPanel {
                                     // TODO: Add keyboard navigation.
                                     match entry {
                                         HistoryEntry::Thread(thread) => {
-                                            PastThread::new(thread, cx.entity().downgrade(), false, vec![], EntryTimeFormat::DateAndTime)
+                                            PastThread::new(thread, cx.entity().downgrade(), false, vec![])
                                                 .into_any_element()
                                         }
                                         HistoryEntry::Context(context) => {
-                                            PastContext::new(context, cx.entity().downgrade(), false, vec![], EntryTimeFormat::DateAndTime)
+                                            PastContext::new(context, cx.entity().downgrade(), false, vec![])
                                                 .into_any_element()
                                         }
                                     }

crates/agent/src/thread_history.rs 🔗

@@ -1,14 +1,11 @@
-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, Empty, Entity, FocusHandle, Focusable, ScrollStrategy, Stateful, Task,
-    UniformListScrollHandle, WeakEntity, Window, uniform_list,
+    App, Entity, FocusHandle, Focusable, ScrollStrategy, Stateful, Task, UniformListScrollHandle,
+    WeakEntity, Window, uniform_list,
 };
 use time::{OffsetDateTime, UtcOffset};
 use ui::{
@@ -26,45 +23,14 @@ pub struct ThreadHistory {
     history_store: Entity<HistoryStore>,
     scroll_handle: UniformListScrollHandle,
     selected_index: usize,
+    search_query: SharedString,
     search_editor: Entity<Editor>,
     all_entries: Arc<Vec<HistoryEntry>>,
-    // 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<HistoryListItem>,
-    _separated_items_task: Option<Task<()>>,
-    search_state: SearchState,
+    matches: Vec<StringMatch>,
+    _subscriptions: Vec<gpui::Subscription>,
+    _search_task: Option<Task<()>>,
     scrollbar_visibility: bool,
     scrollbar_state: ScrollbarState,
-    _subscriptions: Vec<gpui::Subscription>,
-}
-
-enum SearchState {
-    Empty,
-    Searching {
-        query: SharedString,
-        _task: Task<()>,
-    },
-    Searched {
-        query: SharedString,
-        matches: Vec<StringMatch>,
-    },
-}
-
-enum HistoryListItem {
-    BucketSeparator(TimeBucket),
-    Entry {
-        index: usize,
-        format: EntryTimeFormat,
-    },
-}
-
-impl HistoryListItem {
-    fn entry_index(&self) -> Option<usize> {
-        match self {
-            HistoryListItem::BucketSeparator(_) => None,
-            HistoryListItem::Entry { index, .. } => Some(*index),
-        }
-    }
 }
 
 impl ThreadHistory {
@@ -84,10 +50,15 @@ 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.into(), cx);
+                    this.search_query = query.into();
+                    this.update_search(cx);
                 }
             });
 
+        let entries: Arc<Vec<_>> = 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);
         });
@@ -95,22 +66,20 @@ impl ThreadHistory {
         let scroll_handle = UniformListScrollHandle::default();
         let scrollbar_state = ScrollbarState::new(scroll_handle.clone());
 
-        let mut this = Self {
+        Self {
             assistant_panel,
             history_store,
             scroll_handle,
             selected_index: 0,
-            search_state: SearchState::Empty,
-            all_entries: Default::default(),
-            separated_items: Default::default(),
+            search_query: SharedString::new_static(""),
+            all_entries: entries,
+            matches: Vec::new(),
             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<Self>) {
@@ -118,155 +87,88 @@ impl ThreadHistory {
             .history_store
             .update(cx, |store, cx| store.entries(cx))
             .into();
-
-        self.set_selected_index(0, cx);
-        self.update_separated_items(cx);
-
-        match &self.search_state {
-            SearchState::Empty => {}
-            SearchState::Searching { query, .. } | SearchState::Searched { query, .. } => {
-                self.search(query.clone(), cx);
-            }
-        }
-        cx.notify();
+        self.matches.clear();
+        self.update_search(cx);
     }
 
-    fn update_separated_items(&mut self, cx: &mut Context<Self>) {
-        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();
+    fn update_search(&mut self, cx: &mut Context<Self>) {
+        self._search_task.take();
 
-            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 separated_items = bg_task.await;
-            this.update(cx, |this, cx| {
-                this.separated_items = separated_items;
-                cx.notify();
-            })
-            .log_err();
-        });
-        self._separated_items_task = Some(task);
-    }
-
-    fn search(&mut self, query: SharedString, cx: &mut Context<Self>) {
-        if query.is_empty() {
-            self.search_state = SearchState::Empty;
+        if self.has_search_query() {
+            self.perform_search(cx);
+        } else {
+            self.matches.clear();
+            self.set_selected_index(0, cx);
             cx.notify();
-            return;
         }
+    }
 
+    fn perform_search(&mut self, cx: &mut Context<Self>) {
+        let query = self.search_query.clone();
         let all_entries = self.all_entries.clone();
 
-        let fuzzy_search_task = cx.background_spawn({
-            let query = query.clone();
+        let task = cx.spawn(async move |this, cx| {
             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));
+            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
-            }
-        });
+                    const MAX_MATCHES: usize = 100;
 
-        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();
-                    };
+                    fuzzy::match_strings(
+                        &candidates,
+                        &query,
+                        false,
+                        MAX_MATCHES,
+                        &Default::default(),
+                        executor,
+                    )
+                    .await
                 })
-                .log_err();
-            }
+                .await;
+
+            this.update(cx, |this, cx| {
+                this.matches = matches;
+                this.set_selected_index(0, cx);
+                cx.notify();
+            })
+            .log_err();
         });
 
-        self.search_state = SearchState::Searching {
-            query: query.clone(),
-            _task: task,
-        };
-        cx.notify();
+        self._search_task = Some(task);
     }
 
-    fn matched_count(&self) -> usize {
-        match &self.search_state {
-            SearchState::Empty => self.all_entries.len(),
-            SearchState::Searching { .. } => 0,
-            SearchState::Searched { matches, .. } => matches.len(),
-        }
+    fn has_search_query(&self) -> bool {
+        !self.search_query.is_empty()
     }
 
-    fn search_produced_no_matches(&self) -> bool {
-        match &self.search_state {
-            SearchState::Empty => false,
-            SearchState::Searching { .. } => false,
-            SearchState::Searched { matches, .. } => matches.is_empty(),
+    fn matched_count(&self) -> usize {
+        if self.has_search_query() {
+            self.matches.len()
+        } else {
+            self.all_entries.len()
         }
     }
 
     fn get_match(&self, ix: usize) -> Option<&HistoryEntry> {
-        match &self.search_state {
-            SearchState::Empty => self.all_entries.get(ix),
-            SearchState::Searching { .. } => None,
-            SearchState::Searched { matches, .. } => matches
+        if self.has_search_query() {
+            self.matches
                 .get(ix)
-                .and_then(|m| self.all_entries.get(m.candidate_id)),
+                .and_then(|m| self.all_entries.get(m.candidate_id))
+        } else {
+            self.all_entries.get(ix)
         }
     }
 
@@ -409,107 +311,6 @@ impl ThreadHistory {
             cx.notify();
         }
     }
-
-    fn list_items(
-        &mut self,
-        range: Range<usize>,
-        _window: &mut Window,
-        cx: &mut Context<Self>,
-    ) -> Vec<AnyElement> {
-        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<usize>,
-        item: &HistoryListItem,
-        highlight_positions: Vec<usize>,
-        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<usize>,
-        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 {
@@ -520,6 +321,8 @@ impl Focusable for ThreadHistory {
 
 impl Render for ThreadHistory {
     fn render(&mut self, _window: &mut Window, cx: &mut Context<Self>) -> impl IntoElement {
+        let selected_index = self.selected_index;
+
         v_flex()
             .key_context("ThreadHistory")
             .size_full()
@@ -563,7 +366,7 @@ impl Render for ThreadHistory {
                                     .size(LabelSize::Small),
                             ),
                         )
-                } else if self.search_produced_no_matches() {
+                } else if self.has_search_query() && self.matches.is_empty() {
                     view.justify_center().child(
                         h_flex().w_full().justify_center().child(
                             Label::new("No threads match your search.").size(LabelSize::Small),
@@ -576,7 +379,56 @@ impl Render for ThreadHistory {
                                 cx.entity().clone(),
                                 "thread-history",
                                 self.matched_count(),
-                                Self::list_items,
+                                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<usize>|
+                                     -> 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()
+                                    }
+                                },
                             )
                             .p_1()
                             .track_scroll(self.scroll_handle.clone())
@@ -596,7 +448,6 @@ pub struct PastThread {
     assistant_panel: WeakEntity<AssistantPanel>,
     selected: bool,
     highlight_positions: Vec<usize>,
-    timestamp_format: EntryTimeFormat,
 }
 
 impl PastThread {
@@ -605,14 +456,12 @@ impl PastThread {
         assistant_panel: WeakEntity<AssistantPanel>,
         selected: bool,
         highlight_positions: Vec<usize>,
-        timestamp_format: EntryTimeFormat,
     ) -> Self {
         Self {
             thread,
             assistant_panel,
             selected,
             highlight_positions,
-            timestamp_format,
         }
     }
 }
@@ -621,10 +470,13 @@ impl RenderOnce for PastThread {
     fn render(self, _window: &mut Window, cx: &mut App) -> impl IntoElement {
         let summary = self.thread.summary;
 
-        let thread_timestamp = self.timestamp_format.format_timestamp(
-            &self.assistant_panel,
-            self.thread.updated_at.timestamp(),
-            cx,
+        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,
         );
 
         ListItem::new(SharedString::from(self.thread.id.to_string()))
@@ -688,7 +540,6 @@ pub struct PastContext {
     assistant_panel: WeakEntity<AssistantPanel>,
     selected: bool,
     highlight_positions: Vec<usize>,
-    timestamp_format: EntryTimeFormat,
 }
 
 impl PastContext {
@@ -697,14 +548,12 @@ impl PastContext {
         assistant_panel: WeakEntity<AssistantPanel>,
         selected: bool,
         highlight_positions: Vec<usize>,
-        timestamp_format: EntryTimeFormat,
     ) -> Self {
         Self {
             context,
             assistant_panel,
             selected,
             highlight_positions,
-            timestamp_format,
         }
     }
 }
@@ -712,10 +561,13 @@ 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 = self.timestamp_format.format_timestamp(
-            &self.assistant_panel,
-            self.context.mtime.timestamp(),
-            cx,
+        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,
         );
 
         ListItem::new(SharedString::from(
@@ -775,137 +627,3 @@ impl RenderOnce for PastContext {
         })
     }
 }
-
-#[derive(Clone, Copy)]
-pub enum EntryTimeFormat {
-    DateAndTime,
-    TimeOnly,
-}
-
-impl EntryTimeFormat {
-    fn format_timestamp(
-        &self,
-        assistant_panel: &WeakEntity<AssistantPanel>,
-        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<TimeBucket> 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);
-    }
-}

crates/time_format/src/time_format.rs 🔗

@@ -42,82 +42,6 @@ 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(&timestamp);
-        }
-
-        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(&timestamp)
-        }
-    }
-    #[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(&timestamp)
-    }
-    #[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,
@@ -128,22 +52,22 @@ fn format_absolute_timestamp(
         if !enhanced_date_formatting {
             return format!(
                 "{} {}",
-                format_absolute_date(timestamp, reference, enhanced_date_formatting),
-                format_absolute_time(timestamp)
+                macos::format_date(&timestamp),
+                macos::format_time(&timestamp)
             );
         }
 
         let timestamp_date = timestamp.date();
         let reference_date = reference.date();
         if timestamp_date == reference_date {
-            format!("Today at {}", format_absolute_time(timestamp))
+            format!("Today at {}", macos::format_time(&timestamp))
         } else if reference_date.previous_day() == Some(timestamp_date) {
-            format!("Yesterday at {}", format_absolute_time(timestamp))
+            format!("Yesterday at {}", macos::format_time(&timestamp))
         } else {
             format!(
                 "{} {}",
-                format_absolute_date(timestamp, reference, enhanced_date_formatting),
-                format_absolute_time(timestamp)
+                macos::format_date(&timestamp),
+                macos::format_time(&timestamp)
             )
         }
     }
@@ -155,62 +79,13 @@ fn format_absolute_timestamp(
     }
 }
 
-fn format_absolute_date_medium(
-    timestamp: OffsetDateTime,
-    reference: OffsetDateTime,
-    enhanced_formatting: bool,
-) -> String {
-    #[cfg(target_os = "macos")]
-    {
-        if !enhanced_formatting {
-            return macos::format_date_medium(&timestamp);
-        }
-
-        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(&timestamp)
-        }
-    }
-    #[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,
+    #[allow(unused_variables)] reference: OffsetDateTime,
 ) -> String {
     #[cfg(target_os = "macos")]
     {
-        format_absolute_date_medium(timestamp, reference, false)
+        macos::format_date_medium(&timestamp)
     }
     #[cfg(not(target_os = "macos"))]
     {
@@ -303,9 +178,15 @@ 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.
-fn format_timestamp_naive_time(timestamp_local: OffsetDateTime, is_12_hour_time: bool) -> String {
+pub fn format_timestamp_naive(
+    timestamp_local: OffsetDateTime,
+    reference_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 {
@@ -325,103 +206,38 @@ fn format_timestamp_naive_time(timestamp_local: OffsetDateTime, is_12_hour_time:
         (timestamp_local_hour, None)
     };
 
-    match meridiem {
+    let formatted_time = match meridiem {
         Some(meridiem) => format!("{}:{:02} {}", hour, timestamp_local_minute, meridiem),
         None => format!("{:02}:{:02}", hour, timestamp_local_minute),
-    }
-}
-
-#[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!(
+    let formatted_date = match meridiem {
+        Some(_) => format!(
             "{:02}/{:02}/{}",
             timestamp_local_date.month() as u32,
             timestamp_local_date.day(),
             timestamp_local_date.year()
         ),
-        false => format!(
+        None => 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<String> = std::sync::OnceLock::new();
-
 #[cfg(not(target_os = "macos"))]
 fn format_timestamp_fallback(timestamp: OffsetDateTime, reference: OffsetDateTime) -> String {
+    static CURRENT_LOCALE: std::sync::OnceLock<String> = std::sync::OnceLock::new();
     let current_locale = CURRENT_LOCALE
         .get_or_init(|| sys_locale::get_locale().unwrap_or_else(|| String::from("en-US")));
 
@@ -429,8 +245,8 @@ fn format_timestamp_fallback(timestamp: OffsetDateTime, reference: OffsetDateTim
     format_timestamp_naive(timestamp, reference, is_12_hour_time)
 }
 
-/// Returns `true` if the locale is recognized as a 12-hour time locale.
 #[cfg(not(target_os = "macos"))]
+/// Returns `true` if the locale is recognized as a 12-hour time locale.
 fn is_12_hour_time_by_locale(locale: &str) -> bool {
     [
         "es-MX", "es-CO", "es-SV", "es-NI",
@@ -528,131 +344,6 @@ 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);