From f46cc4e7c30400a32a4c5877cfcc5223279d628e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Tue, 6 Oct 2026 16:02:40 -0400 Subject: [PATCH] =?UTF-8?q?Panics=20=C2=B7=20P02=20Typography:=20Link=20ta?= =?UTF-8?q?kes=20its=20handler,=20an=20unknown=20zone=20reads=20as=20UTC?= =?UTF-8?q?=20and=20says=20so,=20a=20long=20highlight=20query=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- AGENTS.md | 2 +- examples/gallery/pages/typography/text.rs | 6 +- src/account/factor.rs | 9 +-- src/account/login.rs | 9 ++- src/account/mail.rs | 4 +- src/forms/date/mod.rs | 2 +- src/typography/fit.rs | 21 +++--- src/typography/format.rs | 31 +++++++-- src/typography/inline.rs | 22 ++++-- src/typography/link.rs | 81 +++++++++++++++-------- tasks/panics.md | 5 +- 11 files changed, 132 insertions(+), 60 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index af61aa5..e5a0cf5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -190,7 +190,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - The gallery's pictures go through `asset!` and `ui::picture` or `ui::resource`: a file natively, on the web an address under `ELY_GALLERY_ASSETS`, since gpui reads no files there. - On the web a paste reaches the focused field as the browser's paste event, through `EntityInputHandler::paste`, so Paste stays unbound there (`TextInput`, `CodeEditor`). Code that reads the clipboard itself awaits `read_from_clipboard_async`: gpui_web's `read_from_clipboard` answers at once and finds nothing (`tables::DataGrid`'s Cmd-V). - The first frame wears the host's mode through `Theme::set_mode_now`, a mode sent before the window opens included; `set_mode` fades from the colors on screen. -- The system time zone comes from `typography::format::system_zone`, which fails loud; jiff's `TimeZone::system()` falls back to UTC in silence. +- The system time zone comes from `typography::format::system_zone`: an unknown zone logs one error naming the first component that asked and reads as jiff's `TimeZone::unknown()`, which is UTC. A time shown in the system zone goes through `typography::format::datetime`, which adds " UTC" in the unknown zone, so the screen never claims local time. jiff's `TimeZone::system()` falls back to UTC in silence. - A Select with nothing to pick stays shut: disabled, it shows its placeholder; open, it fails (`media::DeviceSelector` says no device was found). - A list opens with its cursor on the current choice: Select on the chosen row, Combobox on its value, MultiSelect on the first one ticked, SelectableList on its first selected row. A selection the owner makes, not the echo of the list's own pick, moves SelectableList's cursor there. - An event repeats as RFC 5545 has it: weeks count from the start's own week, and a month or a year without the start's date is passed over (`calendar::Recurrence`). diff --git a/examples/gallery/pages/typography/text.rs b/examples/gallery/pages/typography/text.rs index 46d9eb1..4491946 100644 --- a/examples/gallery/pages/typography/text.rs +++ b/examples/gallery/pages/typography/text.rs @@ -211,9 +211,9 @@ pub fn link(cx: &App) -> impl IntoElement + use<> { .child( row() .child("Read the") - .child( - Link::new("guide", "style guide").on_click(|_, _, _| log::info!("gallery: link")), - ) + .child(Link::new("guide", "style guide", |_, _, _| { + log::info!("gallery: link") + })) .child("before you start."), ) } diff --git a/src/account/factor.rs b/src/account/factor.rs index 5d8e69c..ff3d509 100644 --- a/src/account/factor.rs +++ b/src/account/factor.rs @@ -113,10 +113,11 @@ impl RenderOnce for TwoFactorInput { ) .children(note) .children(self.on_recovery.map(|run| { - div().child( - Link::new((id.clone(), "recovery"), "Use a recovery code") - .on_click(move |_, window, cx| run(window, cx)), - ) + div().child(Link::new( + (id.clone(), "recovery"), + "Use a recovery code", + move |_, window, cx| run(window, cx), + )) })) } } diff --git a/src/account/login.rs b/src/account/login.rs index d54d312..f707fbf 100644 --- a/src/account/login.rs +++ b/src/account/login.rs @@ -48,7 +48,7 @@ pub(super) fn aside( .text_size(theme.text_size(TextSize::Sm)) .text_color(theme.colors.fg_muted) .child(words) - .child(Link::new(id, link).on_click(move |_, window, cx| run(window, cx))) + .child(Link::new(id, link, move |_, window, cx| run(window, cx))) } /// What a sign-in hands its owner. @@ -170,8 +170,11 @@ impl RenderOnce for LoginForm { }), ) .children(self.on_forgot.map(|run| { - Link::new((id.clone(), "forgot"), "Forgot password?") - .on_click(move |_, window, cx| run(window, cx)) + Link::new( + (id.clone(), "forgot"), + "Forgot password?", + move |_, window, cx| run(window, cx), + ) })), ) .children( diff --git a/src/account/mail.rs b/src/account/mail.rs index 6b9be6b..37078f4 100644 --- a/src/account/mail.rs +++ b/src/account/mail.rs @@ -72,7 +72,9 @@ fn email_step(step: Step, window: &mut Window, cx: &mut App) -> AnyElement { ) }; let back = back.map(|(label, run)| { - Link::new((id.clone(), "back"), label).on_click(move |_, window, cx| run(window, cx)) + Link::new((id.clone(), "back"), label, move |_, window, cx| { + run(window, cx) + }) }); if sent { let other = other.map(|(label, run)| { diff --git a/src/forms/date/mod.rs b/src/forms/date/mod.rs index ad18bc3..7a16ba9 100644 --- a/src/forms/date/mod.rs +++ b/src/forms/date/mod.rs @@ -28,7 +28,7 @@ pub use time::{DateTimePicker, DurationPicker, TimePicker}; pub use week::WeekPicker; pub use zone::TimezoneSelect; -/// The system's clock in its own zone. An unknown zone stops here, not in UTC. +/// The system's clock in its own zone, UTC when that zone is unknown. pub(crate) fn zoned_now() -> Zoned { Timestamp::now().to_zoned(system_zone("date pickers")) } diff --git a/src/typography/fit.rs b/src/typography/fit.rs index b5a5227..b0a7fd3 100644 --- a/src/typography/fit.rs +++ b/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() diff --git a/src/typography/format.rs b/src/typography/format.rs index dee1d97..790acb1 100644 --- a/src/typography/format.rs +++ b/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 { - 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" + ); } } diff --git a/src/typography/inline.rs b/src/typography/inline.rs index 76ddfb0..df97ea3 100644 --- a/src/typography/inline.rs +++ b/src/typography/inline.rs @@ -187,13 +187,19 @@ impl Highlight { let ranges = if query.is_empty() { Vec::new() } else { - RegexBuilder::new(®ex::escape(query)) + match RegexBuilder::new(®ex::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"); diff --git a/src/typography/link.rs b/src/typography/link.rs index 06c4de2..6dc4f93 100644 --- a/src/typography/link.rs +++ b/src/typography/link.rs @@ -11,51 +11,34 @@ use crate::{ type ClickHandler = Box; -/// 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, - on_click: Option, + on_click: ClickHandler, external: bool, } impl Link { - pub fn new(id: impl Into, label: impl Into) -> Self { + pub fn new( + id: impl Into, + label: impl Into, + 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) -> 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, url: impl Into, ) -> 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>); + + impl Render for Links { + fn render(&mut self, _: &mut Window, _: &mut Context) -> 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")); + } +} diff --git a/tasks/panics.md b/tasks/panics.md index 85ff3ae..020b087 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -7,6 +7,8 @@ Rules, at the owner's word: - A handler a component needs is an argument of `new`. Leaving it out fails to compile, not at render. - A state live data or the outside world can reach (a key gone from a list that changed, a failed load) logs an error naming the owner and draws a visible state: nothing chosen, or an error line in the component's box. The app keeps running. - An assert on a caller's argument, or on a fact true by construction, stays, as std's do. One that runtime data or input can trip is fixed. Reachable means what a component meets on its own: input inside it, its own later state, a call to the outside, or two arguments that agree at one time and not the next (a selected key and its list). One value checked against itself (a ratio above zero) is the caller's contract. +- An unknown system time zone logs once and reads as UTC; every time shown in the system zone goes through `format::datetime`, which says "UTC" then. Each item routes its own times there. +- A paint call that fails inside gpui (glyphs, the sprite atlas) logs an error naming the component and skips that draw; drawing an error state would meet the same fault. Each item reads every site in its folders, fixes what the rules name, updates callers in the gallery, docs examples and tests, passes `scripts/check.sh`, and passes a codex review (`gpt-6.1-sol`, effort `high`) in a herdr split, five rounds at most. Then it commits and pushes. @@ -14,7 +16,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P00 Policy in AGENTS.md - [x] P01 Primitives — `src/primitives`, `src/lib.rs`, `src/assets.rs` -- [ ] P02 Typography — `src/typography` +- [x] P02 Typography — `src/typography` - [ ] P03 Layout — `src/layout` - [ ] P04 Window & Shell — `src/shell` - [ ] P05 Buttons & Actions — `src/buttons` @@ -64,3 +66,4 @@ Each item reads every site in its folders, fixes what the rules name, updates ca One line per item: what changed, what stayed and why, review rounds. - P00: AGENTS.md holds the three rules; this list follows the PRD's chapters. Review: 2 rounds (round 1 found the runtime clause missing). - P01: `init` returns `anyhow::Result`; a missing app icon logs once and shows `Icon::broken`. Kept: `layer.rs` expects (gpui's element order), `checked_ratio` (one value, the caller's). Review: 2 rounds. +- P02: `Link::new` takes its handler; `system_zone` logs once and reads as jiff's unknown zone, which `datetime` marks UTC; an oversized highlight query logs and marks nothing; a cut line's paint fault logs. Kept: caller contracts (speed, digits, step, keystroke, heading level, pattern) and the exponent parse. Review: 3 rounds.