From 677683b740d926cc63fec5bab67428f5988db7f3 Mon Sep 17 00:00:00 2001 From: ddidderr Date: Sun, 11 Oct 2026 22:57:35 +0200 Subject: [PATCH] refactor(terminal): wire kitty graphics and drop PageList workarounds Integration pass over the terminal core (Codex/gpt-6-astra, reviewed): - Screen now owns the real kitty ImageStorage; new/reset/teardown, erase-display deletes and alternate-screen limit inheritance hook in where Zig does, and Terminal implements ExecTerminal so kitty APC commands execute against it. Exec tests use a real Terminal. - The PageList gains the APIs Screen/Terminal needed (pin iterators, cross-page row/partial-row clone, string allocator probing, dedup before allocating hyperlinks, Top viewport fixup after erase, append_ grapheme no longer marking rows dirty), and the Screen/Terminal workarounds for their absence are removed so the code mirrors Zig. Public signatures are unchanged; downstream crates build as before. Test Plan: - cargo test -p ghostty-terminal (1252 passed, 0 ignored) - cargo clippy -p ghostty-terminal --all-targets -- -D warnings --- .../src/kitty/graphics_exec.rs | 164 +++++++----- crates/ghostty-terminal/src/page.rs | 67 ++++- crates/ghostty-terminal/src/page_list.rs | 237 ++++++++++++++++-- crates/ghostty-terminal/src/pin_iter.rs | 135 +--------- crates/ghostty-terminal/src/render.rs | 5 +- crates/ghostty-terminal/src/screen.rs | 202 ++++++--------- crates/ghostty-terminal/src/selection.rs | 7 +- crates/ghostty-terminal/src/terminal.rs | 129 ++++------ docs/rewrite/notes/ghostty-terminal-kitty.md | 83 +++--- .../ghostty-terminal-pagelist-requests.md | 73 +++++- docs/rewrite/notes/ghostty-terminal-screen.md | 29 ++- .../notes/ghostty-terminal-terminal.md | 53 +++- 12 files changed, 708 insertions(+), 476 deletions(-) diff --git a/crates/ghostty-terminal/src/kitty/graphics_exec.rs b/crates/ghostty-terminal/src/kitty/graphics_exec.rs index 800d6de..7bea18a 100644 --- a/crates/ghostty-terminal/src/kitty/graphics_exec.rs +++ b/crates/ghostty-terminal/src/kitty/graphics_exec.rs @@ -470,50 +470,14 @@ mod tests { use crate::terminal::{Options, Terminal}; use ghostty_vt::kitty::graphics_command::{Parser, TransmissionFormat}; - /// A `Terminal` plus the image storage that, once integrated, lives in - /// `Screen.kitty_images`. - // TODO(integration): replace with `Terminal` once `Terminal` implements - // `ExecTerminal` via `screen.kitty_images: ImageStorage`. - struct Harness { - t: Terminal, - storage: ImageStorage, - } - - impl Harness { - fn new(cols: u16, rows: u16) -> Self { - Harness { - t: Terminal::new(Options::new(cols, rows)).unwrap(), - storage: ImageStorage::default(), - } - } - } - - impl ExecTerminal for Harness { - fn kitty_parts(&mut self) -> (&mut ImageStorage, &mut PageList) { - (&mut self.storage, &mut self.t.screens.active_mut().pages) - } - fn exec_info(&self) -> TerminalInfo { - TerminalInfo::from_terminal(&self.t) - } - fn exec_cursor_pin(&self) -> Pin { - self.t.screens.active().cursor_pin() - } - fn exec_index(&mut self) -> Result<(), crate::page::Error> { - self.t.index() - } - fn exec_set_cursor_pos(&mut self, row: usize, col: usize) { - self.t.set_cursor_pos(row, col); - } - } - - fn run(h: &mut Harness, s: &str) -> Option { + fn run(h: &mut Terminal, s: &str) -> Option { let cmd = Parser::parse_string(s.as_bytes()).unwrap(); - execute(h, &cmd) + h.kitty_graphics_command(&cmd) } #[test] fn kittygfx_more_chunks_with_q_1() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); // Initial chunk has q=1 let resp = run(&mut t, "a=T,f=24,t=d,i=1,s=1,v=2,c=10,r=1,m=1,q=1;////"); @@ -526,7 +490,7 @@ mod tests { #[test] fn kittygfx_more_chunks_with_q_0() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); // Initial chunk has q=0 let resp = run(&mut t, "a=t,f=24,t=d,s=1,v=2,c=10,r=1,m=1,i=1,q=0;////"); @@ -539,7 +503,7 @@ mod tests { #[test] fn kittygfx_more_chunks_with_chunk_increasing_q() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); // Initial chunk has q=0 let resp = run(&mut t, "a=t,f=24,t=d,s=1,v=2,c=10,r=1,m=1,i=1,q=0;////"); @@ -552,18 +516,18 @@ mod tests { #[test] fn kittygfx_default_format_is_rgba() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); let resp = run(&mut t, "a=t,t=d,i=1,s=1,v=2,c=10,r=1;///////////").unwrap(); assert!(resp.ok()); - let img = t.storage.image_by_id(1).unwrap(); + let img = t.screen().kitty_images.image_by_id(1).unwrap(); assert_eq!(img.format, TransmissionFormat::Rgba); } #[test] fn kittygfx_test_valid_u32_expect_invalid_image_id() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); let resp = run(&mut t, "a=p,i=4294967295").unwrap(); assert!(!resp.ok()); @@ -572,7 +536,7 @@ mod tests { #[test] fn kittygfx_test_valid_i32_expect_invalid_image_id() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); let resp = run(&mut t, "a=p,i=1,z=-2147483648").unwrap(); assert!(!resp.ok()); @@ -581,7 +545,7 @@ mod tests { #[test] fn kittygfx_no_response_with_no_image_id_or_number() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); let resp = run(&mut t, "a=t,f=24,t=d,s=1,v=2,c=10,r=1,i=0,I=0;////////"); assert!(resp.is_none()); @@ -589,7 +553,7 @@ mod tests { #[test] fn kittygfx_no_response_with_no_image_id_or_number_load_and_display() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); let resp = run(&mut t, "a=T,f=24,t=d,s=1,v=2,c=10,r=1,i=0,I=0;////////"); assert!(resp.is_none()); @@ -597,19 +561,19 @@ mod tests { #[test] fn kittygfx_disabled_storage_never_responds() { - let mut t = Harness::new(5, 5); - t.storage.total_limit = 0; + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); + t.screen_mut().kitty_images.total_limit = 0; assert!(run(&mut t, "a=q,i=1,s=1,v=1,f=24;AAAA").is_none()); } #[test] fn kittygfx_query_ok_and_error() { - let mut t = Harness::new(5, 5); + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); let resp = run(&mut t, "a=q,i=31,s=1,v=1,f=24;AAAA").unwrap(); assert!(resp.ok()); assert_eq!(resp.id, 31); // Nothing is stored by a query. - assert!(t.storage.images.is_empty()); + assert!(t.screen().kitty_images.images.is_empty()); // Without an ID there is nothing to respond to (error is only logged). assert!(run(&mut t, "a=q,s=1,v=1,f=24;AAAA").is_none()); @@ -617,28 +581,108 @@ mod tests { #[test] fn kittygfx_transmit_and_display_moves_cursor() { - let mut t = Harness::new(10, 10); - t.t.width_px = 100; - t.t.height_px = 100; + let mut t = Terminal::new(Options::new(10, 10)).unwrap(); + t.width_px = 100; + t.height_px = 100; // 1x2 pixel image shown in 3 columns x 2 rows. let resp = run(&mut t, "a=T,f=24,t=d,i=5,s=1,v=2,c=3,r=2;////////").unwrap(); assert!(resp.ok()); assert_eq!(resp.id, 5); - assert_eq!(t.storage.placements.len(), 1); + assert_eq!(t.screen().kitty_images.placements.len(), 1); // The cursor moves to the last row of the image, right of its last column. - let cursor = &t.t.screens.active().cursor; + let cursor = &t.screens.active().cursor; assert_eq!((cursor.x, cursor.y), (3, 1)); } #[test] fn kittygfx_delete_all() { - let mut t = Harness::new(10, 10); + let mut t = Terminal::new(Options::new(10, 10)).unwrap(); run(&mut t, "a=T,f=24,t=d,i=5,s=1,v=2,C=1;////////").unwrap(); - assert_eq!(t.storage.placements.len(), 1); + assert_eq!(t.screen().kitty_images.placements.len(), 1); let resp = run(&mut t, "a=d,d=A"); assert!(resp.is_none()); - assert!(t.storage.placements.is_empty()); - assert!(t.storage.images.is_empty()); + assert!(t.screen().kitty_images.placements.is_empty()); + assert!(t.screen().kitty_images.images.is_empty()); + } + #[test] + fn kittygfx_screen_lifecycle_releases_placement_pins() { + let mut t = Terminal::new(Options::new(10, 10)).unwrap(); + let baseline = t.screen().pages.count_tracked_pins(); + run(&mut t, "a=T,f=24,t=d,i=5,s=1,v=2,C=1;////////").unwrap(); + assert_eq!(t.screen().pages.count_tracked_pins(), baseline + 1); + let cloned = t + .screen() + .clone_screen( + crate::point::Point::Active(crate::point::Coordinate::default()), + None, + ) + .unwrap(); + assert!(cloned.kitty_images.images.is_empty()); + assert!(cloned.kitty_images.placements.is_empty()); + let screen = t.screen_mut(); + screen + .kitty_images + .set_limit(&mut screen.pages, 1024) + .unwrap(); + screen.reset().unwrap(); + assert!(screen.kitty_images.images.is_empty()); + assert!(screen.kitty_images.placements.is_empty()); + assert!(screen.kitty_images.dirty); + assert_eq!( + screen.kitty_images.total_limit, + ImageStorage::default().total_limit + ); + assert_eq!(screen.pages.count_tracked_pins(), baseline); + screen.pages.assert_integrity(); + } + + #[test] + fn kittygfx_erase_display_deletes_at_reference_hooks() { + use ghostty_vt::csi::EraseDisplay; + for mode in [EraseDisplay::Complete, EraseDisplay::ScrollComplete] { + let mut t = Terminal::new(Options::new(10, 10)).unwrap(); + let baseline = t.screen().pages.count_tracked_pins(); + run(&mut t, "a=T,f=24,t=d,i=5,s=1,v=2,C=1;////////").unwrap(); + t.erase_display(EraseDisplay::Below, false); + assert_eq!(t.screen().kitty_images.placements.len(), 1); + t.erase_display(mode, false); + assert!(t.screen().kitty_images.images.is_empty()); + assert!(t.screen().kitty_images.placements.is_empty()); + assert_eq!(t.screen().pages.count_tracked_pins(), baseline); + } + } + + #[test] + fn kittygfx_alternate_inherits_limit_and_keeps_separate_storage() { + use crate::screen_set::Key; + let mut t = Terminal::new(Options::new(10, 10)).unwrap(); + let screen = t.screen_mut(); + screen + .kitty_images + .set_limit(&mut screen.pages, 1024) + .unwrap(); + run(&mut t, "a=T,f=24,t=d,i=5,s=1,v=2,C=1;////////").unwrap(); + t.switch_screen(Key::Alternate).unwrap(); + assert_eq!(t.screen().kitty_images.total_limit, 1024); + assert!(t.screen().kitty_images.dirty); + assert!(t.screen().kitty_images.images.is_empty()); + run(&mut t, "a=T,f=24,t=d,i=7,s=1,v=2,C=1;////////").unwrap(); + t.full_reset(); + assert_eq!(t.screens.active_key, Key::Primary); + assert!(t.screens.get(Key::Alternate).is_none()); + assert!(t.screen().kitty_images.images.is_empty()); + assert_eq!(t.screen().pages.count_tracked_pins(), 2); + } + + #[test] + fn kittygfx_legacy_entrypoint_and_reset_pending_transfer() { + let mut t = Terminal::new(Options::new(5, 5)).unwrap(); + let mut cmd = Parser::parse_string(b"a=t,f=24,t=d,s=1,v=2,i=1;////////").unwrap(); + assert!(t.kitty_graphics(&mut cmd).unwrap().ok()); + assert!(run(&mut t, "a=t,f=24,t=d,s=1,v=2,i=2,m=1;////").is_none()); + assert!(t.screen().kitty_images.loading.is_some()); + t.screen_mut().reset().unwrap(); + assert!(t.screen().kitty_images.loading.is_none()); } } diff --git a/crates/ghostty-terminal/src/page.rs b/crates/ghostty-terminal/src/page.rs index c779645..6f30811 100644 --- a/crates/ghostty-terminal/src/page.rs +++ b/crates/ghostty-terminal/src/page.rs @@ -845,7 +845,6 @@ impl Page { self.grapheme_allocations.insert(i, a); self.cells[i].set_content_tag(ContentTag::CodepointGrapheme); self.rows[y].set_grapheme(true); - self.rows[y].set_dirty(true); Ok(()) } /// Append one additional codepoint, retaining allocation chunk semantics. @@ -873,7 +872,6 @@ impl Page { } self.grapheme_data[range.end] = cp; range.end += 1; - self.rows[y].set_dirty(true); Ok(()) } /// Release a cell's additional codepoints. @@ -930,6 +928,12 @@ impl Page { link: Hyperlink, preferred: Option, ) -> Result { + // Existing live links already own their strings. Retain them before + // attempting another allocation on a potentially full page. + if let Some(id) = self.hyperlink_set.lookup(&link) { + self.hyperlink_set.r#use(id).map_err(|_| Error::InvalidId)?; + return Ok(id); + } let allocations = self.allocate_hyperlink_strings(&link)?; let result = if let Some(id) = preferred { self.hyperlink_set @@ -1398,6 +1402,31 @@ impl Page { self.rows[dst_y] = metadata; Ok(()) } + /// Clone columns within a page with the same row metadata rules as a cross-page copy. + pub(crate) fn clone_partial_row_within( + &mut self, + src_y: usize, + dst_y: usize, + left: usize, + len: usize, + ) -> Result<(), Error> { + let source_row = *self.get_row(src_y).ok_or(Error::InvalidCoordinate)?; + let original_row = *self.get_row(dst_y).ok_or(Error::InvalidCoordinate)?; + self.clone_cells_within(src_y, dst_y, left, left, len)?; + let managed = self.rows[dst_y]; + let mut metadata = source_row; + metadata.set_cells(original_row.cells()); + if len < usize::from(self.size.cols) { + metadata.set_wrap(original_row.wrap()); + metadata.set_wrap_continuation(original_row.wrap_continuation()); + metadata.set_grapheme(managed.grapheme()); + metadata.set_styled(managed.styled()); + metadata.set_hyperlink(managed.hyperlink()); + metadata.set_dirty(source_row.dirty() || original_row.dirty()); + } + self.rows[dst_y] = metadata; + Ok(()) + } /// Clone a range within the same page, including overlapping ranges. pub fn clone_cells_within( &mut self, @@ -2383,4 +2412,38 @@ mod tests { let q = clone_exact(&p, 1); assert_eq!(q.hyperlink_count(), 50); } + #[test] + fn grapheme_append_preserves_row_dirty_state() { + let mut p = page(); + p.cells[0] = Cell::init('a' as u32); + p.rows[0].set_dirty(false); + for _ in 0..6 { + p.append_grapheme(0, 0, 0x301).unwrap(); + assert!(!p.rows[0].dirty()); + } + p.rows[0].set_dirty(true); + p.append_grapheme(0, 0, 0x302).unwrap(); + assert!(p.rows[0].dirty()); + p.assert_integrity(); + } + + #[test] + fn hyperlink_dedup_succeeds_with_full_string_allocator() { + let mut p = Page::init(Capacity { + cols: 2, + rows: 1, + string_bytes: 32, + ..Capacity::default() + }) + .unwrap(); + let link = Hyperlink { + uri: vec![b'a'; Page::layout(p.capacity).string_alloc_layout.bitmap_count * 64 * 32], + id: HyperlinkId::Implicit(1), + }; + let id = p.insert_hyperlink(link.clone()).unwrap(); + assert!(!p.can_allocate_string(32)); + assert_eq!(p.insert_hyperlink(link).unwrap(), id); + assert_eq!(p.hyperlink_set.ref_count(id), 2); + assert!(!p.can_allocate_string(32)); + } } diff --git a/crates/ghostty-terminal/src/page_list.rs b/crates/ghostty-terminal/src/page_list.rs index 8ffb4e9..af8c19b 100644 --- a/crates/ghostty-terminal/src/page_list.rs +++ b/crates/ghostty-terminal/src/page_list.rs @@ -530,10 +530,11 @@ impl PageList { .filter(|s| s.generation == h.generation)? .pin } - /// Update a tracked location to another valid cell. + /// Update a tracked location. The node must be live; row bounds may be + /// temporarily exceeded while a caller repairs cursor state after splitting. pub fn set_tracked_pin(&mut self, h: TrackedPin, p: Pin) -> Result<(), Error> { - if !self.pin_is_valid(p) { - return Err(Error::InvalidCoordinate); + if self.node(p.node).is_none() { + return Err(Error::InvalidId); } let slot = self .pins @@ -813,12 +814,12 @@ impl PageList { } } fn fixup_viewport(&mut self) { - if self.viewport == Viewport::Pin - && self - .viewport_pin - .and_then(|h| self.tracked_pin(h)) - .is_some_and(|p| self.pin_is_active(p)) - { + let pin = match self.viewport { + Viewport::Active => return, + Viewport::Top => self.get_top_left(Tag::Screen), + Viewport::Pin => self.viewport_pin.and_then(|h| self.tracked_pin(h)), + }; + if pin.is_some_and(|p| self.pin_is_active(p)) { self.viewport = Viewport::Active; } } @@ -1054,6 +1055,10 @@ impl PageList { } /// Remove an inclusive row range, regrowing erased active rows. pub fn erase_rows(&mut self, start: Point, end: Option) -> Result<(), Error> { + // An empty history produces no chunks in the reference iterator. + if start.tag() == Tag::History && self.active_offset() == 0 && end.is_none() { + return Ok(()); + } let a = self .pin(start) .and_then(|p| self.screen_row(p)) @@ -1085,9 +1090,7 @@ impl PageList { .store(cached - (b - a), Ordering::Relaxed); } } - if self.viewport == Viewport::Top && self.active_offset() == 0 { - self.viewport = Viewport::Active; - } + self.fixup_viewport(); Ok(()) } /// Remove one row, shifting following rows without changing page dimensions. @@ -1544,6 +1547,79 @@ impl Pin { Some(p) } } +impl Pin { + /// Iterate cells toward an inclusive limit row. The limit column is ignored. + pub fn cell_iterator( + self, + list: &PageList, + direction: Direction, + limit: Option, + ) -> CellIterator<'_> { + let (front, back) = match direction { + Direction::RightDown => ( + Some(self), + limit + .or_else(|| list.get_bottom_right(Tag::Screen)) + .map(|p| Pin { + x: list.cols - 1, + ..p + }), + ), + Direction::LeftUp => ( + limit + .or_else(|| list.get_top_left(Tag::Screen)) + .map(|p| Pin { x: 0, ..p }), + Some(self), + ), + }; + CellIterator { + list, + front, + back, + direction, + } + } + /// Iterate rows directly from a pin, with an inclusive limit row. + pub fn row_iterator( + self, + list: &PageList, + direction: Direction, + limit: Option, + ) -> RowIterator<'_> { + let cells = self.cell_iterator(list, direction, limit); + RowIterator { + list, + front: cells.front.map(|p| Pin { x: 0, ..p }), + back: cells.back.map(|p| Pin { x: 0, ..p }), + direction, + } + } + /// Iterate maximal page chunks directly from a pin. + pub fn page_iterator( + self, + list: &PageList, + direction: Direction, + limit: Option, + ) -> PageIterator<'_> { + PageIterator { + rows: self.row_iterator(list, direction, limit), + } + } + /// Iterate prompt starts directly from a pin, with an inclusive limit row. + pub fn prompt_iterator( + self, + list: &PageList, + direction: Direction, + limit: Option, + ) -> PromptIterator<'_> { + PromptIterator { + list, + current: Some(self), + limit, + direction, + } + } +} /// A double-ended cell iterator yielding stable untracked locations. pub struct CellIterator<'a> { list: &'a PageList, @@ -2350,7 +2426,9 @@ impl PageList { })?; self.resize_cols(cols, opts.cursor)?; } - self.fixup_viewport(); + if self.viewport == Viewport::Pin { + self.fixup_viewport(); + } Ok(()) } fn resize_without_reflow(&mut self, opts: Resize) -> Result<(), Error> { @@ -2400,12 +2478,52 @@ impl PageList { while self.total_rows < usize::from(rows) { self.grow()?; } - self.fixup_viewport(); + if self.viewport == Viewport::Pin { + self.fixup_viewport(); + } } } } Ok(()) } + /// Clone a complete row between two page locations, ignoring columns. + pub fn clone_row_between(&mut self, src: Pin, dst: Pin) -> Result<(), Error> { + if src.node == dst.node { + let cols = usize::from(self.node(src.node).ok_or(Error::InvalidId)?.data.size.cols); + return self.clone_partial_row_between(src, dst, 0, cols); + } + self.copy_row(src.node, src.y, dst.node, dst.y) + } + + /// Clone columns `[left, right)` between rows without a temporary row copy. + pub fn clone_partial_row_between( + &mut self, + src: Pin, + dst: Pin, + left: usize, + right: usize, + ) -> Result<(), Error> { + let len = right.checked_sub(left).ok_or(Error::InvalidCoordinate)?; + if src.node == dst.node { + let page = &mut self.node_mut(src.node).ok_or(Error::InvalidId)?.data; + return page.clone_partial_row_within( + usize::from(src.y), + usize::from(dst.y), + left, + len, + ); + } + let (source, dest) = self.pages_for_copy(src.node, dst.node)?; + dest.clone_partial_row_from( + source, + usize::from(src.y), + usize::from(dst.y), + left, + left, + len, + ) + } + /// Borrow distinct source and destination pages without snapshots or unsafe /// pointers. Their slab slots remain stable for the duration of the copy. fn copy_row(&mut self, src: NodeId, src_y: u16, dst: NodeId, dst_y: u16) -> Result<(), Error> { @@ -2413,7 +2531,9 @@ impl PageList { dest.clone_row_from(source, usize::from(src_y), usize::from(dst_y)) } fn pages_for_copy(&mut self, src: NodeId, dst: NodeId) -> Result<(&Page, &mut Page), Error> { - assert_ne!(src.index, dst.index); + if self.node(src).is_none() || self.node(dst).is_none() || src.index == dst.index { + return Err(Error::InvalidId); + } let (source, dest) = if src.index < dst.index { let (left, right) = self.nodes.split_at_mut(dst.index); (left[src.index].as_ref(), right[0].as_mut()) @@ -7449,4 +7569,91 @@ mod tests { assert_eq!(s.viewport_row_offset(), 0); finish(&s); } + #[test] + fn pin_iterators_use_inclusive_rows_across_pages() { + let mut list = PageList::init(4, 4, None).unwrap(); + list.split(list.pin(active_point(0, 2)).unwrap()).unwrap(); + let start = list.pin(active_point(2, 1)).unwrap(); + let limit = list.pin(active_point(1, 2)).unwrap(); + let cells: Vec<_> = start + .cell_iterator(&list, Direction::RightDown, Some(limit)) + .collect(); + assert_eq!(cells.len(), 6); + assert_eq!(cells[0], start); + assert_eq!(cells[5].x, 3); + let reverse: Vec<_> = limit + .cell_iterator(&list, Direction::LeftUp, Some(start)) + .collect(); + assert_eq!(reverse.len(), 6); + assert_eq!(reverse[0], limit); + assert_eq!(reverse[5].x, 0); + for (start, limit, direction) in [ + (start, limit, Direction::RightDown), + (limit, start, Direction::LeftUp), + ] { + assert_eq!(start.row_iterator(&list, direction, Some(limit)).count(), 2); + let pages: Vec<_> = start.page_iterator(&list, direction, Some(limit)).collect(); + assert_eq!(pages.len(), 2); + assert!(pages.iter().all(|c| c.end - c.start == 1)); + } + } + + #[test] + fn row_copy_between_pages_preserves_managed_values_and_metadata() { + let mut list = PageList::init(4, 4, None).unwrap(); + list.split(list.pin(active_point(0, 2)).unwrap()).unwrap(); + let src = list.pin(active_point(0, 0)).unwrap(); + let dst = list.pin(active_point(0, 2)).unwrap(); + let page = &mut list.node_mut(src.node).unwrap().data; + page.cells[1] = Cell::init('a' as u32); + page.append_grapheme(1, 0, 0x301).unwrap(); + let id = page + .insert_hyperlink(crate::hyperlink::Hyperlink { + uri: b"https://example.com".to_vec(), + id: crate::hyperlink::HyperlinkId::Implicit(1), + }) + .unwrap(); + page.set_hyperlink(1, 0, id).unwrap(); + page.rows[0].set_kitty_virtual_placeholder(true); + page.rows[0].set_dirty(false); + let page = &mut list.node_mut(dst.node).unwrap().data; + page.cells[0] = Cell::init('z' as u32); + page.rows[0].set_wrap(true); + page.rows[0].set_dirty(true); + list.clone_partial_row_between(src, dst, 1, 2).unwrap(); + let page = &list.node(dst.node).unwrap().data; + assert_eq!(page.get_cells(0).unwrap()[0].codepoint(), 'z' as u32); + assert!(page.rows[0].wrap()); + assert!(page.rows[0].dirty()); + assert!(page.rows[0].kitty_virtual_placeholder()); + assert_eq!( + page.lookup_grapheme(page.cell_index(1, 0).unwrap()), + Some(&[0x301][..]) + ); + assert!( + page.lookup_hyperlink(page.cell_index(1, 0).unwrap()) + .is_some() + ); + list.clone_row_between(src, dst).unwrap(); + let page = &list.node(dst.node).unwrap().data; + assert_eq!(page.get_cells(0).unwrap()[0].codepoint(), 0); + assert!(!page.rows[0].wrap()); + assert!(!page.rows[0].dirty()); + list.assert_integrity(); + } + + #[test] + fn erase_empty_history_and_top_viewport() { + let mut list = PageList::init(4, 4, None).unwrap(); + list.erase_rows(Point::History(Coordinate::default()), None) + .unwrap(); + assert_eq!(list.total_rows, 4); + list.grow().unwrap(); + list.scroll(Scroll::Top); + list.erase_rows(Point::History(Coordinate::default()), None) + .unwrap(); + assert_eq!(list.total_rows, 4); + assert_eq!(list.viewport, Viewport::Active); + list.assert_integrity(); + } } diff --git a/crates/ghostty-terminal/src/pin_iter.rs b/crates/ghostty-terminal/src/pin_iter.rs index dcc8474..6d6f1c1 100644 --- a/crates/ghostty-terminal/src/pin_iter.rs +++ b/crates/ghostty-terminal/src/pin_iter.rs @@ -1,148 +1,31 @@ -//! Pin-based iterators. -//! -//! Zig's `PageList.Pin` has `rowIterator`, `cellIterator` and -//! `promptIterator` methods that start from a pin and use an inclusive limit -//! *pin* (the Rust `PageList` iterators are point-based). These are local -//! equivalents used by `Screen` and `Selection`. -//! -//! TODO(integration): replace with `Pin` methods in `page_list.rs` once -//! available (see docs/rewrite/notes/ghostty-terminal-pagelist-requests.md). -use crate::{ - page_list::{Direction, PageList, Pin, PromptIterator}, - point::{Coordinate, Point, Tag}, -}; +//! Compatibility wrappers for the native [`Pin`] iterators. +pub use crate::page_list::{CellIterator as CellIter, RowIterator as RowIter}; +use crate::page_list::{Direction, PageList, Pin, PromptIterator}; -fn same_row(a: Pin, b: Pin) -> bool { - a.node == b.node && a.y == b.y -} - -/// Iterates the rows (x = 0) from `start` toward `limit` (inclusive). -pub struct RowIter<'a> { - list: &'a PageList, - current: Option, - limit: Option, - direction: Direction, -} - -impl Iterator for RowIter<'_> { - type Item = Pin; - - fn next(&mut self) -> Option { - let cur = self.current?; - let result = Pin { x: 0, ..cur }; - if self.limit.is_some_and(|l| same_row(l, cur)) { - self.current = None; - } else { - self.current = match self.direction { - Direction::RightDown => cur.down(self.list, 1), - Direction::LeftUp => cur.up(self.list, 1), - }; - } - Some(result) - } -} - -/// Equivalent of `Pin.rowIterator(direction, limit)`. The limit is inclusive. +/// Iterate rows through an inclusive limit row. pub fn row_iterator( list: &PageList, start: Pin, direction: Direction, limit: Option, ) -> RowIter<'_> { - RowIter { - list, - current: Some(start), - limit, - direction, - } + start.row_iterator(list, direction, limit) } - -/// Iterates cells from `start` toward the end of the `limit` row. -/// -/// Note that, as in Zig, the limit only applies at row granularity: iteration -/// continues to the end (or start, for `LeftUp`) of the limit row. -pub struct CellIter<'a> { - list: &'a PageList, - cell: Option, - limit: Option, - direction: Direction, -} - -impl Iterator for CellIter<'_> { - type Item = Pin; - - fn next(&mut self) -> Option { - let cell = self.cell?; - let cols = self - .list - .node(cell.node) - .map_or(self.list.cols, |n| n.data.size.cols); - let at_limit = self.limit.is_some_and(|l| same_row(l, cell)); - self.cell = match self.direction { - Direction::RightDown => { - if cell.x + 1 < cols { - Some(Pin { - x: cell.x + 1, - ..cell - }) - } else if at_limit { - None - } else { - cell.down(self.list, 1).map(|p| Pin { x: 0, ..p }) - } - } - Direction::LeftUp => { - if cell.x > 0 { - Some(Pin { - x: cell.x - 1, - ..cell - }) - } else if at_limit { - None - } else { - cell.up(self.list, 1).map(|p| { - let cols = self.list.node(p.node).map_or(cols, |n| n.data.size.cols); - Pin { x: cols - 1, ..p } - }) - } - } - }; - Some(cell) - } -} - -/// Equivalent of `Pin.cellIterator(direction, limit)`. +/// Iterate cells through an inclusive limit row. pub fn cell_iterator( list: &PageList, start: Pin, direction: Direction, limit: Option, ) -> CellIter<'_> { - CellIter { - list, - cell: Some(start), - limit, - direction, - } + start.cell_iterator(list, direction, limit) } - -/// Equivalent of `Pin.promptIterator(direction, limit)`. +/// Iterate prompt starts through an inclusive limit row. pub fn prompt_iterator( list: &PageList, start: Pin, direction: Direction, limit: Option, ) -> PromptIterator<'_> { - let pt = |p: Pin| { - list.point_from_pin(Tag::Screen, p) - .unwrap_or(Point::Screen(Coordinate::default())) - }; - match direction { - Direction::RightDown => list.prompt_iterator(direction, pt(start), limit.map(pt)), - Direction::LeftUp => list.prompt_iterator( - direction, - limit.map_or(Point::Screen(Coordinate::default()), pt), - Some(pt(start)), - ), - } + start.prompt_iterator(list, direction, limit) } diff --git a/crates/ghostty-terminal/src/render.rs b/crates/ghostty-terminal/src/render.rs index 17713fd..e1626a3 100644 --- a/crates/ghostty-terminal/src/render.rs +++ b/crates/ghostty-terminal/src/render.rs @@ -20,9 +20,12 @@ //! To use this, initialize the render state to empty, then call `update` //! on each frame to update the state to the latest terminal state. //! -//! ```ignore +//! ``` +//! # use ghostty_terminal::{render::RenderState, terminal::{Terminal, Options}}; +//! # let mut terminal = Terminal::new(Options::new(80, 24))?; //! let mut state = RenderState::new(); //! state.update(&mut terminal)?; +//! # Ok::<(), ghostty_terminal::page::Error>(()) //! ``` //! //! Note: the render state retains as much memory as possible between updates diff --git a/crates/ghostty-terminal/src/screen.rs b/crates/ghostty-terminal/src/screen.rs index b0cdc68..7665902 100644 --- a/crates/ghostty-terminal/src/screen.rs +++ b/crates/ghostty-terminal/src/screen.rs @@ -22,12 +22,12 @@ use crate::{ cursor::Style as CursorStyle, formatter::{Content as FormatContent, Options as FormatOptions, ScreenFormatter}, hyperlink::{self, Hyperlink, HyperlinkId}, + kitty::graphics_storage::ImageStorage, page::{Cell, ContentTag, Page, Row, SemanticContent, SemanticPrompt as RowPrompt, Wide}, page_list::{ CellSubset, Chunk, Direction, IncreaseCapacity, NodeId, PageList, Pin, Scroll as ListScroll, TrackedPin, Viewport, }, - pin_iter, point::{Coordinate, Point, Tag}, selection::{Bounds, Order, Selection}, size, @@ -45,7 +45,7 @@ pub use crate::page::Error; /// The Unicode codepoint used by the Kitty graphics protocol for virtual /// placements (`kitty.graphics.unicode.placeholder`). -const KITTY_PLACEHOLDER: u32 = 0x10EEEE; +use crate::kitty::graphics_unicode::PLACEHOLDER as KITTY_PLACEHOLDER; /// See Terminal.Dirty. This behaves the same way. #[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] @@ -91,11 +91,8 @@ impl SemanticPrompt { }; } -/// Kitty graphics protocol state. -/// -/// TODO(integration): replace with `kitty::graphics_image::ImageStorage` -/// once the kitty graphics modules are ported. Only the dirty flag that -/// `Screen` touches is represented here. +/// Legacy dirty flag value retained for source compatibility. +/// Screens now own the full [`ImageStorage`]. #[derive(Debug, Clone, Default)] pub struct KittyImages { /// Dirty flag for the renderer. @@ -477,7 +474,7 @@ pub struct Screen { pub kitty_keyboard: FlagStack, /// Kitty graphics protocol state. - pub kitty_images: KittyImages, + pub kitty_images: ImageStorage, /// Semantic prompt (OSC133) state. pub semantic_prompt: SemanticPrompt, @@ -486,6 +483,12 @@ pub struct Screen { pub dirty: Dirty, } +impl Drop for Screen { + fn drop(&mut self) { + self.kitty_images.deinit(&mut self.pages); + } +} + impl Screen { /// Initialize a new screen. /// @@ -507,7 +510,7 @@ impl Screen { garbage: false, })?; - Ok(Screen { + let mut result = Screen { pages, no_scrollback: opts.max_scrollback == 0, cursor: Cursor::new(page_pin), @@ -516,10 +519,15 @@ impl Screen { charset: CharsetState::default(), protected_mode: ProtectedMode::Off, kitty_keyboard: FlagStack::default(), - kitty_images: KittyImages::default(), + kitty_images: ImageStorage::default(), semantic_prompt: SemanticPrompt::DISABLED, dirty: Dirty::default(), - }) + }; + result + .kitty_images + .set_limit(&mut result.pages, opts.kitty_image_storage_limit) + .expect("empty image storage accepts any limit"); + Ok(result) } /// The page for a node (panics if the node is stale). @@ -649,7 +657,11 @@ impl Screen { self.cursor = Cursor::new(self.cursor.page_pin); // Reset kitty graphics storage - self.kitty_images = KittyImages { dirty: true }; + self.kitty_images.deinit(&mut self.pages); + self.kitty_images = ImageStorage { + dirty: true, + ..Default::default() + }; // Reset our basic state self.saved_cursor = None; @@ -836,7 +848,7 @@ impl Screen { charset: CharsetState::default(), protected_mode: ProtectedMode::Off, kitty_keyboard: FlagStack::default(), - kitty_images: KittyImages::default(), + kitty_images: ImageStorage::default(), semantic_prompt: SemanticPrompt::DISABLED, dirty: self.dirty, }; @@ -1304,7 +1316,20 @@ impl Screen { cur_page.rows[..n].rotate_right(1); // Copy the last row of the previous page to the top of current. - self.clone_row_between(prev, prev_rows - 1, current, 0)?; + self.pages.clone_row_between( + Pin { + node: prev, + y: prev_rows - 1, + x: 0, + garbage: false, + }, + Pin { + node: current, + y: 0, + x: 0, + garbage: false, + }, + )?; // Mark dirty on the page, since we are dirtying all rows with this. self.page_mut(current).dirty = true; @@ -1328,36 +1353,6 @@ impl Screen { Ok(()) } - /// Clone a row from one page to another (Zig: `Page.cloneRowFrom`). - fn clone_row_between( - &mut self, - src: NodeId, - src_y: u16, - dst: NodeId, - dst_y: u16, - ) -> Result<(), Error> { - let (row, cells) = { - let sp = self.page(src); - let row = *sp - .get_row(usize::from(src_y)) - .ok_or(Error::InvalidCoordinate)?; - let cells: Vec<_> = (0..usize::from(sp.size.cols)) - .map(|x| sp.export_cell(row.cells() + x)) - .collect(); - (row, cells) - }; - let dp = self.page_mut(dst); - let cols = usize::from(dp.size.cols); - dp.clear_cells(usize::from(dst_y), 0, cols, false)?; - for (x, c) in cells.iter().enumerate().take(cols) { - dp.import_cell(x, usize::from(dst_y), c)?; - } - let start = dp.rows[usize::from(dst_y)].cells(); - dp.rows[usize::from(dst_y)] = row; - dp.rows[usize::from(dst_y)].set_cells(start); - Ok(()) - } - /// Move the cursor down if we're not at the bottom of the screen. Otherwise /// scroll. Currently only used for testing. pub fn cursor_down_or_scroll(&mut self) -> Result<(), Error> { @@ -1519,7 +1514,10 @@ impl Screen { /// Mark the cursor position as dirty. pub fn cursor_mark_dirty(&mut self) { - self.cursor_row_mut().set_dirty(true); + let pin = self.cursor_pin(); + // A split can temporarily restore the old cursor pin beyond the live + // row count. Its allocated row still exists, like Zig's cached page_row. + self.page_mut(pin.node).rows[usize::from(pin.y)].set_dirty(true); } /// Reset the cursor row's soft-wrap state and the cursor's pending wrap. @@ -1601,19 +1599,7 @@ impl Screen { /// Like Zig's `PageList.eraseRows` this requires that `tl` is valid. pub fn erase_rows(&mut self, tl: Point, bl: Option) { // Erase the rows - if let Err(err) = self.pages.erase_rows(tl, bl) { - log::warn!("erase_rows failed err={err:?}"); - } - - // Workaround for PageList::fixup_viewport not handling `Viewport::Top` - // (Zig: a top viewport returns to active when the first page is in - // the active area after erasing). - // TODO(integration): remove once page_list.rs handles this. - if self.pages.viewport == Viewport::Top - && self.pages.total_rows <= usize::from(self.pages.rows) - { - self.pages.viewport = Viewport::Active; - } + self.pages.erase_rows(tl, bl).expect("valid erase range"); // Just to be safe, reset our cursor since it is possible depending // on the points that our active area shifted so our pointers are @@ -1988,20 +1974,17 @@ impl Screen { } Redraw::True => { - let start = pin_iter::prompt_iterator( - &self.pages, - self.cursor_pin(), - Direction::LeftUp, - None, - ) - .next(); + let start = self + .cursor_pin() + .prompt_iterator(&self.pages, Direction::LeftUp, None) + .next(); if let Some(start) = start { // Clear cells from our start down. We replace it with spaces, // and do not physically erase the rows (erase_rows) because the // shell is going to expect this space to be available. - let rows: Vec = - pin_iter::row_iterator(&self.pages, start, Direction::RightDown, None) - .collect(); + let rows: Vec = start + .row_iterator(&self.pages, Direction::RightDown, None) + .collect(); for pin in rows { self.clear_cells(pin.node, pin.y, 0, cols); } @@ -2029,13 +2012,10 @@ impl Screen { // If we have no scrollback and we shrunk our rows, we must explicitly // erase our history. This is because PageList always keeps at least // a page size of history. - if self.no_scrollback - && self.pages.get_top_left(Tag::History).is_some() - && let Err(err) = self - .pages + if self.no_scrollback { + self.pages .erase_rows(Point::History(Coordinate::default()), None) - { - log::warn!("failed to erase history on resize err={err:?}"); + .expect("valid history range"); } // If our cursor was updated, we do a full reload so all our cursor @@ -2301,16 +2281,8 @@ impl Screen { // Cursor changed, we need to restore the old pin then use // cursor_change_pin to move to the new pin. The old node is guaranteed // to still exist, just not the row. - // - // The page list validates tracked pins, so the stand-in for the old - // location is clamped into the old node's remaining rows (only the - // node matters to cursor_change_pin). let new_cursor = self.cursor_pin(); - let old_rows = self.page(old_cursor.node).size.rows; - self.set_cursor_pin(Pin { - y: old_cursor.y.min(old_rows - 1), - ..old_cursor - }); + self.set_cursor_pin(old_cursor); self.cursor_change_pin(new_cursor); Ok(()) } @@ -2459,32 +2431,18 @@ impl Screen { // hyperlink_map is out of space, realloc the page to be larger Err(Error::HyperlinkMapOutOfMemory) => { - // Note: Zig first probes the string allocator to decide whether - // string bytes must grow too (because its increaseCapacity - // always adds an extra copy of the URI). The Rust page rebuild - // deduplicates hyperlinks, so only the hyperlink capacity needs - // to grow. - let saved_link = self.cursor.hyperlink.clone(); - self.increase_capacity(p.node, Some(IncreaseCapacity::HyperlinkBytes))?; - - // The page rebuild re-adds the cursor hyperlink, but that can - // fail when the string allocator has no room for the extra - // copy of the URI (Zig grows string_bytes up front for this). - // Re-add it here, growing as needed. - if self.cursor.hyperlink_id == 0 - && let Some(link) = saved_link - { - loop { - let adjustment = match self.start_hyperlink_once(&link) { - Ok(()) => break, - Err(Error::StringsOutOfMemory) => IncreaseCapacity::StringBytes, - Err(Error::SetOutOfMemory) => IncreaseCapacity::HyperlinkBytes, - Err(e) => return Err(e), - }; - let node = self.cursor_pin().node; - self.increase_capacity(node, Some(adjustment))?; + // Probe the URI allocation before rebuilding, just as + // cursorSetHyperlink does in the reference implementation. + while let Some(link) = &self.cursor.hyperlink { + let len = link.uri.len(); + let node = self.cursor_pin().node; + if self.page_mut(node).can_allocate_string(len) { + break; } + self.increase_capacity(node, Some(IncreaseCapacity::StringBytes))?; } + let node = self.cursor_pin().node; + self.increase_capacity(node, Some(IncreaseCapacity::HyperlinkBytes))?; // Retry // @@ -2657,7 +2615,7 @@ impl Screen { // The real start of the row is the first row in the soft-wrap. let start_pin: Pin = 'start_pin: { - let mut it = pin_iter::row_iterator(list, opts.pin, Direction::LeftUp, None); + let mut it = opts.pin.row_iterator(list, Direction::LeftUp, None); let mut it_prev: Pin = it.next().expect("self row"); // skip self // First, check the current row for semantic boundaries before the clicked position. @@ -2714,7 +2672,7 @@ impl Screen { // The real end of the row is the final row in the soft-wrap. let end_pin: Pin = 'end_pin: { - for p in pin_iter::row_iterator(list, opts.pin, Direction::RightDown, None) { + for p in opts.pin.row_iterator(list, Direction::RightDown, None) { let row = self.pin_row(p); let cols = self.pin_cols(p); @@ -2765,7 +2723,7 @@ impl Screen { let Some(whitespace) = opts.whitespace else { break 'start start_pin; }; - for p in pin_iter::cell_iterator(list, start_pin, Direction::RightDown, Some(end_pin)) { + for p in start_pin.cell_iterator(list, Direction::RightDown, Some(end_pin)) { let cell = self.pin_cell(p); if !cell.has_text() { continue; @@ -2787,7 +2745,7 @@ impl Screen { let Some(whitespace) = opts.whitespace else { break 'end end_pin; }; - for p in pin_iter::cell_iterator(list, end_pin, Direction::LeftUp, Some(start_pin)) { + for p in end_pin.cell_iterator(list, Direction::LeftUp, Some(start_pin)) { let cell = self.pin_cell(p); if !cell.has_text() { continue; @@ -2852,7 +2810,7 @@ impl Screen { } else { Direction::LeftUp }; - for pin in pin_iter::cell_iterator(list, start, dir, Some(end)) { + for pin in start.cell_iterator(list, dir, Some(end)) { // Boundary conditions match dir { Direction::RightDown => { @@ -2900,7 +2858,7 @@ impl Screen { // Go forwards to find our end boundary let end: Pin = 'end: { - let mut it = pin_iter::cell_iterator(list, pin, Direction::RightDown, None); + let mut it = pin.cell_iterator(list, Direction::RightDown, None); let mut prev = it.next().expect("start cell"); // Consume one, our start for p in it { let (row, cell) = { @@ -2933,7 +2891,7 @@ impl Screen { // Go backwards to find our start boundary let start: Pin = 'start: { - let mut it = pin_iter::cell_iterator(list, pin, Direction::LeftUp, None); + let mut it = pin.cell_iterator(list, Direction::LeftUp, None); let mut prev = it.next().expect("start cell"); // Consume one, our start for p in it { let (row, cell) = { @@ -2987,20 +2945,22 @@ impl Screen { let prompt_pin: Pin = 'prompt: { // If we have a prompt above this point (including this point), // then thats the prompt we want to capture output from. - if let Some(p) = pin_iter::prompt_iterator(list, pin, Direction::LeftUp, None).next() { + if let Some(p) = pin.prompt_iterator(list, Direction::LeftUp, None).next() { break 'prompt p; } // If we don't have a prompt, then we assume that we're // capturing all the output up to the next prompt. - let next = pin_iter::prompt_iterator(list, pin, Direction::RightDown, None).next()?; + let next = pin + .prompt_iterator(list, Direction::RightDown, None) + .next()?; // We'll capture from the start of the screen to just above // the prompt and will trim the trailing whitespace. let start_pin = self.pages.get_top_left(Tag::Screen)?; let mut end_pin = next.up(list, 1)?; end_pin.x = self.pin_cols(end_pin) - 1; - for p in pin_iter::cell_iterator(list, end_pin, Direction::LeftUp, Some(start_pin)) { + for p in end_pin.cell_iterator(list, Direction::LeftUp, Some(start_pin)) { end_pin = p; if self.pin_cell(p).has_text() { break; @@ -3016,7 +2976,7 @@ impl Screen { .highlight_semantic_content(prompt_pin, SemanticContent::Output)?; // Trim our trailing whitespace - for p in pin_iter::cell_iterator(list, hl_end, Direction::LeftUp, Some(hl_start)) { + for p in hl_end.cell_iterator(list, Direction::LeftUp, Some(hl_start)) { hl_end = p; if self.pin_cell(p).has_text() { break; @@ -3091,7 +3051,7 @@ impl Screen { // We go row-by-row because soft-wrapped rows are still a single // line to a shell, so we can't just look at our page row. 'row_it: for row_pin in - pin_iter::row_iterator(list, cursor_pin, Direction::RightDown, Some(click_pin)) + cursor_pin.row_iterator(list, Direction::RightDown, Some(click_pin)) { let row = self.pin_row(row_pin); let cells = self.pin_row_cells(row_pin); @@ -3177,9 +3137,7 @@ impl Screen { // We go row-by-row because soft-wrapped rows are still a single // line to a shell, so we can't just look at our page row. - 'row_it: for row_pin in - pin_iter::row_iterator(list, cursor_pin, Direction::LeftUp, Some(click_pin)) - { + 'row_it: for row_pin in cursor_pin.row_iterator(list, Direction::LeftUp, Some(click_pin)) { let row = self.pin_row(row_pin); let cells = self.pin_row_cells(row_pin); diff --git a/crates/ghostty-terminal/src/selection.rs b/crates/ghostty-terminal/src/selection.rs index 4480bea..6d612a3 100644 --- a/crates/ghostty-terminal/src/selection.rs +++ b/crates/ghostty-terminal/src/selection.rs @@ -7,7 +7,6 @@ use crate::{ page::{Cell, Error}, page_list::{Direction, PageList, Pin, TrackedPin}, - pin_iter, point::{Coordinate, Point, Tag}, }; @@ -447,7 +446,8 @@ impl Selection { } Adjustment::Left => { - let found = pin_iter::cell_iterator(list, end_pin, Direction::LeftUp, None) + let found = end_pin + .cell_iterator(list, Direction::LeftUp, None) .skip(1) .find(|p| cell_has_text(list, *p)); if let Some(next) = found { @@ -458,7 +458,8 @@ impl Selection { Adjustment::Right => { // Step right, wrapping to the next row down at the start of each new line, // until we find a non-empty cell. - let found = pin_iter::cell_iterator(list, end_pin, Direction::RightDown, None) + let found = end_pin + .cell_iterator(list, Direction::RightDown, None) .skip(1) .find(|p| cell_has_text(list, *p)); if let Some(next) = found { diff --git a/crates/ghostty-terminal/src/terminal.rs b/crates/ghostty-terminal/src/terminal.rs index 118c18e..17ea04b 100644 --- a/crates/ghostty-terminal/src/terminal.rs +++ b/crates/ghostty-terminal/src/terminal.rs @@ -21,7 +21,6 @@ use crate::{ page::{Cell, ContentTag, Page, SemanticContent, SemanticPrompt as RowPrompt, Wide}, page_list::{Direction, IncreaseCapacity, Pin}, - pin_iter, point::{Coordinate, Point}, screen::{ CharsetState, InputClear, Options as ScreenOptions, Resize as ScreenResize, SavedCursor, @@ -53,7 +52,7 @@ pub use crate::page::Error; const TABSTOP_INTERVAL: usize = 8; /// The unicode codepoint for kitty graphics placeholders. -const KITTY_PLACEHOLDER: u32 = 0x10EEEE; +use crate::kitty::graphics_unicode::PLACEHOLDER as KITTY_PLACEHOLDER; /// The set of screens behind this terminal (e.g. primary vs alternate). #[derive(Debug)] @@ -608,15 +607,7 @@ impl Terminal { } } - // Zig's appendGrapheme does not touch the row dirty flag, but the - // Rust Page::append_grapheme marks the row dirty; preserve the old - // flag (see pagelist requests). - let was_dirty = self.screen().cursor_row().dirty(); - let r = self.screen_mut().append_grapheme(prev_x, c); - if !was_dirty { - self.screen_mut().cursor_row_mut().set_dirty(false); - } - return r; + return self.screen_mut().append_grapheme(prev_x, c); } // We have a printable character, save it @@ -2079,60 +2070,6 @@ impl Terminal { }); } - /// Copy columns `[left, right)` of a row of one page to a row of another - /// page in the same list (Zig: `Page.clonePartialRowFrom` across two page - /// nodes). - /// - /// TODO(integration): replace with a `PageList` helper (see - /// ghostty-terminal-pagelist-requests.md). - fn clone_partial_row_between( - &mut self, - src: Pin, - dst: Pin, - left: usize, - right: usize, - ) -> Result<(), Error> { - let len = right - left; - let (source_row, cells) = { - let sp = self.screen().page(src.node); - let source_row = sp.rows[usize::from(src.y)]; - let cells: Vec<_> = (left..right) - .map(|x| { - sp.cell_index(x, usize::from(src.y)) - .map(|i| sp.export_cell(i)) - }) - .collect::>()?; - (source_row, cells) - }; - let dp = self.screen_mut().page_mut(dst.node); - let dy = usize::from(dst.y); - let original_row = dp.rows[dy]; - dp.clear_cells(dy, left, right, false)?; - if source_row.managed_memory() { - for (n, c) in cells.iter().enumerate() { - dp.import_cell(left + n, dy, c)?; - } - } else { - let to = original_row.cells() + left; - for (n, c) in cells.iter().enumerate() { - dp.cells[to + n] = c.cell; - } - } - let managed = dp.rows[dy]; - let mut metadata = source_row; - metadata.set_cells(original_row.cells()); - if len < usize::from(dp.size.cols) { - metadata.set_wrap(original_row.wrap()); - metadata.set_wrap_continuation(original_row.wrap_continuation()); - metadata.set_grapheme(managed.grapheme()); - metadata.set_styled(managed.styled()); - metadata.set_hyperlink(managed.hyperlink()); - metadata.set_dirty(source_row.dirty() || original_row.dirty()); - } - dp.rows[dy] = metadata; - Ok(()) - } - /// Shared implementation of insert_lines / delete_lines. When `insert` is /// true rows are shifted down from the cursor (iterating bottom-up), /// otherwise rows are shifted up (iterating top-down). @@ -2229,7 +2166,7 @@ impl Terminal { // If our page doesn't match, then we need to do a copy from // one page to another. This is the slow path. if src_p.node != dst_p.node { - if let Err(err) = self.clone_partial_row_between( + if let Err(err) = self.screen_mut().pages.clone_partial_row_between( src_p, dst_p, usize::from(sr.left), @@ -2745,7 +2682,9 @@ impl Terminal { // Go from the bottom of the active up and see if we're // at a prompt. // (The Zig loop decides on the first row it sees.) - let Some(p) = pin_iter::row_iterator(&scr.pages, active_br, Direction::LeftUp, top).next() + let Some(p) = active_br + .row_iterator(&scr.pages, Direction::LeftUp, top) + .next() else { return false; }; @@ -2761,11 +2700,14 @@ impl Terminal { } /// Delete all kitty graphics state for the active screen. - /// - /// TODO(integration): call `kitty_images.delete(.{ .all = true })` once - /// kitty graphics storage is ported; for now only the dirty flag exists. fn kitty_images_delete_all(&mut self) { - self.screen_mut().kitty_images.dirty = true; + let info = crate::kitty::graphics_storage::TerminalInfo::from_terminal(self); + let screen = self.screen_mut(); + screen.kitty_images.delete( + &mut screen.pages, + &info, + kitty_graphics_command::Delete::All(true), + ); } /// Resets all margins and fills the whole screen with the character 'E' @@ -2851,15 +2793,19 @@ impl Terminal { /// an error occurred otherwise the behavior of the graphics protocol is /// undefined. /// - /// TODO(integration): call `kitty::graphics::execute(self, cmd)` once - /// kitty graphics_exec is ported. Until then no command has any effect and - /// no response is produced. pub fn kitty_graphics( &mut self, cmd: &mut kitty_graphics_command::Command, ) -> Option { - let _ = cmd; - None + self.kitty_graphics_command(cmd) + } + + /// Execute a borrowed Kitty graphics command. + pub fn kitty_graphics_command( + &mut self, + cmd: &kitty_graphics_command::Command, + ) -> Option { + crate::kitty::graphics_exec::execute(self, cmd) } /// Set a style attribute. @@ -3089,8 +3035,7 @@ impl Terminal { // Inherit our Kitty image storage limit from the primary // screen if we have to initialize. - // TODO(integration): `primary.kitty_images.total_limit`. - kitty_image_storage_limit: ScreenOptions::DEFAULT.kitty_image_storage_limit, + kitty_image_storage_limit: primary.kitty_images.total_limit, }; self.screens.get_init(key, opts)?; } @@ -3232,9 +3177,7 @@ impl Terminal { self.screens.remove(ScreenKey::Alternate); // Reset our screens - if let Err(err) = self.screen_mut().reset() { - log::warn!("screen reset failed err={err:?}"); - } + self.screen_mut().reset().expect("screen reset"); // Rest our basic state self.modes.reset(); @@ -3269,3 +3212,27 @@ impl Terminal { #[cfg(test)] mod tests; + +impl crate::kitty::graphics_exec::ExecTerminal for Terminal { + fn kitty_parts( + &mut self, + ) -> ( + &mut crate::kitty::graphics_storage::ImageStorage, + &mut crate::page_list::PageList, + ) { + let screen = self.screens.active_mut(); + (&mut screen.kitty_images, &mut screen.pages) + } + fn exec_info(&self) -> crate::kitty::graphics_storage::TerminalInfo { + crate::kitty::graphics_storage::TerminalInfo::from_terminal(self) + } + fn exec_cursor_pin(&self) -> Pin { + self.screen().cursor_pin() + } + fn exec_index(&mut self) -> Result<(), Error> { + self.index() + } + fn exec_set_cursor_pos(&mut self, row: usize, col: usize) { + self.set_cursor_pos(row, col); + } +} diff --git a/docs/rewrite/notes/ghostty-terminal-kitty.md b/docs/rewrite/notes/ghostty-terminal-kitty.md index e2babff..a070e30 100644 --- a/docs/rewrite/notes/ghostty-terminal-kitty.md +++ b/docs/rewrite/notes/ghostty-terminal-kitty.md @@ -7,7 +7,7 @@ | graphics_storage.zig | `graphics_storage.rs` (`ImageStorage`, `Placement`, `PlacementKey`, `TerminalInfo`) | 19/19 Zig + 3 extra (placement replace releases pin, eviction order + limit, image_by_number newest) | | graphics_unicode.zig | `graphics_unicode.rs` (`placement_iterator`, `Placement`, `render_placement`) | 16/16 Zig + 2 extra | | graphics_render.zig | `graphics_render.rs` (`Placement`) | no tests in Zig | -| graphics_exec.zig | `graphics_exec.rs` (`execute`, `ExecTerminal` trait) | 8/8 Zig + 4 extra (run against a harness, see below) | +| graphics_exec.zig | `graphics_exec.rs` (`execute`, `ExecTerminal` trait) | 8/8 Zig + 8 extra (real Terminal, including lifecycle integration) | | kitty.zig / graphics.zig | `kitty/mod.rs` (re-exports; the command parser stays in `ghostty_vt::kitty::graphics_command`) | - | `pub mod kitty;` added to `lib.rs`. Test data copied to `kitty/testdata/`. @@ -32,39 +32,52 @@ Zig tests skipped: none. (`debugDump` not ported: debug-only helper.) `evicted > req` (strictly); `set_limit(0)` resets storage to defaults (limit then set to 0); screen reset `.{ .dirty = true }` also resets total_limit to default. - `execute(terminal, &cmd)` has no allocator; `Command` stays borrowed (the loading image copies the payload). -## graphics_exec and Terminal (TODO(integration), needs T3) -`Screen.kitty_images` is still the placeholder `KittyImages { dirty }`, so `Terminal` cannot implement `ExecTerminal` yet. -`execute` is generic over the `ExecTerminal` trait; the exec tests run against a `Harness { t: Terminal, storage: ImageStorage }` -(real `Terminal::index`/`set_cursor_pos`/pages; storage held beside it). No tests are ignored. +## graphics_exec and Terminal integration — complete -### Integration steps -1. `screen.rs`: replace `KittyImages` with `pub kitty_images: crate::kitty::graphics_storage::ImageStorage` (`Default` = Zig `.{}`). - - `Screen::new`: after constructing, `result.kitty_images.set_limit(&mut result.pages, opts.kitty_image_storage_limit).unwrap()` - (infallible there; `set_limit` returns `Result`). Opts default is already 320_000_000. - - `Screen::reset` (Zig `self.kitty_images.deinit(self); self.kitty_images = .{ .dirty = true }`): - `self.kitty_images.deinit(&mut self.pages); self.kitty_images = ImageStorage { dirty: true, ..Default::default() };` - - `Drop`/explicit teardown: `kitty_images.deinit(&mut pages)` must run before pages are dropped (tracked pins). `clone_screen` does not clone images. - - the existing `self.kitty_images.dirty = true` sites keep working unchanged. -2. `terminal.rs`: - - `erase_display` `.complete` and `.scrollback`... (Zig lines ~2495/2552): `let info = TerminalInfo::from_terminal(self); let s = self.screens.active_mut(); s.kitty_images.delete(&mut s.pages, &info, Delete::All(true));` - - `switch_screen_mode`/`set_alternate`: new-screen `kitty_image_storage_limit` = primary `kitty_images.total_limit`; then `new.kitty_images.dirty = true`. - - add `Terminal::kitty_graphics_command(&mut self, cmd: &Command) -> Option { kitty::graphics_exec::execute(self, cmd) }` - (Zig `Terminal.kittyGraphics`). - - `impl ExecTerminal for Terminal`: - ```rust - fn kitty_parts(&mut self) -> (&mut ImageStorage, &mut PageList) { let s = self.screens.active_mut(); (&mut s.kitty_images, &mut s.pages) } - fn exec_info(&self) -> TerminalInfo { TerminalInfo::from_terminal(self) } - fn exec_cursor_pin(&self) -> Pin { self.screens.active().cursor_pin() } - fn exec_index(&mut self) -> Result<(), page::Error> { self.index() } - fn exec_set_cursor_pos(&mut self, row: usize, col: usize) { self.set_cursor_pos(row, col) } - ``` - - `print`: Zig sets row `kitty_virtual_placeholder` when printing `kitty::graphics_unicode::PLACEHOLDER` (already present in Rust, uses its own constant; can switch to `crate::kitty::unicode::PLACEHOLDER`). -3. `stream_readonly`/termio: APC `G` -> `ghostty_vt::apc` already yields a parsed `Command`; termio calls `terminal.kitty_graphics_command(&cmd)` and, if `Some(resp)`, writes `resp.encode(..)` to the pty. - Termio config: `screen.kitty_images.set_limit(&mut screen.pages, limit)` (Termio.zig 265/469); clear-screen binding: `Delete::All(true)` as in step 2 (Termio.zig 615). -4. Renderer (`renderer/image.zig` equivalent): iterate `storage.placements` / `storage.images`, `Placement::rect`/`grid_size`/`calculated_size`, - `graphics_unicode::placement_iterator(&pages, top, Some(bot))` + `Placement::render_placement(storage, img, cell_w, cell_h)`; reset `storage.dirty = false` after upload. - Font shaper uses `kitty::unicode::PLACEHOLDER`. -5. Once Terminal implements the trait, the exec tests' `Harness` can be replaced by `Terminal` directly. +`Screen.kitty_images` is the real `ImageStorage`. Initialization applies the +configured limit, reset deinitializes storage and restores default limits with +`dirty = true`, and Drop releases placement pins before dropping PageList. +`clone_screen` does not copy images. The default is still 320,000,000 bytes. -## Quality gate (2026-10-11) -`cargo build/test/clippy -D warnings -p ghostty-terminal` pass (1243 tests incl. 66 kitty: 54 ported + 12 extra); rustfmt applied to `kitty/*.rs`. +`Terminal` implements `ExecTerminal` by splitting active-screen storage/pages, +snapshotting `TerminalInfo`, and forwarding cursor/index operations. Both +`kitty_graphics(&mut Command)` and the new borrowed +`kitty_graphics_command(&Command)` execute commands and return real responses. +All graphics_exec tests now use Terminal directly; the separate harness is gone. + +Reference hook audit: +- `.complete` and `.scroll_complete` display erasure call `Delete::All(true)`. + `.scrollback` only erases history, as in Terminal.zig. +- Lazy alternate-screen creation inherits the primary image limit; screen + switching marks the new screen's images dirty. +- All existing image dirty hooks in Screen and Terminal remain at their Zig + locations (scrolling, index, insert/delete lines, screen switching). +- Full reset drops alternate storage and resets primary storage. Placeholder + printing uses the shared `graphics_unicode::PLACEHOLDER` constant. + +Four additional integration regressions cover screen clone/reset and pin +release, erasure hooks, alternate-screen isolation/limit inheritance/full +reset, and the legacy command entrypoint plus in-progress transfer reset. + +## API changes and consumer wiring + +Added `Terminal::kitty_graphics_command(&Command)` and `ExecTerminal for +Terminal`; retained `Terminal::kitty_graphics(&mut Command)` unchanged. +`Screen.kitty_images` now has the required concrete ImageStorage type; +existing `.dirty` access remains valid. No downstream callers needed edits. + +Termio can pass APC `G` commands through either terminal entrypoint and encode +returned responses to the pty. Storage limits use +`screen.kitty_images.set_limit(&mut screen.pages, limit)`; clear-screen bindings +use `Delete::All(true)`. Renderer consumers can inspect placements/images and +Unicode placements and clear the storage dirty flag after updating graphics. +Consumer implementation beyond the public terminal API remains owned by the +concurrent renderer/termio agents. + +## Quality gate (2026-10-11 integration pass) + +`cargo test -p ghostty-terminal`: 1252 unit tests (including 70 Kitty tests: +54 source cases + 16 extra) and 1 documentation test pass; 0 failed, 0 ignored. +Clippy with `--all-targets -- -D warnings`, rustfmt, and the combined build of +`ghostty-font`, `ghostty-renderer`, and `ghostty-termio` pass. No commit or version +change was made. Existing non-integration deviations above are unchanged. diff --git a/docs/rewrite/notes/ghostty-terminal-pagelist-requests.md b/docs/rewrite/notes/ghostty-terminal-pagelist-requests.md index 5f22d10..f83d036 100644 --- a/docs/rewrite/notes/ghostty-terminal-pagelist-requests.md +++ b/docs/rewrite/notes/ghostty-terminal-pagelist-requests.md @@ -1,15 +1,62 @@ # Requests for page.rs / page_list.rs (from the Screen port) -1. **Pin-based iterators** (Zig `PageList.zig` ~5100-5170): `Pin::{row_iterator, cell_iterator, page_iterator, prompt_iterator}(direction, limit: Option)`. - The limit pin is inclusive and applies at row granularity. Screen uses a local copy in `pin_iter.rs`. -2. **`PageList::set_tracked_pin` validity**: it rejects pins with `y >= node.size.rows`. Zig writes the pin raw; `Screen::split_for_capacity` temporarily restores the cursor to the pre-split node, which may have shrunk. Worked around with a clamped stand-in. -3. **Cross-page row clone**: Zig `Page.cloneRowFrom(other_page, ...)` between two nodes of one list needs two page borrows. Screen implements it with `export_cell`/`import_cell` (`Screen::clone_row_between`). A `PageList::clone_row_between(src_pin, dst_pin)` would be better. -4. **String allocator probing**: `Page.string_alloc` is private; Zig `cursorSetHyperlink` probes it before growing. Screen instead re-adds the cursor hyperlink after `increase_capacity`. Also `insert_hyperlink` allocates strings before checking set dedup, so near-full pages can fail needlessly. -5. **`fixup_viewport` must handle `Viewport::Top`** after `erase_rows` (Zig PageList.zig ~3043-3070). Worked around in `Screen::erase_rows`. -6. **Resize reflow / no-reflow fewer rows** must drop trailing blank rows, including rows holding only bg-color cells (Zig `resizeCols`/`resizeWithoutReflow`). Three Screen tests are `#[ignore = "pagelist: ..."]`: - `screen_resize_no_reflow_less_rows_trims_blank_lines`, `screen_resize_less_cols_with_reflow_but_row_space`, `screen_resize_less_cols_to_wrap_wide_char`. -7. `PageList::reset` and `erase_rows` return `Result`; `erase_rows` with an empty history start point returns `InvalidCoordinate` (Zig would assert). Screen guards/ignores. -- (T3/p4) **`erase_row_bounded` must reset row flags on the blanked row**: after rotating, the final row is cleared with `Page::clear_cells`, which leaves `row.hyperlink`/`styled`/`grapheme` set. Zig's row is left with the flags cleared (test `Terminal: index bottom of scroll region clear hyperlinks` expects `!row.hyperlink`; ignored as `terminal_index_bottom_of_scroll_region_clear_hyperlinks`). -- (T3/p8) **Reflow to fewer cols with a pending-wrap cursor**: 4x2 screen printed "0123", `resize(2,2)` with reflow gives screen contents "01\n23" but the active area/viewport dump is only "23" (active top is one row too low; Zig yields "01\n23"). Ignored test: `terminal_resize_with_wraparound_on`. -- (T3/p1) **`Page::set_graphemes` / `Page::append_grapheme` must not mark the row dirty** (Zig `Page.appendGrapheme` leaves dirty untouched; callers like `Terminal.print` mark dirty explicitly). Terminal works around it for the zero-width path (test `terminal_ignored_vs16_doesnt_mark_dirty`). -- (T3/terminal) `PageList::clone_partial_row_between(src_pin, dst_pin, left, right)`: Terminal `insert_lines`/`delete_lines` need Zig's `Page.clonePartialRowFrom` between two different page nodes of one list (needs two page borrows). `terminal.rs::clone_partial_row_between` re-implements it via `export_cell`/`import_cell` (now `pub(crate)` use) plus the row-metadata merge from `Page::clone_partial_row_from`. +All requests resolved and verified in the 2026-10-11 integration pass. + +1. **Resolved — pin-based iterators.** `Pin::{row_iterator, cell_iterator, + page_iterator, prompt_iterator}` take the owning `&PageList`, direction, and + an inclusive optional limit pin. Limits apply to whole rows; cell iteration + begins at the starting column. Construction uses pins directly, without + point conversion or a list scan. Screen, Terminal, and Selection use these + methods. Public `pin_iter` functions/types remain as compatibility wrappers. +2. **Resolved — tracked-pin restoration.** `set_tracked_pin` validates the + handle and node lifetime, allowing a temporarily out-of-range row during + split recovery. Screen restores the exact old pin, with no clamped stand-in. + Cursor dirtying accesses the still-allocated row, matching Zig's cached + `page_row`. The maximum-style page-split regression passes. +3. **Resolved — cross-page row clone.** `PageList::clone_row_between(src, dst)` + borrows the two pages directly and calls `Page::clone_row_from`. Screen's + export/import row snapshot helper is removed. +4. **Resolved — string allocation and hyperlink deduplication.** Screen probes + `Page::can_allocate_string` before capacity growth in `cursor_set_hyperlink`, + following Zig's string-growth/hyperlink-growth/retry order. The probe frees + its temporary reservation in Rust; the page is then rebuilt. The cursor-link + recovery workaround is removed. `insert_hyperlink` retains existing live + entries before allocating strings; a full-allocator regression covers this. +5. **Resolved — Top viewport after erasure.** `fixup_viewport` handles `Top` + inside PageList; Screen has no viewport fixup. Resize still only fixes a + pinned viewport, preserving Zig's `Top` behavior (covered by the existing + resize-no-reflow-more-rows-contains-viewport test). +6. **Verified resolved — trailing blank rows during resize.** All three tests + are enabled and pass: `screen_resize_no_reflow_less_rows_trims_blank_lines`, + `screen_resize_less_cols_with_reflow_but_row_space`, and + `screen_resize_less_cols_to_wrap_wide_char`. +7. **Resolved — reset/erase error handling.** Existing public `Result` signatures + remain for backwards compatibility. Erasing empty history is a successful + no-op. Screen's extra history guard and error swallowing are removed; + valid internal erasure/reset calls assert success as in Zig. + +- **Verified resolved (T3/p4):** `erase_row_bounded` clears the blanked row's + managed flags; `terminal_index_bottom_of_scroll_region_clear_hyperlinks` + is enabled and passes. +- **Verified resolved (T3/p8):** pending-wrap reflow retains the correct active + rows; `terminal_resize_with_wraparound_on` is enabled and passes. +- **Resolved (T3/p1):** `set_graphemes` and `append_grapheme` preserve row dirty + state, including allocation growth. Terminal's save/restore workaround is + removed; the VS16 test and a direct append regression pass. +- **Resolved (T3/terminal):** `PageList::clone_partial_row_between(src, dst, + left, right)` directly borrows distinct pages and delegates managed-memory + and row-metadata handling to `Page::clone_partial_row_from`. Terminal's + export/import implementation is removed. The regression checks graphemes, + hyperlinks, untouched columns, dirty/wrap flags, and Kitty placeholder flags. + +## API changes + +Additive methods: the four Pin iterators and both PageList row-copy helpers. +Existing iterator wrappers and `Result` signatures are retained. No downstream +caller edits were needed. + +## Validation + +`cargo test -p ghostty-terminal`: 1252 unit tests and 1 documentation test pass, +0 failed, 0 ignored. Clippy with `-D warnings`, rustfmt, and the combined build +of `ghostty-font`, `ghostty-renderer`, and `ghostty-termio` pass. diff --git a/docs/rewrite/notes/ghostty-terminal-screen.md b/docs/rewrite/notes/ghostty-terminal-screen.md index 174c788..3bf8cf8 100644 --- a/docs/rewrite/notes/ghostty-terminal-screen.md +++ b/docs/rewrite/notes/ghostty-terminal-screen.md @@ -3,13 +3,13 @@ ## Ported | Zig | Rust | tests | |---|---|---| -| Screen.zig | `screen.rs` (+ `cursor.rs` for cursor.zig `Style`, `pin_iter.rs` helper) | 183 of 184 (`screen/tests/p1..p5.rs`) | +| Screen.zig | `screen.rs` (+ `cursor.rs` for cursor.zig `Style`) | 183 of 184 (`screen/tests/p1..p5.rs`) | | ScreenSet.zig | `screen_set.rs` | 1/1 | | Tabstops.zig | `tabstops.rs` | 4/5 | | Selection.zig | `selection.rs` | 18/18 | Skipped: `Tabstops: resize alloc failure preserves state` and `selectionString map allocation failure cleanup` (Zig tripwire failure injection). -Ignored (pagelist limitations, see `ghostty-terminal-pagelist-requests.md`): 3 resize tests. +All three formerly blocked resize tests are enabled and pass. No ignored tests. Deps added: ghostty-unicode (width in `test_write_string`), ghostty-datastruct (unused so far), log. ## Deviations @@ -19,11 +19,11 @@ Deps added: ghostty-unicode (width in `test_write_string`), ghostty-datastruct ( - `clone` is `clone_screen(top, bot)`. `cursor_copy(&Cursor, hyperlink: bool)` copies the hyperlink from the other cursor's heap copy. - `append_grapheme(x, cp)` takes a column of the cursor row. `clear_cells(node, y, x0, x1)` takes a half-open column range. - Selection methods take `&PageList` (`&screen.pages`) since tracked bounds are handles: `sel.start(&pages)`, `top_left`, `order`, `contains`, `contained_row`, `ordered`, `eql(&pages, &other)`, `adjust(&mut pages, adj)`, `track(&mut pages)`, `deinit(&mut pages)`. `Selection` is `Copy`. -- `kitty_images` is a placeholder `KittyImages { dirty }` (TODO(integration) until kitty graphics storage lands). `StringMap` is a local struct (TODO(integration)). -- Minimal plain-text formatter (`plain_format` in screen.rs) backs `selection_string[_map]`, `dump_string`, `dump_string_alloc[_unwrapped]`; TODO(integration): delegate to formatter.rs. +- `kitty_images` owns `kitty::graphics_storage::ImageStorage`: initialization applies the configured limit; reset releases placement pins and restores default storage with `dirty = true`; Drop releases storage before pages. `clone_screen` does not clone images. +- `StringMap` re-exports the real `string_map::StringMap`. +- Selection/dump strings use the real `formatter::ScreenFormatter`. - Tabstops::unset XORs like Zig (unsetting an unset stop sets it). -- `cursor_set_hyperlink` does not probe the string allocator (private); it re-adds the link after growth. -- `Screen::erase_rows` carries a `Viewport::Top` workaround (request 5). +- `cursor_set_hyperlink` follows the reference allocator probe and capacity-growth order. PageList owns viewport repair and cross-page row copying; Screen uses native Pin iterators and exact cursor restoration after page splitting. No PageList workarounds remain. ## API overview for Terminal `Screen` fields: `pages, no_scrollback, cursor, saved_cursor, selection, charset, protected_mode, kitty_keyboard, kitty_images, semantic_prompt, dirty`. @@ -32,3 +32,20 @@ Editing: `clear_cells`, `clear_unprotected_cells`, `clear_rows`, `erase_rows`, ` Style/links: `set_attribute(sgr::Attribute)`, `manual_style_update()`, `start_hyperlink(uri, id)`, `end_hyperlink()`, `cursor_set_hyperlink()`, `cursor_set_semantic_content(..)`. Selection: `select(Option)`, `clear_selection()`, `select_line/all/word/word_between/output`, `line_iterator`, `prompt_click_move`, `selection_string`. Test helper: `test_write_string`. + +## API changes (integration pass) + +`Screen.kitty_images` now exposes the required real `ImageStorage`; its existing +`dirty` field access remains valid. The old standalone `KittyImages` value type +is retained for compatibility, but is no longer used by Screen. Other public +signatures, including `Screen::reset() -> Result`, are unchanged. No renderer, +font, or termio callers needed edits. + +## Integration verification (2026-10-11) + +All Screen/Terminal TODO and Zig-reference comments were reviewed against the +reference. Remaining TODOs also occur in Zig (capacity-growth recovery and a +selection test note); no integration workaround remains. Lifecycle regressions +cover real image storage, clone isolation, reset, and placement-pin release. +Crate gate: 1252 unit + 1 documentation tests pass, none ignored; clippy with +warnings denied, rustfmt, and the combined font/renderer/termio build pass. diff --git a/docs/rewrite/notes/ghostty-terminal-terminal.md b/docs/rewrite/notes/ghostty-terminal-terminal.md index 23273cf..a267079 100644 --- a/docs/rewrite/notes/ghostty-terminal-terminal.md +++ b/docs/rewrite/notes/ghostty-terminal-terminal.md @@ -3,16 +3,15 @@ ## Ported | Zig | Rust | tests | |---|---|---| -| Terminal.zig | `terminal.rs` (+ `terminal/tests/p1..p8.rs`, helpers in `tests/mod.rs`) | 372 of 372 (370 pass, 2 `#[ignore = "pagelist: ..."]`) | +| Terminal.zig | `terminal.rs` (+ `terminal/tests/p1..p8.rs`, helpers in `tests/mod.rs`) | 372 of 372 (all pass) | | stream_readonly.zig | `stream_readonly.rs` | 33 of 33 | | StringMap.zig | `string_map.rs` (uses the `onig` crate; `ghostty-config` is a dev-dependency for the URL regex tests; `Screen`'s stand-in `StringMap` is now a re-export) | 3 of 3 | -Ignored tests (PageList gaps, see `ghostty-terminal-pagelist-requests.md`): -`terminal_index_bottom_of_scroll_region_clear_hyperlinks` (erase_row_bounded keeps the row hyperlink flag) and -`terminal_resize_with_wraparound_on` (reflow to fewer cols loses a row in the active dump). - -Small non-test deviations: `glitch.txt` copied next to the tests (used by the glitch-text test); zero-width attach in -`print` restores the row dirty flag because `Page::append_grapheme` marks rows dirty (Zig does not; request in the pagelist file). +The two formerly blocked tests (`terminal_index_bottom_of_scroll_region_clear_hyperlinks` +and `terminal_resize_with_wraparound_on`) are enabled and pass. `glitch.txt` +remains next to the tests. The zero-width grapheme dirty-state workaround and +local cross-page row snapshot helper have been removed: Page/PageList now own +those semantics. ## API overview (for termio / renderer / core) ```rust @@ -35,8 +34,38 @@ let text = t.plain_string()?; // viewport as text (p - Termio's own handler can call the same `Terminal` methods; it should handle side-effect actions (queries, title, clipboard, bell, APC/DCS) itself and delegate the rest, mirroring `stream_readonly::Handler::vt`. - Tests: Zig tests that mixed `s.nextSlice` and direct `t.` calls use `feed(&mut t, bytes)` (fresh stream per call) or `t.vt_stream()` + `s.handler.terminal`. -## TODO(integration) -- `Terminal::kitty_graphics(cmd)` is a stub returning `None` (Zig `kitty.graphics.execute`); `kitty_images_delete_all` (erase display complete/scroll complete) only sets `kitty_images.dirty`; kitty image storage limit on lazily-created screens uses `Options::DEFAULT`. `kitty_images.dirty = true` hooks are in place at every Zig location (index, scroll_up, insert/delete lines, switch_screen, Screen::scroll). -- tmux control-mode viewer: not required by Terminal/stream_readonly; not ported here. -- `clone_partial_row_between` local helper (pagelist request). -- Oniguruma: `onig` crate lacks the retry-limit-in-search setter; only retry-limit-in-match is set. +## Integration complete (2026-10-11) + +- `Terminal` implements `kitty::graphics_exec::ExecTerminal` using its active + screen's storage/pages and real cursor/index operations. +- `kitty_graphics` executes commands and returns real responses. Display erasure + uses `Delete::All(true)` at exactly the reference `.complete` and + `.scroll_complete` hooks (not `.scrollback`). Lazy alternate-screen creation + inherits the primary image limit. All existing dirty hooks remain at their + reference locations, including both branches of shared insert/delete-lines. +- Placeholder printing uses the shared Kitty Unicode constant. Full reset + tears down alternate-screen images and resets primary-screen storage. +- Pin iteration and partial row copying delegate to PageList. Reset/erase no + longer swallow internal errors. Existing public Result APIs are retained. +- Reviewed all workaround/TODO/stand-in/Zig comments. Remaining functional TODO + comments are inherited from Terminal.zig, not integration gaps. + +## API changes + +Added `kitty_graphics_command(&Command) -> Option` and the +`ExecTerminal` implementation. Existing `kitty_graphics(&mut Command)` is +unchanged and delegates to the new entrypoint. No downstream callers changed. + +## Remaining representation limitations + +- Oniguruma: `onig` lacks the retry-limit-in-search setter; only + retry-limit-in-match is set. +- tmux control-mode viewer is implemented separately in `tmux/`; it is not + part of Terminal/stream_readonly's integration surface. + +## Quality gate + +1252 unit tests and 1 documentation test pass, none failed or ignored. +`cargo clippy -p ghostty-terminal --all-targets -- -D warnings`, rustfmt, and +`cargo build -p ghostty-font -p ghostty-renderer -p ghostty-termio` pass. +No version bump or commit was made.