fix: avoid rendering patch previews every frame

[Context and Fix]

Patch previews can be rendered by external renderers, like `bat`,
`delta`, and `diff-so-fancy`. However, this rendering is done for every
frame and includes processing the whole patch being previewed through
the renderer, then calling `into_text()` (from `ansi_to_tui` crate).
Even though this whole process isn't too costly, making it at every
frame results in ugly slowdowns for not so big patches (around 2000
lines after rendering).

To fix this, move the whole rendering process to when we first
instantiate a `PatchsetDetailsAndActions`, i.e., when consulting
a patchset.

[Collateral Effects]

This basically solves the problem, however, there is still minor
lag, due to ratatui `Paragraph` getting the whole file, and, for big
series, there is a bit a loading when opening patchsets. Also, this
does a bit of coupling of the model (`App`) with the view (`ratatui`).

On the flipside, this collaterally fixes problems of wrong scroll
calculations that happened because they considered the raw patches, not
the rendered ones.

Closes: #77

Signed-off-by: David Tadokoro <davidbtadokoro@usp.br>

src/app.rs

@@ -1,9 +1,11 @@
 use crate::log_on_error;
+use ansi_to_tui::IntoText;
 use color_eyre::eyre::bail;
 use config::Config;
 use logging::Logger;
 use patch_hub::lore::{lore_api_client::BlockingLoreAPIClient, lore_session, patch::Patch};
