Panics · P02 Typography: Link takes its handler, an unknown zone reads as UTC and says so, a long highlight query logs

src/typography/fit.rs

@@ -127,16 +127,17 @@ fn cut_line(text: SharedString, fit: fn(&str, Pixels, &mut Window) -> String) ->
                     let style = window.text_style();
                     let line = style.line_height_in_pixels(window.rem_size());
                     let x = offset(style.text_align, bounds.size.width - shaped.width);
-                    shaped
-                        .paint(
-                            bounds.origin + point(x, Pixels::ZERO),
-                            line,
-                            TextAlign::Left,
-                            None,
-                            window,
-                            cx,
-                        )
-                        .expect("a cut line paints");
+                    let painted = shaped.paint(
+                        bounds.origin + point(x, Pixels::ZERO),
+                        line,
+                        TextAlign::Left,
+                        None,
+                        window,
+                        cx,
+                    );
+                    if let Err(error) = painted {
+                        log::error!("ellipsis: a cut line failed to paint: {error:#}");
+                    }
                 },
             )
             .absolute()

src/typography/format.rs

@@ -1,3 +1,5 @@
+use std::sync::atomic::{AtomicBool, Ordering};
+
 use jiff::{Timestamp, fmt::strtime, tz::TimeZone};
 
 /// Typographic minus; aligns with plus in tabular figures.
@@ -241,16 +243,24 @@ pub(crate) fn span(then: Timestamp, now: Timestamp) -> Option<(u64, &'static str
     Some((count, unit, delta < 0))
 }
 
-/// The system's time zone for `user`. An unknown zone stops here rather than reading as UTC.
+/// The system's zone; an unknown one logs once, naming `user`, and reads as jiff's unknown zone.
 pub(crate) fn system_zone(user: &str) -> TimeZone {
     TimeZone::try_system().unwrap_or_else(|error| {
-        panic!("{user}: the system time zone is unknown ({error}); pass a zone")
+        static TOLD: AtomicBool = AtomicBool::new(false);
+        if !TOLD.swap(true, Ordering::Relaxed) {
+            log::error!("{user}: the system time zone is unknown ({error}); times show in UTC");
+        }
+        TimeZone::unknown()
     })
 }
 
-/// strftime in `zone`; a bad pattern is an error.
+/// strftime in `zone`; a bad pattern is an error. The unknown zone says UTC.
 pub fn datetime(at: Timestamp, zone: &TimeZone, pattern: &str) -> Result<String, jiff::Error> {
-    strtime::format(pattern, &at.to_zoned(zone.clone()))
+    let text = strtime::format(pattern, &at.to_zoned(zone.clone()))?;
+    Ok(match zone.is_unknown() && !text.contains("UTC") {
+        true => format!("{text} UTC"),
+        false => text,
+    })
 }
 
 #[cfg(test)]
@@ -394,5 +404,18 @@ mod tests {
         let formatted = datetime(at, &TimeZone::UTC, "%Y-%m-%d %H:%M").unwrap();
         assert_eq!(formatted, "1970-01-01 00:00");
         assert!(datetime(at, &TimeZone::UTC, "%Y %").is_err());
+        let unknown = TimeZone::unknown();
+        assert_eq!(
+            datetime(at, &unknown, "%H:%M").expect("a pattern"),
+            "00:00 UTC"
+        );
+        let named = datetime(at, &unknown, "%H:%M %Z %%Z").expect("a pattern");
+        assert_eq!(named, "00:00 UTC %Z");
+        let escaped = datetime(at, &unknown, "%H:%M %%Z").expect("a pattern");
+        assert_eq!(escaped, "00:00 %Z UTC");
+        assert_eq!(
+            datetime(at, &TimeZone::UTC, "%H:%M").expect("a pattern"),
+            "00:00"
+        );
     }
 }

src/typography/inline.rs

@@ -187,13 +187,19 @@ impl Highlight {
         let ranges = if query.is_empty() {
             Vec::new()
         } else {
-            RegexBuilder::new(&regex::escape(query))
+            match RegexBuilder::new(&regex::escape(query))
                 .case_insensitive(true)
                 .build()
-                .expect("an escaped literal is a valid regex")
-                .find_iter(&text)
-                .map(|found| found.range())
-                .collect()
+            {
+                Ok(found) => found.find_iter(&text).map(|found| found.range()).collect(),
+                Err(error) => {
+                    log::error!(
+                        "highlight: no marks for a {}-byte query: {error}",
+                        query.len()
+                    );
+                    Vec::new()
+                }
+            }
         };
         Self { text, ranges }
     }
