From e2d880fb4faf81b8ac8e9d83ba223564e38c0a7a 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:49:35 -0400 Subject: [PATCH] =?UTF-8?q?Panics=20=C2=B7=20P06=20Forms:=20roots,=20stale?= =?UTF-8?q?=20selections,=20shaping=20faults=20and=20gone=20units=20log=20?= =?UTF-8?q?instead=20of=20panicking;=20cron=20runs=20say=20UTC=20in=20an?= =?UTF-8?q?=20unknown=20zone?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/forms/date/cron.rs | 47 +++++++++++++++++++++++---- src/forms/files.rs | 17 +++++++--- src/forms/tests/mod.rs | 1 + src/forms/tests/stale.rs | 47 +++++++++++++++++++++++++++ src/forms/text/element.rs | 67 ++++++++++++++++++++++----------------- src/forms/text/mod.rs | 16 +++++++--- src/forms/unit.rs | 15 +++++---- tasks/panics.md | 3 +- 8 files changed, 160 insertions(+), 53 deletions(-) create mode 100644 src/forms/tests/stale.rs diff --git a/src/forms/date/cron.rs b/src/forms/date/cron.rs index 0c6f875..55d8616 100644 --- a/src/forms/date/cron.rs +++ b/src/forms/date/cron.rs @@ -2,7 +2,7 @@ use gpui::{ App, ElementId, Entity, InteractiveElement, IntoElement, ParentElement, RenderOnce, Styled, Window, div, }; -use jiff::civil::DateTime; +use jiff::{civil::DateTime, tz::TimeZone}; use super::{ super::{Input, TextInput}, @@ -11,6 +11,7 @@ use super::{ use crate::{ buttons::{Button, ButtonVariant}, theme::{ActiveTheme, ControlSize, TextSize}, + typography::format::datetime, }; const PRESETS: [(&str, &str); 5] = [ @@ -54,11 +55,29 @@ impl CronEditor { } } +/// A run's time; in the system zone it goes through `datetime`, which says UTC there. +fn shown(at: DateTime, zone: Option<&TimeZone>) -> String { + const PATTERN: &str = "%a %b %-d, %H:%M"; + let Some(zone) = zone else { + return at.strftime(PATTERN).to_string(); + }; + let zoned = at + .to_zoned(zone.clone()) + .expect("a run within five years has a zone time"); + datetime(zoned.timestamp(), zone, PATTERN).expect("the pattern is fixed") +} + impl RenderOnce for CronEditor { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { let text = self.state.read(cx).text().trim().to_string(); let rule = (!text.is_empty()).then(|| CronRule::parse(&text)); - let now = self.now.unwrap_or_else(|| zoned_now().datetime()); + let (now, zone) = match self.now { + Some(now) => (now, None), + None => { + let zoned = zoned_now(); + (zoned.datetime(), Some(zoned.time_zone().clone())) + } + }; let theme = cx.theme(); let colors = &theme.colors; let presets = PRESETS.iter().enumerate().map(|(ix, (label, preset))| { @@ -83,10 +102,8 @@ impl RenderOnce for CronEditor { let runs = if next.is_empty() { "No run in the next five years.".to_string() } else { - let times: Vec = next - .iter() - .map(|at| at.strftime("%a %b %-d, %H:%M").to_string()) - .collect(); + let times: Vec = + next.iter().map(|at| shown(*at, zone.as_ref())).collect(); format!("Next: {}", times.join(" \u{00b7} ")) }; (rule.describe(), colors.fg, Some(runs)) @@ -117,3 +134,21 @@ impl RenderOnce for CronEditor { })) } } + +#[cfg(test)] +mod tests { + use jiff::{civil::date, tz::TimeZone}; + + use super::shown; + + #[test] + fn a_run_in_the_unknown_zone_says_utc() { + let at = date(2026, 10, 6).at(9, 30, 0, 0); + assert_eq!(shown(at, None), "Tue Oct 6, 09:30"); + assert_eq!(shown(at, Some(&TimeZone::UTC)), "Tue Oct 6, 09:30"); + assert_eq!( + shown(at, Some(&TimeZone::unknown())), + "Tue Oct 6, 09:30 UTC" + ); + } +} diff --git a/src/forms/files.rs b/src/forms/files.rs index 6dae1c7..f454661 100644 --- a/src/forms/files.rs +++ b/src/forms/files.rs @@ -120,11 +120,12 @@ fn files(multiple: bool) -> PathPromptOptions { } } +/// The file's name, or the whole path for a root such as `C:\`. fn file_name(path: &Path) -> String { - path.file_name() - .expect("a chosen file has a name") - .to_string_lossy() - .into_owned() + match path.file_name() { + Some(name) => name.to_string_lossy().into_owned(), + None => path.display().to_string(), + } } /// Chosen files in a field. A click opens the dialog; files dropped on it count too. @@ -347,7 +348,13 @@ impl RenderOnce for DropZone { mod tests { use std::fs; - use super::{PICTURES, dropped, taken}; + use super::{PICTURES, dropped, file_name, taken}; + + #[test] + fn a_root_shows_its_whole_path() { + assert_eq!(file_name(std::path::Path::new("/")), "/"); + assert_eq!(file_name(std::path::Path::new("/notes/plan.md")), "plan.md"); + } #[test] fn drops_take_files_and_refuse_folders_extras_and_nothing() { diff --git a/src/forms/tests/mod.rs b/src/forms/tests/mod.rs index e84aac0..1899257 100644 --- a/src/forms/tests/mod.rs +++ b/src/forms/tests/mod.rs @@ -2,6 +2,7 @@ mod choices; mod colors; mod dates; mod other; +mod stale; mod structure; mod tags; mod upload; diff --git a/src/forms/tests/stale.rs b/src/forms/tests/stale.rs new file mode 100644 index 0000000..d23fc59 --- /dev/null +++ b/src/forms/tests/stale.rs @@ -0,0 +1,47 @@ +use gpui::{AppContext, Context, IntoElement, Render, TestAppContext, Window}; + +use super::setup; +use crate::forms::{TextInput, UnitInput}; + +#[gpui::test] +fn a_selection_outside_the_text_or_inside_a_character_is_left(cx: &mut TestAppContext) { + setup(cx); + let (_, cx) = cx.add_window_view(|_, _| gpui::Empty); + let input = cx.update(|window, cx| cx.new(|cx| TextInput::new(window, cx))); + input.update(cx, |input, cx| { + input.set_text("héllo", cx); + input.select(0..1, cx); + input.select(0..2, cx); + input.select(3..99, cx); + assert_eq!(input.selection(), 0..1); + }); +} + +/// A unit field whose list may drop the chosen unit. +struct Units { + shortened: bool, +} + +impl Render for Units { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let units = if self.shortened { + vec!["em"] + } else { + vec!["px", "em"] + }; + UnitInput::new("length", 12.0, units, "px") + } +} + +#[gpui::test] +fn a_unit_gone_from_its_list_keeps_the_field(cx: &mut TestAppContext) { + setup(cx); + let (host, cx) = cx.add_window_view(|_, _| Units { shortened: false }); + cx.run_until_parked(); + host.update(cx, |host, cx| { + host.shortened = true; + cx.notify(); + }); + cx.run_until_parked(); + assert!(cx.debug_bounds("number-root").is_some()); +} diff --git a/src/forms/text/element.rs b/src/forms/text/element.rs index 32287e7..0fe3391 100644 --- a/src/forms/text/element.rs +++ b/src/forms/text/element.rs @@ -178,20 +178,23 @@ impl Element for TextElement { AvailableSpace::Definite(width) => Some(width), _ => None, }); - let lines = window - .text_system() - .shape_text( - SharedString::from(text.clone()), - font_size, - &runs, - width, - None, - ) - .expect("text input shaping failed"); - let rows: usize = lines - .iter() - .map(|line| line.wrap_boundaries().len() + 1) - .sum(); + let shaped = window.text_system().shape_text( + SharedString::from(text.clone()), + font_size, + &runs, + width, + None, + ); + let rows: usize = match shaped { + Ok(lines) => lines + .iter() + .map(|line| line.wrap_boundaries().len() + 1) + .sum(), + Err(error) => { + log::error!("text input: shaping failed: {error:#}"); + 1 + } + }; size( width.unwrap_or_default(), line_height * rows.clamp(min, max) as f32, @@ -220,7 +223,10 @@ impl Element for TextElement { let shaped = window .text_system() .shape_text(text.clone().into(), font_size, &runs, wrap, None) - .expect("text input shaping failed"); + .unwrap_or_else(|error| { + log::error!("text input: shaping failed: {error:#}"); + Default::default() + }); let lines: Rc<[(usize, WrappedLine)]> = line_starts(&text).into_iter().zip(shaped).collect(); let focused = input.focus().is_focused(window); @@ -234,10 +240,11 @@ impl Element for TextElement { }); }); let input = self.input.read(cx); - let caret_at = input - .position_for(input.display_offset(input.cursor())) - .expect("caret offset lies inside the laid-out text"); - let scroll = keep_in_view(input, &lines, caret_at, caret_width, bounds, line_height); + let caret_at = input.position_for(input.display_offset(input.cursor())); + let scroll = match caret_at { + Some(at) => keep_in_view(input, &lines, at, caret_width, bounds, line_height), + None => input.scroll, + }; let selection = input.selection(); let selections = if placeholder || selection.is_empty() { Vec::new() @@ -251,12 +258,9 @@ impl Element for TextElement { .map(|rect| Bounds::new(rect.origin + bounds.origin - scroll, rect.size)) .collect() }; - let caret = (focused && caret_on && selection.is_empty()).then(|| { - Bounds::new( - bounds.origin + caret_at - scroll, - size(caret_width, line_height), - ) - }); + let caret = caret_at + .filter(|_| focused && caret_on && selection.is_empty()) + .map(|at| Bounds::new(bounds.origin + at - scroll, size(caret_width, line_height))); self.input.update(cx, |input, _| input.scroll = scroll); Prepaint { lines, @@ -293,15 +297,20 @@ impl Element for TextElement { top += line.size(line_height).height; } for ((_, line), origin) in prepaint.lines.iter().zip(&origins) { - line.paint_background(*origin, line_height, TextAlign::Left, None, window, cx) - .expect("text input wash paint failed"); + let washed = + line.paint_background(*origin, line_height, TextAlign::Left, None, window, cx); + if let Err(error) = washed { + log::error!("text input: wash paint failed: {error:#}"); + } } for rect in prepaint.selections.drain(..) { window.paint_quad(fill(rect, prepaint.selection_color)); } for ((_, line), origin) in prepaint.lines.iter().zip(&origins) { - line.paint(*origin, line_height, TextAlign::Left, None, window, cx) - .expect("text input paint failed"); + let painted = line.paint(*origin, line_height, TextAlign::Left, None, window, cx); + if let Err(error) = painted { + log::error!("text input: paint failed: {error:#}"); + } } if let Some(caret) = prepaint.caret.take() { window.paint_quad(fill(caret, prepaint.caret_color)); diff --git a/src/forms/text/mod.rs b/src/forms/text/mod.rs index b16247b..4062a98 100644 --- a/src/forms/text/mod.rs +++ b/src/forms/text/mod.rs @@ -266,12 +266,18 @@ impl TextInput { self.replace(self.selection.clone(), text, false, cx); } + /// Selects `range`; one outside the text or inside a character is logged and left. pub fn select(&mut self, range: Range, cx: &mut Context) { - assert!( - range.start <= range.end && range.end <= self.text.len(), - "selection {range:?} is outside {} bytes", - self.text.len() - ); + let fits = range.start <= range.end + && self.text.is_char_boundary(range.start) + && self.text.is_char_boundary(range.end); + if !fits { + log::error!( + "text input: no selection {range:?} in {} bytes", + self.text.len() + ); + return; + } self.selection = range; self.reversed = false; self.restart_blink(cx); diff --git a/src/forms/unit.rs b/src/forms/unit.rs index 1340161..55417a4 100644 --- a/src/forms/unit.rs +++ b/src/forms/unit.rs @@ -28,7 +28,7 @@ impl UnitInput { ) -> Self { let units: Vec = units.into_iter().map(Into::into).collect(); let unit = unit.into(); - assert!(units.contains(&unit), "unit {unit} is not among {units:?}"); + assert!(!units.is_empty(), "unit input has no units"); Self { field: NumberInput::new(id, value), units, @@ -65,12 +65,13 @@ impl UnitInput { impl RenderOnce for UnitInput { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { let theme = cx.theme(); - let position = self - .units - .iter() - .position(|unit| *unit == self.unit) - .expect("checked at construction"); - let next = self.units[(position + 1) % self.units.len()].clone(); + let next = match self.units.iter().position(|unit| *unit == self.unit) { + Some(at) => self.units[(at + 1) % self.units.len()].clone(), + None => { + log::error!("unit input: {} is not among {:?}", self.unit, self.units); + self.units[0].clone() + } + }; let on_unit = self.on_unit; let hover = theme.colors.hover; let unit = div() diff --git a/tasks/panics.md b/tasks/panics.md index a9dd153..d770821 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -20,7 +20,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P03 Layout — `src/layout` - [x] P04 Window & Shell — `src/shell` - [x] P05 Buttons & Actions — `src/buttons` -- [ ] P06 Forms — `src/forms` +- [x] P06 Forms — `src/forms` - [ ] P07 Navigation — `src/navigation` - [ ] P08 Menus — `src/menus` - [ ] P09 Overlays — `src/overlays` @@ -70,3 +70,4 @@ One line per item: what changed, what stayed and why, review rounds. - P03: `PaneGroup` leaves a gone or last pane and logs (`split` returns `Option`); a stale split path sets nothing; `SplitPane` splits evenly when sizes lag or fail, seeds by the owner's raw sizes, and drops a drag over another pane count; masonry sorts with `total_cmp`. Kept: dock ids (panels never leave), one-value contracts, pane-id overflow, workspace JSON. Review: 2 rounds. - P04: no change. Kept: splash progress and column count (caller contracts), the objc class under `Once`, `Tray::pick` (items fixed at creation). Review: 1 round. - P05: a `SegmentedControl` whose choice left its segments marks none; the cause, `motion::slide`, logs and returns no marker (P11 inherits it). Kept: one-value contracts (group and quick-action counts, toggle label or icon, share payload). Review: 1 round. +- P06: a root path shows whole; `TextInput::select` logs a range past the text or inside a character; the text element logs shaping and paint faults; a unit gone from its list logs; cron runs in the system zone go through `datetime`. Kept: empty choice lists (the owner disables, as AGENTS.md says), one-value contracts, facts by construction. Review: 2 rounds.