-use patch_renderer::PatchRenderer;
+use patch_renderer::{render_patch_preview, PatchRenderer};
+use ratatui::text::Text;
 use screens::{
     bookmarked::BookmarkedPatchsetsState,
     details_actions::{PatchsetAction, PatchsetDetailsAndActionsState},
@@ -134,10 +136,27 @@ impl App {
         };
 
         match log_on_error!(lore_session::split_patchset(&patchset_path)) {
-            Ok(patches) => {
+            Ok(raw_patches) => {
+                let mut patches_preview: Vec<Text> = Vec::new();
+                for raw_patch in &raw_patches {
+                    let raw_patch = raw_patch.replace('\t', "        ");
+                    let patch_preview =
+                        match render_patch_preview(&raw_patch, self.config.patch_renderer()) {
+                            Ok(render) => render,
+                            Err(_) => {
+                                Logger::error(
+                                    "Failed to render patch preview with external program",
+                                );
+                                raw_patch
+                            }
+                        }
+                        .into_text()?;
+                    patches_preview.push(patch_preview);
+                }
                 self.patchset_details_and_actions_state = Some(PatchsetDetailsAndActionsState {
                     representative_patch,
-                    patches,
+                    raw_patches,
+                    patches_preview,
                     preview_index: 0,
                     preview_scroll_offset: 0,
                     preview_pan: 0,

src/app/patch_renderer.rs

@@ -1,7 +1,13 @@
-use std::fmt::Display;
+use std::{
+    fmt::Display,
+    io::Write,
+    process::{Command, Stdio},
+};
 
 use serde::{Deserialize, Serialize};
 
+use super::logging::Logger;
+
 #[derive(Debug, Serialize, Deserialize, Clone, Copy, Default)]
 pub enum PatchRenderer {
     #[default]
@@ -47,3 +53,141 @@ impl Display for PatchRenderer {
         }
     }
 }
+
+pub fn render_patch_preview(raw: &str, renderer: &PatchRenderer) -> color_eyre::Result<String> {
+    let text = match renderer {
+        PatchRenderer::Default => Ok(raw.to_string()),
+        PatchRenderer::Bat => bat_patch_renderer(raw),
+        PatchRenderer::Delta => delta_patch_renderer(raw),
+        PatchRenderer::DiffSoFancy => diff_so_fancy_renderer(raw),
+    }?;
+
+    Ok(text)
+}
+
+/// Renders a patch using the `bat` command line tool.
+///
+/// # Errors
+///
+/// If bat isn't installed or if the command fails, an error will be returned.
+fn bat_patch_renderer(patch: &str) -> color_eyre::Result<String> {
+    let mut bat = Command::new("bat")
+        .arg("-pp")
+        .arg("-f")
+        .arg("-l")
+        .arg("patch")
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .spawn()
+        .map_err(|e| {
+            Logger::error(format!("Failed to spawn bat for patch preview: {}", e));
+            e
+        })?;
+
+    bat.stdin.as_mut().unwrap().write_all(patch.as_bytes())?;
+    let output = bat.wait_with_output()?;
+    Ok(String::from_utf8(output.stdout)?)
+}
+
+/// Renders a patch using the `delta` command line tool.
+///
+/// # Errors
+///
+/// If delta isn't installed or if the command fails, an error will be returned.
+fn delta_patch_renderer(patch: &str) -> color_eyre::Result<String> {
+    let mut delta = Command::new("delta")
+        .arg("--pager")
+        .arg("less")
+        .arg("--paging")
+        .arg("never")
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .spawn()
+        .map_err(|e| {
+            Logger::error(format!("Failed to spawn delta for patch preview: {}", e));
+            e
+        })?;
+
+    delta.stdin.as_mut().unwrap().write_all(patch.as_bytes())?;
+    let output = delta.wait_with_output()?;
+    Ok(String::from_utf8(output.stdout)?)
+}
+
+/// Renders a patch using the `diff-so-fancy` command line tool.
+///
+/// # Errors
+///
+/// If diff-so-fancy isn't installed or if the command fails, an error will be returned.
+fn diff_so_fancy_renderer(patch: &str) -> color_eyre::Result<String> {
+    let mut dsf = Command::new("diff-so-fancy")
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .spawn()
+        .map_err(|e| {
+            Logger::error(format!(
+                "Failed to spawn diff-so-fancy for patch preview: {}",
+                e
+            ));
+            e
+        })?;
+
+    dsf.stdin.as_mut().unwrap().write_all(patch.as_bytes())?;
+    let output = dsf.wait_with_output()?;
+    Ok(String::from_utf8(output.stdout)?)
+}
+
+#[cfg(test)]
+mod tests {
+    use std::fs;
+
+    use super::*;
+
+    const PATCH_SAMPLE: &str = "diff --git a/file.txt b/file.txt
+index 83db48f..e3b0c44 100644
+--- a/file.txt
++++ b/file.txt
+@@ -1 +1 @@
+-Hello, world!
++Hello, Rust!
+";
+
+    #[test]
+    #[ignore = "optional-dependency"]
+    fn test_bat_patch_renderer() {
+        let result = bat_patch_renderer(PATCH_SAMPLE);
+        assert!(result.is_ok());
+        let rendered_patch = result.unwrap();
+        assert_eq!(
+            fs::read_to_string("src/test_samples/ui/render_patchset/expected_bat.diff").unwrap(),
+            rendered_patch,
+            "Wrong rendering of bat"
+        );
+    }
+
+    #[test]
+    #[ignore = "optional-dependency"]
+    fn test_delta_patch_renderer() {
+        let result = delta_patch_renderer(PATCH_SAMPLE);
+        assert!(result.is_ok());
+        let rendered_patch = result.unwrap();
+        assert_eq!(
+            fs::read_to_string("src/test_samples/ui/render_patchset/expected_delta.diff").unwrap(),
+            rendered_patch,
+            "Wrong rendering of delta"
+        );
+    }
+
+    #[test]
+    #[ignore = "optional-dependency"]
+    fn test_diff_so_fancy_renderer() {
+        let result = diff_so_fancy_renderer(PATCH_SAMPLE);
+        assert!(result.is_ok());
+        let rendered_patch = result.unwrap();
+        assert_eq!(
+            fs::read_to_string("src/test_samples/ui/render_patchset/expected_diff-so-fancy.diff")
+                .unwrap(),
+            rendered_patch,
+            "Wrong rendering of diff-so-fancy"
+        );
+    }
+}

src/app/screens/details_actions.rs

@@ -1,11 +1,15 @@
 use super::CurrentScreen;
 use ::patch_hub::lore::{lore_api_client::BlockingLoreAPIClient, lore_session, patch::Patch};
 use color_eyre::eyre::bail;
+use ratatui::text::Text;
 use std::{collections::HashMap, path::Path, process::Command};
 
 pub struct PatchsetDetailsAndActionsState {
     pub representative_patch: Patch,
-    pub patches: Vec<String>,
+    /// Raw patches as plain text files
+    pub raw_patches: Vec<String>,
+    /// Patches in the format to be displayed as preview
+    pub patches_preview: Vec<Text<'static>>,
     pub preview_index: usize,
     pub preview_scroll_offset: usize,
     /// Horizontal offset
@@ -27,7 +31,7 @@ pub enum PatchsetAction {
 
 impl PatchsetDetailsAndActionsState {
     pub fn preview_next_patch(&mut self) {
-        if (self.preview_index + 1) < self.patches.len() {
+        if (self.preview_index + 1) < self.patches_preview.len() {
             self.preview_index += 1;
             self.preview_scroll_offset = 0;
             self.preview_pan = 0;
@@ -45,7 +49,7 @@ impl PatchsetDetailsAndActionsState {
     /// Scroll `n` lines down
     pub fn preview_scroll_down(&mut self, n: usize) {
         // TODO: Support for renderers (only considers base preview string)
-        let number_of_lines = self.patches[self.preview_index].lines().count();
+        let number_of_lines = self.patches_preview[self.preview_index].height();
         if (self.preview_scroll_offset + n) <= number_of_lines {
             self.preview_scroll_offset += n;
         }
@@ -59,7 +63,7 @@ impl PatchsetDetailsAndActionsState {
     /// Scroll to the last line
     pub fn go_to_last_line(&mut self) {
         // TODO: Support for renderers (only considers base preview string)
-        let number_of_lines = self.patches[self.preview_index].lines().count();
+        let number_of_lines = self.patches_preview[self.preview_index].height();
         self.preview_scroll_offset = number_of_lines - LAST_LINE_PADDING;
     }
 
@@ -133,7 +137,7 @@ impl PatchsetDetailsAndActionsState {
             &self.lore_api_client,
             tmp_dir,
             target_list,
-            &self.patches,
+            &self.raw_patches,
             &format!("{git_user_name} <{git_user_email}>"),
             git_send_email_options,
         ) {

src/ui.rs

@@ -14,7 +14,6 @@ mod latest;
 pub mod loading_screen;
 mod mail_list;
 mod navigation_bar;
-mod render_patchset;
 
 pub fn draw_ui(f: &mut Frame, app: &App) {
     let chunks = Layout::default()

src/ui/details_actions.rs

@@ -1,14 +1,12 @@
 use ratatui::{
     layout::{Constraint, Direction, Layout, Rect},
     style::{Color, Modifier, Style},
-    text::{Line, Span, Text},
+    text::{Line, Span},
     widgets::{Block, Borders, Padding, Paragraph, Wrap},
     Frame,
 };
 
-use crate::app::{logging::Logger, screens::details_actions::PatchsetAction, App};
-
-use super::render_patchset::render_patch_preview;
+use crate::app::{screens::details_actions::PatchsetAction, App};
 
 fn render_details_and_actions(f: &mut Frame, app: &App, details_chunk: Rect, actions_chunk: Rect) {
     let patchset_details_and_actions = app.patchset_details_and_actions_state.as_ref().unwrap();
@@ -132,16 +130,7 @@ fn render_preview(f: &mut Frame, app: &App, chunk: Rect) {
 
     let preview_offset = patchset_details_and_actions.preview_scroll_offset;
     let preview_pan = patchset_details_and_actions.preview_pan;
-    let patch_preview =
-        patchset_details_and_actions.patches[preview_index].replace('\t', "        ");
-    // TODO: Pass the terminal size to the external program
-    let patch_preview = match render_patch_preview(&patch_preview, app.config.patch_renderer()) {
-        Ok(rendered) => rendered,
-        Err(_) => {
-            Logger::error("Failed to render patch preview with external program");
-            Text::from(patch_preview)
-        }
-    };
+    let patch_preview = patchset_details_and_actions.patches_preview[preview_index].clone();
 
     let patch_preview = Paragraph::new(patch_preview)
         .block(