From f0a235f635562854b9dac4eb80089bd464854f30 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Tue, 21 Jul 2026 15:30:06 -0400 Subject: [PATCH] fix(vterm): harden Stage 1 terminal contracts Add typed IND, NEL, and RI operations with exact screen semantics, preserve application tab stops across resize, and default terminal children to the supported xterm-256color capability set. Make shutdown liveness acceptance portable with kill(pid, 0), and document the public TerminalScreen methods consumed by later stages. --- src/ansi.rs | 6 +++ src/lua_bindings/mod.rs | 3 ++ src/terminal/screen.rs | 75 +++++++++++++++++++++++++++++++- src/terminal/session.rs | 8 +++- tests/vterm_stage1_acceptance.rs | 14 +++--- 5 files changed, 98 insertions(+), 8 deletions(-) diff --git a/src/ansi.rs b/src/ansi.rs index e876b01..f7a74b7 100644 --- a/src/ansi.rs +++ b/src/ansi.rs @@ -176,6 +176,12 @@ pub enum AnsiEvent { /// Full-screen-only terminal operations. Bell, LineFeed, + /// `ESC D`: advance one row, scrolling at the bottom margin. + Index, + /// `ESC E`: return to column zero and advance one row. + NextLine, + /// `ESC M`: move up one row, scrolling down at the top margin. + ReverseIndex, HorizontalTab, SetTabStop, ClearTabStop, diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index f2acb6c..9af5b5b 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -4970,6 +4970,9 @@ fn event_to_lua_table(lua: &Lua, ev: &crate::ansi::AnsiEvent) -> mlua::Result t.set("kind", "bell")?, AnsiEvent::LineFeed => t.set("kind", "line_feed")?, + AnsiEvent::Index => t.set("kind", "index")?, + AnsiEvent::NextLine => t.set("kind", "next_line")?, + AnsiEvent::ReverseIndex => t.set("kind", "reverse_index")?, AnsiEvent::HorizontalTab => t.set("kind", "horizontal_tab")?, AnsiEvent::SetTabStop => t.set("kind", "set_tab_stop")?, AnsiEvent::ClearTabStop => t.set("kind", "clear_tab_stop")?, diff --git a/src/terminal/screen.rs b/src/terminal/screen.rs index f7aafd8..cfc7f3b 100644 --- a/src/terminal/screen.rs +++ b/src/terminal/screen.rs @@ -199,6 +199,8 @@ impl TerminalScreen { }) } + /// Apply one parsed terminal operation and return an optional fixed device + /// reply for the session manager to queue to the child. #[allow(clippy::too_many_lines, clippy::let_and_return, clippy::cast_lossless)] pub fn apply_event(&mut self, event: AnsiEvent) -> Option> { if !matches!(&event, AnsiEvent::Text(_)) { @@ -235,6 +237,19 @@ impl TerminalScreen { self.line_feed(false); None } + AnsiEvent::Index => { + self.line_feed(false); + None + } + AnsiEvent::NextLine => { + self.cursor.col = 0; + self.line_feed(false); + None + } + AnsiEvent::ReverseIndex => { + self.reverse_index(); + None + } AnsiEvent::HorizontalTab => { self.horizontal_tab(); None @@ -410,6 +425,7 @@ impl TerminalScreen { reply } + /// Release a synchronized-output batch at EOF or session completion. pub fn finish_output(&mut self) { if self.modes.synchronized_output { self.modes.synchronized_output = false; @@ -453,6 +469,8 @@ impl TerminalScreen { self.finish_output(); } + /// Resize the main screen with soft-wrap reflow and clip/pad the alternate + /// screen, preserving cursor, history, and application tab-stop state. pub fn resize(&mut self, size: CellSize) -> Result<(), ScreenError> { validate_size(size)?; if size == self.size { @@ -475,8 +493,11 @@ impl TerminalScreen { self.cursor.col = self.cursor.col.min(size.cols as usize - 1); self.cursor.pending_wrap = false; self.tab_stops.retain(|&col| col < size.cols as usize); - for col in (8..size.cols as usize).step_by(8) { - self.tab_stops.insert(col); + if size.cols > old_size.cols { + let first_new_default = (old_size.cols as usize).div_ceil(8) * 8; + for col in (first_new_default..size.cols as usize).step_by(8) { + self.tab_stops.insert(col); + } } self.enforce_history_budget(); self.changed(); @@ -749,6 +770,16 @@ impl TerminalScreen { self.changed(); } + fn reverse_index(&mut self) { + self.cursor.pending_wrap = false; + if self.cursor.row == self.scroll_top { + self.scroll_down(1); + } else if self.cursor.row > 0 { + self.cursor.row -= 1; + self.changed(); + } + } + fn horizontal_tab(&mut self) { let cols = self.size.cols as usize; self.cursor.col = self @@ -1679,6 +1710,46 @@ mod tests { assert!(budget.history().len() * 512 <= MAX_TERMINAL_HISTORY_CELLS); } + #[test] + fn index_next_line_and_reverse_index_respect_scrolling_margins() { + let mut s = screen(4, 4); + for (row, value) in ["aaaa", "bbbb", "cccc", "dddd"].into_iter().enumerate() { + s.apply_event(AnsiEvent::CursorPosition { + row: row as u32 + 1, + col: 1, + }); + s.apply_event(AnsiEvent::Text(value.into())); + } + s.apply_event(AnsiEvent::SetScrollingRegion { + top: 2, + bottom: Some(3), + }); + s.apply_event(AnsiEvent::CursorPosition { row: 2, col: 3 }); + s.apply_event(AnsiEvent::ReverseIndex); + assert_eq!(&text(&s.snapshot()), "aaaa bbbbdddd"); + assert_eq!(s.snapshot().cursor, Some(CellCoord::new(1, 2))); + + s.apply_event(AnsiEvent::Index); + assert_eq!(s.snapshot().cursor, Some(CellCoord::new(2, 2))); + s.apply_event(AnsiEvent::NextLine); + assert_eq!(&text(&s.snapshot()), "aaaabbbb dddd"); + assert_eq!(s.snapshot().cursor, Some(CellCoord::new(2, 0))); + } + + #[test] + fn resize_only_adds_default_tab_stops_in_new_columns() { + let mut s = screen(2, 16); + s.apply_event(AnsiEvent::ClearAllTabStops); + s.apply_event(AnsiEvent::CursorHorizontalAbsolute(4)); + s.apply_event(AnsiEvent::SetTabStop); + s.resize(CellSize::new(2, 32)).unwrap(); + s.apply_event(AnsiEvent::CursorHorizontalAbsolute(1)); + s.apply_event(AnsiEvent::HorizontalTab); + assert_eq!(s.snapshot().cursor, Some(CellCoord::new(0, 3))); + s.apply_event(AnsiEvent::HorizontalTab); + assert_eq!(s.snapshot().cursor, Some(CellCoord::new(0, 16))); + } + #[test] fn annotation_is_default_style_hard_line() { let mut s = screen(3, 20); diff --git a/src/terminal/session.rs b/src/terminal/session.rs index 12bb1ed..8e0e672 100644 --- a/src/terminal/session.rs +++ b/src/terminal/session.rs @@ -34,7 +34,8 @@ pub struct TerminalSpec { pub args: Vec, /// Working directory, or the editor process directory when absent. pub cwd: Option, - /// Environment overrides inherited by the child. + /// Environment overrides inherited by the child. `TERM` defaults to + /// `xterm-256color` when the caller does not provide it. pub env: Vec<(String, String)>, /// Identity-buffer name. Defaults to `*terminal:*`. pub name: Option, @@ -264,6 +265,11 @@ impl TerminalManager { process_spec.args = spec.args; process_spec.cwd = spec.cwd; process_spec.env = spec.env; + if !process_spec.env.iter().any(|(name, _)| name == "TERM") { + process_spec + .env + .push(("TERM".into(), "xterm-256color".into())); + } process_spec.mode = ProcessMode::Pty { rows: spec.rows, cols: spec.cols, diff --git a/tests/vterm_stage1_acceptance.rs b/tests/vterm_stage1_acceptance.rs index f86d429..2ec28a3 100644 --- a/tests/vterm_stage1_acceptance.rs +++ b/tests/vterm_stage1_acceptance.rs @@ -119,7 +119,10 @@ fn strict_owned_spec_rejects_before_spawn_and_is_mutation_independent() { ); assert_eq!( process_spec.env, - [(String::from("PMACS_VTERM_OWNED"), String::from("original"))] + [ + (String::from("PMACS_VTERM_OWNED"), String::from("original")), + (String::from("TERM"), String::from("xterm-256color")), + ] ); } @@ -380,13 +383,14 @@ fn editor_shutdown_kills_term_ignoring_terminal_child() { .pid }; + let pid = nix::unistd::Pid::from_raw(i32::try_from(pid).expect("pid fits i32")); let deadline = Instant::now() + Duration::from_secs(2); - let proc_path = format!("/proc/{pid}"); - while Instant::now() < deadline && std::path::Path::new(&proc_path).exists() { + while Instant::now() < deadline && nix::sys::signal::kill(pid, None).is_ok() { std::thread::sleep(Duration::from_millis(10)); } - assert!( - !std::path::Path::new(&proc_path).exists(), + assert_eq!( + nix::sys::signal::kill(pid, None), + Err(nix::errno::Errno::ESRCH), "terminal child {pid} survived EditorState shutdown" ); }