diff --git a/Cargo.lock b/Cargo.lock index 7e9644be7..5d5d71440 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2572,6 +2572,7 @@ dependencies = [ "comrak", "glob", "insta", + "jp_term", "proptest", "syntect", "two-face", diff --git a/crates/jp_cli/src/cmd/query/tool/coordinator.rs b/crates/jp_cli/src/cmd/query/tool/coordinator.rs index 7c6908fde..a5ba55b28 100644 --- a/crates/jp_cli/src/cmd/query/tool/coordinator.rs +++ b/crates/jp_cli/src/cmd/query/tool/coordinator.rs @@ -553,13 +553,14 @@ impl ToolCoordinator { interactive: bool, turn_state: &mut TurnState, tool_renderer: &ToolRenderer, + printer: &Printer, ) -> ToolCallDecision { // A tool call reached from a reasoning block sits inside that block's - // shading, and a prompt is a visual row like any other (RFD 095). The - // prompter writes through the printer rather than through the - // renderer, so it has to be handed the background rather than finding - // it. - prompter.set_background(tool_renderer.current_region()); + // shading, and a prompt is a visual row like any other (RFD 095). This + // is the one place holding both the renderer that owns the region and + // the printer that draws the prompts, and the region is per tool, so + // the read happens here rather than at the tool-call boundary. + printer.set_prompt_background(tool_renderer.current_region()); // Step 1: decide. let decision = self.decide_permission(executor, interactive, turn_state); @@ -886,6 +887,7 @@ impl ToolCoordinator { interactive: bool, turn_state: &mut TurnState, tool_renderer: &ToolRenderer, + printer: &Printer, ) -> ( Vec<(usize, Box)>, Vec<(usize, ToolCallResponse)>, @@ -905,6 +907,7 @@ impl ToolCoordinator { interactive, turn_state, tool_renderer, + printer, ) .await; diff --git a/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs b/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs index b0565e273..f0650f3c6 100644 --- a/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs +++ b/crates/jp_cli/src/cmd/query/tool/coordinator_tests.rs @@ -493,7 +493,14 @@ async fn test_resolve_tool_call_decision_invalidates_prerender_on_edit() { let mut turn_state = TurnState::default(); let decision = coordinator - .resolve_tool_call_decision(executor, &prompter, true, &mut turn_state, &tool_renderer) + .resolve_tool_call_decision( + executor, + &prompter, + true, + &mut turn_state, + &tool_renderer, + &printer, + ) .await; match decision { diff --git a/crates/jp_cli/src/cmd/query/tool/prompter.rs b/crates/jp_cli/src/cmd/query/tool/prompter.rs index b0f30f77e..b316ae88a 100644 --- a/crates/jp_cli/src/cmd/query/tool/prompter.rs +++ b/crates/jp_cli/src/cmd/query/tool/prompter.rs @@ -10,11 +10,7 @@ //! imperative shell" principle. //! The `jp_llm` crate remains pure. -use std::{ - fmt::Write as _, - io::{self, Write as _}, - sync::{Arc, Mutex, PoisonError}, -}; +use std::{io::Write as _, sync::Arc}; use crossterm::style::Stylize as _; use jp_config::conversation::tool::{RunMode, ToolSource}; @@ -22,7 +18,6 @@ use jp_conversation::event::SelectOption; use jp_editor::{EditOutcome, EditorBackend}; use jp_inquire::{InlineOption, ReplyEditMode, ReplyOutcome, prompt::PromptBackend}; use jp_llm::tool::executor::PermissionInfo; -use jp_md::{format::DefaultBackground, shade::ShadedWriter}; use jp_printer::{Printer, PromptWriter}; use jp_tool::AnswerType; use serde_json::Value; @@ -91,63 +86,6 @@ pub struct ToolPrompter { edit_mode: ReplyEditMode, printer: Arc, - - /// The reasoning-region background prompts are drawn against. - /// - /// Set per tool call by the coordinator, which is the only place that knows - /// both this prompter and the renderer holding the region. - background: Mutex>, -} - -/// A prompt writer carrying the reasoning background, when one is open. -/// -/// A prompt drawn inside a reasoning block shows that block's background like -/// every other row (RFD 095), and a widget owns its own cursor — it rewrites -/// its line with `\r\x1b[K` on each keystroke, and that erase fills with -/// whatever background is active. -/// [`ShadedWriter`] is what keeps the fill right across the widget's own -/// escapes, including the resets it emits mid-line. -enum PromptCanvas<'a> { - /// No reasoning block is open; writes pass straight through. - Plain(PromptWriter<'a>), - - /// Writes are shaded with the open block's background. - Shaded(Box>>), -} - -impl io::Write for PromptCanvas<'_> { - fn write(&mut self, buf: &[u8]) -> io::Result { - match self { - Self::Plain(writer) => io::Write::write(writer, buf), - Self::Shaded(writer) => { - let text = str::from_utf8(buf).map_err(io::Error::other)?; - writer.write_str(text).map_err(io::Error::other)?; - Ok(buf.len()) - } - } - } - - /// A widget flushes before it reads a key, and on this path a flush is a - /// barrier rather than a buffer drain: it blocks until the printer's worker - /// has written everything queued ahead of it. - /// Shading decorates the writer and must not swallow that. - fn flush(&mut self) -> io::Result<()> { - match self { - Self::Plain(writer) => io::Write::flush(writer), - Self::Shaded(writer) => io::Write::flush(writer.get_mut()), - } - } -} - -impl Drop for PromptCanvas<'_> { - /// Close the background here rather than at the end of each prompt: a - /// prompt can end by cancellation or by an error, and a background left - /// open paints everything printed after it. - fn drop(&mut self) { - if let Self::Shaded(writer) = self { - let _err = writer.finish(); - } - } } impl ToolPrompter { @@ -166,34 +104,6 @@ impl ToolPrompter { prompt_backend, edit_mode, printer, - background: Mutex::new(None), - } - } - - /// Draw prompts against `background` until it is replaced. - /// - /// `None` while no reasoning block is open, which is the common case. - pub(crate) fn set_background(&self, background: Option) { - *self - .background - .lock() - .unwrap_or_else(PoisonError::into_inner) = background; - } - - /// A prompt writer shaded with the open reasoning block, if there is one. - fn canvas(&self) -> PromptCanvas<'_> { - let writer = self.printer.prompt_writer(); - - match self - .background - .lock() - .unwrap_or_else(PoisonError::into_inner) - .as_ref() - { - Some(background) => { - PromptCanvas::Shaded(Box::new(ShadedWriter::new(writer, background))) - } - None => PromptCanvas::Plain(writer), } } @@ -205,7 +115,6 @@ impl ToolPrompter { prompt_backend: Arc, ) -> Self { Self { - background: Mutex::new(None), editor, prompt_backend, edit_mode: ReplyEditMode::Emacs, @@ -280,7 +189,7 @@ impl ToolPrompter { let inline_options = select_options_to_inline(&Self::permission_options()); - let mut writer = self.canvas(); + let mut writer = self.printer.prompt_writer(); match self .prompt_backend @@ -498,7 +407,7 @@ impl ToolPrompter { /// - `Ok(true)` if user confirms delivery /// - `Ok(false)` if user skips delivery pub fn prompt_result_confirmation(&self, tool_name: &str) -> Result { - let mut writer = self.canvas(); + let mut writer = self.printer.prompt_writer(); let question = format!("Deliver {} result to assistant?", tool_name.yellow().bold()); @@ -539,7 +448,7 @@ impl ToolPrompter { /// A `QuestionResult` containing the answer and `persist_level` which /// indicates whether the answer should be remembered for this turn. pub fn prompt_question(&self, question: &jp_tool::Question) -> Result { - let mut writer = self.canvas(); + let mut writer = self.printer.prompt_writer(); if let Some(pre_amble) = &question.pre_amble { writeln!(writer, "{pre_amble}")?; @@ -600,7 +509,7 @@ impl ToolPrompter { fn prompt_boolean_git_style( &self, question: &jp_tool::Question, - writer: &mut PromptCanvas<'_>, + writer: &mut PromptWriter<'_>, ) -> Result { let options = vec![ InlineOption::new('y', "yes, just this once"), diff --git a/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs b/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs index b0a0d1117..50d3c66a7 100644 --- a/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs +++ b/crates/jp_cli/src/cmd/query/tool/prompter_tests.rs @@ -1,9 +1,9 @@ -use std::{sync::Arc, time::Duration}; +use std::sync::Arc; use jp_editor::MockEditorBackend; use jp_inquire::{ReplyOutcome, prompt::MockPromptBackend}; -use jp_md::format::BackgroundFill; -use jp_printer::{OutputFormat, PrintableExt as _, SharedBuffer}; +use jp_printer::{OutputFormat, SharedBuffer}; +use jp_term::background::{BackgroundFill, DefaultBackground}; use serde_json::json; use super::*; @@ -32,65 +32,16 @@ fn terminal_region() -> DefaultBackground { } #[test] -fn a_prompt_inside_a_reasoning_block_carries_its_background() { - let (prompter, out) = prompter_with_output(MockPromptBackend::new()); - prompter.set_background(Some(terminal_region())); - - { - let mut canvas = prompter.canvas(); - write!(canvas, "Run local shell tool?").unwrap(); - } - prompter.printer.flush(); - - // The background is asserted before the text and closed once the widget is - // done with the terminal, so the row is shaded and nothing after it is. - assert_eq!(*out.lock(), "\x1b[48;5;236mRun local shell tool?\x1b[49m"); -} - -#[test] -fn a_prompt_outside_a_reasoning_block_is_unshaded() { - let (prompter, out) = prompter_with_output(MockPromptBackend::new()); - - { - let mut canvas = prompter.canvas(); - write!(canvas, "Run local shell tool?").unwrap(); - } - prompter.printer.flush(); - - assert_eq!(*out.lock(), "Run local shell tool?"); -} - -#[test] -fn a_cancelled_prompt_still_closes_its_background() { - // A widget can end by `Ctrl+C` or by an error, neither of which returns - // through the normal path. The close lives in `Drop` so the background - // cannot outlive the prompt and paint whatever is printed next. - let (prompter, out) = prompter_with_output(MockPromptBackend::new()); - prompter.set_background(Some(terminal_region())); - - { - let mut canvas = prompter.canvas(); - write!(canvas, "Deliver result?").unwrap(); - // No further writes: the prompt is abandoned mid-session. - } - prompter.printer.flush(); - - let rendered = out.lock().clone(); - assert!( - rendered.ends_with("\x1b[49m"), - "an abandoned prompt must still close its background, got {rendered:?}" - ); -} - -#[test] -fn prompting_a_question_shades_through_the_canvas() { - // The tests above drive the canvas directly, so they hold even if a prompt - // method still reached for a bare prompt writer. This one goes through - // `prompt_question`, whose pre-amble is the one part of a prompt written by - // the prompter rather than by the widget. +fn prompting_a_question_carries_the_reasoning_background() { + // `jp_printer` owns the shading and pins its escapes; what this covers is + // that a real prompt method reaches it, rather than building a writer of + // its own. The pre-amble is the one part of a prompt written by the + // prompter rather than by the widget. let (prompter, out) = prompter_with_output(MockPromptBackend::new().with_inline_responses(['y'])); - prompter.set_background(Some(terminal_region())); + prompter + .printer + .set_prompt_background(Some(terminal_region())); let mut question = jp_tool::Question::boolean("confirm", "Proceed?").expect("valid question"); question.pre_amble = Some("About to run a shell command".to_owned()); @@ -110,53 +61,30 @@ fn prompting_a_question_shades_through_the_canvas() { } #[test] -fn flushing_a_shaded_canvas_waits_for_the_printer() { - // A widget flushes its writer before it reads a key, and on this path a - // flush is a barrier rather than a buffer drain: it is what makes the bytes - // have landed before the widget takes the cursor and the terminal's mode. - // Shading is a decoration over that writer and has no business swallowing - // it. - let (prompter, out) = prompter_with_output(MockPromptBackend::new()); - prompter.set_background(Some(terminal_region())); - - let mut canvas = prompter.canvas(); - - // Queued after acquisition drained the printer, and slow enough that the - // worker is certainly still inside it: without a real barrier the prompt's - // own text cannot have reached the terminal yet. +fn the_inline_reply_widget_carries_the_reasoning_background() { + // `edit_result` runs the inline reply widget, which owns its output stream + // and so takes `owned_prompt_writer` rather than the borrowed one. It is + // reached by `e` from a permission prompt, inside the same block. + let (prompter, out) = prompter_with_output( + MockPromptBackend::new().with_reply_outcomes([ReplyOutcome::Submit("edited".into())]), + ); prompter .printer - .print("slow".typewriter(Duration::from_millis(100))); + .set_prompt_background(Some(terminal_region())); - write!(canvas, "Run local shell tool?").unwrap(); - canvas.flush().unwrap(); + prompter + .edit_result("original") + .expect("the mock submits the edit"); + prompter.printer.flush(); - // Deliberately no `printer.flush()`, which is the assertion. + let rendered = out.lock().clone(); assert!( - out.lock().contains("Run local shell tool?"), - "flushing the canvas must drain the printer, got {:?}", - *out.lock() + rendered.starts_with("\x1b[48;5;236m"), + "the widget's writes must open under the background, got {rendered:?}" ); -} - -#[test] -fn clearing_the_background_unshades_later_prompts() { - let (prompter, out) = prompter_with_output(MockPromptBackend::new()); - - prompter.set_background(Some(terminal_region())); - drop(prompter.canvas()); - prompter.set_background(None); - - { - let mut canvas = prompter.canvas(); - write!(canvas, "after").unwrap(); - } - prompter.printer.flush(); - assert!( - out.lock().ends_with("after"), - "a prompt after the block closed carries no background: {:?}", - *out.lock() + rendered.ends_with("\x1b[49m"), + "and close it when the widget gives the terminal back, got {rendered:?}" ); } diff --git a/crates/jp_cli/src/cmd/query/turn/coordinator.rs b/crates/jp_cli/src/cmd/query/turn/coordinator.rs index 1c6051476..2f657ac4e 100644 --- a/crates/jp_cli/src/cmd/query/turn/coordinator.rs +++ b/crates/jp_cli/src/cmd/query/turn/coordinator.rs @@ -9,8 +9,8 @@ use jp_llm::{ event::{Event, EventPart, FinishReason}, event_builder::EventBuilder, }; -use jp_md::format::DefaultBackground; use jp_printer::Printer; +use jp_term::background::DefaultBackground; use crate::{ cmd::query::{interrupt::InterruptAction, stream::TurnView}, diff --git a/crates/jp_cli/src/cmd/query/turn_loop.rs b/crates/jp_cli/src/cmd/query/turn_loop.rs index 85c0072f3..37068c68e 100644 --- a/crates/jp_cli/src/cmd/query/turn_loop.rs +++ b/crates/jp_cli/src/cmd/query/turn_loop.rs @@ -637,6 +637,7 @@ pub(super) async fn run_turn_loop( interactive, &mut turn_state, &tool_renderer, + &printer, ) .await; @@ -729,6 +730,7 @@ pub(super) async fn run_turn_loop( interactive, &mut turn_state, &tool_renderer, + &printer, ) .await; diff --git a/crates/jp_cli/src/render/chat.rs b/crates/jp_cli/src/render/chat.rs index 878d5001d..e302adc2a 100644 --- a/crates/jp_cli/src/render/chat.rs +++ b/crates/jp_cli/src/render/chat.rs @@ -38,13 +38,11 @@ use jp_config::style::{ use jp_conversation::event::ChatResponse; use jp_md::{ buffer::{Buffer, Event, Fixups}, - format::{ - BackgroundFill, CodeBlockState, DefaultBackground, Formatter, TerminalOptions, - render_separator, - }, + format::{CodeBlockState, Formatter, TerminalOptions, render_separator}, theme, }; use jp_printer::{OutputWidth, PrintableExt as _, Printer, RegionStyle, StatusRegion}; +use jp_term::background::{BackgroundFill, DefaultBackground}; use tracing::warn; /// The kind of content last pushed into the renderer. diff --git a/crates/jp_cli/src/render/tool.rs b/crates/jp_cli/src/render/tool.rs index e499ec5a2..1c71dd8d9 100644 --- a/crates/jp_cli/src/render/tool.rs +++ b/crates/jp_cli/src/render/tool.rs @@ -22,12 +22,9 @@ use jp_config::{ }; use jp_conversation::event::ToolCallResponse; use jp_llm::{CommandResult, run_tool_command, tool::InvocationContext}; -use jp_md::{ - format::{DefaultBackground, Formatter}, - shade::ShadedWriter, -}; +use jp_md::format::Formatter; use jp_printer::{ErrChannel, LineSink, OutputLines, RegionStyle, StatusRegion}; -use jp_term::osc::hyperlink; +use jp_term::{background::DefaultBackground, osc::hyperlink, shade::ShadedWriter}; use serde_json::{Map, Value}; use tokio_util::sync::CancellationToken; use tracing::warn; diff --git a/crates/jp_cli/src/render/tool_tests.rs b/crates/jp_cli/src/render/tool_tests.rs index d3651d50a..36e7c678d 100644 --- a/crates/jp_cli/src/render/tool_tests.rs +++ b/crates/jp_cli/src/render/tool_tests.rs @@ -7,8 +7,8 @@ use jp_config::{ style::stderr_rows::{RowCount, StderrRows}, }; use jp_conversation::event::ToolCallResponse; -use jp_md::format::{BackgroundFill, DefaultBackground}; use jp_printer::{ErrChannel, OutputFormat, Printer, SharedBuffer, TerminalCapability}; +use jp_term::background::{BackgroundFill, DefaultBackground}; use serde_json::{Map, Value}; use super::*; diff --git a/crates/jp_cli/src/render/turn_view.rs b/crates/jp_cli/src/render/turn_view.rs index b47c88b43..64dfbc1de 100644 --- a/crates/jp_cli/src/render/turn_view.rs +++ b/crates/jp_cli/src/render/turn_view.rs @@ -20,8 +20,8 @@ use std::sync::{ use jp_config::style::StyleConfig; use jp_conversation::event::{ChatRequest, ChatResponse}; -use jp_md::format::DefaultBackground; use jp_printer::Printer; +use jp_term::background::DefaultBackground; use super::{ChatRenderer, RenderFlow, StructuredRenderer}; diff --git a/crates/jp_inquire/src/prompt.rs b/crates/jp_inquire/src/prompt.rs index b6be20e20..c41d65281 100644 --- a/crates/jp_inquire/src/prompt.rs +++ b/crates/jp_inquire/src/prompt.rs @@ -287,15 +287,23 @@ impl PromptBackend for MockPromptBackend { .ok_or(InquireError::OperationCanceled) } + /// Writes `message` to `output` before answering. + /// + /// The real widget draws on the stream it is handed, so a mock that never + /// touched it could not tell a caller's writer apart from any other — a + /// test asserting on what a prompt renders would pass against no prompt at + /// all. fn inline_reply( &self, - _message: &str, + message: &str, _initial_text: &str, _edit_mode: ReplyEditMode, _editor_escape: bool, _help: Option<&str>, - _output: Box, + mut output: Box, ) -> Result { + write!(output, "{message}")?; + self.reply_outcomes .lock() .pop_front() diff --git a/crates/jp_md/Cargo.toml b/crates/jp_md/Cargo.toml index d5ed2805b..633a8ece9 100644 --- a/crates/jp_md/Cargo.toml +++ b/crates/jp_md/Cargo.toml @@ -13,6 +13,8 @@ repository.workspace = true version.workspace = true [dependencies] +jp_term = { workspace = true } + comrak = { workspace = true } syntect = { workspace = true, features = ["regex-fancy", "default-themes", "default-syntaxes"] } two-face = { workspace = true, features = ["syntect-fancy"] } diff --git a/crates/jp_md/src/format.rs b/crates/jp_md/src/format.rs index 01b9ee02e..87bde5260 100644 --- a/crates/jp_md/src/format.rs +++ b/crates/jp_md/src/format.rs @@ -1,17 +1,20 @@ //! Markdown formatting utilities. -use std::{borrow::Cow, fmt, sync::LazyLock}; +use std::{fmt, sync::LazyLock}; use comrak::{ Arena, nodes::{NodeList, NodeValue}, options::{Extension, ListStyleType, Render}, }; +use jp_term::{ + ansi::{self, AnsiState, Segment}, + background::{DefaultBackground, line_fill}, +}; use syntect::{highlighting::Theme, parsing::SyntaxSet}; use two_face::syntax; use crate::{ - ansi::{self, AnsiState, Segment}, render::{self, HrOptions, RenderOptions}, table::TableOptions, theme, @@ -33,19 +36,6 @@ const DEFAULT_WIDTH: usize = 80; /// Default maximum column width for tables. const DEFAULT_TABLE_MAX_COL_WIDTH: usize = 40; -/// How a default background color fills each line. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BackgroundFill { - /// Fill to the last visible character on the line. - Content, - - /// Fill to a fixed column width (padding with spaces if needed). - Column(usize), - - /// Fill to the end of the terminal window via `\x1b[K`. - Terminal, -} - /// Controls how horizontal rules (`---`) are rendered in terminal output. #[derive(Debug, Clone, Copy, Default)] pub enum HrStyle { @@ -58,17 +48,6 @@ pub enum HrStyle { Line, } -/// A default background color applied to all content, with a fill mode -/// controlling how far it extends on each line. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct DefaultBackground { - /// SGR background parameter, e.g. `"48;5;236"` or `"48;2;80;73;69"`. - pub param: String, - - /// How far the background extends on each line. - pub fill: BackgroundFill, -} - /// Per-call options for [`Formatter::format_terminal_with`]. #[derive(Debug, Default, Clone, PartialEq, Eq)] pub struct TerminalOptions { @@ -484,28 +463,6 @@ fn ends_with_tight_list<'a>(root: &'a comrak::nodes::AstNode<'a>) -> bool { ) } -/// The text that extends a background from `column` to the end of the line. -/// -/// The one place [`BackgroundFill`] is interpreted. -/// Every writer that maintains a region background consults this so the three -/// modes cannot drift apart: `Content` adds nothing, `Terminal` defers to the -/// terminal's erase-to-end-of- line, and `Column` pads with real spaces — the -/// only form a host that lays out its own sub-window (an `fzf` preview pane) -/// renders, since it does not implement the erase. -/// -/// The caller is responsible for having the background active before writing -/// the result. -pub(crate) fn line_fill(fill: BackgroundFill, column: usize) -> Cow<'static, str> { - match fill { - BackgroundFill::Content => Cow::Borrowed(""), - BackgroundFill::Terminal => Cow::Borrowed("\x1b[K"), - BackgroundFill::Column(target) => match target.saturating_sub(column) { - 0 => Cow::Borrowed(""), - pad => Cow::Owned(" ".repeat(pad)), - }, - } -} - /// Render an inter-block separator (blank line) with optional background fill. /// Used between blocks and after closing code fences. /// diff --git a/crates/jp_md/src/format_tests.rs b/crates/jp_md/src/format_tests.rs index 3c15df71d..59460d306 100644 --- a/crates/jp_md/src/format_tests.rs +++ b/crates/jp_md/src/format_tests.rs @@ -1,3 +1,5 @@ +use jp_term::background::BackgroundFill; + use super::*; struct TestCase { diff --git a/crates/jp_md/src/lib.rs b/crates/jp_md/src/lib.rs index d611d0641..471599ea6 100644 --- a/crates/jp_md/src/lib.rs +++ b/crates/jp_md/src/lib.rs @@ -27,12 +27,10 @@ reason = "we don't host the docs, and use them mainly for LSP integration" )] -mod ansi; pub mod buffer; pub mod format; pub mod heading; mod render; -pub mod shade; mod table; pub mod theme; mod writer; diff --git a/crates/jp_md/src/render.rs b/crates/jp_md/src/render.rs index 39b500b0b..3935d9916 100644 --- a/crates/jp_md/src/render.rs +++ b/crates/jp_md/src/render.rs @@ -34,14 +34,17 @@ use comrak::{ NodeTaskItem, NodeValue, }, }; -use syntect::highlighting::Theme; - -use crate::{ +use jp_term::{ ansi::{ BG_END, BOLD_END, BOLD_START, FG_END, ITALIC_END, ITALIC_START, STRIKETHROUGH_END, STRIKETHROUGH_START, UNDERLINE_END, UNDERLINE_START, }, - format::{DefaultBackground, HrStyle, SYNTAXES}, + background::DefaultBackground, +}; +use syntect::highlighting::Theme; + +use crate::{ + format::{HrStyle, SYNTAXES}, table, writer::TerminalWriter, }; diff --git a/crates/jp_md/src/table.rs b/crates/jp_md/src/table.rs index 920662174..3d07616c6 100644 --- a/crates/jp_md/src/table.rs +++ b/crates/jp_md/src/table.rs @@ -22,13 +22,11 @@ use std::{cmp::min, fmt::Write as _}; use comrak::nodes::{NodeValue, TableAlignment}; +use jp_term::ansi::{self, AnsiState, RESET, Segment}; use unicode_segmentation::UnicodeSegmentation as _; use unicode_width::UnicodeWidthStr as _; -use crate::{ - ansi::{self, AnsiState, RESET, Segment}, - render::{RenderOptions, TerminalFormatter}, -}; +use crate::render::{RenderOptions, TerminalFormatter}; /// Type alias for comrak AST node references. type Node<'a> = &'a comrak::nodes::AstNode<'a>; diff --git a/crates/jp_md/src/writer.rs b/crates/jp_md/src/writer.rs index 70e15cc6c..9c0a01328 100644 --- a/crates/jp_md/src/writer.rs +++ b/crates/jp_md/src/writer.rs @@ -17,11 +17,13 @@ use std::{ fmt::{self, Write}, }; -use crate::{ +use jp_term::{ ansi::{self, AnsiState, RESET, Segment}, - format::{self, BackgroundFill, DefaultBackground}, + background::{BackgroundFill, DefaultBackground, line_fill}, }; +use crate::format; + /// ANSI-aware terminal writer with word-wrapping support. #[expect(clippy::struct_excessive_bools)] pub struct TerminalWriter<'w> { @@ -432,8 +434,8 @@ impl<'w> TerminalWriter<'w> { /// Extend the active background to the end of the current line. /// - /// The fill itself comes from [`format::line_fill`], the single - /// interpretation of [`BackgroundFill`]. + /// The fill itself comes from [`line_fill`], the single interpretation of + /// [`BackgroundFill`]. /// The background escape always precedes it so a temporary background /// (inline code, say) can't bleed into the fill. /// @@ -444,7 +446,7 @@ impl<'w> TerminalWriter<'w> { return Ok(()); }; - let fill = format::line_fill(bg.fill, self.column); + let fill = line_fill(bg.fill, self.column); if fill.is_empty() { return Ok(()); } diff --git a/crates/jp_md/src/writer_tests.rs b/crates/jp_md/src/writer_tests.rs index 27a65046c..070f9ac53 100644 --- a/crates/jp_md/src/writer_tests.rs +++ b/crates/jp_md/src/writer_tests.rs @@ -1,5 +1,4 @@ use super::*; -use crate::format::{BackgroundFill, DefaultBackground}; /// Regression test: a 1-byte prefix used to underflow in `write_prefix` because /// `prefix.len() - 2` wraps to `usize::MAX`. diff --git a/crates/jp_printer/src/printer.rs b/crates/jp_printer/src/printer.rs index 8f8bb725d..5cad8d2a7 100644 --- a/crates/jp_printer/src/printer.rs +++ b/crates/jp_printer/src/printer.rs @@ -1,6 +1,7 @@ //! The printer module. use std::{ + collections::VecDeque, fmt::{self, Write}, io, sync::{ @@ -12,6 +13,7 @@ use std::{ time::{Duration, Instant}, }; +use jp_term::{background::DefaultBackground, shade::ShadedWriter}; use parking_lot::{Condvar, Mutex}; use tracing::{debug, error}; @@ -109,6 +111,12 @@ pub struct Printer { /// even a currently-running typewriter task wakes up immediately. /// Cleared by the worker after processing `FlushInstant`. delay_control: Arc, + + /// The region background prompts are drawn against, when one is open. + /// + /// Shared across clones: whoever owns the region sets it once, and every + /// prompt taken from any handle picks it up. + prompt_background: Arc>>, } impl Clone for Printer { @@ -122,6 +130,7 @@ impl Clone for Printer { terminal: self.terminal, worker_handle: self.worker_handle.clone(), delay_control: self.delay_control.clone(), + prompt_background: self.prompt_background.clone(), } } } @@ -158,6 +167,7 @@ impl Printer { rx, delay_control, regions: RegionStack::new(), + held: VecDeque::new(), }; worker.run(); }) @@ -172,6 +182,7 @@ impl Printer { terminal: TerminalCapability::default(), worker_handle: Arc::new(Mutex::new(Some(handle))), delay_control, + prompt_background: Arc::new(Mutex::new(None)), } } @@ -292,13 +303,17 @@ impl Printer { StatusRegion::new(id, self.tx.clone(), buffer, refresh) } - /// Suspend status-region rendering until the returned guard drops. + /// Hand the terminal to a writer outside the printer until the returned + /// guard drops. /// - /// Blocks until the worker has erased any drawn region and entered the - /// suspended state, so a caller handing the terminal to a writer outside - /// the printer — an external `$EDITOR`, say — knows the rows are gone + /// Blocks until the worker has erased any drawn status region, so a caller + /// handing the terminal to an external `$EDITOR` knows the rows are gone /// before the child paints. - /// Returns immediately when regions are disabled. + /// For the guard's lifetime the worker also holds back ordinary output, + /// writing it once the terminal comes back: whoever holds the terminal owns + /// the cursor, and JP keeps producing while they have it. + /// + /// Handovers nest; the terminal comes back when the outermost guard drops. #[must_use] pub fn suspend_status(&self) -> SuspendGuard { self.suspend_regions() @@ -306,10 +321,6 @@ impl Printer { /// Enqueue a suspension and wait for the worker to apply it. fn suspend_regions(&self) -> SuspendGuard { - if !self.chrome_repaints() || !self.terminal.permits_regions(self.format) { - return SuspendGuard::inert(); - } - let (ack, rx) = mpsc::channel(); if self @@ -493,6 +504,7 @@ impl Printer { PrinterWriter { printer: self, target: PrintTarget::Out, + origin: PrintOrigin::Content, } } @@ -502,6 +514,7 @@ impl Printer { PrinterWriter { printer: self, target: PrintTarget::Err, + origin: PrintOrigin::Content, } } @@ -546,18 +559,36 @@ impl Printer { /// erased, and no region redraws until the writer drops: a prompt session /// is a run of small writes with the widget owning the cursor in between, /// and anything landing between them corrupts it. + /// + /// Writes carry whatever background [`Self::set_prompt_background`] last + /// named. #[must_use] pub fn prompt_writer(&self) -> PromptWriter<'_> { + let writer = PrinterWriter { + printer: self, + target: self.prompt_target(), + origin: PrintOrigin::Prompt, + }; + let suspension = self.begin_prompt_session(); + PromptWriter { - writer: PrinterWriter { - printer: self, - target: self.prompt_target(), - }, - _suspension: self.begin_prompt_session(), + writer: Canvas::new(writer, self.prompt_background.lock().as_ref()), + _suspension: suspension, _trace: PromptTrace::open(), } } + /// Draw prompts against `background` until it is replaced. + /// + /// `None` while no shaded region is open, which is the common case. + /// A prompt drawn inside one is a visual row like any other and shows the + /// same background to the right edge (RFD 095); whoever owns the region + /// names it here, and every prompt taken afterwards picks it up without its + /// call site knowing a region exists. + pub fn set_prompt_background(&self, background: Option) { + *self.prompt_background.lock() = background; + } + /// Print a line on the prompt stream. /// /// For the context a question needs to be answerable — the identity of a @@ -573,6 +604,7 @@ impl Printer { let mut task = p.into_task(); task.content.push('\n'); task.target = self.prompt_target(); + task.origin = PrintOrigin::Prompt; self.send(Command::Print(task)); } @@ -588,10 +620,15 @@ impl Printer { /// ordered with the printer's other output. #[must_use] pub fn owned_prompt_writer(&self) -> Box { - Box::new(OwnedPrinterWriter { + let writer = OwnedPrinterWriter { tx: self.tx.clone(), target: self.prompt_target(), - _suspension: self.begin_prompt_session(), + }; + let suspension = self.begin_prompt_session(); + + Box::new(OwnedPromptWriter { + writer: Canvas::new(writer, self.prompt_background.lock().as_ref()), + _suspension: suspension, _trace: PromptTrace::open(), }) } @@ -769,6 +806,9 @@ pub struct PrinterWriter<'a> { /// The target output stream. target: PrintTarget, + + /// Who the writes belong to. + origin: PrintOrigin, } impl fmt::Write for PrinterWriter<'_> { @@ -784,6 +824,7 @@ impl fmt::Write for PrinterWriter<'_> { content: s.to_owned(), mode: PrintMode::Instant, target: self.target, + origin: self.origin, }; self.printer @@ -854,23 +895,85 @@ impl Drop for PromptTrace { } } -/// A writer for interactive prompt output that suspends status regions. +/// A prompt's writes, shaded with the open region background if there is one. /// -/// Region rows are erased when the writer is acquired and no redraw lands until -/// it drops, so a widget owning the cursor between writes is never interrupted. +/// A prompt drawn inside a shaded region shows that region's background like +/// every other row (RFD 095), and a widget owns its own cursor — it rewrites +/// its line with `\r\x1b[K` on each keystroke, and that erase fills with +/// whatever background is active. +/// [`ShadedWriter`] is what keeps the fill right across the widget's own +/// escapes, including the resets it emits mid-line. +enum Canvas { + /// No region is open; writes pass straight through. + Plain(W), + + /// Writes are shaded with the open region's background. + Shaded(Box>), +} + +impl Canvas { + /// Wrap `writer`, shading it when a region is open. + fn new(writer: W, background: Option<&DefaultBackground>) -> Self { + match background { + Some(background) => Self::Shaded(Box::new(ShadedWriter::new(writer, background))), + None => Self::Plain(writer), + } + } + + /// The writer underneath the shading. + fn get_mut(&mut self) -> &mut W { + match self { + Self::Plain(writer) => writer, + Self::Shaded(writer) => writer.get_mut(), + } + } + + /// Close the background, if one was opened. + /// + /// Called from the prompt writer's `Drop` rather than at the end of each + /// prompt: a prompt can end by cancellation or by an error, and a + /// background left open paints everything printed after it. + fn finish(&mut self) { + if let Self::Shaded(writer) = self { + let _err = writer.finish(); + } + } +} + +impl fmt::Write for Canvas { + fn write_str(&mut self, s: &str) -> fmt::Result { + match self { + Self::Plain(writer) => writer.write_str(s), + Self::Shaded(writer) => writer.write_str(s), + } + } +} + +/// A writer for interactive prompt output. +/// +/// The terminal is quiet and clean when the writer is acquired, and stays that +/// way until it drops: status rows are erased, ordinary output waits, and no +/// redraw lands between the widget's own writes. +/// Writes carry the open region background, when there is one. /// Returned by [`Printer::prompt_writer`]. -#[derive(Debug)] pub struct PromptWriter<'a> { - /// The underlying writer, targeting the TTY or `out`. - writer: PrinterWriter<'a>, + /// The underlying writer, targeting the TTY or `out`, shaded when a region + /// is open. + writer: Canvas>, - /// Holds the suspension for the writer's lifetime. + /// Holds the terminal for the writer's lifetime. _suspension: SuspendGuard, /// Records the session's lifetime for the trace log. _trace: PromptTrace, } +impl Drop for PromptWriter<'_> { + fn drop(&mut self) { + self.writer.finish(); + } +} + impl fmt::Write for PromptWriter<'_> { fn write_str(&mut self, s: &str) -> fmt::Result { self.writer.write_str(s) @@ -879,17 +982,21 @@ impl fmt::Write for PromptWriter<'_> { impl io::Write for PromptWriter<'_> { fn write(&mut self, buf: &[u8]) -> io::Result { - io::Write::write(&mut self.writer, buf) + let s = str::from_utf8(buf).map_err(io::Error::other)?; + self.writer.write_str(s).map_err(io::Error::other)?; + Ok(s.len()) } + /// A widget flushes before it reads a key, and on this path a flush is a + /// barrier rather than a buffer drain: it blocks until the printer's worker + /// has written everything queued ahead of it. fn flush(&mut self) -> io::Result<()> { - io::Write::flush(&mut self.writer) + io::Write::flush(self.writer.get_mut()) } } -/// An owned writer targeting one of the [`Printer`]'s streams. +/// An owned channel into one of the [`Printer`]'s streams. /// -/// Returned (boxed) by [`Printer::owned_prompt_writer`]. /// Owns a clone of the printer's command channel, so it is `'static` and /// `Send`; writes flow through the printer's serialized worker just like /// [`PrinterWriter`]. @@ -899,25 +1006,24 @@ struct OwnedPrinterWriter { /// The target output stream. target: PrintTarget, - - /// Holds the status-region suspension for the writer's lifetime. - _suspension: SuspendGuard, - - /// Records the session's lifetime for the trace log. - _trace: PromptTrace, } -impl io::Write for OwnedPrinterWriter { - fn write(&mut self, buf: &[u8]) -> io::Result { - let s = str::from_utf8(buf).map_err(io::Error::other)?; +impl fmt::Write for OwnedPrinterWriter { + fn write_str(&mut self, s: &str) -> fmt::Result { let task = PrintTask { content: s.to_owned(), mode: PrintMode::Instant, target: self.target, + origin: PrintOrigin::Prompt, }; - self.tx - .send(Command::Print(task)) - .map_err(|_| io::Error::new(io::ErrorKind::BrokenPipe, "printer shutdown"))?; + self.tx.send(Command::Print(task)).map_err(|_| fmt::Error) + } +} + +impl io::Write for OwnedPrinterWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + let s = str::from_utf8(buf).map_err(io::Error::other)?; + self.write_str(s).map_err(io::Error::other)?; Ok(s.len()) } @@ -931,6 +1037,40 @@ impl io::Write for OwnedPrinterWriter { } } +/// The owned counterpart of [`PromptWriter`], for components that need to own +/// their output stream. +/// +/// Returned (boxed) by [`Printer::owned_prompt_writer`], and carries the same +/// guarantees. +struct OwnedPromptWriter { + /// The underlying channel, shaded when a region is open. + writer: Canvas, + + /// Holds the terminal for the writer's lifetime. + _suspension: SuspendGuard, + + /// Records the session's lifetime for the trace log. + _trace: PromptTrace, +} + +impl Drop for OwnedPromptWriter { + fn drop(&mut self) { + self.writer.finish(); + } +} + +impl io::Write for OwnedPromptWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + let s = str::from_utf8(buf).map_err(io::Error::other)?; + self.writer.write_str(s).map_err(io::Error::other)?; + Ok(s.len()) + } + + fn flush(&mut self) -> io::Result<()> { + io::Write::flush(self.writer.get_mut()) + } +} + /// The worker thread that processes print tasks. struct Worker { /// The `out` writer; strips ANSI escapes for non-pretty formats. @@ -950,6 +1090,11 @@ struct Worker { /// The claimed status regions and the rows currently painted for them. regions: RegionStack, + + /// Ordinary output produced while the terminal is handed to someone else. + /// + /// Written, in the order it was produced, once the terminal comes back. + held: VecDeque, } impl Worker { @@ -977,8 +1122,18 @@ impl Worker { }; match cmd { - Command::Print(task) => self.process_task(&task), - Command::Region(cmd) => self.regions.apply(cmd, &mut self.err), + Command::Print(task) => { + if let Some(task) = self.admit(task) { + self.process_task(&task); + } + } + Command::Region(cmd) => { + let resuming = matches!(cmd, RegionCommand::Resume); + self.regions.apply(cmd, &mut self.err); + if resuming { + self.release_held(); + } + } Command::Flush(tx) => { // We don't need to do anything specific to flush out/err // because we flush after every write in `process_task`. We @@ -995,9 +1150,47 @@ impl Worker { } } + // A run can end with a widget still holding the terminal — a `Ctrl+C` + // at a prompt, an error unwinding past one. Nothing is going to give it + // back, and what is held is the answer the user asked for, so it goes + // out anyway: a last frame the widget left untidy is a smaller loss + // than the output. + self.drain_held(); self.regions.erase(&mut self.err); } + /// Decide whether `task` may be written now, holding it back if not. + /// + /// Only the prompt session's own writes reach a terminal someone else owns; + /// everything else waits for it to come back. + fn admit(&mut self, task: PrintTask) -> Option { + if !self.regions.is_suspended() || task.origin == PrintOrigin::Prompt { + return Some(task); + } + + self.held.push_back(task); + None + } + + /// Write everything held back, now that the terminal has come back. + /// + /// A no-op while it is still away, so the inner close of a nested handover + /// releases nothing. + fn release_held(&mut self) { + if self.regions.is_suspended() { + return; + } + + self.drain_held(); + } + + /// Write everything held back, in the order it was produced. + fn drain_held(&mut self) { + while let Some(task) = self.held.pop_front() { + self.process_task(&task); + } + } + /// Drain all pending commands, printing instantly. /// /// Processes queued `Print` tasks as `Instant` (ignoring typewriter @@ -1007,8 +1200,18 @@ impl Worker { fn drain_instant(&mut self) { while let Ok(cmd) = self.rx.try_recv() { match cmd { - Command::Print(task) => self.process_task_instant(&task), - Command::Region(cmd) => self.regions.apply(cmd, &mut self.err), + Command::Print(task) => { + if let Some(task) = self.admit(task) { + self.process_task_instant(&task); + } + } + Command::Region(cmd) => { + let resuming = matches!(cmd, RegionCommand::Resume); + self.regions.apply(cmd, &mut self.err); + if resuming { + self.release_held(); + } + } Command::Flush(tx) => { let _ = tx.send(()); } @@ -1092,6 +1295,7 @@ impl Worker { content, mode, target, + .. } = task; let writer: &mut dyn io::Write = match target { @@ -1293,6 +1497,23 @@ pub enum PrintTarget { Tty, } +/// Who a print task belongs to. +/// +/// While the terminal is handed to a widget, ordinary output waits and the +/// widget's own writes go straight through: it is drawing the screen the user +/// is answering on, and content landing in the middle of that is what the wait +/// exists to prevent. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum PrintOrigin { + /// Ordinary output: assistant responses, chrome, structured records. + #[default] + Content, + + /// Part of a prompt session — the widget's own drawing, or a line written + /// to give its question the context it needs to be answerable. + Prompt, +} + #[derive(Debug, Clone)] /// A task to be printed. pub struct PrintTask { @@ -1304,6 +1525,10 @@ pub struct PrintTask { /// The target output stream. pub target: PrintTarget, + + /// Who the task belongs to, which decides whether it may land while a + /// widget owns the terminal. + pub origin: PrintOrigin, } impl Default for PrintTask { @@ -1312,6 +1537,7 @@ impl Default for PrintTask { content: String::new(), mode: PrintMode::Instant, target: PrintTarget::Out, + origin: PrintOrigin::Content, } } } diff --git a/crates/jp_printer/src/printer_tests.rs b/crates/jp_printer/src/printer_tests.rs index f407d5272..e70ec9a05 100644 --- a/crates/jp_printer/src/printer_tests.rs +++ b/crates/jp_printer/src/printer_tests.rs @@ -1,3 +1,5 @@ +use jp_term::background::BackgroundFill; + use super::*; use crate::region::OutputLines; @@ -873,6 +875,203 @@ fn a_prompt_that_ends_mid_line_does_not_hold_the_region_hostage() { ); } +#[test] +fn output_produced_while_a_prompt_is_open_waits_for_it() { + let (printer, out, err) = Printer::memory(OutputFormat::TextPretty); + + { + let mut prompt = printer.prompt_writer(); + write!(prompt, "Run local shell tool? [y/n] ").unwrap(); + + // A streaming response, an MCP server's stderr, a title generator: all + // of them keep producing while the user thinks about the question. + printer.println("assistant content"); + printer.eprintln("chrome"); + printer.flush(); + + assert_eq!( + *out.lock(), + "Run local shell tool? [y/n] ", + "only the widget writes while it owns the terminal" + ); + assert_eq!(*err.lock(), ""); + } + + printer.flush(); + assert_eq!( + *out.lock(), + "Run local shell tool? [y/n] assistant content\n" + ); + assert_eq!(*err.lock(), "chrome\n"); +} + +#[test] +fn a_nested_prompt_does_not_release_the_outer_hold() { + // A permission prompt holds its writer and opens the inline reply widget + // on top of it, so sessions nest. The terminal comes back when the + // outermost one gives it back, not the first. + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + + let outer = printer.prompt_writer(); + { + let _inner = printer.owned_prompt_writer(); + printer.println("assistant content"); + } + + printer.flush(); + assert_eq!(*out.lock(), "", "the outer session still owns the terminal"); + + drop(outer); + printer.flush(); + assert_eq!(*out.lock(), "assistant content\n"); +} + +#[test] +fn a_nested_acquisition_does_not_flush_held_output() { + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + + let _outer = printer.prompt_writer(); + printer.println("assistant content"); + + // Acquisition drains the queue instantly, which must not mean pushing held + // output onto the screen a widget is already drawing on. + let _inner = printer.owned_prompt_writer(); + + assert_eq!(*out.lock(), ""); +} + +#[test] +fn output_held_by_a_prompt_is_not_lost_at_shutdown() { + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + + let _prompt = printer.prompt_writer(); + printer.println("assistant content"); + printer.shutdown(); + + assert_eq!( + *out.lock(), + "assistant content\n", + "held output is deferred, not discarded" + ); +} + +/// A full-width reasoning-region background. +fn shaded_region() -> DefaultBackground { + DefaultBackground { + param: "48;5;236".into(), + fill: BackgroundFill::Terminal, + } +} + +#[test] +fn a_prompt_inside_a_shaded_region_carries_its_background() { + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + printer.set_prompt_background(Some(shaded_region())); + + { + let mut prompt = printer.prompt_writer(); + write!(prompt, "Run local shell tool?").unwrap(); + } + printer.flush(); + + // The background is asserted before the text and closed once the widget is + // done with the terminal, so the row is shaded and nothing after it is. + assert_eq!(*out.lock(), "\x1b[48;5;236mRun local shell tool?\x1b[49m"); +} + +#[test] +fn an_owned_prompt_writer_carries_the_background_too() { + // The inline reply widget owns its output stream, so it takes this writer + // rather than the borrowed one. Both are prompts and both are inside the + // region. + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + printer.set_prompt_background(Some(shaded_region())); + + { + let mut prompt = printer.owned_prompt_writer(); + write!(prompt, "Edit arguments").unwrap(); + } + printer.flush(); + + assert_eq!(*out.lock(), "\x1b[48;5;236mEdit arguments\x1b[49m"); +} + +#[test] +fn a_prompt_outside_a_shaded_region_is_unshaded() { + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + + { + let mut prompt = printer.prompt_writer(); + write!(prompt, "Run local shell tool?").unwrap(); + } + printer.flush(); + + assert_eq!(*out.lock(), "Run local shell tool?"); +} + +#[test] +fn clearing_the_background_unshades_later_prompts() { + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + + printer.set_prompt_background(Some(shaded_region())); + drop(printer.prompt_writer()); + printer.set_prompt_background(None); + + { + let mut prompt = printer.prompt_writer(); + write!(prompt, "after").unwrap(); + } + printer.flush(); + + assert_eq!(*out.lock(), "after"); +} + +#[test] +fn an_abandoned_prompt_still_closes_its_background() { + // A widget can end by `Ctrl+C` or by an error, neither of which returns + // through the normal path. The close lives in `Drop` so the background + // cannot outlive the prompt and paint whatever is printed next. + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + printer.set_prompt_background(Some(shaded_region())); + + { + let mut prompt = printer.prompt_writer(); + write!(prompt, "Deliver result?").unwrap(); + // No further writes: the prompt is abandoned mid-session. + } + printer.set_prompt_background(None); + printer.println("after"); + printer.flush(); + + assert_eq!(*out.lock(), "\x1b[48;5;236mDeliver result?\x1b[49mafter\n"); +} + +#[test] +fn flushing_a_shaded_prompt_waits_for_the_printer() { + // A widget flushes its writer before it reads a key, and on this path a + // flush is a barrier rather than a buffer drain: the bytes have to have + // landed before the widget takes the cursor and the terminal's mode. + // Shading decorates that writer and must not swallow it. + let (printer, out, _err) = Printer::memory(OutputFormat::TextPretty); + printer.set_prompt_background(Some(shaded_region())); + + let mut prompt = printer.prompt_writer(); + + // Queued after acquisition drained the printer, and slow enough that the + // worker is certainly still inside it. + printer.print("slow".typewriter(Duration::from_millis(100))); + + write!(prompt, "Run local shell tool?").unwrap(); + io::Write::flush(&mut prompt).unwrap(); + + // Deliberately no `printer.flush()`, which is the assertion. + assert!( + out.lock().contains("Run local shell tool?"), + "flushing the prompt must drain the printer, got {:?}", + *out.lock() + ); +} + #[test] fn suspend_status_erases_before_it_returns() { let (printer, _out, err) = region_printer(); diff --git a/crates/jp_printer/src/region.rs b/crates/jp_printer/src/region.rs index 9b2a773a1..609b1fa65 100644 --- a/crates/jp_printer/src/region.rs +++ b/crates/jp_printer/src/region.rs @@ -838,6 +838,16 @@ impl RegionStack { } } + /// Whether the terminal is currently handed to a writer outside the + /// printer. + /// + /// A suspension is taken for exactly as long as someone else — a prompt + /// widget, an external `$EDITOR` — owns the cursor, so this answers both + /// "are the rows hidden" and "may ordinary output land". + pub const fn is_suspended(&self) -> bool { + self.suspensions > 0 + } + /// Record whether the persistent write that just landed left the cursor /// part-way along a row. /// diff --git a/crates/jp_md/src/ansi.rs b/crates/jp_term/src/ansi.rs similarity index 90% rename from crates/jp_md/src/ansi.rs rename to crates/jp_term/src/ansi.rs index 224185f29..16b1ba4e6 100644 --- a/crates/jp_md/src/ansi.rs +++ b/crates/jp_term/src/ansi.rs @@ -1,8 +1,11 @@ -//! Shared ANSI SGR escape constants and state tracking. +//! ANSI SGR escape constants, state tracking, and escape-aware measurement. //! -//! This module provides the escape sequences, state tracking, and visual width -//! computation used by both the terminal renderer (`render.rs`) and the table -//! formatter (`table.rs`). +//! [`segments`] is the tokenizer everything else here is built on: it splits a +//! byte stream into visible text and complete escape sequences. +//! [`AnsiState`] tracks which attributes a stream has left active, so a writer +//! can close them at a line break and re-open them on the next line. +//! [`visual_width`] and [`advance_column`] measure text as the display lays it +//! out, skipping escapes and advancing a tab to its stop. use unicode_width::UnicodeWidthStr as _; @@ -47,9 +50,10 @@ pub const RESET: &str = "\x1b[0m"; /// Tracks which ANSI SGR attributes are currently active. /// -/// Used to close formatting at line breaks and re-open it on the next line, -/// both for the terminal renderer's incremental wrapping and the table -/// formatter's batch wrapping. +/// Feed it every escape a stream emits, and it answers what a line break has to +/// close and what the next line has to re-open. +/// Attributes combined into one escape are tracked individually, so +/// `\x1b[1;48;5;236m` registers as both bold and a background. #[derive(Debug, Clone, Default)] #[expect(clippy::struct_excessive_bools)] pub struct AnsiState { @@ -80,7 +84,8 @@ pub struct AnsiState { impl AnsiState { /// Returns `true` if any attribute is currently active. - pub(crate) const fn is_active(&self) -> bool { + #[must_use] + pub const fn is_active(&self) -> bool { self.bold || self.italic || self.underline @@ -102,7 +107,7 @@ impl AnsiState { /// Returns `true` when the escape resets all attributes or sets/clears the /// background — the signal a default-background overlay uses to know it /// must re-assert its fill after the escape is forwarded. - pub(crate) fn update(&mut self, esc: &str) -> bool { + pub fn update(&mut self, esc: &str) -> bool { let Some(params) = esc.strip_prefix("\x1b[").and_then(|s| s.strip_suffix('m')) else { return false; }; @@ -165,7 +170,7 @@ impl AnsiState { } /// Update state by scanning all ANSI escape sequences in `s`. - pub(crate) fn update_from_str(&mut self, s: &str) { + pub fn update_from_str(&mut self, s: &str) { for segment in segments(s) { if let Segment::Escape(esc) = segment { let _affects_background = self.update(esc); @@ -174,7 +179,8 @@ impl AnsiState { } /// Builds a string that re-activates all currently active attributes. - pub(crate) fn restore_sequence(&self) -> String { + #[must_use] + pub fn restore_sequence(&self) -> String { let mut s = String::new(); if self.bold { s.push_str(BOLD_START); @@ -232,6 +238,7 @@ fn consume_color<'a, I: Iterator>(prefix: &str, tokens: &mut I) /// SGR is the only family [`AnsiState`] tracks, so this doubles as the test for /// whether an escape's effect can be closed with [`RESET`] or re-opened after a /// line break. +#[must_use] pub fn is_sgr(esc: &str) -> bool { esc.starts_with("\x1b[") && esc.ends_with('m') } @@ -250,10 +257,10 @@ pub enum Segment<'a> { /// Split `s` into visible-text runs and ANSI escape sequences. /// /// An escape sequence runs from `\x1b` through the first ASCII letter or `~` — -/// sufficient for the SGR/CSI sequences this crate emits and consumes. -/// This is the single tokenizer for every escape-aware routine in the crate -/// (width computation, state tracking, table wrapping), so the termination rule -/// cannot drift between call sites. +/// sufficient for the SGR/CSI sequences JP emits and consumes. +/// This is the single tokenizer every escape-aware routine goes through, so the +/// termination rule cannot drift between call sites. +#[must_use] pub const fn segments(s: &str) -> Segments<'_> { Segments { rest: s } } @@ -328,6 +335,7 @@ fn osc_terminator_end(body: &str) -> Option { /// Grapheme cluster boundaries are a property of this text, not of the escape /// separated runs it was built from, so anything measuring or cutting on /// cluster boundaries has to work from here. +#[must_use] pub fn visible_text(s: &str) -> String { let mut plain = String::new(); for segment in segments(s) { @@ -343,6 +351,7 @@ pub fn visible_text(s: &str) -> String { /// A tab counts as a single column. /// Use [`advance_column`] where the resulting cursor position matters, since a /// tab moves the cursor to the next tab stop instead. +#[must_use] pub fn visual_width(s: &str) -> usize { visible_text(s).width() } @@ -353,6 +362,7 @@ pub fn visual_width(s: &str) -> usize { /// of [`TAB_STOP`], and a carriage return returns to column 0 — the positions /// the display actually arrives at, so text padded to a fixed column lands /// there instead of overshooting. +#[must_use] pub fn advance_column(column: usize, s: &str) -> usize { let plain = visible_text(s); let mut column = column; diff --git a/crates/jp_md/src/ansi_tests.rs b/crates/jp_term/src/ansi_tests.rs similarity index 100% rename from crates/jp_md/src/ansi_tests.rs rename to crates/jp_term/src/ansi_tests.rs diff --git a/crates/jp_term/src/background.rs b/crates/jp_term/src/background.rs new file mode 100644 index 000000000..be8b03fba --- /dev/null +++ b/crates/jp_term/src/background.rs @@ -0,0 +1,58 @@ +//! A background colour applied to a region of terminal output. +//! +//! A region background is the fill behind a run of rows — the shading a +//! reasoning block sits in, the tint a status row is drawn against. +//! It is described by two things: the SGR parameter that sets the colour, and +//! how far along each row the colour extends. +//! +//! [`line_fill`] is the one place that second question is answered, so every +//! writer that maintains a background agrees on what a filled row looks like. + +use std::borrow::Cow; + +/// How a default background colour fills each line. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BackgroundFill { + /// Fill to the last visible character on the line. + Content, + + /// Fill to a fixed column width (padding with spaces if needed). + Column(usize), + + /// Fill to the end of the terminal window via `\x1b[K`. + Terminal, +} + +/// A default background colour applied to all content, with a fill mode +/// controlling how far it extends on each line. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct DefaultBackground { + /// SGR background parameter, e.g. `"48;5;236"` or `"48;2;80;73;69"`. + pub param: String, + + /// How far the background extends on each line. + pub fill: BackgroundFill, +} + +/// The text that extends a background from `column` to the end of the line. +/// +/// The one place [`BackgroundFill`] is interpreted. +/// Every writer that maintains a region background consults this so the three +/// modes cannot drift apart: `Content` adds nothing, `Terminal` defers to the +/// terminal's erase-to-end-of-line, and `Column` pads with real spaces — the +/// only form a host that lays out its own sub-window (an `fzf` preview pane) +/// renders, since it does not implement the erase. +/// +/// The caller is responsible for having the background active before writing +/// the result. +#[must_use] +pub fn line_fill(fill: BackgroundFill, column: usize) -> Cow<'static, str> { + match fill { + BackgroundFill::Content => Cow::Borrowed(""), + BackgroundFill::Terminal => Cow::Borrowed("\x1b[K"), + BackgroundFill::Column(target) => match target.saturating_sub(column) { + 0 => Cow::Borrowed(""), + pad => Cow::Owned(" ".repeat(pad)), + }, + } +} diff --git a/crates/jp_term/src/lib.rs b/crates/jp_term/src/lib.rs index 4f79c0fb8..a7c90f05e 100644 --- a/crates/jp_term/src/lib.rs +++ b/crates/jp_term/src/lib.rs @@ -1,5 +1,8 @@ #![allow(clippy::print_stdout, clippy::print_stderr)] +pub mod ansi; +pub mod background; pub mod osc; +pub mod shade; pub mod table; pub mod width; diff --git a/crates/jp_md/src/shade.rs b/crates/jp_term/src/shade.rs similarity index 96% rename from crates/jp_md/src/shade.rs rename to crates/jp_term/src/shade.rs index e97b7e73f..1ec26fa57 100644 --- a/crates/jp_md/src/shade.rs +++ b/crates/jp_term/src/shade.rs @@ -9,18 +9,15 @@ //! no row past the region is painted. //! [`shade`] is the buffer-at-a-time convenience built on the same core. //! -//! Unlike the line-oriented [`apply_line_background`], the writer holds its -//! state across `write_str` calls, so it can keep `B` active across cursor -//! rewrites and escape sequences split between writes — the cases a pure -//! per-line transform cannot express. -//! -//! [`apply_line_background`]: crate::format::apply_line_background +//! The writer holds its state across `write_str` calls, so `B` survives a +//! cursor rewrite or an escape sequence split between two writes — the cases a +//! per-line transform over a finished string cannot express. use std::fmt::{self, Write}; use crate::{ ansi::{self, AnsiState, Segment, is_sgr}, - format::{self, BackgroundFill, DefaultBackground}, + background::{BackgroundFill, DefaultBackground, line_fill}, }; /// Wraps a writer and maintains a default-background invariant across the byte @@ -199,7 +196,7 @@ impl ShadedWriter { /// Fill the rest of the current line with the active background, ahead of a /// newline. fn fill_line(&mut self) -> fmt::Result { - let fill = format::line_fill(self.fill, self.column); + let fill = line_fill(self.fill, self.column); if fill.is_empty() { return Ok(()); } diff --git a/crates/jp_md/src/shade_tests.rs b/crates/jp_term/src/shade_tests.rs similarity index 100% rename from crates/jp_md/src/shade_tests.rs rename to crates/jp_term/src/shade_tests.rs diff --git a/docs/rfd/084-configurable-markdown-element-coloring.md b/docs/rfd/084-configurable-markdown-element-coloring.md index d8cbbecad..0dbc98723 100644 --- a/docs/rfd/084-configurable-markdown-element-coloring.md +++ b/docs/rfd/084-configurable-markdown-element-coloring.md @@ -369,7 +369,7 @@ build themes against it. ### Renderer changes -`AnsiState` (`crates/jp_md/src/ansi.rs`) grows an `intensity: Intensity` field +`AnsiState` (`crates/jp_term/src/ansi.rs`) grows an `intensity: Intensity` field in place of its existing `bold: bool`. `restore_sequence` re-emits SGR 1 or SGR 2 as appropriate after any SGR 22 transition, so wrap-break and pop-style restoration handle dim with the same @@ -778,7 +778,7 @@ Depends on Phase 2. This RFD extends that renderer's styling surface. - `jp_md::render::TerminalFormatter` — the AST walker whose hard-coded SGR calls this RFD replaces. -- `jp_md::ansi::AnsiState` — the existing state-tracking primitive that the +- `jp_term::ansi::AnsiState` — the existing state-tracking primitive that the proposed writer stack builds on. - `termimad`'s `MadSkin` — referenced as schema inspiration; not adopted. diff --git a/docs/rfd/091-printer-owned-status-region.md b/docs/rfd/091-printer-owned-status-region.md index 4ac4a4d13..2fa545b67 100644 --- a/docs/rfd/091-printer-owned-status-region.md +++ b/docs/rfd/091-printer-owned-status-region.md @@ -273,7 +273,7 @@ A line that leaves an attribute open is terminated with a reset, so child state cannot bleed into JP's own chrome below it. This is the policy `jp_md::table` already applies when truncating cells (retain -SGR, drop the rest, close with a reset), and `jp_md::ansi::is_sgr` is the same +SGR, drop the rest, close with a reset), and `jp_term::ansi::is_sgr` is the same predicate. In `jp_printer` it is a second policy over the existing `vte` parser that backs `AnsiStripper` — same crate, same parser, no new dependency. @@ -347,7 +347,7 @@ temp and progress rows as part of it: while a reasoning region with background produced by cursor-relative rewrites, and including the `\x1b[K` that erases them, which fills with whatever background is active when it runs. `ToolRenderer` holds that invariant today by routing its writes through -`jp_md::shade::ShadedWriter`. +`jp_term::shade::ShadedWriter`. A worker that draws and erases those rows itself, knowing nothing about the reasoning region, would punch an unshaded hole in the middle of a shaded one — the exact gap RFD 095 closed. @@ -1068,12 +1068,12 @@ contracts commits to. replaces. - `crates/jp_cli/src/cmd/query/stream/retry.rs` — `notify` and `clear_line`, the ninth hand-rolled mechanism. -- `crates/jp_md/src/shade.rs` — `ShadedWriter`, which holds the background +- `crates/jp_term/src/shade.rs` — `ShadedWriter`, which holds the background invariant for tool chrome today. - `crates/jp_printer/src/printer.rs` — the worker loop this RFD extends. - `crates/jp_printer/src/ansi.rs` — the `vte`-based `AnsiStripper` the SGR allowlist extends. -- `crates/jp_md/src/ansi.rs` — `is_sgr`, the predicate the allowlist reuses, +- `crates/jp_term/src/ansi.rs` — `is_sgr`, the predicate the allowlist reuses, and the retain-SGR-drop-the-rest precedent in `jp_md/src/table.rs`. - `crates/jp_mcp/src/client.rs` — `spawn_stderr_forwarder`, the stderr ring buffer, and `StartupSet`. diff --git a/docs/ticket/0fg8cjs-the-reasoning-background-drops-out-while-a-prompt-is-up.md b/docs/ticket/0fg8cjs-the-reasoning-background-drops-out-while-a-prompt-is-up.md index 8026a17e1..e0c4e13f6 100644 --- a/docs/ticket/0fg8cjs-the-reasoning-background-drops-out-while-a-prompt-is-up.md +++ b/docs/ticket/0fg8cjs-the-reasoning-background-drops-out-while-a-prompt-is-up.md @@ -1,6 +1,6 @@ # The reasoning background drops out while a prompt is up -- **Status**: Todo +- **Status**: Done - **Kind**: Bug - **Authors**: jp - **Date**: 2026-09-08 @@ -126,3 +126,61 @@ painted the row rather than clearing it, and the shell prompt inherited it after `prompter_tests.rs` has a test that abandons a prompt mid-session and asserts the close still lands; worth extending to the editor path rather than writing a new one. + +----- + +- **From**: jp +- **Date**: 2026-09-08T13:57:29Z + +Done, including the `Ctrl+X` / inline-reply path this ticket left open. + +## Where it landed + +The background moved into `jp_printer`, which is where this ticket's "shape of a +fix" pointed: `Printer::set_prompt_background` holds it, and both +`prompt_writer` and `owned_prompt_writer` wrap their writer in a `ShadedWriter` +when one is set. +Every prompt taken from the printer is shaded without its call site knowing a +region exists, so the reply widget is covered by the same code as the approval +prompt rather than needing its own. + +That required `ShadedWriter` to be reachable from `jp_printer`, which depends on +`jp_term` and deliberately not on `jp_md`. +`ansi` and `shade` moved to `jp_term` along with `DefaultBackground`, +`BackgroundFill` and `line_fill` (now `jp_term::background`). +None of them were markdown: `segments` tokenizes an escape stream, `AnsiState` +tracks what a stream left active. + +The `owned_prompt_writer` trait problem this ticket recorded resolved itself in +the move. +`OwnedPrinterWriter` gained a `fmt::Write` impl and became the inner writer of a +shared `Canvas`, so the adapter is a trait impl on the type that needs it +rather than a wrapper at the call site. + +`ToolPrompter::set_background`, `canvas()`, and `PromptCanvas` are deleted. +The coordinator still names the background at the same point in +`resolve_tool_call_decision` — the region is per tool call, so the read has to +happen there — but it now hands it to the printer. + +## Two things found on the way + +The `PromptCanvas::flush` this ticket's earlier comment described was swallowing +the flush for the shaded variant, on the grounds that the printer holds nothing +back a flush could release. +True of buffering, false of ordering: on the prompt path a flush is a barrier, +and `inquire` flushes before it reads a key. +Inside a reasoning block that was the only difference between the two variants, +which is why prompt output staircased there and nowhere else. +Fixed separately, along with making prompt-writer acquisition quiesce the +terminal. + +`MockPromptBackend::inline_reply` wrote nothing to the stream it was handed, so +a test asserting on what the reply widget renders passed against no prompt at +all. +It now writes its message, like the real widget does. + +## Not covered + +The eight prompts in `T-0ffsv2r` still build their own `io::stderr()` writer, so +they are still unshaded. +They pick this up for free once they go through the printer. diff --git a/docs/ticket/0fk64t3-jp_term-measures-display-width-two-ways.md b/docs/ticket/0fk64t3-jp_term-measures-display-width-two-ways.md new file mode 100644 index 000000000..b4b8078a8 --- /dev/null +++ b/docs/ticket/0fk64t3-jp_term-measures-display-width-two-ways.md @@ -0,0 +1,43 @@ +# `jp_term` measures display width two ways + +- **Status**: Todo +- **Kind**: Chore +- **Authors**: jp +- **Date**: 2026-09-08 +- **Label**: package=jp_term +- **Label**: type=task + +`jp_term` now holds two independent implementations of "how wide is this text on +screen", which arrived from different directions and have never been compared. + +- `jp_term::width::display_width` strips ANSI with `strip-ansi-escapes`, then + measures with `unicode-width`. + Its neighbours (`truncate_to_width`, `wrap_ranges`, `prefix_end_for_width`) + walk grapheme clusters and probe for ligatures that collapse. +- `jp_term::ansi::visual_width` tokenizes with `ansi::segments`, joins the + visible runs, and measures the result. + `advance_column` builds on it and adds tab stops and carriage returns. + +They disagree in at least one way that matters: `visual_width` counts a tab as +one column and `advance_column` exists because of it, while the `width` module +has no notion of a tab at all. +Whether they disagree on OSC 8 hyperlinks, partial escapes, or ligature collapse +is unknown — nothing tests them against each other. + +## Why it is worth doing + +The duplication was invisible while the two lived in different crates. +Now a reader of `jp_term` has to pick one, and the names give no help: +`display_width` and `visual_width` are the same phrase. + +## Shape of a fix + +Start with a test that runs both over the same inputs — plain ASCII, CJK, VS16 +emoji, ZWJ sequences, tabs, OSC 8 links, escapes split mid-sequence — and +records where they differ. +That answers whether this is one function with two names or two functions with +one job each. + +If they agree except on tabs, collapse to one and keep `advance_column` as the +cursor-position variant. +If they genuinely differ, the names have to say how.