ui: make status() return a dyn Write, add status_formatter()

When the caller needs a formatter, it's because they're doing
something non-trivial. When the user passed `--quiet` (see upcoming
patch), we should ideally skip doing related work for print the
formatting output. It helps if the `Ui` object doesn't even return a
`Formatter` then, so the caller is forced to handle the quiet case
differently.

Thanks to Yuya for the suggestion.

cli/src/cli_util.rs

@@ -1147,16 +1147,17 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
             new_commit,
         )?;
         if Some(new_commit) != maybe_old_commit {
-            let mut formatter = ui.status();
-            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!(formatter, "Parent commit      : ")?;
-                template.format(&parent, formatter.as_mut())?;
+            if let Some(mut formatter) = ui.status_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!(formatter, "Parent commit      : ")?;
+                    template.format(&parent, formatter.as_mut())?;
+                    writeln!(formatter)?;
+                }
             }
         }
         if let Some(stats) = stats {
@@ -1236,6 +1237,9 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
         ui: &mut Ui,
         old_repo: &Arc<ReadonlyRepo>,
     ) -> Result<(), CommandError> {
+        let Some(mut fmt) = ui.status_formatter() else {
+            return Ok(());
+        };
         let old_view = old_repo.view();
         let new_repo = self.repo().as_ref();
         let new_view = new_repo.view();
@@ -1279,7 +1283,6 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin
             .retain(|change_id, _commits| !removed_conflicts_by_change_id.contains_key(change_id));
 
         // TODO: Also report new divergence and maybe resolved divergence
-        let mut fmt = ui.status();
         let template = self.commit_summary_template();
         if !resolved_conflicts_by_change_id.is_empty() {
             writeln!(
@@ -1642,12 +1645,12 @@ pub fn print_trackable_remote_branches(ui: &Ui, view: &View) -> io::Result<()> {
         ui.hint_default(),
         "The following remote branches aren't associated with the existing local branches:"
     )?;
-    let mut formatter = ui.status();
-    for full_name in &remote_branch_names {
-        write!(formatter, "  ")?;
-        writeln!(formatter.labeled("branch"), "{full_name}")?;
+    if let Some(mut formatter) = ui.status_formatter() {
+        for full_name in &remote_branch_names {
+            write!(formatter, "  ")?;
+            writeln!(formatter.labeled("branch"), "{full_name}")?;
+        }
     }
-    drop(formatter);
     writeln!(
         ui.hint_default(),
         "Run `jj branch track {names}` to keep local branches updated on future pulls.",

cli/src/commands/abandon.rs

@@ -66,28 +66,29 @@ pub(crate) fn cmd_abandon(
     }
     let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
 
-    if to_abandon.len() == 1 {
-        write!(ui.status(), "Abandoned commit ")?;
-        tx.base_workspace_helper()
-            .write_commit_summary(ui.status().as_mut(), &to_abandon[0])?;
-        writeln!(ui.status())?;
-    } else if !args.summary {
-        let mut formatter = ui.status();
-        let template = tx.base_workspace_helper().commit_summary_template();
-        writeln!(formatter, "Abandoned the following commits:")?;
-        for commit in &to_abandon {
-            write!(formatter, "  ")?;
-            template.format(commit, formatter.as_mut())?;
-            writeln!(formatter)?;
+    if let Some(mut formatter) = ui.status_formatter() {
+        if to_abandon.len() == 1 {
+            write!(formatter, "Abandoned commit ")?;
+            tx.base_workspace_helper()
+                .write_commit_summary(formatter.as_mut(), &to_abandon[0])?;
+            writeln!(ui.status())?;
+        } else if !args.summary {
+            let template = tx.base_workspace_helper().commit_summary_template();
+            writeln!(formatter, "Abandoned the following commits:")?;
+            for commit in &to_abandon {
+                write!(formatter, "  ")?;
+                template.format(commit, formatter.as_mut())?;
+                writeln!(formatter)?;
+            }
+        } else {
+            writeln!(formatter, "Abandoned {} commits.", &to_abandon.len())?;
+        }
+        if num_rebased > 0 {
+            writeln!(
+                formatter,
+                "Rebased {num_rebased} descendant commits onto parents of abandoned commits"
+            )?;
         }
-    } else {
-        writeln!(ui.status(), "Abandoned {} commits.", &to_abandon.len())?;
-    }
-    if num_rebased > 0 {
-        writeln!(
-            ui.status(),
-            "Rebased {num_rebased} descendant commits onto parents of abandoned commits"
-        )?;
     }
     let transaction_description = if to_abandon.len() == 1 {
         format!("abandon commit {}", to_abandon[0].id().hex())

cli/src/commands/diffedit.rs

@@ -107,11 +107,13 @@ don't make any changes, then the operation will be aborted.",
         // rebase_descendants early; otherwise `new_commit` would always have
         // a conflicted change id at this point.
         let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
-        write!(ui.status(), "Created ")?;
-        tx.write_commit_summary(ui.status().as_mut(), &new_commit)?;
-        writeln!(ui.status())?;
-        if num_rebased > 0 {
-            writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?;
+        if let Some(mut formatter) = ui.status_formatter() {
+            write!(formatter, "Created ")?;
+            tx.write_commit_summary(formatter.as_mut(), &new_commit)?;
+            writeln!(formatter)?;
+            if num_rebased > 0 {
+                writeln!(formatter, "Rebased {num_rebased} descendant commits")?;
+            }
         }
         tx.finish(ui, format!("edit commit {}", target_commit.id().hex()))?;
     }

cli/src/commands/duplicate.rs

@@ -77,10 +77,12 @@ pub(crate) fn cmd_duplicate(
         duplicated_old_to_new.insert(original_commit_id, new_commit);
     }
 
-    for (old_id, new_commit) in &duplicated_old_to_new {
-        write!(ui.status(), "Duplicated {} as ", short_commit_hash(old_id))?;
-        tx.write_commit_summary(ui.status().as_mut(), new_commit)?;
-        writeln!(ui.status())?;
+    if let Some(mut formatter) = ui.status_formatter() {
+        for (old_id, new_commit) in &duplicated_old_to_new {
+            write!(formatter, "Duplicated {} as ", short_commit_hash(old_id))?;
+            tx.write_commit_summary(formatter.as_mut(), new_commit)?;
+            writeln!(formatter)?;
+        }
     }
     tx.finish(ui, format!("duplicate {} commit(s)", to_duplicate.len()))?;
     Ok(())

cli/src/commands/new.rs

@@ -189,9 +189,11 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
     }
     num_rebased += tx.mut_repo().rebase_descendants(command.settings())?;
     if args.no_edit {
-        write!(ui.status(), "Created new commit ")?;
-        tx.write_commit_summary(ui.status().as_mut(), &new_commit)?;
-        writeln!(ui.status())?;
+        if let Some(mut formatter) = ui.status_formatter() {
+            write!(formatter, "Created new commit ")?;
+            tx.write_commit_summary(formatter.as_mut(), &new_commit)?;
+            writeln!(formatter)?;
+        }
     } else {
         tx.edit(&new_commit).unwrap();
         // The description of the new commit will be printed by tx.finish()

cli/src/commands/rebase.rs

@@ -33,6 +33,7 @@ use crate::cli_util::{
     RevisionArg, WorkspaceCommandHelper,
 };
 use crate::command_error::{user_error, CommandError};
+use crate::formatter::Formatter;
 use crate::ui::Ui;
 
 /// Move revisions to different parent(s)
@@ -295,7 +296,9 @@ fn rebase_descendants(
         .iter()
         .partition::<Vec<_>, _>(|commit| commit.parents() == new_parents);
     if !skipped_commits.is_empty() {
-        log_skipped_rebase_commits_message(ui, workspace_command, &skipped_commits)?;
+        if let Some(mut fmt) = ui.status_formatter() {
+            log_skipped_rebase_commits_message(fmt.as_mut(), workspace_command, &skipped_commits)?;
+        }
     }
     if old_commits.is_empty() {
         return Ok(());
@@ -447,9 +450,11 @@ fn rebase_revision(
     // longer have any children; they have all been rebased and the originals
     // have been abandoned.
     let skipped_commit_rebase = if old_commit.parents() == new_parents {
-        write!(ui.status(), "Skipping rebase of commit ")?;
-        tx.write_commit_summary(ui.status().as_mut(), &old_commit)?;
-        writeln!(ui.status())?;
+        if let Some(mut formatter) = ui.status_formatter() {
+            write!(formatter, "Skipping rebase of commit ")?;
+            tx.write_commit_summary(formatter.as_mut(), &old_commit)?;
+            writeln!(formatter)?;
+        }
         true
     } else {
         rebase_commit(settings, tx.mut_repo(), &old_commit, &new_parents)?;
@@ -496,21 +501,20 @@ fn check_rebase_destinations(
 }
 
 fn log_skipped_rebase_commits_message(
-    ui: &Ui,
+    fmt: &mut dyn Formatter,
     workspace_command: &WorkspaceCommandHelper,
     commits: &[&Commit],
 ) -> Result<(), CommandError> {
-    let mut fmt = ui.status();
     let template = workspace_command.commit_summary_template();
     if commits.len() == 1 {
         write!(fmt, "Skipping rebase of commit ")?;
-        template.format(commits[0], fmt.as_mut())?;
+        template.format(commits[0], fmt)?;
         writeln!(fmt)?;
     } else {
         writeln!(fmt, "Skipping rebase of commits:")?;
         for commit in commits {
             write!(fmt, "  ")?;
-            template.format(commit, fmt.as_mut())?;
+            template.format(commit, fmt)?;
             writeln!(fmt)?;
         }
     }

cli/src/commands/resolve.rs

@@ -115,15 +115,18 @@ pub(crate) fn cmd_resolve(
         format!("Resolve conflicts in commit {}", commit.id().hex()),
     )?;
 
+    // TODO: Delete local `--quiet`
     if !args.quiet {
-        let new_tree = new_commit.tree()?;
-        let new_conflicts = new_tree.conflicts().collect_vec();
-        if !new_conflicts.is_empty() {
-            writeln!(
-                ui.status(),
-                "After this operation, some files at this revision still have conflicts:"
-            )?;
-            print_conflicted_paths(&new_conflicts, ui.status().as_mut(), &workspace_command)?;
+        if let Some(mut formatter) = ui.status_formatter() {
+            let new_tree = new_commit.tree()?;
+            let new_conflicts = new_tree.conflicts().collect_vec();
+            if !new_conflicts.is_empty() {
+                writeln!(
+                    formatter,
+                    "After this operation, some files at this revision still have conflicts:"
+                )?;
+                print_conflicted_paths(&new_conflicts, formatter.as_mut(), &workspace_command)?;
+            }
         }
     };
     Ok(())

cli/src/commands/restore.rs

@@ -112,11 +112,13 @@ pub(crate) fn cmd_restore(
         // rebase_descendants early; otherwise `new_commit` would always have
         // a conflicted change id at this point.
         let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
-        write!(ui.status(), "Created ")?;
-        tx.write_commit_summary(ui.status().as_mut(), &new_commit)?;
-        writeln!(ui.status())?;
-        if num_rebased > 0 {
-            writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?;
+        if let Some(mut formatter) = ui.status_formatter() {
+            write!(formatter, "Created ")?;
+            tx.write_commit_summary(formatter.as_mut(), &new_commit)?;
+            writeln!(formatter)?;
+            if num_rebased > 0 {
+                writeln!(formatter, "Rebased {num_rebased} descendant commits")?;
+            }
         }
         tx.finish(ui, format!("restore into commit {}", to_commit.id().hex()))?;
     }

cli/src/commands/split.rs

@@ -149,14 +149,16 @@ don't make any changes, then the operation will be aborted.
     tx.mut_repo()
         .set_rewritten_commit(commit.id().clone(), second_commit.id().clone());
     let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?;
-    if num_rebased > 0 {
-        writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?;
+    if let Some(mut formatter) = ui.status_formatter() {
+        if num_rebased > 0 {
+            writeln!(formatter, "Rebased {num_rebased} descendant commits")?;
+        }
+        write!(formatter, "First part: ")?;
+        tx.write_commit_summary(formatter.as_mut(), &first_commit)?;
+        write!(formatter, "\nSecond part: ")?;
+        tx.write_commit_summary(formatter.as_mut(), &second_commit)?;
+        writeln!(formatter)?;
     }
-    write!(ui.status(), "First part: ")?;
-    tx.write_commit_summary(ui.status().as_mut(), &first_commit)?;
-    write!(ui.status(), "\nSecond part: ")?;
-    tx.write_commit_summary(ui.status().as_mut(), &second_commit)?;
-    writeln!(ui.status())?;
     tx.finish(ui, format!("split commit {}", commit.id().hex()))?;
     Ok(())
 }

cli/src/commands/workspace.rs

@@ -424,11 +424,13 @@ fn cmd_workspace_update_stale(
                     )
                 })?;
             locked_ws.finish(repo.op_id().clone())?;
-            write!(ui.status(), "Working copy now at: ")?;
-            ui.status().with_label("working_copy", |fmt| {
-                workspace_command.write_commit_summary(fmt, &desired_wc_commit)
-            })?;
-            writeln!(ui.status())?;
+            if let Some(mut formatter) = ui.status_formatter() {
+                write!(formatter, "Working copy now at: ")?;
+                formatter.with_label("working_copy", |fmt| {
+                    workspace_command.write_commit_summary(fmt, &desired_wc_commit)
+                })?;
+                writeln!(formatter)?;
+            }
             print_checkout_stats(ui, stats, &desired_wc_commit)?;
         }
     }

cli/src/git_util.rs

@@ -256,6 +256,9 @@ pub fn print_git_import_stats(
     stats: &GitImportStats,
     show_ref_stats: bool,
 ) -> Result<(), CommandError> {
+    let Some(mut formatter) = ui.status_formatter() else {
+        return Ok(());
+    };
     if show_ref_stats {
         let refs_stats = stats
             .changed_remote_refs
@@ -274,7 +277,6 @@ pub fn print_git_import_stats(
 
         let max_width = refs_stats.iter().map(|x| x.ref_name.width()).max();
         if let Some(max_width) = max_width {
-            let mut formatter = ui.status();
             for status in refs_stats {
                 status.output(max_width, has_both_ref_kinds, &mut *formatter)?;
             }
@@ -283,7 +285,7 @@ pub fn print_git_import_stats(
 
     if !stats.abandoned_commits.is_empty() {
         writeln!(
-            ui.status(),
+            formatter,
             "Abandoned {} commits that are no longer reachable.",
             stats.abandoned_commits.len()
         )?;

cli/src/ui.rs

@@ -373,10 +373,16 @@ impl Ui {
         })
     }
 
-    /// Writes a message that's a status update not part of the command's main
-    /// output.
-    pub fn status(&self) -> Box<dyn Formatter + '_> {
-        self.stderr_formatter()
+    /// Writer to print an update that's not part of the command's main output.
+    pub fn status(&self) -> Box<dyn Write + '_> {
+        Box::new(self.stderr())
+    }
+
+    /// A formatter to print an update that's not part of the command's main
+    /// output. Returns `None` if `--quiet` was requested.
+    // TODO: Actually support `--quiet`
+    pub fn status_formatter(&self) -> Option<Box<dyn Formatter + '_>> {
+        Some(self.stderr_formatter())
     }
 
     /// Writer to print hint with the default "Hint: " heading.
@@ -388,7 +394,7 @@ impl Ui {
 
     /// Writer to print hint without the "Hint: " heading.
     pub fn hint_no_heading(&self) -> LabeledWriter<Box<dyn Formatter + '_>, &'static str> {
-        LabeledWriter::new(self.status(), "hint")
+        LabeledWriter::new(self.stderr_formatter(), "hint")
     }
 
     /// Writer to print hint with the given heading.
@@ -396,7 +402,7 @@ impl Ui {
         &self,
         heading: H,
     ) -> HeadingLabeledWriter<Box<dyn Formatter + '_>, &'static str, H> {
-        HeadingLabeledWriter::new(self.status(), "hint", heading)
+        HeadingLabeledWriter::new(self.stderr_formatter(), "hint", heading)
     }
 
     /// Writer to print warning with the default "Warning: " heading.