Panics · P06 Forms: roots, stale selections, shaping faults and gone units log instead of panicking; cron runs say UTC in an unknown zone
This commit is contained in:
1 parent
73a8bf2e34
commit
e2d880fb4f
8 files changed
+160
-53
No files matched your search
+41
-6
@@ -2,7 +2,7 @@ use gpui::{
|
|||||||
App, ElementId, Entity, InteractiveElement, IntoElement, ParentElement, RenderOnce, Styled,
|
App, ElementId, Entity, InteractiveElement, IntoElement, ParentElement, RenderOnce, Styled,
|
||||||
Window, div,
|
Window, div,
|
||||||
};
|
};
|
||||||
use jiff::civil::DateTime;
|
use jiff::{civil::DateTime, tz::TimeZone};
|
||||||
|
|
||||||
use super::{
|
use super::{
|
||||||
super::{Input, TextInput},
|
super::{Input, TextInput},
|
||||||
@@ -11,6 +11,7 @@ use super::{
|
|||||||
use crate::{
|
use crate::{
|
||||||
buttons::{Button, ButtonVariant},
|
buttons::{Button, ButtonVariant},
|
||||||
theme::{ActiveTheme, ControlSize, TextSize},
|
theme::{ActiveTheme, ControlSize, TextSize},
|
||||||
|
typography::format::datetime,
|
||||||
};
|
};
|
||||||
|
|
||||||
const PRESETS: [(&str, &str); 5] = [
|
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 {
|
impl RenderOnce for CronEditor {
|
||||||
fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
|
fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
|
||||||
let text = self.state.read(cx).text().trim().to_string();
|
let text = self.state.read(cx).text().trim().to_string();
|
||||||
let rule = (!text.is_empty()).then(|| CronRule::parse(&text));
|
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 theme = cx.theme();
|
||||||
let colors = &theme.colors;
|
let colors = &theme.colors;
|
||||||
let presets = PRESETS.iter().enumerate().map(|(ix, (label, preset))| {
|
let presets = PRESETS.iter().enumerate().map(|(ix, (label, preset))| {
|
||||||
@@ -83,10 +102,8 @@ impl RenderOnce for CronEditor {
|
|||||||
let runs = if next.is_empty() {
|
let runs = if next.is_empty() {
|
||||||
"No run in the next five years.".to_string()
|
"No run in the next five years.".to_string()
|
||||||
} else {
|
} else {
|
||||||
let times: Vec<String> = next
|
let times: Vec<String> =
|
||||||
.iter()
|
next.iter().map(|at| shown(*at, zone.as_ref())).collect();
|
||||||
.map(|at| at.strftime("%a %b %-d, %H:%M").to_string())
|
|
||||||
.collect();
|
|
||||||
format!("Next: {}", times.join(" \u{00b7} "))
|
format!("Next: {}", times.join(" \u{00b7} "))
|
||||||
};
|
};
|
||||||
(rule.describe(), colors.fg, Some(runs))
|
(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"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
+12
-5
@@ -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 {
|
fn file_name(path: &Path) -> String {
|
||||||
path.file_name()
|
match path.file_name() {
|
||||||
.expect("a chosen file has a name")
|
Some(name) => name.to_string_lossy().into_owned(),
|
||||||
.to_string_lossy()
|
None => path.display().to_string(),
|
||||||
.into_owned()
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Chosen files in a field. A click opens the dialog; files dropped on it count too.
|
/// 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 {
|
mod tests {
|
||||||
use std::fs;
|
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]
|
#[test]
|
||||||
fn drops_take_files_and_refuse_folders_extras_and_nothing() {
|
fn drops_take_files_and_refuse_folders_extras_and_nothing() {
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ mod choices;
|
|||||||
mod colors;
|
mod colors;
|
||||||
mod dates;
|
mod dates;
|
||||||
mod other;
|
mod other;
|
||||||
|
mod stale;
|
||||||
mod structure;
|
mod structure;
|
||||||
mod tags;
|
mod tags;
|
||||||
mod upload;
|
mod upload;
|
||||||
|
|||||||
@@ -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<Self>) -> 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());
|
||||||
|
}
|
||||||
+38
-29
@@ -178,20 +178,23 @@ impl Element for TextElement {
|
|||||||
AvailableSpace::Definite(width) => Some(width),
|
AvailableSpace::Definite(width) => Some(width),
|
||||||
_ => None,
|
_ => None,
|
||||||
});
|
});
|
||||||
let lines = window
|
let shaped = window.text_system().shape_text(
|
||||||
.text_system()
|
SharedString::from(text.clone()),
|
||||||
.shape_text(
|
font_size,
|
||||||
SharedString::from(text.clone()),
|
&runs,
|
||||||
font_size,
|
width,
|
||||||
&runs,
|
None,
|
||||||
width,
|
);
|
||||||
None,
|
let rows: usize = match shaped {
|
||||||
)
|
Ok(lines) => lines
|
||||||
.expect("text input shaping failed");
|
.iter()
|
||||||
let rows: usize = lines
|
.map(|line| line.wrap_boundaries().len() + 1)
|
||||||
.iter()
|
.sum(),
|
||||||
.map(|line| line.wrap_boundaries().len() + 1)
|
Err(error) => {
|
||||||
.sum();
|
log::error!("text input: shaping failed: {error:#}");
|
||||||
|
1
|
||||||
|
}
|
||||||
|
};
|
||||||
size(
|
size(
|
||||||
width.unwrap_or_default(),
|
width.unwrap_or_default(),
|
||||||
line_height * rows.clamp(min, max) as f32,
|
line_height * rows.clamp(min, max) as f32,
|
||||||
@@ -220,7 +223,10 @@ impl Element for TextElement {
|
|||||||
let shaped = window
|
let shaped = window
|
||||||
.text_system()
|
.text_system()
|
||||||
.shape_text(text.clone().into(), font_size, &runs, wrap, None)
|
.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)]> =
|
let lines: Rc<[(usize, WrappedLine)]> =
|
||||||
line_starts(&text).into_iter().zip(shaped).collect();
|
line_starts(&text).into_iter().zip(shaped).collect();
|
||||||
let focused = input.focus().is_focused(window);
|
let focused = input.focus().is_focused(window);
|
||||||
@@ -234,10 +240,11 @@ impl Element for TextElement {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
let input = self.input.read(cx);
|
let input = self.input.read(cx);
|
||||||
let caret_at = input
|
let caret_at = input.position_for(input.display_offset(input.cursor()));
|
||||||
.position_for(input.display_offset(input.cursor()))
|
let scroll = match caret_at {
|
||||||
.expect("caret offset lies inside the laid-out text");
|
Some(at) => keep_in_view(input, &lines, at, caret_width, bounds, line_height),
|
||||||
let scroll = keep_in_view(input, &lines, caret_at, caret_width, bounds, line_height);
|
None => input.scroll,
|
||||||
|
};
|
||||||
let selection = input.selection();
|
let selection = input.selection();
|
||||||
let selections = if placeholder || selection.is_empty() {
|
let selections = if placeholder || selection.is_empty() {
|
||||||
Vec::new()
|
Vec::new()
|
||||||
@@ -251,12 +258,9 @@ impl Element for TextElement {
|
|||||||
.map(|rect| Bounds::new(rect.origin + bounds.origin - scroll, rect.size))
|
.map(|rect| Bounds::new(rect.origin + bounds.origin - scroll, rect.size))
|
||||||
.collect()
|
.collect()
|
||||||
};
|
};
|
||||||
let caret = (focused && caret_on && selection.is_empty()).then(|| {
|
let caret = caret_at
|
||||||
Bounds::new(
|
.filter(|_| focused && caret_on && selection.is_empty())
|
||||||
bounds.origin + caret_at - scroll,
|
.map(|at| Bounds::new(bounds.origin + at - scroll, size(caret_width, line_height)));
|
||||||
size(caret_width, line_height),
|
|
||||||
)
|
|
||||||
});
|
|
||||||
self.input.update(cx, |input, _| input.scroll = scroll);
|
self.input.update(cx, |input, _| input.scroll = scroll);
|
||||||
Prepaint {
|
Prepaint {
|
||||||
lines,
|
lines,
|
||||||
@@ -293,15 +297,20 @@ impl Element for TextElement {
|
|||||||
top += line.size(line_height).height;
|
top += line.size(line_height).height;
|
||||||
}
|
}
|
||||||
for ((_, line), origin) in prepaint.lines.iter().zip(&origins) {
|
for ((_, line), origin) in prepaint.lines.iter().zip(&origins) {
|
||||||
line.paint_background(*origin, line_height, TextAlign::Left, None, window, cx)
|
let washed =
|
||||||
.expect("text input wash paint failed");
|
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(..) {
|
for rect in prepaint.selections.drain(..) {
|
||||||
window.paint_quad(fill(rect, prepaint.selection_color));
|
window.paint_quad(fill(rect, prepaint.selection_color));
|
||||||
}
|
}
|
||||||
for ((_, line), origin) in prepaint.lines.iter().zip(&origins) {
|
for ((_, line), origin) in prepaint.lines.iter().zip(&origins) {
|
||||||
line.paint(*origin, line_height, TextAlign::Left, None, window, cx)
|
let painted = line.paint(*origin, line_height, TextAlign::Left, None, window, cx);
|
||||||
.expect("text input paint failed");
|
if let Err(error) = painted {
|
||||||
|
log::error!("text input: paint failed: {error:#}");
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if let Some(caret) = prepaint.caret.take() {
|
if let Some(caret) = prepaint.caret.take() {
|
||||||
window.paint_quad(fill(caret, prepaint.caret_color));
|
window.paint_quad(fill(caret, prepaint.caret_color));
|
||||||
|
|||||||
+11
-5
@@ -266,12 +266,18 @@ impl TextInput {
|
|||||||
self.replace(self.selection.clone(), text, false, cx);
|
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<usize>, cx: &mut Context<Self>) {
|
pub fn select(&mut self, range: Range<usize>, cx: &mut Context<Self>) {
|
||||||
assert!(
|
let fits = range.start <= range.end
|
||||||
range.start <= range.end && range.end <= self.text.len(),
|
&& self.text.is_char_boundary(range.start)
|
||||||
"selection {range:?} is outside {} bytes",
|
&& self.text.is_char_boundary(range.end);
|
||||||
self.text.len()
|
if !fits {
|
||||||
);
|
log::error!(
|
||||||
|
"text input: no selection {range:?} in {} bytes",
|
||||||
|
self.text.len()
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
self.selection = range;
|
self.selection = range;
|
||||||
self.reversed = false;
|
self.reversed = false;
|
||||||
self.restart_blink(cx);
|
self.restart_blink(cx);
|
||||||
|
|||||||
+8
-7
@@ -28,7 +28,7 @@ impl UnitInput {
|
|||||||
) -> Self {
|
) -> Self {
|
||||||
let units: Vec<SharedString> = units.into_iter().map(Into::into).collect();
|
let units: Vec<SharedString> = units.into_iter().map(Into::into).collect();
|
||||||
let unit = unit.into();
|
let unit = unit.into();
|
||||||
assert!(units.contains(&unit), "unit {unit} is not among {units:?}");
|
assert!(!units.is_empty(), "unit input has no units");
|
||||||
Self {
|
Self {
|
||||||
field: NumberInput::new(id, value),
|
field: NumberInput::new(id, value),
|
||||||
units,
|
units,
|
||||||
@@ -65,12 +65,13 @@ impl UnitInput {
|
|||||||
impl RenderOnce for UnitInput {
|
impl RenderOnce for UnitInput {
|
||||||
fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
|
fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
|
||||||
let theme = cx.theme();
|
let theme = cx.theme();
|
||||||
let position = self
|
let next = match self.units.iter().position(|unit| *unit == self.unit) {
|
||||||
.units
|
Some(at) => self.units[(at + 1) % self.units.len()].clone(),
|
||||||
.iter()
|
None => {
|
||||||
.position(|unit| *unit == self.unit)
|
log::error!("unit input: {} is not among {:?}", self.unit, self.units);
|
||||||
.expect("checked at construction");
|
self.units[0].clone()
|
||||||
let next = self.units[(position + 1) % self.units.len()].clone();
|
}
|
||||||
|
};
|
||||||
let on_unit = self.on_unit;
|
let on_unit = self.on_unit;
|
||||||
let hover = theme.colors.hover;
|
let hover = theme.colors.hover;
|
||||||
let unit = div()
|
let unit = div()
|
||||||
|
|||||||
+2
-1
@@ -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] P03 Layout — `src/layout`
|
||||||
- [x] P04 Window & Shell — `src/shell`
|
- [x] P04 Window & Shell — `src/shell`
|
||||||
- [x] P05 Buttons & Actions — `src/buttons`
|
- [x] P05 Buttons & Actions — `src/buttons`
|
||||||
- [ ] P06 Forms — `src/forms`
|
- [x] P06 Forms — `src/forms`
|
||||||
- [ ] P07 Navigation — `src/navigation`
|
- [ ] P07 Navigation — `src/navigation`
|
||||||
- [ ] P08 Menus — `src/menus`
|
- [ ] P08 Menus — `src/menus`
|
||||||
- [ ] P09 Overlays — `src/overlays`
|
- [ ] 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.
|
- 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.
|
- 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.
|
- 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.
|
||||||
Reference in new issue
Block a user