diff_util: Move diff renderer format length check to constructor

cli/src/cli_util.rs

@@ -1010,6 +1010,19 @@ impl WorkspaceCommandEnvironment {
     pub fn operation_template_extensions(&self) -> &[Arc<dyn OperationTemplateLanguageExtension>] {
         &self.command.data.operation_template_extensions
     }
+
+    pub fn maybe_diff_renderer<'a>(
+        &'a self,
+        repo: &'a dyn Repo,
+        formats: Vec<DiffFormat>,
+    ) -> Option<DiffRenderer<'a>> {
+        DiffRenderer::new_if_non_empty(
+            repo,
+            formats,
+            &self.path_converter,
+            self.conflict_marker_style,
+        )
+    }
 }
 
 /// Provides utilities for writing a command that works on a [`Workspace`]
@@ -1464,23 +1477,11 @@ to the current parents may contain changes from multiple commits.
         Ok(git_ignores)
     }
 
-    /// Creates textual diff renderer of the specified `formats`.
-    pub fn diff_renderer(&self, formats: Vec<DiffFormat>) -> DiffRenderer<'_> {
-        DiffRenderer::new(
-            self.repo().as_ref(),
-            self.path_converter(),
-            self.env.conflict_marker_style(),
-            formats,
-        )
-    }
-
-    /// Loads textual diff renderer from the settings and command arguments.
-    pub fn diff_renderer_for(
-        &self,
-        args: &DiffFormatArgs,
-    ) -> Result<DiffRenderer<'_>, CommandError> {
-        let formats = diff_util::diff_formats_for(self.settings(), args)?;
-        Ok(self.diff_renderer(formats))
+    /// Creates textual diff renderer for a single `format`.
+    pub fn diff_renderer(&self, format: DiffFormat) -> DiffRenderer<'_> {
+        self.env
+            .maybe_diff_renderer(self.repo().as_ref(), vec![format])
+            .unwrap()
     }
 
     /// Loads textual diff renderer from the settings and log-like command
@@ -1492,7 +1493,20 @@ to the current parents may contain changes from multiple commits.
         patch: bool,
     ) -> Result<Option<DiffRenderer<'_>>, CommandError> {
         let formats = diff_util::diff_formats_for_log(self.settings(), args, patch)?;
-        Ok((!formats.is_empty()).then(|| self.diff_renderer(formats)))
+        Ok(self.env.maybe_diff_renderer(self.repo().as_ref(), formats))
+    }
+
+    /// Loads textual diff renderer from the repo and command arguments.
+    pub fn diff_renderer_for(
+        &self,
+        args: &DiffFormatArgs,
+    ) -> Result<DiffRenderer<'_>, CommandError> {
+        let formats = diff_util::diff_formats_for(self.settings(), args)?;
+        // `diff_formats_for` sets a default value if args were empty.
+        Ok(self
+            .env
+            .maybe_diff_renderer(self.repo().as_ref(), formats)
+            .unwrap())
     }
 
     /// Loads diff editor from the settings.

cli/src/commands/absorb.rs

@@ -131,7 +131,7 @@ pub(crate) fn cmd_absorb(
             let repo = workspace_command.repo().as_ref();
             if !commit.is_empty(repo)? {
                 writeln!(formatter, "Remaining changes:")?;
-                let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]);
+                let diff_renderer = workspace_command.diff_renderer(DiffFormat::Summary);
                 let matcher = &EverythingMatcher; // also print excluded paths
                 let width = ui.term_width();
                 diff_renderer.show_patch(ui, formatter.as_mut(), commit, matcher, width)?;

cli/src/commands/operation/diff.rs

