diff --git a/crates/neomacs-tui-tests/tests/send_string_to_terminal.rs b/crates/neomacs-tui-tests/tests/send_string_to_terminal.rs new file mode 100644 index 0000000000..436e2757f2 --- /dev/null +++ b/crates/neomacs-tui-tests/tests/send_string_to_terminal.rs @@ -0,0 +1,58 @@ +#![cfg(unix)] +//! Issue #453: `send-string-to-terminal` must reach the real terminal. +//! +//! KKP (`kkp.el`) negotiates with the terminal exclusively through this +//! primitive: the support query, the device-attributes request and the +//! `CSI > flags u` enable sequence all leave Lisp as bytes handed to +//! `send-string-to-terminal`. GNU's `Fsend_string_to_terminal` +//! (src/dispnew.c:6808) writes those bytes to the tty; a stub that drops +//! them leaves the terminal silent and raw ESC as prefix input. This +//! regression drives real PTYs and requires the marker bytes on BOTH +//! sides, exactly as a terminal would observe them. + +use crate::support::{boot_pair, eval_expression_one, settle_session}; +use neomacs_tui_tests::TuiSession; +use std::time::{Duration, Instant}; + +/// Unlikely enough that no startup, mode-line or dialog text contains it. +const MARKER: &[u8] = b"KKP-PROBE-4Q3F-MARKER"; + +/// The marker is raw PTY output, not screen content -- bytes that corrupt +/// the grid are exactly the interesting ones here -- so poll the transport +/// instead of waiting on a rendered predicate. +fn send_marker_and_read(session: &mut TuiSession, label: &str) { + session.clear_recent_output(); + eval_expression_one( + session, + &format!( + "(send-string-to-terminal \"{}\")", + std::str::from_utf8(MARKER).unwrap() + ), + ); + let deadline = Instant::now() + Duration::from_secs(15); + loop { + session.read(Duration::from_millis(100)); + if session + .recent_output() + .windows(MARKER.len()) + .any(|window| window == MARKER) + { + return; + } + assert!( + Instant::now() < deadline, + "{label}: send-string-to-terminal bytes never reached the PTY; \ + recent output: {:?}", + String::from_utf8_lossy(session.recent_output()) + ); + } +} + +#[test] +fn send_string_to_terminal_reaches_the_pty_like_gnu() { + let (mut gnu, mut neo) = boot_pair(""); + settle_session(&mut gnu); + settle_session(&mut neo); + send_marker_and_read(&mut gnu, "GNU"); + send_marker_and_read(&mut neo, "NEO"); +} diff --git a/crates/neomacs-tui-tests/tests/tui.rs b/crates/neomacs-tui-tests/tests/tui.rs index 7112087188..fabe0447d9 100644 --- a/crates/neomacs-tui-tests/tests/tui.rs +++ b/crates/neomacs-tui-tests/tests/tui.rs @@ -93,6 +93,8 @@ mod saving_insert; mod scroll_bar_tty; #[path = "search.rs"] mod search; +#[path = "send_string_to_terminal.rs"] +mod send_string_to_terminal; #[path = "shell_compile.rs"] mod shell_compile; #[path = "source_navigation.rs"] diff --git a/crates/neomacs/src/secondary_tty.rs b/crates/neomacs/src/secondary_tty.rs index f533406374..3e6ed1a711 100644 --- a/crates/neomacs/src/secondary_tty.rs +++ b/crates/neomacs/src/secondary_tty.rs @@ -123,6 +123,23 @@ impl SecondaryTtyRegistry { Err("additional text terminals are not supported on this platform".to_string()) } + #[cfg(unix)] + fn write(&self, terminal_id: u64, bytes: &[u8]) -> Result<(), String> { + let mut sessions = self + .sessions + .lock() + .map_err(|_| "secondary TTY registry poisoned".to_string())?; + sessions + .get_mut(&terminal_id) + .ok_or_else(|| "TTY terminal host unavailable".to_string())? + .write(bytes) + } + + #[cfg(not(unix))] + fn write(&self, _terminal_id: u64, _bytes: &[u8]) -> Result<(), String> { + Err("additional text terminals are not supported on this platform".to_string()) + } + fn remove(&self, terminal_id: u64) -> Result<(), String> { let session = self .sessions @@ -217,6 +234,10 @@ impl TerminalHost for SecondaryTtyHost { fn delete_terminal(&mut self) -> Result<(), String> { self.registry.remove(self.terminal_id) } + + fn write_bytes(&mut self, bytes: &[u8]) -> Result<(), String> { + self.registry.write(self.terminal_id, bytes) + } } struct SecondaryTtySession { @@ -448,6 +469,18 @@ impl SecondaryTtySession { self.paused.store(false, Ordering::Release); Ok(()) } + + /// Raw unbuffered terminal output, GNU's `fwrite`+`fflush` on + /// `tty->output' (src/dispnew.c:6838-6843): this terminal's own device, + /// never the primary stdout. + fn write(&mut self, bytes: &[u8]) -> Result<(), String> { + use std::io::Write as _; + self.device + .file + .write_all(bytes) + .and_then(|()| self.device.file.flush()) + .map_err(|error| format!("cannot write to the terminal: {error}")) + } } #[cfg(unix)] diff --git a/crates/neomacs/src/tty_frontend.rs b/crates/neomacs/src/tty_frontend.rs index 54ea4d4eb7..d83a2e331b 100644 --- a/crates/neomacs/src/tty_frontend.rs +++ b/crates/neomacs/src/tty_frontend.rs @@ -115,4 +115,17 @@ impl neovm_core::emacs_core::terminal::pure::TerminalHost for TtyTerminalHost { .send(RenderCommand::Lifecycle(LifecycleCommand::Shutdown)) .map_err(|err| format!("failed to delete tty terminal frontend: {err}")) } + + // GNU writes `tty->output' directly (src/dispnew.c:6838-6843); here that + // stream is stdout, and this builtin already runs on the same evaluator + // thread that performs every other TtyRif stdout write, so ordering with + // redisplay output is inherent. Same precedent as the popup-menu path. + fn write_bytes(&mut self, bytes: &[u8]) -> Result<(), String> { + use std::io::Write as _; + let mut stdout = io::stdout(); + stdout + .write_all(bytes) + .and_then(|()| stdout.flush()) + .map_err(|err| format!("failed to write to the terminal: {err}")) + } } diff --git a/crates/neovm-core/src/emacs_core/display/dispnew/pure.rs b/crates/neovm-core/src/emacs_core/display/dispnew/pure.rs index 7fa8b1ceeb..8be28434d0 100644 --- a/crates/neovm-core/src/emacs_core/display/dispnew/pure.rs +++ b/crates/neovm-core/src/emacs_core/display/dispnew/pure.rs @@ -8,6 +8,7 @@ use crate::emacs_core::display::live_frame_designator_p; use crate::emacs_core::error::LispCondition; use crate::emacs_core::error::{EvalResult, Flow, signal}; use crate::emacs_core::error::{expect_args, expect_args_range}; +use crate::emacs_core::terminal::pure::decode_terminal_id_eval; use crate::emacs_core::terminal::pure::expect_terminal_designator_eval; use crate::emacs_core::value::ValueKind; use crate::emacs_core::value::*; @@ -137,18 +138,34 @@ pub(crate) fn builtin_send_string_to_terminal( args: Vec, ) -> EvalResult { expect_args_range("send-string-to-terminal", &args, 1, 2)?; - match args[0].kind() { - ValueKind::String => { - if let Some(terminal) = args.get(1) { - expect_terminal_designator_eval(eval, terminal)?; - } - Ok(Value::NIL) + // GNU writes SBYTES raw, "without alteration" -- internal encoding bytes, + // never a lossy UTF-8 re-encoding (src/dispnew.c:6820-6821). + let string = match args[0].kind() { + ValueKind::String => args[0] + .as_lisp_string() + .expect("ValueKind::String must carry LispString payload"), + _other => { + return Err(signal( + LispCondition::WrongTypeArgument, + vec![Value::symbol("stringp"), args[0]], + )); } - _other => Err(signal( - LispCondition::WrongTypeArgument, - vec![Value::symbol("stringp"), args[0]], - )), + }; + if let Some(terminal) = args.get(1) { + expect_terminal_designator_eval(eval, terminal)?; } + // GNU `decode_live_terminal': nil is the selected frame's terminal + // (src/terminal.c:238-245); a garbage or deleted designator already + // failed `terminal-live-p' above. + let designator = args.get(1).copied().unwrap_or(Value::NIL); + let Some(terminal_id) = decode_terminal_id_eval(eval, &designator) else { + return Err(signal( + LispCondition::WrongTypeArgument, + vec![Value::symbol("terminal-live-p"), designator], + )); + }; + crate::emacs_core::terminal::pure::write_bytes_to_terminal(terminal_id, string.as_bytes())?; + Ok(Value::NIL) } /// Context-aware variant of `internal-show-cursor`. diff --git a/crates/neovm-core/src/emacs_core/display/dispnew/tests/mod.rs b/crates/neovm-core/src/emacs_core/display/dispnew/tests/mod.rs index ff6721248d..15266edfb7 100644 --- a/crates/neovm-core/src/emacs_core/display/dispnew/tests/mod.rs +++ b/crates/neovm-core/src/emacs_core/display/dispnew/tests/mod.rs @@ -75,6 +75,150 @@ fn send_string_to_terminal_accepts_string() { assert!(result.is_nil()); } +// Issue #453: GNU's `Fsend_string_to_terminal' writes the string to the +// terminal (`src/dispnew.c:6808'). A stub that validates arguments and drops +// the bytes silently breaks every Lisp-side terminal negotiation -- `kkp.el' +// enables the Kitty Keyboard Protocol and queries capabilities exclusively +// through this primitive, so nothing it sends can reach the tty. + +struct ByteRecordingHost { + writes: std::rc::Rc>>>, +} + +impl crate::emacs_core::terminal::pure::TerminalHost for ByteRecordingHost { + fn suspend_tty(&mut self) -> Result<(), String> { + Ok(()) + } + + fn resume_tty(&mut self) -> Result<(), String> { + Ok(()) + } + + fn write_bytes(&mut self, bytes: &[u8]) -> Result<(), String> { + self.writes.borrow_mut().push(bytes.to_vec()); + Ok(()) + } +} + +fn configure_interactive_tty_terminal() { + use crate::emacs_core::terminal::pure::TerminalRuntimeConfig; + use crate::emacs_core::terminal::pure::configure_terminal_runtime; + configure_terminal_runtime(TerminalRuntimeConfig::interactive( + Some("xterm-256color".to_string()), + neomacs_display_protocol::tty_capabilities::TtyAttributeCapabilities::full_with_color_cells( + 256, + ), + )); +} + +fn reset_terminal_to_bootstrap() { + use crate::emacs_core::terminal::pure::{reset_terminal_host, reset_terminal_runtime}; + reset_terminal_host(); + reset_terminal_runtime(); +} + +fn signal_message(result: crate::emacs_core::error::EvalResult) -> String { + match result.kinded() { + Err(FlowKind::Signal(sig)) => { + assert_eq!(sig.symbol_name(), "error"); + sig.data + .first() + .and_then(Value::as_lisp_string) + .map(|message| String::from_utf8_lossy(message.as_bytes()).into_owned()) + .unwrap_or_default() + } + other => panic!("expected an error signal, got {other:?}"), + } +} + +#[test] +fn send_string_to_terminal_delivers_bytes_to_the_termcap_host() { + crate::test_utils::init_test_tracing(); + configure_interactive_tty_terminal(); + let writes = std::rc::Rc::new(std::cell::RefCell::new(Vec::new())); + crate::emacs_core::terminal::pure::set_terminal_host(Box::new(ByteRecordingHost { + writes: std::rc::Rc::clone(&writes), + })); + let mut eval = crate::emacs_core::eval::Context::new(); + builtin_send_string_to_terminal(&mut eval, vec![Value::string("PROBE-BYTES")]).unwrap(); + assert_eq!( + writes.borrow().as_slice(), + [b"PROBE-BYTES".to_vec()], + "send-string-to-terminal must deliver its bytes to the terminal" + ); +} + +#[test] +fn send_string_to_terminal_writes_to_stdout_for_the_initial_terminal() { + crate::test_utils::init_test_tracing(); + reset_terminal_to_bootstrap(); + // GNU: `out = stdout' for `output_initial' -- what batch, daemon and + // pre-tty sessions run on (src/dispnew.c:6823-6824). nextest runs every + // test in its own process, so borrowing fd 1 for this assertion cannot + // leak into sibling tests. + let mut capture = tempfile::tempfile().unwrap(); + use std::io::{Read as _, Seek as _, Write as _}; + use std::os::fd::AsRawFd as _; + capture.flush().unwrap(); + let saved = unsafe { libc::dup(1) }; + assert!(saved >= 0); + unsafe { libc::dup2(capture.as_raw_fd(), 1) }; + let result = builtin_send_string_to_terminal( + &mut crate::emacs_core::eval::Context::new(), + vec![Value::string("PROBE-INITIAL-STDOUT")], + ); + unsafe { + libc::dup2(saved, 1); + libc::close(saved); + } + result.unwrap(); + capture.seek(std::io::SeekFrom::Start(0)).unwrap(); + let mut captured = Vec::new(); + capture.read_to_end(&mut captured).unwrap(); + assert_eq!( + captured, b"PROBE-INITIAL-STDOUT", + "the initial terminal's output is stdout" + ); +} + +#[test] +fn send_string_to_terminal_rejects_a_window_system_terminal() { + crate::test_utils::init_test_tracing(); + use crate::emacs_core::terminal::pure::TerminalRuntimeConfig; + use crate::emacs_core::terminal::pure::configure_terminal_runtime; + configure_terminal_runtime(TerminalRuntimeConfig::window_system( + neomacs_display_protocol::GraphicalDisplayIdentity::named( + neomacs_display_protocol::GraphicalBackend::X11, + ":0", + ) + .unwrap(), + )); + let mut eval = crate::emacs_core::eval::Context::new(); + let result = builtin_send_string_to_terminal(&mut eval, vec![Value::string("x")]); + assert_eq!( + signal_message(result), + "Device 0 is not a termcap terminal device", + "GNU's error for a non-termcap device (src/dispnew.c:6827)" + ); +} + +#[test] +fn send_string_to_terminal_rejects_a_suspended_terminal() { + crate::test_utils::init_test_tracing(); + configure_interactive_tty_terminal(); + crate::emacs_core::terminal::pure::set_terminal_host(Box::new(ByteRecordingHost { + writes: std::rc::Rc::default(), + })); + let mut eval = crate::emacs_core::eval::Context::new(); + crate::emacs_core::terminal::pure::builtin_suspend_tty(&mut eval, vec![]).unwrap(); + let result = builtin_send_string_to_terminal(&mut eval, vec![Value::string("x")]); + assert_eq!( + signal_message(result), + "Terminal is currently suspended", + "GNU's error for a tty without `tty->output' (src/dispnew.c:6835)" + ); +} + #[test] fn internal_show_cursor_tracks_visibility() { crate::test_utils::init_test_tracing(); diff --git a/crates/neovm-core/src/emacs_core/display/terminal/pure.rs b/crates/neovm-core/src/emacs_core/display/terminal/pure.rs index fe823f0f03..57cbbf3d2c 100644 --- a/crates/neovm-core/src/emacs_core/display/terminal/pure.rs +++ b/crates/neovm-core/src/emacs_core/display/terminal/pure.rs @@ -170,6 +170,15 @@ pub trait TerminalHost { fn delete_terminal(&mut self) -> Result<(), String> { Ok(()) } + /// GNU `Fsend_string_to_terminal`'s `fwrite`+`fflush` on the terminal's + /// output stream (src/dispnew.c:6838-6843). The host owns the fd, so the + /// write and its flush belong behind this method. A host that owns no + /// output stream rejects; the caller distinguishes GNU's "not a termcap + /// terminal device" from "Terminal is currently suspended" using the + /// terminal record, not this error. + fn write_bytes(&mut self, _bytes: &[u8]) -> Result<(), String> { + Err("terminal host has no output stream".to_owned()) + } } /// Validated identity of a text terminal a frontend must open for one frame. @@ -806,7 +815,10 @@ fn selected_terminal_id(eval: &crate::emacs_core::eval::Context) -> Option }) } -fn decode_terminal_id_eval(eval: &crate::emacs_core::eval::Context, value: &Value) -> Option { +pub(crate) fn decode_terminal_id_eval( + eval: &crate::emacs_core::eval::Context, + value: &Value, +) -> Option { if value.is_nil() { return selected_terminal_id(eval); } @@ -995,6 +1007,45 @@ fn terminal_output_method_for_id(id: u64) -> Option { with_terminal_manager(|slot| slot.borrow().get(id).map(|terminal| terminal.output_method)) } +/// GNU `Fsend_string_to_terminal`'s terminal dispatch +/// (src/dispnew.c:6819-6843): route STRING's bytes to the terminal's output +/// stream, unaltered. The Lisp-visible argument decoding lives with the +/// builtin; everything that reads the terminal record lives here. +pub(crate) fn write_bytes_to_terminal(terminal_id: u64, bytes: &[u8]) -> Result<(), Flow> { + match terminal_output_method_for_id(terminal_id) { + // GNU: `out = stdout' for `output_initial' -- what batch, daemon and + // pre-tty sessions run on (src/dispnew.c:6823-6824). + Some(TerminalOutputMethod::Initial) => { + use std::io::Write as _; + // GNU ignores the fwrite result here; the tty has nowhere better + // to report a failed diagnostic write. + let _ = std::io::stdout().write_all(bytes); + let _ = std::io::stdout().flush(); + Ok(()) + } + // GNU: `error("Device %d is not a termcap terminal device", t->id)' + // (src/dispnew.c:6826-6827). `None' cannot survive the caller's + // live-terminal decode; treat it with the same answer for totality. + Some(TerminalOutputMethod::WindowSystem) | None => Err(signal( + "error", + vec![Value::string(format!( + "Device {terminal_id} is not a termcap terminal device" + ))], + )), + Some(TerminalOutputMethod::Termcap) => { + // GNU: `!tty->output' means the terminal is suspended + // (src/dispnew.c:6833-6835). + if terminal_runtime_for_id(terminal_id).suspended { + return Err(signal( + "error", + vec![Value::string("Terminal is currently suspended")], + )); + } + with_terminal_host_for_id(terminal_id, |host| host.write_bytes(bytes)) + } + } +} + /// Mark the selected terminal as having a controlling tty, so it can host a /// text-terminal frame. Used by tests that exercise `make-frame` / /// `make-terminal-frame`, which in a real session run on an interactive