From 66250d6c400b971d8819803f586b7d91ce1c12fb Mon Sep 17 00:00:00 2001 From: Valdemar Lorenzen Date: Fri, 16 Oct 2026 18:41:24 +0200 Subject: [PATCH] refactor: encapsulate diff document drawing on WheelPanels --- docs/architecture.md | 2 + src/tui.rs | 743 ++++++++++++++++++++++--------------------- 2 files changed, 385 insertions(+), 360 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index 04503b9..54e1f1d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -90,6 +90,8 @@ Conflict History alignment lives in `diff::HistoryComparison`: it joins the sele Content painting and native caret overlays consume a borrowed `diff::ColumnView` for both ordinary comparisons and Conflict History. It resolves folds, gaps, source text and coordinates, native terminal rows, newline state, and side-specific change ranges without copying text or building a row catalog. Native caret mapping remains separate from review membership: unified equal context can carry anchors from either original source. Review freshness/filtering, pane chrome, viewport policy, wrapping, scope labels, and colors stay with their existing owners; the content painter receives only the view and already-filtered review annotations. History retains its internal pairwise `DiffState` storage. +`WheelPanels::draw_documents` owns the complete ordinary/Resolution document frame: tree preparation, viewport anchoring and scroll projection, visible-row highlighting, content/native caret painting, scope overlays, and final geometry publication. It returns only the surviving document caret for outer popup placement. Selectors, cached-error notices, and file navigation remain in outer orchestration; cached syntax errors are read before preparation to preserve notice-layout timing. The existing owner also retains immediate ordered wheel handling and previous-frame geometry; Conflict History keeps its separate presentation flow. + `Editor::scroll_distance()` supplies the session-wide optional edge margin. The TUI applies it to document/diff visual rows and unwrapped text columns, and retains independent picker offsets across navigation frames. Picker windows exclude their fixed chrome and align whole multi-row entries. File projections carry a monotonic viewport revision, so catalog/filter/expansion changes invalidate picker identity without hashing the entire catalog on every redraw. Metadata-only status changes do not move the viewport. Explicit LSP/wrap-toggle anchors and detached wheel views retain precedence. Keyboard viewport actions queue per-view row and configured-step counts without moving carets. `Editor::take_viewport_scroll(height)` resolves step counts using the session-wide `scroll_step_lines` preference, or half the focused viewport when unset, and saturates multiplication before the frontend clamps movement to content bounds. Ordinary, diff, and conflict-history rendering share this resolution. diff --git a/src/tui.rs b/src/tui.rs index e7295d6..e8098fb 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -2218,7 +2218,7 @@ fn draw_document( ); return; } - panels.diff_syntax = None; + panels.cached_diff_error(None); if overlay_area.width == 0 || overlay_area.height == 0 { return; } @@ -3309,9 +3309,7 @@ fn draw_diff( } let comparison = diff.comparison.as_ref(); let selected = comparison.and_then(|comparison| comparison.files.get(diff.file)); - if diff.phase != Phase::View || !diff.selected_file_loaded() { - panels.diff_syntax = None; - } + let syntax_error = panels.cached_diff_error(Some(diff)); if area.width == 0 || area.height == 0 { return; } @@ -3598,10 +3596,6 @@ fn draw_diff( .and_then(|file| file.error.as_deref()) }) }); - let syntax_error = panels - .diff_syntax - .as_ref() - .and_then(|syntax| syntax.before.error().or_else(|| syntax.after.error())); let notice = diff.error.as_deref().or(resolver_error).or_else(|| { if diff.loading || resolver.is_some_and(|resolver| resolver.loading) { Some("Loading file…") @@ -3649,367 +3643,396 @@ fn draw_diff( comparison.map(|comparison| (comparison.files.as_slice(), &diff.reviewed_files)), ); } - if let Some(resolver) = resolver.filter(|resolver| { - resolver - .files - .get(resolver.current_file) - .is_some_and(|file| !file.text) - }) { - draw_resolver_choices(frame, resolver, panes); - panels.diff = Some(DiffGeometry { - regions: [ - (navigator.unwrap_or_default(), Focus::Files), - (Rect::default(), Focus::Before), - (Rect::default(), Focus::After), - ], - viewport: None, - text_widths: [0; 2], - wrapping: diff.word_wrap, - view_mode: diff.view_mode, - split_axis: diff.split_axis, - caret_row: None, + let document_caret = panels.draw_documents(frame, diff, panes, navigator, editor); + if let Some(editor) = editor { + draw_diff_popups(frame, editor, area, document_caret, panels); + } +} + +impl WheelPanels { + /// Read the previous draw's error without changing notice-layout timing. + fn cached_diff_error(&mut self, diff: Option<&DiffState>) -> Option<&str> { + if diff.is_none_or(|diff| diff.phase != Phase::View || !diff.selected_file_loaded()) { + self.diff_syntax = None; + } + self.diff_syntax + .as_ref() + .and_then(|syntax| syntax.before.error().or_else(|| syntax.after.error())) + } + + /// Publish geometry and return the caret only after all document overlays. + fn draw_documents( + &mut self, + frame: &mut Frame, + diff: &DiffState, + panes: Rect, + navigator: Option, + editor: Option<&Editor>, + ) -> Option<(Rect, Position)> { + self.cached_diff_error(Some(diff)); + let selected = diff + .comparison + .as_ref() + .and_then(|comparison| comparison.files.get(diff.file)); + let resolver = editor + .and_then(Editor::resolver) + .filter(|resolver| resolver.view == ResolverView::Resolution); + if let Some(resolver) = resolver.filter(|resolver| { + resolver + .files + .get(resolver.current_file) + .is_some_and(|file| !file.text) + }) { + draw_resolver_choices(frame, resolver, panes); + self.diff = Some(DiffGeometry { + regions: [ + (navigator.unwrap_or_default(), Focus::Files), + (Rect::default(), Focus::Before), + (Rect::default(), Focus::After), + ], + viewport: None, + text_widths: [0; 2], + wrapping: diff.word_wrap, + view_mode: diff.view_mode, + split_axis: diff.split_axis, + caret_row: None, + }); + return None; + } + let pane_layout = diff.pane_layout(); + let unified = diff.view_mode == ViewMode::Unified; + let [before_area, after_area] = if unified { + [panes, panes] + } else { + match pane_layout { + PaneLayout::Split => { + split_rect(pane_rect(panes), diff.split_axis).map(terminal_rect) + } + PaneLayout::Before | PaneLayout::Empty => [panes, Rect::default()], + PaneLayout::After => [Rect::default(), panes], + } + }; + // Parse complete trees before projecting context; color only visible source rows below. + if let Some(id) = diff.comparison_id + && let Some(file) = selected.filter(|file| file.loaded) + { + self.diff_syntax + .get_or_insert_with(DiffSyntaxCache::default) + .update( + id, + diff.file, + file, + diff.after_source_lines(file), + diff.live_revision, + Some(DiffSyntaxViewport { + diff, + top: diff.top, + areas: [Rect::default(); 2], + wrapped: None, + }), + ); + } + let mut document_caret = None; + let mut top = diff.top; + let mut before_left = diff.left_before; + let mut after_left = diff.left_after; + let wrapping = diff.word_wrap; + let toggle_row = self + .previous_diff + .as_ref() + .filter(|previous| { + (previous.wrapping != wrapping + || previous.view_mode != diff.view_mode + || previous.split_axis != diff.split_axis) + && previous.viewport.is_some_and(|(id, file, _, _, _)| { + Some(id) == diff.comparison_id && file == diff.file + }) + }) + .and_then(|previous| { + editor?.diff_buffer_focus()?; + previous.caret_row + }); + let center_row = editor.and_then(|editor| { + let focus = editor.diff_buffer_focus()?; + let pane = if focus == Focus::Before { + before_area + } else { + after_area + }; + let height = usize::from(diff_inner(pane).height); + let file = selected.filter(|file| file.loaded)?; + (editor.diff_interacting() + && diff.follow_caret + && height > 0 + && diff_pane_text_width(diff, file, pane, focus) > 0 + && diff + .source_display_row(focus, editor.cursor().row) + .is_some() + && editor.take_lsp_center()) + .then_some(height / 2) }); - if let Some(editor) = editor { - draw_diff_popups(frame, editor, area, None, panels); - } - return; - } - let pane_layout = diff.pane_layout(); - let unified = diff.view_mode == ViewMode::Unified; - let [before_area, after_area] = if unified { - [panes, panes] - } else { - match pane_layout { - PaneLayout::Split => split_rect(pane_rect(panes), diff.split_axis).map(terminal_rect), - PaneLayout::Before | PaneLayout::Empty => [panes, Rect::default()], - PaneLayout::After => [Rect::default(), panes], - } - }; - // Parse complete trees before projecting context; color only visible source rows below. - if let Some(id) = diff.comparison_id - && let Some(file) = selected.filter(|file| file.loaded) - { - panels - .diff_syntax - .get_or_insert_with(DiffSyntaxCache::default) - .update( - id, - diff.file, - file, - diff.after_source_lines(file), - diff.live_revision, - Some(DiffSyntaxViewport { - diff, - top: diff.top, - areas: [Rect::default(); 2], - wrapped: None, - }), + let toggle_row = center_row.or(toggle_row); + if let Some(editor) = + editor.filter(|editor| !wrapping && editor.diff_interacting() && diff.follow_caret) + && let Some(focus) = editor.diff_buffer_focus() + { + let (pane, left) = match focus { + Focus::Before => (before_area, &mut before_left), + Focus::After => (after_area, &mut after_left), + Focus::Files => unreachable!("native diff buffers belong to a text pane"), + }; + let source_len = selected.map_or(0, |file| match focus { + Focus::Before => file.before.len(), + Focus::After => file.after.len(), + Focus::Files => unreachable!("native buffer belongs to a text pane"), + }); + let width = selected.map_or_else( + || diff_text_width(pane, source_len), + |file| diff_pane_text_width(diff, file, pane, focus), ); - } - let mut document_caret = None; - let mut top = diff.top; - let mut before_left = diff.left_before; - let mut after_left = diff.left_after; - let wrapping = diff.word_wrap; - let toggle_row = panels - .previous_diff - .as_ref() - .filter(|previous| { - (previous.wrapping != wrapping - || previous.view_mode != diff.view_mode - || previous.split_axis != diff.split_axis) - && previous.viewport.is_some_and(|(id, file, _, _, _)| { + let column = + display_column(&editor.lines()[editor.cursor().row], editor.cursor().column); + *left = scroll_offset(*left, column, width, editor.scroll_distance()); + } + if let Some(file) = selected.filter(|file| file.loaded) + && let Some(syntax) = &self.diff_syntax + { + let caret = editor + .filter(|editor| editor.diff_interacting() && diff.follow_caret) + .and_then(|editor| { + editor + .diff_buffer_focus() + .map(|focus| (focus, editor.cursor())) + }); + let viewport = self.diff_wrap.get_or_insert_with(DiffWrapViewport::default); + viewport.lefts = [before_left, after_left]; + viewport.update( + diff, + file, + [before_area, after_area], + caret, + toggle_row, + editor.map_or(Some(0), Editor::scroll_distance), + syntax, + ); + let height = editor + .and_then(|editor| editor.diff_buffer_focus()) + .map_or(0, |focus| { + usize::from( + diff_inner(if focus == Focus::Before { + before_area + } else { + after_area + }) + .height, + ) + }); + if height > 0 { + let scroll = editor.map_or(0, |editor| editor.take_viewport_scroll(height)); + for _ in 0..scroll.unsigned_abs() { + let next = viewport.step(diff, file, viewport.top, scroll > 0); + if next == viewport.top { + break; + } + viewport.top = next; + } + if scroll != 0 { + viewport.fill( + diff, + file, + viewport.heights[0].max(viewport.heights[1]), + syntax, + ); + } + } + top = viewport.top.display; + } else { + self.diff_wrap = None; + } + if let Some(id) = diff.comparison_id + && let Some(file) = selected.filter(|file| file.loaded) + { + let wrapped = self + .diff_wrap + .as_ref() + .map(|viewport| viewport.rows.as_slice()); + self.diff_syntax + .get_or_insert_with(DiffSyntaxCache::default) + .update( + id, + diff.file, + file, + diff.after_source_lines(file), + diff.live_revision, + Some(DiffSyntaxViewport { + diff, + top, + areas: [before_area, after_area], + wrapped, + }), + ); + } + let mut text_widths = [0; 2]; + let previous_text_widths = self + .previous_diff + .as_ref() + .filter(|geometry| { + geometry.viewport.is_some_and(|(id, file, _, _, _)| { Some(id) == diff.comparison_id && file == diff.file }) - }) - .and_then(|previous| { - editor?.diff_buffer_focus()?; - previous.caret_row - }); - let center_row = editor.and_then(|editor| { - let focus = editor.diff_buffer_focus()?; - let pane = if focus == Focus::Before { - before_area - } else { - after_area - }; - let height = usize::from(diff_inner(pane).height); - let file = selected.filter(|file| file.loaded)?; - (editor.diff_interacting() - && diff.follow_caret - && height > 0 - && diff_pane_text_width(diff, file, pane, focus) > 0 - && diff - .source_display_row(focus, editor.cursor().row) - .is_some() - && editor.take_lsp_center()) - .then_some(height / 2) - }); - let toggle_row = center_row.or(toggle_row); - if let Some(editor) = - editor.filter(|editor| !wrapping && editor.diff_interacting() && diff.follow_caret) - && let Some(focus) = editor.diff_buffer_focus() - { - let (pane, left) = match focus { - Focus::Before => (before_area, &mut before_left), - Focus::After => (after_area, &mut after_left), - Focus::Files => unreachable!("native diff buffers belong to a text pane"), - }; - let source_len = selected.map_or(0, |file| match focus { - Focus::Before => file.before.len(), - Focus::After => file.after.len(), - Focus::Files => unreachable!("native buffer belongs to a text pane"), - }); - let width = selected.map_or_else( - || diff_text_width(pane, source_len), - |file| diff_pane_text_width(diff, file, pane, focus), - ); - let column = display_column(&editor.lines()[editor.cursor().row], editor.cursor().column); - *left = scroll_offset(*left, column, width, editor.scroll_distance()); - } - if let Some(file) = selected.filter(|file| file.loaded) - && let Some(syntax) = &panels.diff_syntax - { - let caret = editor - .filter(|editor| editor.diff_interacting() && diff.follow_caret) - .and_then(|editor| { - editor - .diff_buffer_focus() - .map(|focus| (focus, editor.cursor())) - }); - let viewport = panels - .diff_wrap - .get_or_insert_with(DiffWrapViewport::default); - viewport.lefts = [before_left, after_left]; - viewport.update( - diff, - file, - [before_area, after_area], - caret, - toggle_row, - editor.map_or(Some(0), Editor::scroll_distance), - syntax, - ); - let height = editor - .and_then(|editor| editor.diff_buffer_focus()) - .map_or(0, |focus| { - usize::from( - diff_inner(if focus == Focus::Before { - before_area - } else { - after_area - }) - .height, - ) - }); - if height > 0 { - let scroll = editor.map_or(0, |editor| editor.take_viewport_scroll(height)); - for _ in 0..scroll.unsigned_abs() { - let next = viewport.step(diff, file, viewport.top, scroll > 0); - if next == viewport.top { - break; - } - viewport.top = next; - } - if scroll != 0 { - viewport.fill( - diff, - file, - viewport.heights[0].max(viewport.heights[1]), - syntax, - ); - } - } - top = viewport.top.display; - } else { - panels.diff_wrap = None; - } - if let Some(id) = diff.comparison_id - && let Some(file) = selected.filter(|file| file.loaded) - { - let wrapped = panels + }) + .map(|geometry| geometry.text_widths); + let wrapped = self .diff_wrap .as_ref() .map(|viewport| viewport.rows.as_slice()); - panels - .diff_syntax - .get_or_insert_with(DiffSyntaxCache::default) - .update( - id, - diff.file, - file, - diff.after_source_lines(file), - diff.live_revision, - Some(DiffSyntaxViewport { - diff, - top, - areas: [before_area, after_area], - wrapped, - }), - ); - } - let mut text_widths = [0; 2]; - let previous_text_widths = panels - .previous_diff - .as_ref() - .filter(|geometry| { - geometry.viewport.is_some_and(|(id, file, _, _, _)| { - Some(id) == diff.comparison_id && file == diff.file - }) - }) - .map(|geometry| geometry.text_widths); - let wrapped = panels - .diff_wrap - .as_ref() - .map(|viewport| viewport.rows.as_slice()); - if let Some(file) = selected.filter(|file| file.loaded) - && let Some(syntax) = &panels.diff_syntax - { - if pane_layout == PaneLayout::Empty { - diff_pane_panel(frame, diff, Some(file), before_area, true, resolver); - } else { - if before_area != Rect::default() && (!unified || diff.focus == Focus::Before) { - let text_area = draw_diff_pane( - frame, - diff, - file, - before_area, - true, - [&syntax.before, &syntax.after], - [ - editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::Before)), - editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::After)), - ], - ( - top, - &mut before_left, - previous_text_widths.map(|widths| widths[0]), - ), - wrapped, - panels.diff_colors, - editor.is_some_and(Editor::highlight_trailing_whitespace), - editor.map(Editor::whitespace_characters), - editor.map_or(WhitespaceScope::All, Editor::whitespace_scope), - resolver, - false, - ); - text_widths[0] = text_area.width; - if let Some(editor) = - editor.filter(|editor| editor.diff_buffer_focus() == Some(Focus::Before)) - { - document_caret = paint_diff_editor( - frame, - editor, - diff.column_view(Focus::Before).unwrap(), - text_area, - (top, before_left), - wrapped, - ) - .map(|position| (text_area, position)); - } - } - if after_area != Rect::default() && (!unified || diff.focus != Focus::Before) { - let text_area = draw_diff_pane( - frame, - diff, - file, - after_area, - false, - [&syntax.before, &syntax.after], - [ - editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::Before)), - editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::After)), - ], - ( - top, - &mut after_left, - previous_text_widths.map(|widths| widths[1]), - ), - wrapped, - panels.diff_colors, - editor.is_some_and(Editor::highlight_trailing_whitespace), - editor.map(Editor::whitespace_characters), - editor.map_or(WhitespaceScope::All, Editor::whitespace_scope), - resolver, - false, - ); - text_widths[1] = text_area.width; - if let Some(editor) = - editor.filter(|editor| editor.diff_buffer_focus() == Some(Focus::After)) - { - document_caret = paint_diff_editor( - frame, - editor, - diff.column_view(Focus::After).unwrap(), - text_area, - (top, after_left), - wrapped, - ) - .map(|position| (text_area, position)); - } - } - } - } else { - let content = diff_pane_panel(frame, diff, selected, panes, true, resolver); - frame.render_widget( - Paragraph::new(if diff.loading { - "Loading file…" - } else if selected.is_some() { - "File details unavailable · select the file to retry" + if let Some(file) = selected.filter(|file| file.loaded) + && let Some(syntax) = &self.diff_syntax + { + if pane_layout == PaneLayout::Empty { + diff_pane_panel(frame, diff, Some(file), before_area, true, resolver); } else { - "No changed files" - }), - content, - ); - } - if let (Some(file), Some(syntax), Some(viewport)) = ( - selected.filter(|file| file.loaded), - panels.diff_syntax.as_ref(), - panels.diff_wrap.as_ref(), - ) { - draw_diff_scope_labels( - frame, - diff, - file, - syntax, - viewport, - [before_area, after_area], - [before_left, after_left], - &mut document_caret, - ); - } - panels.diff = Some(DiffGeometry { - regions: [ - (navigator.unwrap_or_default(), Focus::Files), - ( - if unified && diff.focus != Focus::Before { - Rect::default() + if before_area != Rect::default() && (!unified || diff.focus == Focus::Before) { + let text_area = draw_diff_pane( + frame, + diff, + file, + before_area, + true, + [&syntax.before, &syntax.after], + [ + editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::Before)), + editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::After)), + ], + ( + top, + &mut before_left, + previous_text_widths.map(|widths| widths[0]), + ), + wrapped, + self.diff_colors, + editor.is_some_and(Editor::highlight_trailing_whitespace), + editor.map(Editor::whitespace_characters), + editor.map_or(WhitespaceScope::All, Editor::whitespace_scope), + resolver, + false, + ); + text_widths[0] = text_area.width; + if let Some(editor) = + editor.filter(|editor| editor.diff_buffer_focus() == Some(Focus::Before)) + { + document_caret = paint_diff_editor( + frame, + editor, + diff.column_view(Focus::Before).unwrap(), + text_area, + (top, before_left), + wrapped, + ) + .map(|position| (text_area, position)); + } + } + if after_area != Rect::default() && (!unified || diff.focus != Focus::Before) { + let text_area = draw_diff_pane( + frame, + diff, + file, + after_area, + false, + [&syntax.before, &syntax.after], + [ + editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::Before)), + editor.map_or(&[], |editor| editor.diff_semantic_tokens(Focus::After)), + ], + ( + top, + &mut after_left, + previous_text_widths.map(|widths| widths[1]), + ), + wrapped, + self.diff_colors, + editor.is_some_and(Editor::highlight_trailing_whitespace), + editor.map(Editor::whitespace_characters), + editor.map_or(WhitespaceScope::All, Editor::whitespace_scope), + resolver, + false, + ); + text_widths[1] = text_area.width; + if let Some(editor) = + editor.filter(|editor| editor.diff_buffer_focus() == Some(Focus::After)) + { + document_caret = paint_diff_editor( + frame, + editor, + diff.column_view(Focus::After).unwrap(), + text_area, + (top, after_left), + wrapped, + ) + .map(|position| (text_area, position)); + } + } + } + } else { + let content = diff_pane_panel(frame, diff, selected, panes, true, resolver); + frame.render_widget( + Paragraph::new(if diff.loading { + "Loading file…" + } else if selected.is_some() { + "File details unavailable · select the file to retry" } else { - before_area - }, - Focus::Before, - ), - ( - if unified && diff.focus == Focus::Before { - Rect::default() - } else { - after_area - }, - Focus::After, - ), - ], - viewport: diff - .comparison_id - .map(|id| (id, diff.file, top, before_left, after_left)), - text_widths, - wrapping, - view_mode: diff.view_mode, - split_axis: diff.split_axis, - caret_row: document_caret - .map(|(text_area, caret)| usize::from(caret.y.saturating_sub(text_area.y))), - }); - if let Some(editor) = editor { - draw_diff_popups(frame, editor, area, document_caret, panels); + "No changed files" + }), + content, + ); + } + if let (Some(file), Some(syntax), Some(viewport)) = ( + selected.filter(|file| file.loaded), + self.diff_syntax.as_ref(), + self.diff_wrap.as_ref(), + ) { + draw_diff_scope_labels( + frame, + diff, + file, + syntax, + viewport, + [before_area, after_area], + [before_left, after_left], + &mut document_caret, + ); + } + self.diff = Some(DiffGeometry { + regions: [ + (navigator.unwrap_or_default(), Focus::Files), + ( + if unified && diff.focus != Focus::Before { + Rect::default() + } else { + before_area + }, + Focus::Before, + ), + ( + if unified && diff.focus == Focus::Before { + Rect::default() + } else { + after_area + }, + Focus::After, + ), + ], + viewport: diff + .comparison_id + .map(|id| (id, diff.file, top, before_left, after_left)), + text_widths, + wrapping, + view_mode: diff.view_mode, + split_axis: diff.split_axis, + caret_row: document_caret + .map(|(text_area, caret)| usize::from(caret.y.saturating_sub(text_area.y))), + }); + document_caret } }