diff --git a/src/ui/diff_side_by_side.rs b/src/ui/diff_side_by_side.rs index 30ff941a..211740f5 100644 --- a/src/ui/diff_side_by_side.rs +++ b/src/ui/diff_side_by_side.rs @@ -3,26 +3,134 @@ use ratatui::{ layout::Rect, style::{Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, Paragraph, Wrap}, + widgets::{Block, Borders, Paragraph}, }; use unicode_width::UnicodeWidthStr; use crate::app::{ App, DiffSource, ExpandDirection, FocusedPanel, GAP_EXPAND_BATCH, GapId, InputMode, }; -use crate::model::{FileStatus, LineOrigin, LineRange, LineSide}; +use crate::model::{DiffLine, FileStatus, LineOrigin, LineRange, LineSide}; use crate::theme::Theme; use crate::ui::comment_panel; use crate::ui::diff_view::{ apply_horizontal_scroll, comment_type_presentation, cursor_indicator, cursor_indicator_spaced, - diff_stat_title, hunk_header_text_and_style, is_line_highlighted, + diff_stat_title, hunk_header_text_and_style, paint_cursor_line_highlight, paint_visual_selection_overlay, populate_row_to_annotation, render_expander_line, render_hidden_lines, scroll_comment_input_into_view, }; use crate::ui::styles; -use crate::ui::text_utils::{truncate_or_pad, truncate_or_pad_spans}; +use crate::ui::text_utils::{truncate_or_pad, truncate_or_pad_spans, wrap_spans}; use crate::vcs::git::calculate_gap; +#[derive(Clone, Default)] +struct SbsRowMeta { + left_content: Vec>, + right_content: Vec>, + left_prefix: Vec>, + right_prefix: Vec>, + left_pad_style: Style, + right_pad_style: Style, +} + +fn content_spans_for_diff_line( + theme: &Theme, + dl: &DiffLine, + origin: LineOrigin, +) -> Vec> { + let base = match origin { + LineOrigin::Context => styles::diff_context_style(theme), + LineOrigin::Addition => styles::diff_add_style(theme), + LineOrigin::Deletion => styles::diff_del_style(theme), + }; + if let Some(ref h) = dl.highlighted_spans { + h.iter().map(|(s, t)| Span::styled(t.clone(), *s)).collect() + } else { + vec![Span::styled(dl.content.clone(), base)] + } +} + +fn column_pad_style(theme: &Theme, dl: &DiffLine, origin: LineOrigin) -> Style { + match origin { + LineOrigin::Context => styles::diff_context_style(theme), + LineOrigin::Addition => { + if dl.highlighted_spans.is_some() { + Style::default().fg(theme.diff_add).bg(theme.syntax_add_bg) + } else { + styles::diff_add_style(theme) + } + } + LineOrigin::Deletion => { + if dl.highlighted_spans.is_some() { + Style::default().fg(theme.diff_del).bg(theme.syntax_del_bg) + } else { + styles::diff_del_style(theme) + } + } + } +} + +fn pad_spans_to_width( + mut spans: Vec>, + width: usize, + pad_style: Style, +) -> Vec> { + let cur: usize = spans.iter().map(|s| s.content.width()).sum(); + if cur < width { + spans.push(Span::styled(" ".repeat(width - cur), pad_style)); + } + spans +} + +struct SideSpec { + lineno: Option, + marker: &'static str, + marker_style: Style, +} + +fn sbs_row_prefixes( + theme: &Theme, + indicator: &'static str, + left: SideSpec, + right: SideSpec, + lw: usize, +) -> (Vec>, Vec>) { + let dim = styles::dim_style(theme); + let old_num = left + .lineno + .map(|n| format!("{n:>lw$}")) + .unwrap_or_else(|| " ".repeat(lw)); + let new_num = right + .lineno + .map(|n| format!("{n:>lw$}")) + .unwrap_or_else(|| " ".repeat(lw)); + + let left_prefix = vec![ + Span::styled(indicator, styles::current_line_indicator_style(theme)), + Span::styled(format!("{old_num} "), dim), + Span::styled(left.marker.to_string(), left.marker_style), + ]; + let right_prefix = vec![ + Span::styled(" │ ", dim), + Span::styled(format!("{new_num} "), dim), + Span::styled(right.marker.to_string(), right.marker_style), + ]; + (left_prefix, right_prefix) +} + +/// Continuation-row prefixes shared by every wrapped line: blank in place of +/// the line numbers (same width, so columns stay aligned) with the center +/// divider preserved. +fn sbs_blank_prefixes(theme: &Theme, lw: usize) -> (Vec>, Vec>) { + let dim = styles::dim_style(theme); + let left = vec![Span::styled(" ".repeat(lw + 3), Style::default())]; + let right = vec![ + Span::styled(" │ ", dim), + Span::styled(" ".repeat(lw + 2), Style::default()), + ]; + (left, right) +} + /// Cursor info for the inline comment input box in side-by-side view: /// (cursor_logical_line, cursor_column, box_start_line, box_end_line) type SideBySideCursorInfo = (usize, u16, usize, usize, usize); @@ -47,6 +155,7 @@ struct SideBySideContext<'a> { // RefCell so deeply-nested rendering helpers can push without each // intermediate function needing a `&mut Vec` parameter threaded through. comment_bars: std::cell::RefCell>, + sbs_meta: std::cell::RefCell>, // Only fully build spans for diff lines whose `line_idx` falls in this // half-open range; off-screen rows push `Line::default()` placeholders. visible_start: usize, @@ -108,6 +217,7 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R editing_comment_id: app.editing_comment_id.as_deref(), current_file_idx: app.diff_state.current_file_idx, comment_bars: std::cell::RefCell::new(Vec::new()), + sbs_meta: std::cell::RefCell::new(std::collections::HashMap::new()), visible_start, visible_end, }; @@ -488,11 +598,8 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R render_sbs_expanded_context_line( &mut lines, &mut line_idx, - ctx.current_line_idx, expanded_line, - ctx.content_width, - &app.theme, - lw, + &ctx, ); } } @@ -564,11 +671,8 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R render_sbs_expanded_context_line( &mut lines, &mut line_idx, - ctx.current_line_idx, expanded_line, - ctx.content_width, - &app.theme, - lw, + &ctx, ); } } @@ -642,11 +746,8 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R render_sbs_expanded_context_line( &mut lines, &mut line_idx, - ctx.current_line_idx, expanded_line, - ctx.content_width, - &app.theme, - lw, + &ctx, ); } } @@ -678,11 +779,8 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R render_sbs_expanded_context_line( &mut lines, &mut line_idx, - ctx.current_line_idx, expanded_line, - ctx.content_width, - &app.theme, - lw, + &ctx, ); } } @@ -702,6 +800,10 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R let mut bars = ctx.comment_bars.borrow_mut(); std::mem::take(&mut *bars) }; + let sbs_meta = { + let mut m = ctx.sbs_meta.borrow_mut(); + std::mem::take(&mut *m) + }; drop(ctx); app.comment_input_annotation_offset = annotation_offset; @@ -738,22 +840,73 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R let scroll_offset = app.diff_state.scroll_offset; let wrap = app.diff_state.wrap_lines; - // Word-wrap-accurate visual row count per line; see `compute_row_heights`. - let row_heights = crate::ui::diff_view::compute_row_heights( - &visible_lines_unscrolled, - wrap, - inner.width as usize, - ); + let viewport_width = inner.width as usize; + let visible_lines_unscrolled_for_overlay = visible_lines_unscrolled.clone(); + // Single pass: wrap each logical line once, producing both the visual + // rows to render and the per-line height used by every row-mapping + // consumer below, so the two can't disagree. + let (row_heights, wrapped_lines): (Vec, Option>) = if wrap && content_width > 0 + { + let mut heights = Vec::with_capacity(visible_lines_unscrolled_for_overlay.len()); + let mut out: Vec = Vec::new(); + let (left_prefix_blank, right_prefix_blank) = sbs_blank_prefixes(&app.theme, lw); + for (i, line) in visible_lines_unscrolled_for_overlay.iter().enumerate() { + let logical_idx = scroll_offset + i; + match sbs_meta.get(&logical_idx) { + Some(m) => { + let left_rows = if m.left_content.is_empty() { + vec![Vec::new()] + } else { + wrap_spans(&m.left_content, content_width) + }; + let right_rows = if m.right_content.is_empty() { + vec![Vec::new()] + } else { + wrap_spans(&m.right_content, content_width) + }; + let n = left_rows.len().max(right_rows.len()).max(1); + heights.push(n); + let empty_row: Vec = Vec::new(); + for k in 0..n { + let left_content_row = left_rows.get(k).unwrap_or(&empty_row).clone(); + let right_content_row = right_rows.get(k).unwrap_or(&empty_row).clone(); + let left_padded = + pad_spans_to_width(left_content_row, content_width, m.left_pad_style); + let right_padded = + pad_spans_to_width(right_content_row, content_width, m.right_pad_style); + let (left_prefix, right_prefix) = if k == 0 { + (m.left_prefix.clone(), m.right_prefix.clone()) + } else { + (left_prefix_blank.clone(), right_prefix_blank.clone()) + }; + let mut spans = left_prefix; + spans.extend(left_padded); + spans.extend(right_prefix); + spans.extend(right_padded); + out.push(Line::from(spans)); + } + } + None => { + let rows = wrap_spans(&line.spans, viewport_width); + heights.push(rows.len()); + out.extend(rows.into_iter().map(Line::from)); + } + } + } + (heights, Some(out)) + } else { + (vec![1; visible_lines_unscrolled_for_overlay.len()], None) + }; app.diff_state.visible_line_count = populate_row_to_annotation( &mut app.diff_row_to_annotation, &row_heights, - inner.width as usize, + viewport_width, inner.height as usize, wrap, scroll_offset, ); - let max_scroll_x = max_content_width.saturating_sub(inner.width as usize); + let max_scroll_x = max_content_width.saturating_sub(viewport_width); if app.diff_state.scroll_x > max_scroll_x { app.diff_state.scroll_x = max_scroll_x; } @@ -762,14 +915,12 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R } let scroll_x = app.diff_state.scroll_x; - let visible_lines_unscrolled_for_overlay = visible_lines_unscrolled.clone(); - let visible_lines: Vec = if app.diff_state.wrap_lines { - visible_lines_unscrolled - } else { - visible_lines_unscrolled + let visible_lines: Vec = match wrapped_lines { + Some(out) => out, + None => visible_lines_unscrolled .into_iter() .map(|line| apply_horizontal_scroll(line, scroll_x)) - .collect() + .collect(), }; let overlay_ctx = crate::ui::diff_view::DiffOverlayPaint { @@ -788,28 +939,16 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R // Section-marker row tint (hunk headers + expand/hidden stubs). crate::ui::diff_view::paint_section_highlight(frame, &overlay_ctx); - let mut diff = Paragraph::new(visible_lines).style(styles::panel_style(&app.theme)); - if app.diff_state.wrap_lines { - diff = diff.wrap(Wrap { trim: false }); - } + let diff = Paragraph::new(visible_lines).style(styles::panel_style(&app.theme)); frame.render_widget(diff, inner); - if app.cursor_line_highlight { - let viewport_height = inner.height as usize; - for offset in 0..viewport_height { - if is_line_highlighted(app, offset) { - let row_rect = Rect { - x: inner.x, - y: inner.y + offset as u16, - width: inner.width, - height: 1, - }; - frame - .buffer_mut() - .set_style(row_rect, Style::default().bg(app.theme.cursor_line_bg)); - } - } - } + paint_cursor_line_highlight( + frame, + inner, + &visible_lines_unscrolled_for_overlay, + &row_heights, + app, + ); // Painted last so the cell overlay wins over cursor-line bg on overlap. if let Some(sel) = app.visual_selection { @@ -859,13 +998,13 @@ pub(super) fn render_side_by_side_diff(frame: &mut Frame, app: &mut App, area: R fn render_sbs_expanded_context_line( lines: &mut Vec>, line_idx: &mut usize, - current_line_idx: usize, expanded_line: &crate::model::DiffLine, - content_width: usize, - theme: &Theme, - lw: usize, + ctx: &SideBySideContext, ) { - let indicator = cursor_indicator(*line_idx, current_line_idx); + let theme = ctx.theme; + let lw = ctx.lineno_width; + let content_width = ctx.content_width; + let indicator = cursor_indicator(*line_idx, ctx.current_line_idx); let old_line_num = expanded_line .old_lineno .map(|n| format!("{n:>lw$} ")) @@ -874,23 +1013,48 @@ fn render_sbs_expanded_context_line( .new_lineno .map(|n| format!("{n:>lw$} ")) .unwrap_or_else(|| " ".repeat(lw + 1)); + let ec_style = styles::expanded_context_style(theme); let line_spans = vec![ Span::styled(indicator, styles::current_line_indicator_style(theme)), - Span::styled(old_line_num, styles::expanded_context_style(theme)), - Span::styled(" ", styles::expanded_context_style(theme)), + Span::styled(old_line_num.clone(), ec_style), + Span::styled(" ", ec_style), Span::styled( truncate_or_pad(&expanded_line.content, content_width), - styles::expanded_context_style(theme), + ec_style, ), Span::styled(" │ ", styles::dim_style(theme)), - Span::styled(new_line_num, styles::expanded_context_style(theme)), - Span::styled(" ", styles::expanded_context_style(theme)), + Span::styled(new_line_num.clone(), ec_style), + Span::styled(" ", ec_style), Span::styled( truncate_or_pad(&expanded_line.content, content_width), - styles::expanded_context_style(theme), + ec_style, ), ]; lines.push(Line::from(line_spans)); + + let dim = styles::dim_style(theme); + let left_prefix = vec![ + Span::styled(indicator, styles::current_line_indicator_style(theme)), + Span::styled(old_line_num, ec_style), + Span::styled(" ", ec_style), + ]; + let right_prefix = vec![ + Span::styled(" │ ", dim), + Span::styled(new_line_num, ec_style), + Span::styled(" ", ec_style), + ]; + let content = vec![Span::styled(expanded_line.content.clone(), ec_style)]; + ctx.sbs_meta.borrow_mut().insert( + *line_idx, + SbsRowMeta { + left_content: content.clone(), + right_content: content, + left_prefix, + right_prefix, + left_pad_style: ec_style, + right_pad_style: ec_style, + }, + ); *line_idx += 1; } @@ -1030,6 +1194,35 @@ fn render_context_line_side_by_side( } lines.push(Line::from(spans)); + + let content = content_spans_for_diff_line(ctx.theme, diff_line, LineOrigin::Context); + let ctx_style = styles::diff_context_style(ctx.theme); + let (lp, rp) = sbs_row_prefixes( + ctx.theme, + indicator, + SideSpec { + lineno: diff_line.old_lineno, + marker: " ", + marker_style: ctx_style, + }, + SideSpec { + lineno: diff_line.new_lineno, + marker: " ", + marker_style: ctx_style, + }, + w, + ); + ctx.sbs_meta.borrow_mut().insert( + line_idx, + SbsRowMeta { + left_content: content.clone(), + right_content: content, + left_prefix: lp, + right_prefix: rp, + left_pad_style: ctx_style, + right_pad_style: ctx_style, + }, + ); } else { lines.push(Line::default()); } @@ -1095,6 +1288,8 @@ fn render_deletion_addition_pair_side_by_side( // Render each pair of deletion/addition for offset in 0..max_lines { + let del_opt = (offset < del_count).then(|| &hunk_lines[start_idx + offset]); + let add_opt = (offset < add_count).then(|| &hunk_lines[add_start + offset]); if ctx.is_visible(line_idx) { let indicator = cursor_indicator(line_idx, ctx.current_line_idx); @@ -1104,8 +1299,7 @@ fn render_deletion_addition_pair_side_by_side( )]; // Left side (deletion) - if offset < del_count { - let del_line = &hunk_lines[start_idx + offset]; + if let Some(del_line) = del_opt { add_deletion_spans( ctx.theme, &mut spans, @@ -1120,8 +1314,7 @@ fn render_deletion_addition_pair_side_by_side( spans.push(Span::styled(" │ ", styles::dim_style(ctx.theme))); // Right side (addition) - if offset < add_count { - let add_line = &hunk_lines[add_start + offset]; + if let Some(add_line) = add_opt { add_addition_spans( ctx.theme, &mut spans, @@ -1134,68 +1327,116 @@ fn render_deletion_addition_pair_side_by_side( } lines.push(Line::from(spans)); + + let w = ctx.lineno_width; + let (left_content, left_pad, left_marker, left_lineno, left_marker_style) = + match del_opt { + Some(dl) => ( + content_spans_for_diff_line(ctx.theme, dl, LineOrigin::Deletion), + column_pad_style(ctx.theme, dl, LineOrigin::Deletion), + "▌", + dl.old_lineno, + styles::diff_del_style(ctx.theme), + ), + None => (Vec::new(), Style::default(), " ", None, Style::default()), + }; + let (right_content, right_pad, right_marker, right_lineno, right_marker_style) = + match add_opt { + Some(al) => ( + content_spans_for_diff_line(ctx.theme, al, LineOrigin::Addition), + column_pad_style(ctx.theme, al, LineOrigin::Addition), + "▌", + al.new_lineno, + styles::diff_add_style(ctx.theme), + ), + None => (Vec::new(), Style::default(), " ", None, Style::default()), + }; + let (lp, rp) = sbs_row_prefixes( + ctx.theme, + indicator, + SideSpec { + lineno: left_lineno, + marker: left_marker, + marker_style: left_marker_style, + }, + SideSpec { + lineno: right_lineno, + marker: right_marker, + marker_style: right_marker_style, + }, + w, + ); + ctx.sbs_meta.borrow_mut().insert( + line_idx, + SbsRowMeta { + left_content, + right_content, + left_prefix: lp, + right_prefix: rp, + left_pad_style: left_pad, + right_pad_style: right_pad, + }, + ); } else { lines.push(Line::default()); } line_idx += 1; // Add comments for deletion - if offset < del_count { - let del_line = &hunk_lines[start_idx + offset]; - if let Some(old_ln) = del_line.old_lineno { - let (new_line_idx, cursor_info) = add_comments_to_line( + if let Some(del_line) = del_opt + && let Some(old_ln) = del_line.old_lineno + { + let (new_line_idx, cursor_info) = add_comments_to_line( + old_ln, + line_comments, + LineSide::Old, + ctx, + file_idx, + line_idx, + lines, + ); + line_idx = new_line_idx; + if cursor_info.is_some() { + cursor_info_out = cursor_info; + } + if let Some(file) = ctx.app.diff_files.get(file_idx) { + line_idx = add_remote_threads_to_line( old_ln, - line_comments, LineSide::Old, ctx, - file_idx, + file.display_path(), line_idx, lines, ); - line_idx = new_line_idx; - if cursor_info.is_some() { - cursor_info_out = cursor_info; - } - if let Some(file) = ctx.app.diff_files.get(file_idx) { - line_idx = add_remote_threads_to_line( - old_ln, - LineSide::Old, - ctx, - file.display_path(), - line_idx, - lines, - ); - } } } // Add comments for addition - if offset < add_count { - let add_line = &hunk_lines[add_start + offset]; - if let Some(new_ln) = add_line.new_lineno { - let (new_line_idx, cursor_info) = add_comments_to_line( + if let Some(add_line) = add_opt + && let Some(new_ln) = add_line.new_lineno + { + let (new_line_idx, cursor_info) = add_comments_to_line( + new_ln, + line_comments, + LineSide::New, + ctx, + file_idx, + line_idx, + lines, + ); + line_idx = new_line_idx; + if cursor_info.is_some() { + cursor_info_out = cursor_info; + } + if let Some(file) = ctx.app.diff_files.get(file_idx) { + line_idx = add_remote_threads_to_line( new_ln, - line_comments, LineSide::New, ctx, - file_idx, + file.display_path(), line_idx, lines, ); - line_idx = new_line_idx; - if cursor_info.is_some() { - cursor_info_out = cursor_info; - } - if let Some(file) = ctx.app.diff_files.get(file_idx) { - line_idx = add_remote_threads_to_line( - new_ln, - LineSide::New, - ctx, - file.display_path(), - line_idx, - lines, - ); - } } } } @@ -1231,6 +1472,36 @@ fn render_standalone_addition_side_by_side( ); lines.push(Line::from(spans)); + + let w = ctx.lineno_width; + let right_content = content_spans_for_diff_line(ctx.theme, diff_line, LineOrigin::Addition); + let right_pad = column_pad_style(ctx.theme, diff_line, LineOrigin::Addition); + let (lp, rp) = sbs_row_prefixes( + ctx.theme, + indicator, + SideSpec { + lineno: None, + marker: " ", + marker_style: Style::default(), + }, + SideSpec { + lineno: diff_line.new_lineno, + marker: "▌", + marker_style: styles::diff_add_style(ctx.theme), + }, + w, + ); + ctx.sbs_meta.borrow_mut().insert( + line_idx, + SbsRowMeta { + left_content: Vec::new(), + right_content, + left_prefix: lp, + right_prefix: rp, + left_pad_style: Style::default(), + right_pad_style: right_pad, + }, + ); } else { lines.push(Line::default()); } @@ -1571,6 +1842,7 @@ mod remote_comments_side_by_side_snapshot_tests { use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::buffer::Buffer; + use ratatui::layout::Rect; use std::path::{Path, PathBuf}; struct SnapshotVcs { @@ -1772,4 +2044,206 @@ mod remote_comments_side_by_side_snapshot_tests { "remote comment leaked under Hide:\n{body}" ); } + + fn diff_file_with_pair(left: &str, right: &str) -> DiffFile { + let lines = vec![ + DiffLine { + origin: LineOrigin::Deletion, + content: left.to_string(), + old_lineno: Some(1), + new_lineno: None, + highlighted_spans: None, + }, + DiffLine { + origin: LineOrigin::Addition, + content: right.to_string(), + old_lineno: None, + new_lineno: Some(1), + highlighted_spans: None, + }, + ]; + let hunks = vec![DiffHunk { + header: "@@ -1,1 +1,1 @@".to_string(), + lines, + old_start: 1, + old_count: 1, + new_start: 1, + new_count: 1, + }]; + let content_hash = DiffFile::compute_content_hash(&hunks); + DiffFile { + old_path: Some(PathBuf::from("src/lib.rs")), + new_path: Some(PathBuf::from("src/lib.rs")), + status: FileStatus::Modified, + hunks, + is_binary: false, + is_too_large: false, + is_commit_message: false, + content_hash, + } + } + + fn diff_file_with_standalone_deletion(left: &str) -> DiffFile { + let lines = vec![DiffLine { + origin: LineOrigin::Deletion, + content: left.to_string(), + old_lineno: Some(1), + new_lineno: None, + highlighted_spans: None, + }]; + let hunks = vec![DiffHunk { + header: "@@ -1,1 +0,0 @@".to_string(), + lines, + old_start: 1, + old_count: 1, + new_start: 0, + new_count: 0, + }]; + let content_hash = DiffFile::compute_content_hash(&hunks); + DiffFile { + old_path: Some(PathBuf::from("src/lib.rs")), + new_path: Some(PathBuf::from("src/lib.rs")), + status: FileStatus::Modified, + hunks, + is_binary: false, + is_too_large: false, + is_commit_message: false, + content_hash, + } + } + + fn draw_sbs(app: &mut App, w: u16, h: u16) -> Buffer { + let backend = TestBackend::new(w, h); + let mut terminal = Terminal::new(backend).unwrap(); + terminal + .draw(|frame| super::render_side_by_side_diff(frame, app, Rect::new(0, 0, w, h))) + .expect("draw sbs"); + terminal.backend().buffer().clone() + } + + fn char_at(buf: &Buffer, x: u16, y: u16) -> String { + buf[(x, y)].symbol().to_string() + } + + #[test] + fn should_wrap_long_line_in_side_by_side_view_when_wrap_enabled() { + let long_left = "L".repeat(200); + let mut app = make_pr_app(); + app.diff_files = vec![diff_file_with_pair(&long_left, "short")]; + app.set_diff_wrap(true); + app.rebuild_annotations(); + + let buf = draw_sbs(&mut app, 160, 20); + + let mut rows_with_l = 0u16; + for y in 0..buf.area.height { + let row: String = (0..buf.area.width).map(|x| char_at(&buf, x, y)).collect(); + if row.contains("LLLLLLLLLL") { + rows_with_l += 1; + } + } + assert!( + rows_with_l >= 2, + "expected long left content to span >=2 visual rows, got {rows_with_l}" + ); + } + + #[test] + fn should_not_wrap_when_wrap_disabled_in_side_by_side() { + let long_left = "L".repeat(200); + let mut app = make_pr_app(); + app.diff_files = vec![diff_file_with_pair(&long_left, "short")]; + app.set_diff_wrap(false); + app.rebuild_annotations(); + + let buf = draw_sbs(&mut app, 160, 20); + + let rows_with_l: u16 = (0..buf.area.height) + .filter(|&y| { + (0..buf.area.width) + .map(|x| char_at(&buf, x, y)) + .collect::() + .contains("LLLLLLLLLL") + }) + .count() as u16; + assert_eq!( + rows_with_l, 1, + "wrap-off should produce exactly one row of L, got {rows_with_l}" + ); + } + + #[test] + fn should_align_divider_on_wrapped_rows_in_side_by_side() { + let long_left = "L".repeat(200); + let mut app = make_pr_app(); + app.diff_files = vec![diff_file_with_pair(&long_left, "short")]; + app.set_diff_wrap(true); + app.rebuild_annotations(); + + let lw = 1usize; + let inner_w = 158usize; + let content_width = (inner_w - crate::app::sbs_overhead(lw) as usize) / 2; + let divider_x_inner = crate::app::sbs_left_gutter(lw) as usize + content_width; + let divider_glyph_x = 1 + divider_x_inner + 1; + + let buf = draw_sbs(&mut app, 160, 20); + + let mut rows_with_l: Vec = Vec::new(); + for y in 0..buf.area.height { + let row: String = (0..buf.area.width).map(|x| char_at(&buf, x, y)).collect(); + if row.contains("LLLLLLLLLL") { + rows_with_l.push(y); + } + } + assert!( + rows_with_l.len() >= 2, + "expected ≥2 wrapped rows, got {}", + rows_with_l.len() + ); + for y in &rows_with_l { + let glyph = char_at(&buf, divider_glyph_x as u16, *y); + assert_eq!( + glyph, "│", + "expected │ at col {divider_glyph_x} on row {y}, got {glyph:?}" + ); + } + } + + #[test] + fn should_pad_shorter_column_on_wrapped_rows_in_side_by_side() { + let long_left = "L".repeat(200); + let mut app = make_pr_app(); + app.diff_files = vec![diff_file_with_standalone_deletion(&long_left)]; + app.set_diff_wrap(true); + app.rebuild_annotations(); + + let buf = draw_sbs(&mut app, 160, 20); + + let lw = 1usize; + let inner_w = 158usize; + let content_width = (inner_w - crate::app::sbs_overhead(lw) as usize) / 2; + let divider_glyph_x = 1 + crate::app::sbs_left_gutter(lw) as usize + content_width + 1; + let right_content_start = divider_glyph_x + 2 + lw + 1 + 1; + let right_content_end = right_content_start + content_width; + + let mut checked = 0; + for y in 0..buf.area.height { + let row: String = (0..buf.area.width).map(|x| char_at(&buf, x, y)).collect(); + if !row.contains("LLLLLLLLLL") { + continue; + } + checked += 1; + let right: String = (right_content_start..right_content_end) + .map(|x| char_at(&buf, x as u16, y)) + .collect(); + assert!( + right.trim().is_empty(), + "right column should be blank on wrapped L row {y}, got {right:?}" + ); + } + assert!( + checked >= 2, + "expected ≥2 wrapped rows to check, got {checked}" + ); + } } diff --git a/src/ui/diff_unified.rs b/src/ui/diff_unified.rs index 756dc005..35bd61e7 100644 --- a/src/ui/diff_unified.rs +++ b/src/ui/diff_unified.rs @@ -3,7 +3,7 @@ use ratatui::{ layout::Rect, style::{Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, Paragraph, Wrap}, + widgets::{Block, Borders, Paragraph}, }; use unicode_width::UnicodeWidthStr; @@ -16,9 +16,9 @@ use crate::theme::Theme; use crate::ui::comment_panel; use crate::ui::diff_view::{ apply_horizontal_scroll, comment_type_presentation, cursor_indicator, cursor_indicator_spaced, - diff_stat_title, hunk_header_text_and_style, is_line_highlighted, paint_unified_diff_rows_with, - paint_visual_selection_overlay, populate_row_to_annotation, push_comment_bar, - render_expander_line, render_hidden_lines, scroll_comment_input_into_view, + diff_stat_title, hunk_header_text_and_style, paint_cursor_line_highlight, + paint_unified_diff_rows_with, paint_visual_selection_overlay, populate_row_to_annotation, + push_comment_bar, render_expander_line, render_hidden_lines, scroll_comment_input_into_view, unified_line_bg_style, }; use crate::ui::styles; @@ -1119,24 +1119,34 @@ pub(super) fn render_unified_diff(frame: &mut Frame, app: &mut App, area: Rect) let scroll_offset = app.diff_state.scroll_offset; let wrap = app.diff_state.wrap_lines; - // Word-wrap-accurate visual row count per line, shared by every row-mapping - // consumer below so per-row backgrounds and overlays align with the - // rendered (word-wrapped) paragraph rather than drifting on prose lines. - let row_heights = crate::ui::diff_view::compute_row_heights( - &visible_lines_unscrolled, - wrap, - inner.width as usize, - ); + let viewport_width = inner.width as usize; + let visible_lines_unscrolled_for_bg = visible_lines_unscrolled.clone(); + // Single pass: wrap each logical line once, producing both the visual + // rows to render and the per-line height used by every row-mapping + // consumer below, so the two can't disagree. + let (row_heights, wrapped_lines): (Vec, Option>) = + if wrap && viewport_width > 0 { + let mut heights = Vec::with_capacity(visible_lines_unscrolled_for_bg.len()); + let mut out: Vec = Vec::new(); + for line in &visible_lines_unscrolled_for_bg { + let rows = crate::ui::text_utils::wrap_spans(&line.spans, viewport_width); + heights.push(rows.len()); + out.extend(rows.into_iter().map(Line::from)); + } + (heights, Some(out)) + } else { + (vec![1; visible_lines_unscrolled_for_bg.len()], None) + }; app.diff_state.visible_line_count = populate_row_to_annotation( &mut app.diff_row_to_annotation, &row_heights, - inner.width as usize, + viewport_width, inner.height as usize, wrap, scroll_offset, ); - let max_scroll_x = max_content_width.saturating_sub(inner.width as usize); + let max_scroll_x = max_content_width.saturating_sub(viewport_width); if app.diff_state.scroll_x > max_scroll_x { app.diff_state.scroll_x = max_scroll_x; } @@ -1145,14 +1155,12 @@ pub(super) fn render_unified_diff(frame: &mut Frame, app: &mut App, area: Rect) } let scroll_x = app.diff_state.scroll_x; - let visible_lines_unscrolled_for_bg = visible_lines_unscrolled.clone(); - let visible_lines: Vec = if app.diff_state.wrap_lines { - visible_lines_unscrolled - } else { - visible_lines_unscrolled + let visible_lines: Vec = match wrapped_lines { + Some(out) => out, + None => visible_lines_unscrolled .into_iter() .map(|line| apply_horizontal_scroll(line, scroll_x)) - .collect() + .collect(), }; // Paint per-visual-row add/del backgrounds across full row width. @@ -1183,25 +1191,18 @@ pub(super) fn render_unified_diff(frame: &mut Frame, app: &mut App, area: Rect) crate::ui::diff_view::paint_section_highlight(frame, &overlay_ctx); // Keep paragraph bg unset so pre-painted per-row diff backgrounds remain visible. - let mut diff = Paragraph::new(visible_lines).style(Style::default().fg(app.theme.fg_primary)); - if app.diff_state.wrap_lines { - diff = diff.wrap(Wrap { trim: false }); - } + let diff = Paragraph::new(visible_lines).style(Style::default().fg(app.theme.fg_primary)); frame.render_widget(diff, inner); // Cursor-line bg has to land after the paragraph: spans on +/- lines carry // explicit diff_add_bg/diff_del_bg that would mask a pre-paint over the code. - if app.cursor_line_highlight { - paint_unified_diff_rows_with( - frame, - inner, - &visible_lines_unscrolled_for_bg, - &row_heights, - |idx, _line| { - is_line_highlighted(app, idx).then(|| Style::default().bg(app.theme.cursor_line_bg)) - }, - ); - } + paint_cursor_line_highlight( + frame, + inner, + &visible_lines_unscrolled_for_bg, + &row_heights, + app, + ); if let Some(sel) = app.visual_selection { paint_visual_selection_overlay(frame, inner, app, sel, &app.theme); @@ -1776,4 +1777,148 @@ mod remote_comments_snapshot_tests { "review-level thread leaked under Hide:\n{body}" ); } + + #[test] + fn should_wrap_long_line_in_unified_view_when_wrap_enabled() { + let long: String = "x".repeat(200); + let hunk = DiffHunk { + header: "@@ -0,0 +1,1 @@".to_string(), + lines: vec![DiffLine { + origin: LineOrigin::Addition, + content: long.clone(), + old_lineno: None, + new_lineno: Some(1), + highlighted_spans: None, + }], + old_start: 0, + old_count: 0, + new_start: 1, + new_count: 1, + }; + let hunks = vec![hunk]; + let content_hash = DiffFile::compute_content_hash(&hunks); + let file = DiffFile { + old_path: Some(PathBuf::from("src/lib.rs")), + new_path: Some(PathBuf::from("src/lib.rs")), + status: FileStatus::Modified, + hunks, + is_binary: false, + is_too_large: false, + is_commit_message: false, + content_hash, + }; + let mut app = make_revision_app(vec![file]); + app.set_diff_wrap(true); + app.rebuild_annotations(); + + let backend = TestBackend::new(80, 20); + let mut terminal = Terminal::new(backend).unwrap(); + terminal + .draw(|frame| super::render_unified_diff(frame, &mut app, Rect::new(0, 0, 80, 20))) + .expect("draw"); + let body = body_text(terminal.backend().buffer()); + + let tail: String = long.chars().rev().take(20).collect::(); + let tail: String = tail.chars().rev().collect(); + assert!( + body.contains(&tail), + "tail of wrapped long line should appear in body:\n{body}" + ); + + assert!( + app.diff_state.visible_line_count > 0 && app.diff_state.visible_line_count < 20, + "expected logical visible_line_count 1..20, got {}", + app.diff_state.visible_line_count + ); + } + + #[test] + fn should_extend_comment_bar_over_wrapped_rows_when_wrap_enabled() { + use crate::model::{Comment, CommentType}; + + let long: String = "x".repeat(200); + let hunk = DiffHunk { + header: "@@ -0,0 +1,1 @@".to_string(), + lines: vec![DiffLine { + origin: LineOrigin::Addition, + content: long.clone(), + old_lineno: None, + new_lineno: Some(1), + highlighted_spans: None, + }], + old_start: 0, + old_count: 0, + new_start: 1, + new_count: 1, + }; + let hunks = vec![hunk]; + let content_hash = DiffFile::compute_content_hash(&hunks); + let path = PathBuf::from("src/lib.rs"); + let file = DiffFile { + old_path: Some(path.clone()), + new_path: Some(path.clone()), + status: FileStatus::Modified, + hunks, + is_binary: false, + is_too_large: false, + is_commit_message: false, + content_hash, + }; + let mut app = make_revision_app(vec![file]); + app.set_diff_wrap(true); + + let file_review = app + .session + .get_file_mut(&path) + .expect("file registered in session"); + file_review.add_line_comment( + 1, + Comment::new("needs a rename".to_string(), CommentType::Note, None), + ); + app.rebuild_annotations(); + + let backend = TestBackend::new(80, 20); + let mut terminal = Terminal::new(backend).unwrap(); + terminal + .draw(|frame| super::render_unified_diff(frame, &mut app, Rect::new(0, 0, 80, 20))) + .expect("draw"); + let buffer = terminal.backend().buffer(); + + let mut cap: Option<(u16, u16)> = None; + let mut cap_count = 0; + for y in 0..buffer.area.height { + for x in 0..buffer.area.width { + if buffer[(x, y)].symbol() == "╭" { + cap = Some((x, y)); + cap_count += 1; + } + } + } + assert_eq!( + cap_count, 1, + "expected exactly one ╭ cap in the gutter, got {cap_count}" + ); + let (bar_x, cap_y) = cap.unwrap(); + + let mut box_top_y: Option = None; + for y in (cap_y + 1)..buffer.area.height { + if buffer[(bar_x, y)].symbol() == "├" { + box_top_y = Some(y); + break; + } + } + let box_top_y = box_top_y.expect("expected ├ box top below the ╭ cap"); + assert!( + box_top_y > cap_y + 1, + "test needs at least one row between cap and box top; cap_y={cap_y} box_top_y={box_top_y}" + ); + + for y in (cap_y + 1)..box_top_y { + let glyph = buffer[(bar_x, y)].symbol(); + assert_eq!( + glyph, "│", + "expected │ at ({bar_x},{y}) between cap ({cap_y}) and box top ({box_top_y}), got {glyph:?}" + ); + } + } } diff --git a/src/ui/diff_view.rs b/src/ui/diff_view.rs index 34d31908..4db8699a 100644 --- a/src/ui/diff_view.rs +++ b/src/ui/diff_view.rs @@ -3,7 +3,6 @@ use ratatui::{ layout::Rect, style::Style, text::{Line, Span}, - widgets::{Paragraph, Wrap}, }; use crate::app::{ @@ -504,6 +503,31 @@ pub(super) fn unified_line_bg_style(line: &Line, theme: &Theme) -> Option