refactor: encapsulate diff document drawing on WheelPanels

2026-10-16 18:41:24 +02:00
parent befa432095
commit 66250d6c40
2 changed files with 385 additions and 360 deletions
@@ -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.
@@ -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,6 +3643,40 @@ fn draw_diff(
            comparison.map(|comparison| (comparison.files.as_slice(), &diff.reviewed_files)),
        );
    }
+    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<Rect>,
+        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
@@ -3656,7 +3684,7 @@ fn draw_diff(
                .is_some_and(|file| !file.text)
        }) {
            draw_resolver_choices(frame, resolver, panes);
-        panels.diff = Some(DiffGeometry {
+            self.diff = Some(DiffGeometry {
                regions: [
                    (navigator.unwrap_or_default(), Focus::Files),
                    (Rect::default(), Focus::Before),
@@ -3669,10 +3697,7 @@ fn draw_diff(
                split_axis: diff.split_axis,
                caret_row: None,
            });
-        if let Some(editor) = editor {
-            draw_diff_popups(frame, editor, area, None, panels);
-        }
-        return;
+            return None;
        }
        let pane_layout = diff.pane_layout();
        let unified = diff.view_mode == ViewMode::Unified;
@@ -3680,7 +3705,9 @@ fn draw_diff(
            [panes, panes]
        } else {
            match pane_layout {
-            PaneLayout::Split => split_rect(pane_rect(panes), diff.split_axis).map(terminal_rect),
+                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],
            }
@@ -3689,8 +3716,7 @@ fn draw_diff(
        if let Some(id) = diff.comparison_id
            && let Some(file) = selected.filter(|file| file.loaded)
        {
-        panels
-            .diff_syntax
+            self.diff_syntax
                .get_or_insert_with(DiffSyntaxCache::default)
                .update(
                    id,
@@ -3711,7 +3737,7 @@ fn draw_diff(
        let mut before_left = diff.left_before;
        let mut after_left = diff.left_after;
        let wrapping = diff.word_wrap;
-    let toggle_row = panels
+        let toggle_row = self
            .previous_diff
            .as_ref()
            .filter(|previous| {
@@ -3764,11 +3790,12 @@ fn draw_diff(
                || 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);
+            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 Some(syntax) = &self.diff_syntax
        {
            let caret = editor
                .filter(|editor| editor.diff_interacting() && diff.follow_caret)
@@ -3777,9 +3804,7 @@ fn draw_diff(
                        .diff_buffer_focus()
                        .map(|focus| (focus, editor.cursor()))
                });
-        let viewport = panels
-            .diff_wrap
-            .get_or_insert_with(DiffWrapViewport::default);
+            let viewport = self.diff_wrap.get_or_insert_with(DiffWrapViewport::default);
            viewport.lefts = [before_left, after_left];
            viewport.update(
                diff,
@@ -3822,17 +3847,16 @@ fn draw_diff(
            }
            top = viewport.top.display;
        } else {
-        panels.diff_wrap = None;
+            self.diff_wrap = None;
        }
        if let Some(id) = diff.comparison_id
            && let Some(file) = selected.filter(|file| file.loaded)
        {
-        let wrapped = panels
+            let wrapped = self
                .diff_wrap
                .as_ref()
                .map(|viewport| viewport.rows.as_slice());
-        panels
-            .diff_syntax
+            self.diff_syntax
                .get_or_insert_with(DiffSyntaxCache::default)
                .update(
                    id,
@@ -3849,7 +3873,7 @@ fn draw_diff(
                );
        }
        let mut text_widths = [0; 2];
-    let previous_text_widths = panels
+        let previous_text_widths = self
            .previous_diff
            .as_ref()
            .filter(|geometry| {
@@ -3858,12 +3882,12 @@ fn draw_diff(
                })
            })
            .map(|geometry| geometry.text_widths);
-    let wrapped = panels
+        let wrapped = self
            .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
+            && let Some(syntax) = &self.diff_syntax
        {
            if pane_layout == PaneLayout::Empty {
                diff_pane_panel(frame, diff, Some(file), before_area, true, resolver);
@@ -3886,7 +3910,7 @@ fn draw_diff(
                            previous_text_widths.map(|widths| widths[0]),
                        ),
                        wrapped,
-                    panels.diff_colors,
+                        self.diff_colors,
                        editor.is_some_and(Editor::highlight_trailing_whitespace),
                        editor.map(Editor::whitespace_characters),
                        editor.map_or(WhitespaceScope::All, Editor::whitespace_scope),
@@ -3926,7 +3950,7 @@ fn draw_diff(
                            previous_text_widths.map(|widths| widths[1]),
                        ),
                        wrapped,
-                    panels.diff_colors,
+                        self.diff_colors,
                        editor.is_some_and(Editor::highlight_trailing_whitespace),
                        editor.map(Editor::whitespace_characters),
                        editor.map_or(WhitespaceScope::All, Editor::whitespace_scope),
@@ -3964,8 +3988,8 @@ fn draw_diff(
        }
        if let (Some(file), Some(syntax), Some(viewport)) = (
            selected.filter(|file| file.loaded),
-        panels.diff_syntax.as_ref(),
-        panels.diff_wrap.as_ref(),
+            self.diff_syntax.as_ref(),
+            self.diff_wrap.as_ref(),
        ) {
            draw_diff_scope_labels(
                frame,
@@ -3978,7 +4002,7 @@ fn draw_diff(
                &mut document_caret,
            );
        }
-    panels.diff = Some(DiffGeometry {
+        self.diff = Some(DiffGeometry {
            regions: [
                (navigator.unwrap_or_default(), Focus::Files),
                (
@@ -4008,8 +4032,7 @@ fn draw_diff(
            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);
+        document_caret
    }
 }