@@ -216,6 +222,12 @@ impl RenderOnce for Highlight {
 mod tests {
     use super::{Highlight, Range};
 
+    #[test]
+    fn a_query_past_the_regex_limit_marks_nothing() {
+        let long = "a".repeat(500_000);
+        assert!(Highlight::matching("aaa", &long).ranges.is_empty());
+    }
+
     #[test]
     fn matching_finds_every_case_insensitive_hit() {
         let found = Highlight::matching("Ely, ely, ELY and élan", "ely");

src/typography/link.rs

@@ -11,51 +11,34 @@ use crate::{
 
 type ClickHandler = Box<dyn Fn(&ClickEvent, &mut Window, &mut App) + 'static>;
 
-/// Inline link. Give it an `href` or an `on_click`.
+/// Inline link that runs `on_click`; `ExternalLink` opens an address.
 #[derive(IntoElement)]
 pub struct Link {
     id: ElementId,
     label: SharedString,
-    href: Option<SharedString>,
-    on_click: Option<ClickHandler>,
+    on_click: ClickHandler,
     external: bool,
 }
 
 impl Link {
-    pub fn new(id: impl Into<ElementId>, label: impl Into<SharedString>) -> Self {
+    pub fn new(
+        id: impl Into<ElementId>,
+        label: impl Into<SharedString>,
+        on_click: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static,
+    ) -> Self {
         Self {
             id: id.into(),
             label: label.into(),
-            href: None,
-            on_click: None,
+            on_click: Box::new(on_click),
             external: false,
         }
     }
-
-    /// Opens in the system browser.
-    pub fn href(mut self, url: impl Into<SharedString>) -> Self {
-        self.href = Some(url.into());
-        self
-    }
-
-    pub fn on_click(
-        mut self,
-        handler: impl Fn(&ClickEvent, &mut Window, &mut App) + 'static,
-    ) -> Self {
-        self.on_click = Some(Box::new(handler));
-        self
-    }
 }
 
 impl RenderOnce for Link {
     fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
         let theme = cx.theme();
         let color = theme.colors.link;
-        let action: ClickHandler = match (self.on_click, self.href) {
-            (Some(handler), _) => handler,
-            (None, Some(url)) => Box::new(move |_, _, cx| cx.open_url(&url)),
-            (None, None) => panic!("link {:?} has neither href nor on_click", self.id),
-        };
         div()
             .id(self.id)
             .flex()
@@ -71,7 +54,7 @@ impl RenderOnce for Link {
             .focus_ring(cx)
             .hover(|style| style.underline())
             .on_mouse_down(MouseButton::Left, |_, window, _| window.prevent_default())
-            .on_click(action)
+            .on_click(self.on_click)
             .child(self.label)
             .when(self.external, |link| {
                 link.child(
@@ -93,7 +76,8 @@ impl ExternalLink {
         label: impl Into<SharedString>,
         url: impl Into<SharedString>,
     ) -> Self {
-        let mut link = Link::new(id, label).href(url);
+        let url: SharedString = url.into();
+        let mut link = Link::new(id, label, move |_, _, cx| cx.open_url(&url));
         link.external = true;
         Self(link)
     }
@@ -104,3 +88,46 @@ impl RenderOnce for ExternalLink {
         self.0
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::{cell::Cell, rc::Rc};
+
+    use gpui::{
+        Context, IntoElement, KeyUpEvent, Keystroke, ParentElement, Render, TestAppContext, Window,
+        div,
+    };
+
+    use super::{ExternalLink, Link};
+    use crate::theme::Theme;
+
+    struct Links(Rc<Cell<u32>>);
+
+    impl Render for Links {
+        fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+            let presses = self.0.clone();
+            div()
+                .child(Link::new("guide", "guide", move |_, _, _| {
+                    presses.set(presses.get() + 1)
+                }))
+                .child(ExternalLink::new("site", "site", "https://example.com"))
+        }
+    }
+
+    #[gpui::test]
+    fn a_link_runs_its_handler_and_an_external_link_opens_its_address(cx: &mut TestAppContext) {
+        cx.update(Theme::init);
+        let presses = Rc::new(Cell::new(0));
+        let seen = presses.clone();
+        let (_, cx) = cx.add_window_view(|_, _| Links(seen));
+        for _ in 0..2 {
+            cx.update(|window, cx| window.focus_next(cx));
+            cx.simulate_keystrokes("enter");
+            cx.simulate_event(KeyUpEvent {
+                keystroke: Keystroke::parse("enter").expect("a key"),
+            });
+        }
+        assert_eq!(presses.get(), 1);
+        assert_eq!(cx.opened_url().as_deref(), Some("https://example.com"));
+    }
+}