@@ -121,13 +121,8 @@ pub fn cmd_op_diff(
     tx.repo_mut().merge_index(&from_repo);
     let merged_repo = tx.repo();
 
-    let diff_renderer = {
-        let formats = diff_formats_for_log(settings, &args.diff_format, args.patch)?;
-        let path_converter = workspace_env.path_converter();
-        let conflict_marker_style = workspace_env.conflict_marker_style();
-        (!formats.is_empty())
-            .then(|| DiffRenderer::new(merged_repo, path_converter, conflict_marker_style, formats))
-    };
+    let formats = diff_formats_for_log(settings, &args.diff_format, args.patch)?;
+    let diff_renderer = workspace_env.maybe_diff_renderer(merged_repo, formats);
     let id_prefix_context = workspace_env.new_id_prefix_context();
     let commit_summary_template = {
         let language = workspace_env.commit_template_language(merged_repo, &id_prefix_context);

cli/src/commands/operation/log.rs

@@ -36,7 +36,6 @@ use crate::commit_templater::CommitTemplateLanguage;
 use crate::complete;
 use crate::diff_util::diff_formats_for_log;
 use crate::diff_util::DiffFormatArgs;
-use crate::diff_util::DiffRenderer;
 use crate::formatter::Formatter;
 use crate::graphlog::get_graphlog;
 use crate::graphlog::GraphStyle;
@@ -176,16 +175,8 @@ fn do_op_log(
                     CommitTemplateLanguage::wrap_commit,
                 )?
             };
-            let path_converter = workspace_env.path_converter();
-            let conflict_marker_style = workspace_env.conflict_marker_style();
-            let diff_renderer = (!diff_formats.is_empty()).then(|| {
-                DiffRenderer::new(
-                    repo.as_ref(),
-                    path_converter,
-                    conflict_marker_style,
-                    diff_formats.clone(),
-                )
-            });
+            let diff_renderer =
+                workspace_env.maybe_diff_renderer(repo.as_ref(), diff_formats.clone());
 
             show_op_diff(
                 ui,

cli/src/commands/operation/show.rs

@@ -23,7 +23,6 @@ use crate::commit_templater::CommitTemplateLanguage;
 use crate::complete;
 use crate::diff_util::diff_formats_for_log;
 use crate::diff_util::DiffFormatArgs;
-use crate::diff_util::DiffRenderer;
 use crate::graphlog::GraphStyle;
 use crate::ui::Ui;
 
@@ -71,19 +70,8 @@ pub fn cmd_op_show(
 
     let graph_style = GraphStyle::from_settings(settings)?;
     let with_content_format = LogContentFormat::new(ui, settings)?;
-    let diff_renderer = {
-        let formats = diff_formats_for_log(settings, &args.diff_format, args.patch)?;
-        let path_converter = workspace_env.path_converter();
-        let conflict_marker_style = workspace_env.conflict_marker_style();
-        (!formats.is_empty()).then(|| {
-            DiffRenderer::new(
-                repo.as_ref(),
-                path_converter,
-                conflict_marker_style,
-                formats,
-            )
-        })
-    };
+    let formats = diff_formats_for_log(settings, &args.diff_format, args.patch)?;
+    let diff_renderer = workspace_env.maybe_diff_renderer(repo.as_ref(), formats);
 
     // TODO: Should we make this customizable via clap arg?
     let template = {

cli/src/commands/status.rs

@@ -87,7 +87,7 @@ pub(crate) fn cmd_status(
                     let records = get_copy_records(repo.store(), parent, wc_commit.id(), &matcher)?;
                     copy_records.add_records(records)?;
                 }
-                let diff_renderer = workspace_command.diff_renderer(vec![DiffFormat::Summary]);
+                let diff_renderer = workspace_command.diff_renderer(DiffFormat::Summary);
                 let width = ui.term_width();
                 diff_renderer.show_diff(
                     ui,

cli/src/diff_util.rs

@@ -287,23 +287,28 @@ pub enum DiffRenderError {
 /// Configuration and environment to render textual diff.
 pub struct DiffRenderer<'a> {
     repo: &'a dyn Repo,
+    formats: Vec<DiffFormat>,
     path_converter: &'a RepoPathUiConverter,
     conflict_marker_style: ConflictMarkerStyle,
-    formats: Vec<DiffFormat>,
 }
 
 impl<'a> DiffRenderer<'a> {
-    pub fn new(
+    /// Create a new textual diff renderer if the formats vector is non-empty.
+    pub fn new_if_non_empty(
         repo: &'a dyn Repo,
+        formats: Vec<DiffFormat>,
         path_converter: &'a RepoPathUiConverter,
         conflict_marker_style: ConflictMarkerStyle,
-        formats: Vec<DiffFormat>,
-    ) -> Self {
-        DiffRenderer {
-            repo,
-            path_converter,
-            conflict_marker_style,
-            formats,
+    ) -> Option<Self> {
+        if formats.is_empty() {
+            None
+        } else {
+            Some(Self {
+                repo,
+                formats,
+                path_converter,
+                conflict_marker_style,
+            })
         }
     }