From f6ad8d1e50336de3297c7e7f11983608a5c94742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 26 Sep 2026 01:44:04 -0400 Subject: [PATCH] T18 review fixes: a missing folder stops the start, combining marks paint on their cell, the terminal is a Tab stop and keeps its wheel, clear drops find matches; the gallery starts a fresh shell per pick and picks the shell tab before typing --- TASKS.md | 3 +- examples/gallery/pages/terminal/live.rs | 15 ++++---- examples/gallery/pages/terminal/mod.rs | 3 ++ src/terminal/frame.rs | 47 ++++++++++++++++++++++-- src/terminal/input.rs | 1 + src/terminal/paint.rs | 48 +++++++++++++++++-------- src/terminal/pty.rs | 20 +++++++++++ src/terminal/tests.rs | 20 +++++++++++ src/terminal/view.rs | 3 +- 9 files changed, 134 insertions(+), 26 deletions(-) diff --git a/TASKS.md b/TASKS.md index 97778e2..8bac575 100644 --- a/TASKS.md +++ b/TASKS.md @@ -239,4 +239,5 @@ Per-component lines live in `tasks/`. Tags there: | T17d | 1 | FAIL | a path match marked a project name mid-character; pin and remove also opened the row; number settings showed no places | | T17d | 2 | PASS | | | T17e | 1 | PASS | | -| T18 | 1 | — | pending review | +| T18 | 1 | FAIL | a missing folder ran in the app's own; combining marks dropped; Tab skipped the terminal; its wheel scrolled the page too; clear kept find matches; New bash reused the old shell; the dark script typed into the replay | +| T18 | 2 | — | pending review | diff --git a/examples/gallery/pages/terminal/live.rs b/examples/gallery/pages/terminal/live.rs index 1e28400..b2e9edb 100644 --- a/examples/gallery/pages/terminal/live.rs +++ b/examples/gallery/pages/terminal/live.rs @@ -50,10 +50,11 @@ fn launch(shell: usize) -> Launch { /// A pane group of shells, and the shell each pane holds. type Panes = (Entity, Entity)>>); -fn panes(shell: usize, window: &mut Window, cx: &mut App) -> Panes { - let sessions = window.use_keyed_state(("terminal-sessions", shell), cx, |_, _| Vec::new()); +/// Panes of `shell`, fresh for each `start`: a pick starts new shells even of the same kind. +fn panes(shell: usize, start: usize, window: &mut Window, cx: &mut App) -> Panes { + let sessions = window.use_keyed_state(("terminal-sessions", start), cx, |_, _| Vec::new()); let held = sessions.clone(); - let group = window.use_keyed_state(("terminal-panes", shell), cx, move |_, _| { + let group = window.use_keyed_state(("terminal-panes", start), cx, move |_, _| { PaneGroup::new(move |pane, _, cx| session(&held, pane, shell, cx).into_any_element()) }); (group, sessions) @@ -85,9 +86,9 @@ fn field(key: &'static str, text: &str, window: &mut Window, cx: &mut App) -> En pub fn live(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> { let tab = keep("terminal-tab", || SharedString::from("shell"), window, cx); - let shell = keep("terminal-shell-choice", || 0usize, window, cx); - let (now_tab, now_shell) = (tab.read(cx).clone(), *shell.read(cx)); - let (group, sessions) = panes(now_shell, window, cx); + let shell = keep("terminal-shell-choice", || (0usize, 0usize), window, cx); + let (now_tab, (now_shell, start)) = (tab.read(cx).clone(), *shell.read(cx)); + let (group, sessions) = panes(now_shell, start, window, cx); let build = window.use_keyed_state("terminal-build", cx, |_, cx| { Terminal::replay(BUILD.as_bytes(), 92, 12, cx) }); @@ -124,7 +125,7 @@ pub fn live(window: &mut Window, cx: &mut App) -> impl IntoElement + use<> { }), ) .default_shell(now_shell) - .on_pick(move |ix, _, cx| set(&choose, ix, cx)); + .on_pick(move |ix, _, cx| set(&choose, (ix, start + 1), cx)); let on_shell = now_tab.as_ref() == "shell"; let exit = active .read(cx) diff --git a/examples/gallery/pages/terminal/mod.rs b/examples/gallery/pages/terminal/mod.rs index 6065d4c..938276b 100644 --- a/examples/gallery/pages/terminal/mod.rs +++ b/examples/gallery/pages/terminal/mod.rs @@ -18,6 +18,9 @@ pub const PAGE: Page = Page { const SCRIPT: &[Step] = &[ Step::Rest, + Step::DownAt("terminal-tabs", 46.0, 16.0), + Step::UpAt("terminal-tabs", 46.0, 16.0), + Step::Wait(300), Step::DownAt("terminal", 200.0, 150.0), Step::UpAt("terminal", 200.0, 150.0), Step::Wait(400), diff --git a/src/terminal/frame.rs b/src/terminal/frame.rs index d0d6dcf..e3de176 100644 --- a/src/terminal/frame.rs +++ b/src/terminal/frame.rs @@ -27,6 +27,8 @@ pub(crate) struct Style { pub(crate) struct Row { pub text: String, pub runs: Vec<(usize, Style)>, + /// Cells with combining marks, painted on their own at their column: the column, the cluster and its style. + pub clusters: Vec<(usize, String, Style)>, } /// Why cells are lit: selected, or a find match, the current one stronger. @@ -50,7 +52,17 @@ pub(crate) struct Frame { impl Frame { /// The screen's text, trailing spaces gone. pub(crate) fn text(&self) -> String { - let lines: Vec<&str> = self.rows.iter().map(|row| row.text.trim_end()).collect(); + let lines: Vec = self + .rows + .iter() + .map(|row| { + let mut cells: Vec = row.text.chars().map(String::from).collect(); + for (column, cluster, _) in &row.clusters { + cells[*column] = cluster.clone(); + } + cells.concat().trim_end().to_string() + }) + .collect(); lines.join("\n").trim_end().to_string() } } @@ -78,11 +90,27 @@ pub(crate) fn frame( let spacer = cell .flags .intersects(Flags::WIDE_CHAR_SPACER | Flags::LEADING_WIDE_CHAR_SPACER); + let look = style(cell, ink, content.colors); + let cluster = cell + .zerowidth() + .filter(|marks| !marks.is_empty()) + .map(|marks| { + std::iter::once(cell.c) + .chain(marks.iter().copied()) + .collect::() + }); push( &mut rows[row], - if spacer { ' ' } else { cell.c }, - style(cell, ink, content.colors), + if spacer || cluster.is_some() { + ' ' + } else { + cell.c + }, + look, ); + if let Some(cluster) = cluster { + rows[row].clusters.push((point.column.0, cluster, look)); + } let why = if content.selection.is_some_and(|range| range.contains(point)) { Some(Lit::Selected) } else { @@ -178,6 +206,19 @@ mod tests { } } + #[test] + fn combining_marks_stay_on_their_cell() { + let term = replayed("e\u{301}x".as_bytes()); + let shown = frame(&term, &ink(), &[], None); + assert_eq!(shown.text(), "e\u{301}x"); + assert_eq!( + &shown.rows[0].text[..2], + " x", + "the cluster's cell keeps its column" + ); + assert_eq!(shown.rows[0].clusters[0].1, "e\u{301}"); + } + #[test] fn cells_join_into_runs_of_one_style() { let term = replayed(b"\x1b[31mred\x1b[0m ok\r\n\x1b[7minv\x1b[0m"); diff --git a/src/terminal/input.rs b/src/terminal/input.rs index e77700c..f86f8e6 100644 --- a/src/terminal/input.rs +++ b/src/terminal/input.rs @@ -198,6 +198,7 @@ impl Terminal { } fn wheel(&mut self, event: &ScrollWheelEvent, _: &mut Window, cx: &mut Context) { + cx.stop_propagation(); self.rest += event.delta.pixel_delta(self.cell.height).y; let lines = (self.rest / self.cell.height).trunc() as i32; if lines == 0 { diff --git a/src/terminal/paint.rs b/src/terminal/paint.rs index bcbd5ad..5db8749 100644 --- a/src/terminal/paint.rs +++ b/src/terminal/paint.rs @@ -8,7 +8,7 @@ use gpui::{ }; use super::{ - frame::{Frame, Lit}, + frame::{Frame, Lit, Style}, links::Target, view::Terminal, }; @@ -28,6 +28,23 @@ pub(crate) struct Look { pub link: Hsla, } +/// The face a style prints in. +fn font(style: &Style, base: &Font) -> Font { + Font { + weight: if style.bold { + FontWeight::BOLD + } else { + FontWeight::NORMAL + }, + style: if style.italic { + FontStyle::Italic + } else { + FontStyle::Normal + }, + ..base.clone() + } +} + /// The grid, fitted to its box: it tells the terminal its size and origin, then paints each row. pub(crate) fn grid( terminal: Entity, @@ -63,19 +80,7 @@ pub(crate) fn grid( .iter() .map(|(len, style)| TextRun { len: *len, - font: Font { - weight: if style.bold { - FontWeight::BOLD - } else { - FontWeight::NORMAL - }, - style: if style.italic { - FontStyle::Italic - } else { - FontStyle::Normal - }, - ..look.font.clone() - }, + font: font(style, &look.font), color: style.fg, background_color: style.bg, underline: style.underline.then_some(UnderlineStyle { @@ -110,6 +115,21 @@ pub(crate) fn grid( shaped .paint(at(ix, 0), cell.height, window, cx) .expect("a terminal row paints"); + for (column, cluster, style) in &row.clusters { + let run = TextRun { + len: cluster.len(), + font: font(style, &look.font), + color: style.fg, + background_color: None, + underline: None, + strikethrough: None, + }; + window + .text_system() + .shape_line(cluster.clone().into(), look.size, &[run], None) + .paint(at(ix, *column), cell.height, window, cx) + .expect("a terminal cluster paints"); + } } if let Some((row, columns, _)) = &hovered { let under = at(*row, columns.start) + point(Pixels::ZERO, cell.height - look.rule); diff --git a/src/terminal/pty.rs b/src/terminal/pty.rs index 9acddea..3abf02a 100644 --- a/src/terminal/pty.rs +++ b/src/terminal/pty.rs @@ -49,6 +49,13 @@ pub(crate) fn detached(size: TermSize) -> Session { /// Starts `launch` on a pseudo-terminal of `size` cells, each `cell` pixels. pub(crate) fn spawn(launch: &Launch, size: TermSize, cell: (u16, u16)) -> anyhow::Result { + if let Some(cwd) = &launch.cwd { + anyhow::ensure!( + cwd.is_dir(), + "a terminal cannot start in {}: no such folder", + cwd.display() + ); + } let (sender, events) = unbounded(); let listener = Listener(sender); let term = Arc::new(FairMutex::new(Term::new( @@ -103,6 +110,19 @@ mod tests { use super::*; + #[test] + fn a_missing_folder_stops_the_start() { + let launch = Launch { + program: Some(("/bin/sh".into(), vec!["-c".into(), "true".into()])), + cwd: Some("/nonexistent/ely".into()), + ..Launch::default() + }; + let error = spawn(&launch, TermSize::new(20, 4), (8, 16)) + .err() + .expect("a missing folder is an error"); + assert!(error.to_string().contains("/nonexistent/ely")); + } + #[test] fn a_program_prints_into_the_grid_through_a_pseudo_terminal() { let launch = Launch { diff --git a/src/terminal/tests.rs b/src/terminal/tests.rs index 6cd8547..18e85f1 100644 --- a/src/terminal/tests.rs +++ b/src/terminal/tests.rs @@ -49,3 +49,23 @@ fn find_counts_matches_and_steps_round(cx: &mut TestAppContext) { ); }); } + +#[gpui::test] +fn tab_reaches_the_terminal(cx: &mut TestAppContext) { + let (terminal, cx) = replayed(b"ok", cx); + let focused = cx.update(|window, cx| { + window.focus_next(); + terminal.read(cx).focus.is_focused(window) + }); + assert!(focused); +} + +#[gpui::test] +fn clearing_drops_the_matches(cx: &mut TestAppContext) { + let (terminal, cx) = replayed(b"one two one", cx); + terminal.update(cx, |terminal, cx| { + assert_eq!(terminal.find("one", FindOptions::default(), cx), Ok(2)); + terminal.clear(cx); + assert_eq!(terminal.matches(), (0, None)); + }); +} diff --git a/src/terminal/view.rs b/src/terminal/view.rs index c03e94a..086ee06 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -126,7 +126,7 @@ impl Terminal { Self { term, pty, - focus: cx.focus_handle(), + focus: cx.focus_handle().tab_stop(true), title, exited: None, size, @@ -166,6 +166,7 @@ impl Terminal { /// Clears the screen and the scrollback; a shell redraws its prompt. pub fn clear(&mut self, cx: &mut Context) { + self.search = Search::default(); { let mut term = self.term.lock(); term.clear_screen(ClearMode::Saved);