From 59a61a26d9b462556a42bc4d294e488d9e6b96f4 Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Mon, 4 Mar 2024 13:19:23 +0900
Subject: [PATCH] cli: reuse commit summary template and formatter in a loop

The performance wouldn't matter, but the new code doesn't look bad either.
---
 cli/src/cli_util.rs           | 31 +++++++++++++++++++------------
 cli/src/commands/abandon.rs   | 11 ++++++-----
 cli/src/commands/next.rs      |  3 ++-
 cli/src/commands/status.rs    |  7 +++----
 cli/src/commands/workspace.rs |  8 +++++---
 5 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs
index c654bc1a7c..ea6de91831 100644
--- a/cli/src/cli_util.rs
+++ b/cli/src/cli_util.rs
@@ -906,12 +906,16 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
         self.parse_template(&language, template_text)
     }
 
-    fn commit_summary_template(&self) -> Box<dyn Template<Commit> + '_> {
+    /// Template for one-line summary of a commit.
+    pub fn commit_summary_template(&self) -> Box<dyn Template<Commit> + '_> {
         self.parse_commit_template(&self.commit_summary_template_text)
             .expect("parse error should be confined by WorkspaceCommandHelper::new()")
     }
 
     /// Returns one-line summary of the given `commit`.
+    ///
+    /// Use `write_commit_summary()` to get colorized output. Use
+    /// `commit_summary_template()` if you have many commits to process.
     pub fn format_commit_summary(&self, commit: &Commit) -> String {
         let mut output = Vec::new();
         self.write_commit_summary(&mut PlainTextFormatter::new(&mut output), commit)
@@ -920,6 +924,8 @@ Set which revision the branch points to with `jj branch set {branch_name} -r <RE
     }
 
     /// Writes one-line summary of the given `commit`.
+    ///
+    /// Use `commit_summary_template()` if you have many commits to process.
     #[instrument(skip_all)]
     pub fn write_commit_summary(
         &self,
@@ -1098,16 +1104,16 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
             new_commit,
         )?;
         if Some(new_commit) != maybe_old_commit {
-            write!(ui.stderr(), "Working copy now at: ")?;
-            ui.stderr_formatter().with_label("working_copy", |fmt| {
-                self.write_commit_summary(fmt, new_commit)
-            })?;
-            writeln!(ui.stderr())?;
+            let mut formatter = ui.stderr_formatter();
+            let template = self.commit_summary_template();
+            write!(formatter, "Working copy now at: ")?;
+            formatter.with_label("working_copy", |fmt| template.format(new_commit, fmt))?;
+            writeln!(formatter)?;
             for parent in new_commit.parents() {
-                //                  "Working copy now at: "
-                write!(ui.stderr(), "Parent commit      : ")?;
-                self.write_commit_summary(ui.stderr_formatter().as_mut(), &parent)?;
-                writeln!(ui.stderr())?;
+                //                "Working copy now at: "
+                write!(formatter, "Parent commit      : ")?;
+                template.format(&parent, formatter.as_mut())?;
+                writeln!(formatter)?;
             }
         }
         if let Some(stats) = stats {
@@ -1231,6 +1237,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
 
         // TODO: Also report new divergence and maybe resolved divergence
         let mut fmt = ui.stderr_formatter();
+        let template = self.commit_summary_template();
         if !resolved_conflicts_by_change_id.is_empty() {
             writeln!(
                 fmt,
@@ -1242,7 +1249,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
                 // repo, which isn't currently supported by Google's revset engine.
                 for commit in old_commits {
                     write!(fmt, "  ")?;
-                    self.write_commit_summary(fmt.as_mut(), commit)?;
+                    template.format(commit, fmt.as_mut())?;
                     writeln!(fmt)?;
                 }
             }
@@ -1252,7 +1259,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
             for (_, new_commits) in &new_conflicts_by_change_id {
                 for commit in new_commits {
                     write!(fmt, "  ")?;
-                    self.write_commit_summary(fmt.as_mut(), commit)?;
+                    template.format(commit, fmt.as_mut())?;
                     writeln!(fmt)?;
                 }
             }
diff --git a/cli/src/commands/abandon.rs b/cli/src/commands/abandon.rs
index ee76c0d7db..d795e3de20 100644
--- a/cli/src/commands/abandon.rs
+++ b/cli/src/commands/abandon.rs
@@ -63,12 +63,13 @@ pub(crate) fn cmd_abandon(
             .write_commit_summary(ui.stderr_formatter().as_mut(), &to_abandon[0])?;
         writeln!(ui.stderr())?;
     } else if !args.summary {
-        writeln!(ui.stderr(), "Abandoned the following commits:")?;
+        let mut formatter = ui.stderr_formatter();
+        let template = tx.base_workspace_helper().commit_summary_template();
+        writeln!(formatter, "Abandoned the following commits:")?;
         for commit in &to_abandon {
-            write!(ui.stderr(), "  ")?;
-            tx.base_workspace_helper()
-                .write_commit_summary(ui.stderr_formatter().as_mut(), commit)?;
-            writeln!(ui.stderr())?;
+            write!(formatter, "  ")?;
+            template.format(commit, formatter.as_mut())?;
+            writeln!(formatter)?;
         }
     } else {
         writeln!(ui.stderr(), "Abandoned {} commits.", &to_abandon.len())?;
diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs
index d1193e4ee6..520f2fc697 100644
--- a/cli/src/commands/next.rs
+++ b/cli/src/commands/next.rs
@@ -73,10 +73,11 @@ pub fn choose_commit<'a>(
 ) -> Result<&'a Commit, CommandError> {
     writeln!(ui.stdout(), "ambiguous {cmd} commit, choose one to target:")?;
     let mut formatter = ui.stdout_formatter();
+    let template = workspace_command.commit_summary_template();
     let mut choices: Vec<String> = Default::default();
     for (i, commit) in commits.iter().enumerate() {
         write!(formatter, "{}: ", i + 1)?;
-        workspace_command.write_commit_summary(formatter.as_mut(), commit)?;
+        template.format(commit, formatter.as_mut())?;
         writeln!(formatter)?;
         choices.push(format!("{}", i + 1));
     }
diff --git a/cli/src/commands/status.rs b/cli/src/commands/status.rs
index 07fa3224c0..163d4db7d0 100644
--- a/cli/src/commands/status.rs
+++ b/cli/src/commands/status.rs
@@ -75,14 +75,13 @@ pub(crate) fn cmd_status(
             resolve::print_conflicted_paths(&conflicts, formatter, &workspace_command)?
         }
 
+        let template = workspace_command.commit_summary_template();
         formatter.write_str("Working copy : ")?;
-        formatter.with_label("working_copy", |fmt| {
-            workspace_command.write_commit_summary(fmt, wc_commit)
-        })?;
+        formatter.with_label("working_copy", |fmt| template.format(wc_commit, fmt))?;
         formatter.write_str("\n")?;
         for parent in wc_commit.parents() {
             formatter.write_str("Parent commit: ")?;
-            workspace_command.write_commit_summary(formatter, &parent)?;
+            template.format(&parent, formatter)?;
             formatter.write_str("\n")?;
         }
     } else {
diff --git a/cli/src/commands/workspace.rs b/cli/src/commands/workspace.rs
index 3701df5a8f..b90793564a 100644
--- a/cli/src/commands/workspace.rs
+++ b/cli/src/commands/workspace.rs
@@ -275,11 +275,13 @@ fn cmd_workspace_list(
 ) -> Result<(), CommandError> {
     let workspace_command = command.workspace_helper(ui)?;
     let repo = workspace_command.repo();
+    let mut formatter = ui.stdout_formatter();
+    let template = workspace_command.commit_summary_template();
     for (workspace_id, wc_commit_id) in repo.view().wc_commit_ids().iter().sorted() {
-        write!(ui.stdout(), "{}: ", workspace_id.as_str())?;
+        write!(formatter, "{}: ", workspace_id.as_str())?;
         let commit = repo.store().get_commit(wc_commit_id)?;
-        workspace_command.write_commit_summary(ui.stdout_formatter().as_mut(), &commit)?;
-        writeln!(ui.stdout())?;
+        template.format(&commit, formatter.as_mut())?;
+        writeln!(formatter)?;
     }
     Ok(())
 }