From 2bb9f5053ecbe3bb5d280f01977560814f03825d Mon Sep 17 00:00:00 2001 From: Simon Peter Rothgang Date: Mon, 20 Apr 2026 21:02:55 +0200 Subject: [PATCH] fix(ui): preserve indentation in diff-backed tool rendering - keep leading source indentation when wrapping numbered diff rows - retain nested tool-card diff indent and aligned write omission markers - add regression tests for raw diff indentation and tool-call wrapped diff rendering --- src/ui/diff.rs | 58 +++++++++++++++++++++-- src/ui/tool_call/mod.rs | 92 ++++++++++++++++++++++++++++++++++++ src/ui/tool_call/standard.rs | 23 ++++++--- 3 files changed, 164 insertions(+), 9 deletions(-) diff --git a/src/ui/diff.rs b/src/ui/diff.rs index b05925c8..21035520 100644 --- a/src/ui/diff.rs +++ b/src/ui/diff.rs @@ -3,7 +3,7 @@ use crate::agent::model; use crate::ui::theme; -use crate::ui::wrap::{StyledChunk, wrap_styled_chunks}; +use crate::ui::wrap::{StyledChunk, display_width, wrap_styled_chunks}; use ratatui::style::{Color, Modifier, Style}; use ratatui::text::{Line, Span}; use similar::TextDiff; @@ -195,10 +195,13 @@ fn render_wrapped_diff_row( content_width: usize, ) -> Vec> { let number_style = Style::default().fg(theme::DIM); - let content_lines = if value.is_empty() { + let (leading_indent, content) = split_leading_whitespace(value); + let leading_indent_width = display_width(leading_indent); + let content_lines = if content.is_empty() { vec![Line::default()] } else { - wrap_styled_chunks(&[StyledChunk { text: value.to_owned(), style }], content_width) + let wrapped_width = content_width.saturating_sub(leading_indent_width).max(1); + wrap_styled_chunks(&[StyledChunk { text: content.to_owned(), style }], wrapped_width) }; let line_number_text = line_number.map_or_else( @@ -221,12 +224,23 @@ fn render_wrapped_diff_row( } else { vec![Span::styled(continuation_prefix.clone(), number_style)] }; + if !leading_indent.is_empty() { + spans.push(Span::styled(leading_indent.to_owned(), style)); + } spans.extend(content_line.spans); Line::from(spans) }) .collect() } +fn split_leading_whitespace(text: &str) -> (&str, &str) { + let split_at = text + .char_indices() + .find_map(|(idx, ch)| (!ch.is_whitespace()).then_some(idx)) + .unwrap_or(text.len()); + text.split_at(split_at) +} + /// Check if a tool call title references a markdown file. #[allow(clippy::case_sensitive_file_extension_comparisons)] pub fn is_markdown_file(title: &str) -> bool { @@ -370,6 +384,44 @@ mod tests { assert!(!rendered.iter().any(|line| line == "tmp.md")); } + #[test] + fn render_diff_preserves_source_indentation() { + let lines = render_diff( + &model::Diff::new( + "tmp.rs", + "fn main() {\n if true {\n return;\n }\n}\n".to_owned(), + ), + 80, + ); + let rendered: Vec = lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.as_ref()).collect()) + .collect(); + + assert!(rendered.iter().any(|line| line.contains("+ if true {"))); + assert!(rendered.iter().any(|line| line.contains("+ return;"))); + } + + #[test] + fn render_diff_preserves_source_indentation_for_wrapped_lines() { + let lines = render_diff( + &model::Diff::new( + "tmp.rs", + " This is a long added line that should wrap with indentation preserved.\n" + .to_owned(), + ), + 28, + ); + let rendered: Vec = lines + .iter() + .map(|line| line.spans.iter().map(|span| span.content.as_ref()).collect()) + .collect(); + + assert!(rendered.iter().any(|line| line.contains("+ This is a"))); + assert!(rendered.iter().any(|line| line.contains("indentation"))); + assert!(rendered.iter().any(|line| line.starts_with(" "))); + } + #[test] fn compact_hunk_header_omits_empty_side_and_uses_ranges() { assert_eq!(format_compact_hunk_header("@@ -0,0 +1,7 @@"), "lines +1-7"); diff --git a/src/ui/tool_call/mod.rs b/src/ui/tool_call/mod.rs index 2062cde0..210fce89 100644 --- a/src/ui/tool_call/mod.rs +++ b/src/ui/tool_call/mod.rs @@ -349,6 +349,7 @@ mod tests { use super::*; use crate::app::BlockCache; use pretty_assertions::assert_eq; + use std::fmt::Write as _; fn test_tool_call( id: &str, @@ -842,6 +843,97 @@ mod tests { assert!(text.len() > 2); } + #[test] + fn diff_tool_body_adds_nested_indent_inside_tool_prefix() { + let mut tc = test_tool_call("tc-diff-indent", "Edit", model::ToolCallStatus::Completed); + tc.content = vec![model::ToolCallContent::Diff( + model::Diff::new("src/main.rs", "new".to_owned()) + .old_text(Some("old".to_owned())) + .repository(Some("acme/project".to_owned())), + )]; + + let body = standard::render_tool_call_body(&tc, 80); + let rendered: Vec = body + .iter() + .map(|line| line.spans.iter().map(|span| span.content.as_ref()).collect()) + .collect(); + + assert!(rendered.iter().any(|line| line.starts_with(" │ [acme/project]"))); + assert!(rendered.iter().any(|line| line.starts_with(" │ lines "))); + assert!(rendered.iter().any(|line| { + (line.starts_with(" │ ") || line.starts_with(" └─ ")) && line.contains("+ new") + })); + } + + #[test] + fn diff_tool_body_preserves_source_code_indentation() { + let mut tc = + test_tool_call("tc-diff-code-indent", "Edit", model::ToolCallStatus::Completed); + tc.content = vec![model::ToolCallContent::Diff(model::Diff::new( + "src/main.rs", + "fn main() {\n if true {\n return;\n }\n}\n".to_owned(), + ))]; + + let body = standard::render_tool_call_body(&tc, 80); + let rendered: Vec = body + .iter() + .map(|line| line.spans.iter().map(|span| span.content.as_ref()).collect()) + .collect(); + + assert!(rendered.iter().any(|line| line.contains("+ if true {"))); + assert!(rendered.iter().any(|line| line.contains("+ return;"))); + } + + #[test] + fn diff_tool_body_preserves_nested_indent_for_wrapped_continuations() { + let mut tc = test_tool_call("tc-diff-wrap", "Edit", model::ToolCallStatus::Completed); + tc.content = vec![model::ToolCallContent::Diff(model::Diff::new( + "src/main.rs", + " This is a long added line that should wrap onto another visual line.\n" + .to_owned(), + ))]; + + let body = standard::render_tool_call_body(&tc, 28); + let rendered: Vec = body + .iter() + .map(|line| line.spans.iter().map(|span| span.content.as_ref()).collect()) + .collect(); + + assert!(rendered.iter().any(|line| line.contains("+ This"))); + assert!( + rendered.iter().any(|line| line.starts_with(" │ ")) + || rendered.iter().any(|line| line.starts_with(" └─ ")) + ); + assert!(rendered.iter().any(|line| line.contains("another"))); + assert!(rendered.iter().any(|line| line.contains("line."))); + } + + #[test] + fn write_diff_cap_keeps_omission_marker_nested_indented() { + let new_text = (0..120).fold(String::new(), |mut text, idx| { + let _ = writeln!(&mut text, "line {idx}"); + text + }); + let mut tc = test_tool_call("tc-diff-cap", "Write", model::ToolCallStatus::Completed); + tc.content = vec![model::ToolCallContent::Diff(model::Diff::new("src/main.rs", new_text))]; + + let body = standard::render_tool_call_body(&tc, 80); + let rendered: Vec = body + .iter() + .map(|line| line.spans.iter().map(|span| span.content.as_ref()).collect()) + .collect(); + + assert!( + rendered + .iter() + .any(|line| line.starts_with(" │ ... ") && line.contains("diff lines omitted")) + || rendered + .iter() + .any(|line| line.starts_with(" └─ ... ") + && line.contains("diff lines omitted")) + ); + } + #[test] fn plan_files_render_markdown_instead_of_diff() { let mut tc = test_tool_call( diff --git a/src/ui/tool_call/standard.rs b/src/ui/tool_call/standard.rs index 82f7134b..31d36dc8 100644 --- a/src/ui/tool_call/standard.rs +++ b/src/ui/tool_call/standard.rs @@ -27,6 +27,8 @@ pub(super) const WRITE_DIFF_MAX_LINES: usize = 50; pub(super) const WRITE_DIFF_HEAD_LINES: usize = 10; const DEFAULT_COLLAPSED_TEXT_SUMMARY_LIMIT: usize = 60; const IN_PROGRESS_SUBAGENT_COLLAPSED_TEXT_SUMMARY_LIMIT: usize = 180; +const DIFF_BODY_INDENT: &str = " "; +const DIFF_BODY_INDENT_WIDTH: u16 = 2; /// Render just the title line for a non-Execute tool call (the line containing the spinner icon). /// Used for in-progress tool calls where only the spinner changes each frame. @@ -249,12 +251,10 @@ fn render_tool_content(tc: &ToolCallInfo, width: u16) -> Vec> { if is_plan_file_path(&diff.path) { lines.extend(render_plan_content(&diff.new_text)); } else { - let raw = render_diff(diff, width); - if tc.sdk_tool_name == "Write" { - lines.extend(cap_write_diff_lines(raw)); - } else { - lines.extend(raw); - } + let raw = render_diff(diff, width.saturating_sub(DIFF_BODY_INDENT_WIDTH)); + let raw = + if tc.sdk_tool_name == "Write" { cap_write_diff_lines(raw) } else { raw }; + lines.extend(indent_rendered_lines(raw, DIFF_BODY_INDENT)); } } model::ToolCallContent::McpResource(resource) => { @@ -347,6 +347,17 @@ fn render_mcp_resource_content( lines } +fn indent_rendered_lines(lines: Vec>, indent: &str) -> Vec> { + lines + .into_iter() + .map(|line| { + let mut spans = vec![Span::raw(indent.to_owned())]; + spans.extend(line.spans); + Line::from(spans) + }) + .collect() +} + /// Returns `true` for paths inside `.claude/plans/` (cross-platform). /// These files render as markdown plan content instead of unified diffs. fn is_plan_file_path(path: &std::path::Path) -> bool {