From 0bb3f4e8581303ac68b9f3bc2f594f2991eb460a 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 21:34:44 -0400 Subject: [PATCH] =?UTF-8?q?Panics=20=C2=B7=20P18=20Terminal:=20paint=20fau?= =?UTF-8?q?lts=20log,=20a=20pty=20side=20caps=20at=20what=20it=20holds,=20?= =?UTF-8?q?a=20default=20shell=20past=20the=20list=20keeps=20the=20last?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/terminal/paint.rs | 30 +++++++++++++++++++++--------- src/terminal/pty.rs | 32 ++++++++++++++++++++++++++++---- src/terminal/shells.rs | 28 ++++++++++++++++++++++++---- src/terminal/tests.rs | 31 +++++++++++++++++++++++++++++++ src/terminal/view.rs | 13 ++++++++----- tasks/panics.md | 3 ++- 6 files changed, 114 insertions(+), 23 deletions(-) diff --git a/src/terminal/paint.rs b/src/terminal/paint.rs index 865d455..ac79737 100644 --- a/src/terminal/paint.rs +++ b/src/terminal/paint.rs @@ -100,9 +100,17 @@ pub(crate) fn grid( &runs, Some(cell.width), ); - shaped - .paint_background(at(ix, 0), cell.height, TextAlign::Left, None, window, cx) - .expect("a terminal row's ground paints"); + let ground = shaped.paint_background( + at(ix, 0), + cell.height, + TextAlign::Left, + None, + window, + cx, + ); + if let Err(error) = ground { + log::error!("terminal: row {ix}'s ground failed to paint: {error:#}"); + } for (_, columns, why) in frame.lit.iter().filter(|(row, ..)| *row == ix) { let wash = match why { Lit::Selected => look.selection, @@ -112,9 +120,11 @@ pub(crate) fn grid( let extent = size(cell.width * columns.len() as f32, cell.height); window.paint_quad(fill(Bounds::new(at(ix, columns.start), extent), wash)); } - shaped - .paint(at(ix, 0), cell.height, TextAlign::Left, None, window, cx) - .expect("a terminal row paints"); + if let Err(error) = + shaped.paint(at(ix, 0), cell.height, TextAlign::Left, None, window, cx) + { + log::error!("terminal: row {ix} failed to paint: {error:#}"); + } for (column, cluster, style) in &row.clusters { let run = TextRun { len: cluster.len(), @@ -124,7 +134,7 @@ pub(crate) fn grid( underline: None, strikethrough: None, }; - window + let painted = window .text_system() .shape_line(cluster.clone().into(), look.size, &[run], None) .paint( @@ -134,8 +144,10 @@ pub(crate) fn grid( None, window, cx, - ) - .expect("a terminal cluster paints"); + ); + if let Err(error) = painted { + log::error!("terminal: a cluster in row {ix} failed to paint: {error:#}"); + } } } if let Some((row, columns, _)) = &hovered { diff --git a/src/terminal/pty.rs b/src/terminal/pty.rs index 30591c5..6759f24 100644 --- a/src/terminal/pty.rs +++ b/src/terminal/pty.rs @@ -99,16 +99,40 @@ pub(crate) fn spawn(launch: &Launch, size: TermSize, cell: (u16, u16)) -> anyhow } pub(crate) fn window_size(size: &TermSize, cell: (u16, u16)) -> WindowSize { - let cells = - |count: usize| u16::try_from(count).expect("a terminal is under 65536 cells a side"); + let cells = |count: usize, side: u16| { + let most = (i16::MAX as u16).min(u16::MAX / side.max(1)); + let kept = u16::try_from(count).unwrap_or(u16::MAX).min(most); + if usize::from(kept) < count { + log::error!("terminal: {count} cells of {side} px pass what a pty holds; {kept} kept"); + } + kept + }; WindowSize { - num_lines: cells(size.screen_lines), - num_cols: cells(size.columns), + num_lines: cells(size.screen_lines, cell.1), + num_cols: cells(size.columns, cell.0), cell_width: cell.0, cell_height: cell.1, } } +#[cfg(test)] +mod sizes { + use alacritty_terminal::term::test::TermSize; + + use super::window_size; + + #[test] + fn more_cells_than_a_pty_holds_cap_at_its_most() { + let size = window_size(&TermSize::new(70_000, 24), (8, 16)); + assert_eq!((size.num_cols, size.num_lines), (u16::MAX / 8, 24)); + let tiny = window_size(&TermSize::new(70_000, 70_000), (1, 1)); + assert_eq!( + (tiny.num_cols, tiny.num_lines), + (i16::MAX as u16, i16::MAX as u16) + ); + } +} + #[cfg(all(test, unix))] mod tests { use alacritty_terminal::index::{Column, Line}; diff --git a/src/terminal/shells.rs b/src/terminal/shells.rs index b7799c6..971d613 100644 --- a/src/terminal/shells.rs +++ b/src/terminal/shells.rs @@ -46,10 +46,13 @@ impl ShellSelector { } pub fn default_shell(mut self, default: usize) -> Self { - assert!( - default < self.shells.len(), - "the default is one of the shells" - ); + if default >= self.shells.len() { + log::error!( + "shell selector: no shell {default} of {}; the default stays", + self.shells.len() + ); + return self; + } self.default = default; self } @@ -193,3 +196,20 @@ impl RenderOnce for TerminalToolbar { })) } } + +#[cfg(test)] +mod tests { + use super::{ShellChoice, ShellSelector}; + + #[test] + fn a_default_past_the_shells_keeps_the_one_before() { + let shell = |name: &'static str| ShellChoice { + name: name.into(), + program: name.into(), + }; + let picker = ShellSelector::new("shells", [shell("zsh"), shell("bash")]) + .default_shell(1) + .default_shell(5); + assert_eq!(picker.default, 1); + } +} diff --git a/src/terminal/tests.rs b/src/terminal/tests.rs index c3e3dac..be96c64 100644 --- a/src/terminal/tests.rs +++ b/src/terminal/tests.rs @@ -79,3 +79,34 @@ fn a_link_keeps_its_combining_marks(cx: &mut TestAppContext) { assert_eq!(target, Target::Url("https://e.dev/cafe\u{301}/x".into())); }); } + +#[gpui::test] +fn a_pixel_size_query_of_a_wide_grid_answers(cx: &mut TestAppContext) { + use gpui::AppContext as _; + cx.update(Theme::init); + for columns in [80, 8192] { + let terminal = cx.new(|cx| Terminal::replay(b"\x1b[14t", columns, 1, cx)); + cx.run_until_parked(); + terminal.read_with(cx, |terminal, _| { + assert_eq!(terminal.title().as_ref(), "Output") + }); + } +} + +#[cfg(windows)] +#[test] +fn more_cells_than_a_windows_pty_holds_resize_it() { + use alacritty_terminal::{event::OnResize, term::test::TermSize, tty}; + let options = tty::Options { + shell: Some(tty::Shell::new( + "cmd.exe".into(), + vec!["/c".into(), "exit".into()], + )), + escape_args: true, + ..tty::Options::default() + }; + let size = |columns| super::pty::window_size(&TermSize::new(columns, 24), (8, 16)); + let mut pty = tty::new(&options, size(80), 0).expect("a pty starts"); + pty.on_resize(size(80)); + pty.on_resize(size(70_000)); +} diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 4f77482..cc7041f 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -282,11 +282,14 @@ impl Render for Terminal { let text = theme.text_size(TextSize::Sm); let mono = font(theme.mono_family.clone()); let pixels = text.to_pixels(window.rem_size()); - let advance = window - .text_system() - .advance(window.text_system().resolve_font(&mono), pixels, 'm') - .expect("the code font has an m") - .width; + let resolved = window.text_system().resolve_font(&mono); + let advance = match window.text_system().advance(resolved, pixels, 'm') { + Ok(advance) => advance.width, + Err(error) => { + log::error!("terminal: no advance for the code font: {error:#}; cells guessed"); + pixels * 0.6 + } + }; self.cell = size(advance, (pixels * LEADING).round()); let (found, current) = self.search.visible(); self.shown = Rc::new(frame(&self.term.lock(), &ink, found, current)); diff --git a/tasks/panics.md b/tasks/panics.md index cc75e7a..83997ad 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -32,7 +32,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P15 Charts — `src/charts` - [x] P16 Finance — `src/finance` - [x] P17 Code Editor — `src/editor` -- [ ] P18 Terminal — `src/terminal` +- [x] P18 Terminal — `src/terminal` - [ ] P19 Git — `src/git` - [ ] P20 Debug — `src/debug` - [ ] P21 Documents — `src/documents` @@ -83,3 +83,4 @@ One line per item: what changed, what stayed and why, review rounds. - P16: crossed and locked books, empty sizes, leverage and margin past their bounds, stale range and interval keys, short layouts and comparisons, growth from zero, long QR addresses and grids too fine to draw all log and show; ladders mark their middle rung and key cells by row; every system-zone time goes through `datetime`; `nice_step` stays positive. Kept: one-snapshot shapes and one-value contracts. Review: 5 rounds, passed on the fifth; it also passed P15's last fix. - Noted for later items: `chat/params.rs` and `editor/settings.rs` pass `value.abs()` to `decimals`, which asserts a positive step. - P17: the editor keeps every owner offset on whole characters of its text (checked on intake, carried through edits by `editor::anchors`, cleared when the whole text changes); stale selections and edit batches are dropped; a painted row whose line the text lost takes no click, drag or IME query; the sticky header reads this frame; a glob too big to compile matches nothing; a missing font advance logs. Kept: owner contracts and construction. Review: 5 rounds, passed on the fifth. +- P18: row paints and the font advance log gpui faults; a pty side caps at what ConPTY and alacritty's pixel sizes hold; a default shell past the list keeps the one before. Kept: constant regexes, captured matches, keyed rows, non-empty shells. Review: 2 rounds.