From c1c010726293ca81a0633f7826b73c65f67b31a1 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 14 Feb 2024 05:52:56 -0800 Subject: [PATCH 1/6] cli: add a Ui::status() helper for writing non-error to stderr This clarifies that status messages are not errors, and allows us to implement a global `--quiet` flag for silencing status messages. --- cli/examples/custom-command/main.rs | 2 +- cli/src/cli_util.rs | 24 ++++++++++++------------ cli/src/commands/abandon.rs | 14 +++++++------- cli/src/commands/bench.rs | 6 +++--- cli/src/commands/branch.rs | 8 ++++---- cli/src/commands/debug.rs | 4 ++-- cli/src/commands/describe.rs | 2 +- cli/src/commands/diffedit.rs | 10 +++++----- cli/src/commands/duplicate.rs | 8 ++++---- cli/src/commands/edit.rs | 2 +- cli/src/commands/git.rs | 26 +++++++++++++------------- cli/src/commands/init.rs | 2 +- cli/src/commands/new.rs | 8 ++++---- cli/src/commands/operation.rs | 4 ++-- cli/src/commands/rebase.rs | 14 +++++++------- cli/src/commands/resolve.rs | 10 +++------- cli/src/commands/restore.rs | 10 +++++----- cli/src/commands/split.rs | 14 +++++++------- cli/src/commands/untrack.rs | 2 +- cli/src/commands/workspace.rs | 14 +++++++------- cli/src/git_util.rs | 10 +++++----- cli/src/ui.rs | 10 ++++++++-- 22 files changed, 103 insertions(+), 101 deletions(-) diff --git a/cli/examples/custom-command/main.rs b/cli/examples/custom-command/main.rs index bf8abba7dc..e7c4069285 100644 --- a/cli/examples/custom-command/main.rs +++ b/cli/examples/custom-command/main.rs @@ -48,7 +48,7 @@ fn run_custom_command( .write()?; tx.finish(ui, "Frobnicate")?; writeln!( - ui.stderr(), + ui.status(), "Frobnicated revision: {}", workspace_command.format_commit_summary(&new_commit) )?; diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 71af456307..251140c865 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -327,7 +327,7 @@ impl CommandHelper { repo_loader.op_store(), |op_heads| { writeln!( - ui.stderr(), + ui.status(), "Concurrent modification detected, resolving automatically.", )?; let base_repo = repo_loader.load_at(&op_heads[0])?; @@ -339,7 +339,7 @@ impl CommandHelper { let num_rebased = tx.mut_repo().rebase_descendants(&self.settings)?; if num_rebased > 0 { writeln!( - ui.stderr(), + ui.status(), "Rebased {num_rebased} descendant commits onto commits rewritten \ by other operation" )?; @@ -531,7 +531,7 @@ impl WorkspaceCommandHelper { locked_ws.finish(self.user_repo.repo.op_id().clone())?; if old_git_head.is_present() { writeln!( - ui.stderr(), + ui.status(), "Reset the working copy parent to the new Git HEAD." )?; } else { @@ -570,13 +570,13 @@ impl WorkspaceCommandHelper { let num_rebased = tx.mut_repo().rebase_descendants(&self.settings)?; if num_rebased > 0 { writeln!( - ui.stderr(), + ui.status(), "Rebased {num_rebased} descendant commits off of commits rewritten from git" )?; } self.finish_transaction(ui, tx, "import git refs")?; writeln!( - ui.stderr(), + ui.status(), "Done importing changes from the underlying Git repo." )?; Ok(()) @@ -1117,7 +1117,7 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin let num_rebased = mut_repo.rebase_descendants(&self.settings)?; if num_rebased > 0 { writeln!( - ui.stderr(), + ui.status(), "Rebased {num_rebased} descendant commits onto updated working copy" )?; } @@ -1147,7 +1147,7 @@ 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.stderr_formatter(); + 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))?; @@ -1177,12 +1177,12 @@ See https://github.com/martinvonz/jj/blob/main/docs/working-copy.md#stale-workin description: impl Into, ) -> Result<(), CommandError> { if !tx.mut_repo().has_changes() { - writeln!(ui.stderr(), "Nothing changed.")?; + writeln!(ui.status(), "Nothing changed.")?; return Ok(()); } let num_rebased = tx.mut_repo().rebase_descendants(&self.settings)?; if num_rebased > 0 { - writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?; + writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?; } let old_repo = tx.base_repo().clone(); @@ -1279,7 +1279,7 @@ 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.stderr_formatter(); + let mut fmt = ui.status(); let template = self.commit_summary_template(); if !resolved_conflicts_by_change_id.is_empty() { writeln!( @@ -1597,7 +1597,7 @@ pub fn print_checkout_stats( ) -> Result<(), std::io::Error> { if stats.added_files > 0 || stats.updated_files > 0 || stats.removed_files > 0 { writeln!( - ui.stderr(), + ui.status(), "Added {} files, modified {} files, removed {} files", stats.added_files, stats.updated_files, @@ -1642,7 +1642,7 @@ 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.stderr_formatter(); + let mut formatter = ui.status(); for full_name in &remote_branch_names { write!(formatter, " ")?; writeln!(formatter.labeled("branch"), "{full_name}")?; diff --git a/cli/src/commands/abandon.rs b/cli/src/commands/abandon.rs index be0cb71db9..c9c1e718bc 100644 --- a/cli/src/commands/abandon.rs +++ b/cli/src/commands/abandon.rs @@ -55,7 +55,7 @@ pub(crate) fn cmd_abandon( .evaluate_to_commits()? .try_collect()?; if to_abandon.is_empty() { - writeln!(ui.stderr(), "No revisions to abandon.")?; + writeln!(ui.status(), "No revisions to abandon.")?; return Ok(()); } workspace_command.check_rewritable(&to_abandon)?; @@ -67,12 +67,12 @@ pub(crate) fn cmd_abandon( let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?; if to_abandon.len() == 1 { - write!(ui.stderr(), "Abandoned commit ")?; + write!(ui.status(), "Abandoned commit ")?; tx.base_workspace_helper() - .write_commit_summary(ui.stderr_formatter().as_mut(), &to_abandon[0])?; - writeln!(ui.stderr())?; + .write_commit_summary(ui.status().as_mut(), &to_abandon[0])?; + writeln!(ui.status())?; } else if !args.summary { - let mut formatter = ui.stderr_formatter(); + 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 { @@ -81,11 +81,11 @@ pub(crate) fn cmd_abandon( writeln!(formatter)?; } } else { - writeln!(ui.stderr(), "Abandoned {} commits.", &to_abandon.len())?; + writeln!(ui.status(), "Abandoned {} commits.", &to_abandon.len())?; } if num_rebased > 0 { writeln!( - ui.stderr(), + ui.status(), "Rebased {num_rebased} descendant commits onto parents of abandoned commits" )?; } diff --git a/cli/src/commands/bench.rs b/cli/src/commands/bench.rs index 9503479fa7..77ebdc947d 100644 --- a/cli/src/commands/bench.rs +++ b/cli/src/commands/bench.rs @@ -116,7 +116,7 @@ where let result = routine(); let after = Instant::now(); writeln!( - ui.stderr(), + ui.status(), "First run took {:?} and produced: {:?}", after.duration_since(before), result @@ -203,7 +203,7 @@ fn bench_revset( group: &mut BenchmarkGroup, revset: &str, ) -> Result<(), CommandError> { - writeln!(ui.stderr(), "----------Testing revset: {revset}----------")?; + writeln!(ui.status(), "----------Testing revset: {revset}----------")?; let expression = revset::optimize(workspace_command.parse_revset(revset)?.expression().clone()); // Time both evaluation and iteration. let routine = |workspace_command: &WorkspaceCommandHelper, expression: Rc| { @@ -220,7 +220,7 @@ fn bench_revset( let result = routine(workspace_command, expression.clone()); let after = Instant::now(); writeln!( - ui.stderr(), + ui.status(), "First run took {:?} and produced {result} commits", after.duration_since(before), )?; diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index e8f8100b71..4f74384600 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -497,7 +497,7 @@ fn cmd_branch_delete( } tx.finish(ui, format!("delete {}", make_branch_term(&names)))?; if names.len() > 1 { - writeln!(ui.stderr(), "Deleted {} branches.", names.len())?; + writeln!(ui.status(), "Deleted {} branches.", names.len())?; } Ok(()) } @@ -523,7 +523,7 @@ fn cmd_branch_forget( } tx.finish(ui, format!("forget {}", make_branch_term(&names)))?; if names.len() > 1 { - writeln!(ui.stderr(), "Forgot {} branches.", names.len())?; + writeln!(ui.status(), "Forgot {} branches.", names.len())?; } Ok(()) } @@ -554,7 +554,7 @@ fn cmd_branch_track( tx.finish(ui, format!("track remote {}", make_branch_term(&names)))?; if names.len() > 1 { writeln!( - ui.stderr(), + ui.status(), "Started tracking {} remote branches.", names.len() )?; @@ -594,7 +594,7 @@ fn cmd_branch_untrack( tx.finish(ui, format!("untrack remote {}", make_branch_term(&names)))?; if names.len() > 1 { writeln!( - ui.stderr(), + ui.status(), "Stopped tracking {} remote branches.", names.len() )?; diff --git a/cli/src/commands/debug.rs b/cli/src/commands/debug.rs index 6308d12e49..1c65897833 100644 --- a/cli/src/commands/debug.rs +++ b/cli/src/commands/debug.rs @@ -259,7 +259,7 @@ fn cmd_debug_reindex( .build_index_at_operation(&op, repo_loader.store()) .map_err(internal_error)?; writeln!( - ui.stderr(), + ui.status(), "Finished indexing {:?} commits.", default_index.as_composite().stats().num_commits )?; @@ -358,7 +358,7 @@ fn cmd_debug_watchman( }; locked_local_wc.reset_watchman()?; locked_ws.finish(repo.op_id().clone())?; - writeln!(ui.stderr(), "Reset Watchman clock")?; + writeln!(ui.status(), "Reset Watchman clock")?; } } Ok(()) diff --git a/cli/src/commands/describe.rs b/cli/src/commands/describe.rs index 694836874e..b746a92829 100644 --- a/cli/src/commands/describe.rs +++ b/cli/src/commands/describe.rs @@ -83,7 +83,7 @@ pub(crate) fn cmd_describe( edit_description(workspace_command.repo(), &template, command.settings())? }; if description == *commit.description() && !args.reset_author { - writeln!(ui.stderr(), "Nothing changed.")?; + writeln!(ui.status(), "Nothing changed.")?; } else { let mut tx = workspace_command.start_transaction(); let mut commit_builder = tx diff --git a/cli/src/commands/diffedit.rs b/cli/src/commands/diffedit.rs index 1cb8a326ca..1a76bfbd85 100644 --- a/cli/src/commands/diffedit.rs +++ b/cli/src/commands/diffedit.rs @@ -97,7 +97,7 @@ don't make any changes, then the operation will be aborted.", let tree = target_commit.tree()?; let tree_id = diff_editor.edit(&base_tree, &tree, &EverythingMatcher, Some(&instructions))?; if tree_id == *target_commit.tree_id() { - writeln!(ui.stderr(), "Nothing changed.")?; + writeln!(ui.status(), "Nothing changed.")?; } else { let mut_repo = tx.mut_repo(); let new_commit = mut_repo @@ -107,11 +107,11 @@ 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.stderr(), "Created ")?; - tx.write_commit_summary(ui.stderr_formatter().as_mut(), &new_commit)?; - writeln!(ui.stderr())?; + write!(ui.status(), "Created ")?; + tx.write_commit_summary(ui.status().as_mut(), &new_commit)?; + writeln!(ui.status())?; if num_rebased > 0 { - writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?; + writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?; } tx.finish(ui, format!("edit commit {}", target_commit.id().hex()))?; } diff --git a/cli/src/commands/duplicate.rs b/cli/src/commands/duplicate.rs index 1457490386..248eff58e2 100644 --- a/cli/src/commands/duplicate.rs +++ b/cli/src/commands/duplicate.rs @@ -47,7 +47,7 @@ pub(crate) fn cmd_duplicate( .evaluate_to_commit_ids()? .collect(); // in reverse topological order if to_duplicate.is_empty() { - writeln!(ui.stderr(), "No revisions to duplicate.")?; + writeln!(ui.status(), "No revisions to duplicate.")?; return Ok(()); } if to_duplicate.last() == Some(workspace_command.repo().store().root_commit_id()) { @@ -78,9 +78,9 @@ pub(crate) fn cmd_duplicate( } for (old_id, new_commit) in &duplicated_old_to_new { - write!(ui.stderr(), "Duplicated {} as ", short_commit_hash(old_id))?; - tx.write_commit_summary(ui.stderr_formatter().as_mut(), new_commit)?; - writeln!(ui.stderr())?; + write!(ui.status(), "Duplicated {} as ", short_commit_hash(old_id))?; + tx.write_commit_summary(ui.status().as_mut(), new_commit)?; + writeln!(ui.status())?; } tx.finish(ui, format!("duplicate {} commit(s)", to_duplicate.len()))?; Ok(()) diff --git a/cli/src/commands/edit.rs b/cli/src/commands/edit.rs index 3ab8495ca2..a245bdafcf 100644 --- a/cli/src/commands/edit.rs +++ b/cli/src/commands/edit.rs @@ -46,7 +46,7 @@ pub(crate) fn cmd_edit( let new_commit = workspace_command.resolve_single_rev(&args.revision)?; workspace_command.check_rewritable([&new_commit])?; if workspace_command.get_wc_commit_id() == Some(new_commit.id()) { - writeln!(ui.stderr(), "Already editing that commit")?; + writeln!(ui.status(), "Already editing that commit")?; } else { let mut tx = workspace_command.start_transaction(); tx.edit(&new_commit)?; diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 205e6258bd..2cc975f9ca 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -482,7 +482,7 @@ fn init_git_refs( } let repo = tx.commit("import git refs"); writeln!( - ui.stderr(), + ui.status(), "Done importing changes from the underlying Git repo." )?; Ok(repo) @@ -509,7 +509,7 @@ fn cmd_git_init( let relative_wc_path = file_util::relative_path(cwd, &wc_path); writeln!( - ui.stderr(), + ui.status(), r#"Initialized repo in "{}""#, relative_wc_path.display() )?; @@ -747,7 +747,7 @@ fn do_git_clone( }; let git_repo = get_git_repo(repo.store())?; writeln!( - ui.stderr(), + ui.status(), r#"Fetching into new repo in "{}""#, wc_path.display() )?; @@ -857,7 +857,7 @@ fn cmd_git_push( match classify_branch_update(branch_name, &remote, targets) { Ok(Some(update)) => branch_updates.push((branch_name.to_owned(), update)), Ok(None) => writeln!( - ui.stderr(), + ui.status(), "Branch {branch_name}@{remote} already matches {branch_name}", )?, Err(reason) => return Err(reason.into()), @@ -896,7 +896,7 @@ fn cmd_git_push( ); } if branch_updates.is_empty() { - writeln!(ui.stderr(), "Nothing changed.")?; + writeln!(ui.status(), "Nothing changed.")?; return Ok(()); } @@ -958,20 +958,20 @@ fn cmd_git_push( } } - writeln!(ui.stderr(), "Branch changes to push to {}:", &remote)?; + writeln!(ui.status(), "Branch changes to push to {}:", &remote)?; for (branch_name, update) in &branch_updates { match (&update.old_target, &update.new_target) { (Some(old_target), Some(new_target)) => { if force_pushed_branches.contains(branch_name) { writeln!( - ui.stderr(), + ui.status(), " Force branch {branch_name} from {} to {}", short_commit_hash(old_target), short_commit_hash(new_target) )?; } else { writeln!( - ui.stderr(), + ui.status(), " Move branch {branch_name} from {} to {}", short_commit_hash(old_target), short_commit_hash(new_target) @@ -980,14 +980,14 @@ fn cmd_git_push( } (Some(old_target), None) => { writeln!( - ui.stderr(), + ui.status(), " Delete branch {branch_name} from {}", short_commit_hash(old_target) )?; } (None, Some(new_target)) => { writeln!( - ui.stderr(), + ui.status(), " Add branch {branch_name} to {}", short_commit_hash(new_target) )?; @@ -999,7 +999,7 @@ fn cmd_git_push( } if args.dry_run { - writeln!(ui.stderr(), "Dry-run requested, not pushing.")?; + writeln!(ui.status(), "Dry-run requested, not pushing.")?; return Ok(()); } @@ -1129,7 +1129,7 @@ fn update_change_branches( } if view.get_local_branch(&branch_name).is_absent() { writeln!( - ui.stderr(), + ui.status(), "Creating branch {} for revision {}", branch_name, change_str.deref() @@ -1268,7 +1268,7 @@ fn cmd_git_submodule_print_gitmodules( let gitmodules_path = RepoPath::from_internal_string(".gitmodules"); let mut gitmodules_file = match tree.path_value(gitmodules_path).into_resolved() { Ok(None) => { - writeln!(ui.stderr(), "No submodules!")?; + writeln!(ui.status(), "No submodules!")?; return Ok(()); } Ok(Some(TreeValue::File { id, .. })) => repo.store().read_file(gitmodules_path, &id)?, diff --git a/cli/src/commands/init.rs b/cli/src/commands/init.rs index 4963163930..0e6d952dcb 100644 --- a/cli/src/commands/init.rs +++ b/cli/src/commands/init.rs @@ -79,7 +79,7 @@ Set `ui.allow-init-native` to allow initializing a repo with the native backend. let relative_wc_path = file_util::relative_path(cwd, &wc_path); writeln!( - ui.stderr(), + ui.status(), "Initialized repo in \"{}\"", relative_wc_path.display() )?; diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index 1b10eca862..45700e04f8 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -189,15 +189,15 @@ 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.stderr(), "Created new commit ")?; - tx.write_commit_summary(ui.stderr_formatter().as_mut(), &new_commit)?; - writeln!(ui.stderr())?; + write!(ui.status(), "Created new commit ")?; + tx.write_commit_summary(ui.status().as_mut(), &new_commit)?; + writeln!(ui.status())?; } else { tx.edit(&new_commit).unwrap(); // The description of the new commit will be printed by tx.finish() } if num_rebased > 0 { - writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?; + writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?; } tx.finish(ui, "new empty commit")?; Ok(()) diff --git a/cli/src/commands/operation.rs b/cli/src/commands/operation.rs index 6089192910..105008bc1e 100644 --- a/cli/src/commands/operation.rs +++ b/cli/src/commands/operation.rs @@ -364,11 +364,11 @@ fn cmd_op_abandon( )?; let [new_head_id]: [OperationId; 1] = stats.new_head_ids.try_into().unwrap(); if current_head_op.id() == &new_head_id { - writeln!(ui.stderr(), "Nothing changed.")?; + writeln!(ui.status(), "Nothing changed.")?; return Ok(()); } writeln!( - ui.stderr(), + ui.status(), "Abandoned {} operations and reparented {} descendant operations.", stats.unreachable_count, stats.rewritten_count, diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index d1d5d6727c..f9013720fd 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -318,7 +318,7 @@ fn rebase_descendants( let num_rebased = old_commits.len() + tx.mut_repo() .rebase_descendants_with_options(settings, rebase_options)?; - writeln!(ui.stderr(), "Rebased {num_rebased} commits")?; + writeln!(ui.status(), "Rebased {num_rebased} commits")?; let tx_message = if old_commits.len() == 1 { format!( "rebase commit {} and descendants", @@ -447,9 +447,9 @@ 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.stderr(), "Skipping rebase of commit ")?; - tx.write_commit_summary(ui.stderr_formatter().as_mut(), &old_commit)?; - writeln!(ui.stderr())?; + write!(ui.status(), "Skipping rebase of commit ")?; + tx.write_commit_summary(ui.status().as_mut(), &old_commit)?; + writeln!(ui.status())?; true } else { rebase_commit(settings, tx.mut_repo(), &old_commit, &new_parents)?; @@ -460,12 +460,12 @@ fn rebase_revision( if num_rebased_descendants > 0 { if skipped_commit_rebase { writeln!( - ui.stderr(), + ui.status(), "Rebased {num_rebased_descendants} descendant commits onto parent of commit" )?; } else { writeln!( - ui.stderr(), + ui.status(), "Also rebased {num_rebased_descendants} descendant commits onto parent of rebased \ commit" )?; @@ -500,7 +500,7 @@ fn log_skipped_rebase_commits_message( workspace_command: &WorkspaceCommandHelper, commits: &[&Commit], ) -> Result<(), CommandError> { - let mut fmt = ui.stderr_formatter(); + let mut fmt = ui.status(); let template = workspace_command.commit_summary_template(); if commits.len() == 1 { write!(fmt, "Skipping rebase of commit ")?; diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index b91a9985ee..3b9a9a0db6 100644 --- a/cli/src/commands/resolve.rs +++ b/cli/src/commands/resolve.rs @@ -99,7 +99,7 @@ pub(crate) fn cmd_resolve( workspace_command.check_rewritable([&commit])?; let merge_editor = workspace_command.merge_editor(ui, args.tool.as_deref())?; writeln!( - ui.stderr(), + ui.status(), "Resolving conflicts in: {}", workspace_command.format_file_path(repo_path) )?; @@ -120,14 +120,10 @@ pub(crate) fn cmd_resolve( let new_conflicts = new_tree.conflicts().collect_vec(); if !new_conflicts.is_empty() { writeln!( - ui.stderr(), + ui.status(), "After this operation, some files at this revision still have conflicts:" )?; - print_conflicted_paths( - &new_conflicts, - ui.stderr_formatter().as_mut(), - &workspace_command, - )?; + print_conflicted_paths(&new_conflicts, ui.status().as_mut(), &workspace_command)?; } }; Ok(()) diff --git a/cli/src/commands/restore.rs b/cli/src/commands/restore.rs index 5c71005f17..d486fac4cc 100644 --- a/cli/src/commands/restore.rs +++ b/cli/src/commands/restore.rs @@ -101,7 +101,7 @@ pub(crate) fn cmd_restore( let to_tree = to_commit.tree()?; let new_tree_id = restore_tree(&from_tree, &to_tree, matcher.as_ref())?; if &new_tree_id == to_commit.tree_id() { - writeln!(ui.stderr(), "Nothing changed.")?; + writeln!(ui.status(), "Nothing changed.")?; } else { let mut tx = workspace_command.start_transaction(); let mut_repo = tx.mut_repo(); @@ -112,11 +112,11 @@ 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.stderr(), "Created ")?; - tx.write_commit_summary(ui.stderr_formatter().as_mut(), &new_commit)?; - writeln!(ui.stderr())?; + write!(ui.status(), "Created ")?; + tx.write_commit_summary(ui.status().as_mut(), &new_commit)?; + writeln!(ui.status())?; if num_rebased > 0 { - writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?; + writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?; } tx.finish(ui, format!("restore into commit {}", to_commit.id().hex()))?; } diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index 89e8cf3673..fbb47300a2 100644 --- a/cli/src/commands/split.rs +++ b/cli/src/commands/split.rs @@ -88,7 +88,7 @@ don't make any changes, then the operation will be aborted. diff_selector.select(&base_tree, &end_tree, matcher.as_ref(), Some(&instructions))?; if &selected_tree_id == commit.tree_id() && diff_selector.is_interactive() { // The user selected everything from the original commit. - writeln!(ui.stderr(), "Nothing changed.")?; + writeln!(ui.status(), "Nothing changed.")?; return Ok(()); } if selected_tree_id == base_tree.id() { @@ -150,13 +150,13 @@ don't make any changes, then the operation will be aborted. .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.stderr(), "Rebased {num_rebased} descendant commits")?; + writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?; } - write!(ui.stderr(), "First part: ")?; - tx.write_commit_summary(ui.stderr_formatter().as_mut(), &first_commit)?; - write!(ui.stderr(), "\nSecond part: ")?; - tx.write_commit_summary(ui.stderr_formatter().as_mut(), &second_commit)?; - writeln!(ui.stderr())?; + 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(()) } diff --git a/cli/src/commands/untrack.rs b/cli/src/commands/untrack.rs index 207d5a3126..bc637ab072 100644 --- a/cli/src/commands/untrack.rs +++ b/cli/src/commands/untrack.rs @@ -100,7 +100,7 @@ Make sure they're ignored, then try again.", } let num_rebased = tx.mut_repo().rebase_descendants(command.settings())?; if num_rebased > 0 { - writeln!(ui.stderr(), "Rebased {num_rebased} descendant commits")?; + writeln!(ui.status(), "Rebased {num_rebased} descendant commits")?; } let repo = tx.commit("untrack paths"); locked_ws.finish(repo.op_id().clone())?; diff --git a/cli/src/commands/workspace.rs b/cli/src/commands/workspace.rs index 9160281246..81c67c63b0 100644 --- a/cli/src/commands/workspace.rs +++ b/cli/src/commands/workspace.rs @@ -166,7 +166,7 @@ fn cmd_workspace_add( workspace_id, )?; writeln!( - ui.stderr(), + ui.status(), "Created workspace in \"{}\"", file_util::relative_path(old_workspace_command.workspace_root(), &destination_path) .display() @@ -329,7 +329,7 @@ fn create_and_check_out_recovery_commit( locked_workspace.finish(repo.op_id().clone())?; writeln!( - ui.stderr(), + ui.status(), "Created and checked out recovery commit {}", short_commit_hash(new_commit.id()) )?; @@ -357,7 +357,7 @@ fn for_stale_working_copy( ), Err(e @ OpStoreError::ObjectNotFound { .. }) => { writeln!( - ui.stderr(), + ui.status(), "Failed to read working copy's current operation; attempting recovery. Error \ message from read attempt: {e}" )?; @@ -401,7 +401,7 @@ fn cmd_workspace_update_stale( match check_stale_working_copy(locked_ws.locked_wc(), &desired_wc_commit, &repo)? { WorkingCopyFreshness::Fresh | WorkingCopyFreshness::Updated(_) => { writeln!( - ui.stderr(), + ui.status(), "Nothing to do (the working copy is not stale)." )?; } @@ -424,11 +424,11 @@ fn cmd_workspace_update_stale( ) })?; locked_ws.finish(repo.op_id().clone())?; - write!(ui.stderr(), "Working copy now at: ")?; - ui.stderr_formatter().with_label("working_copy", |fmt| { + 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.stderr())?; + writeln!(ui.status())?; print_checkout_stats(ui, stats, &desired_wc_commit)?; } } diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 5bd9db3554..bc74d5cb5f 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -192,7 +192,7 @@ impl GitSidebandProgressMessageWriter { } self.scratch.extend_from_slice(&progress_message[i..i + 1]); - ui.stderr().write_all(&self.scratch)?; + ui.status().write_all(&self.scratch)?; self.scratch.clear(); index = i + 1; @@ -212,7 +212,7 @@ impl GitSidebandProgressMessageWriter { pub fn flush(&mut self, ui: &Ui) -> std::io::Result<()> { if !self.scratch.is_empty() { self.scratch.push(b'\n'); - ui.stderr().write_all(&self.scratch)?; + ui.status().write_all(&self.scratch)?; self.scratch.clear(); } @@ -274,16 +274,16 @@ 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 stderr = ui.stderr_formatter(); + let mut formatter = ui.status(); for status in refs_stats { - status.output(max_width, has_both_ref_kinds, &mut *stderr)?; + status.output(max_width, has_both_ref_kinds, &mut *formatter)?; } } } if !stats.abandoned_commits.is_empty() { writeln!( - ui.stderr(), + ui.status(), "Abandoned {} commits that are no longer reachable.", stats.abandoned_commits.len() )?; diff --git a/cli/src/ui.rs b/cli/src/ui.rs index f6d4488e3c..14fde6b940 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -373,6 +373,12 @@ impl Ui { }) } + /// Writes a message that's a status update not part of the command's main + /// output. + pub fn status(&self) -> Box { + self.stderr_formatter() + } + /// Writer to print hint with the default "Hint: " heading. pub fn hint_default( &self, @@ -382,7 +388,7 @@ impl Ui { /// Writer to print hint without the "Hint: " heading. pub fn hint_no_heading(&self) -> LabeledWriter, &'static str> { - LabeledWriter::new(self.stderr_formatter(), "hint") + LabeledWriter::new(self.status(), "hint") } /// Writer to print hint with the given heading. @@ -390,7 +396,7 @@ impl Ui { &self, heading: H, ) -> HeadingLabeledWriter, &'static str, H> { - HeadingLabeledWriter::new(self.stderr_formatter(), "hint", heading) + HeadingLabeledWriter::new(self.status(), "hint", heading) } /// Writer to print warning with the default "Warning: " heading. From 48c15f90c62cfc9d1223de64b1c2ea49189a30b2 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 31 Mar 2024 08:51:33 -0700 Subject: [PATCH 2/6] errors: don't use the `ui.hint*()` helpers I'm about to make hints not get printed with `--quiet`, but error hints are probably still useful to get. They shouldn't be a problem for scripts since the script would have to deal with the error anyway. --- cli/src/command_error.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/cli/src/command_error.rs b/cli/src/command_error.rs index d0aa801ab5..4aa87af6a1 100644 --- a/cli/src/command_error.rs +++ b/cli/src/command_error.rs @@ -561,7 +561,7 @@ fn try_handle_command_result( CommandErrorKind::Config => { print_error(ui, "Config error: ", err, hints)?; writeln!( - ui.hint_no_heading(), + ui.stderr_formatter().labeled("hint"), "For help, see https://github.com/martinvonz/jj/blob/main/docs/config.md." )?; Ok(ExitCode::from(1)) @@ -619,13 +619,14 @@ fn print_error_sources(ui: &Ui, source: Option<&dyn error::Error>) -> io::Result fn print_error_hints(ui: &Ui, hints: &[ErrorHint]) -> io::Result<()> { for hint in hints { - match hint { - ErrorHint::PlainText(message) => { - writeln!(ui.hint_default(), "{message}")?; - } - ErrorHint::Formatted(recorded) => { - ui.stderr_formatter().with_label("hint", |formatter| { - write!(formatter.labeled("heading"), "Hint: ")?; + ui.stderr_formatter().with_label("hint", |formatter| { + write!(formatter.labeled("heading"), "Hint: ")?; + match hint { + ErrorHint::PlainText(message) => { + writeln!(formatter, "{message}")?; + Ok(()) + } + ErrorHint::Formatted(recorded) => { recorded.replay(formatter)?; // Formatted hint is usually multi-line text, and it's // convenient if trailing "\n" doesn't have to be omitted. @@ -633,9 +634,9 @@ fn print_error_hints(ui: &Ui, hints: &[ErrorHint]) -> io::Result<()> { writeln!(formatter)?; } Ok(()) - })?; + } } - } + })?; } Ok(()) } From d5dc37b87780516ef09645736851dbe2e4fa3e89 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 31 Mar 2024 09:08:39 -0700 Subject: [PATCH 3/6] 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 | 33 +++++++++++++++------------ cli/src/commands/abandon.rs | 43 ++++++++++++++++++----------------- cli/src/commands/diffedit.rs | 12 ++++++---- cli/src/commands/duplicate.rs | 10 ++++---- cli/src/commands/new.rs | 8 ++++--- cli/src/commands/rebase.rs | 20 +++++++++------- cli/src/commands/resolve.rs | 19 +++++++++------- cli/src/commands/restore.rs | 12 ++++++---- cli/src/commands/split.rs | 16 +++++++------ cli/src/commands/workspace.rs | 12 ++++++---- cli/src/git_util.rs | 6 +++-- cli/src/ui.rs | 18 ++++++++++----- 12 files changed, 120 insertions(+), 89 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 251140c865..c1f3541067 100644 --- a/cli/src/cli_util.rs +++ b/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, ) -> 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.", diff --git a/cli/src/commands/abandon.rs b/cli/src/commands/abandon.rs index c9c1e718bc..e46a7898ec 100644 --- a/cli/src/commands/abandon.rs +++ b/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()) diff --git a/cli/src/commands/diffedit.rs b/cli/src/commands/diffedit.rs index 1a76bfbd85..d97d6aed30 100644 --- a/cli/src/commands/diffedit.rs +++ b/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()))?; } diff --git a/cli/src/commands/duplicate.rs b/cli/src/commands/duplicate.rs index 248eff58e2..0ebe30336c 100644 --- a/cli/src/commands/duplicate.rs +++ b/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(()) diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index 45700e04f8..94b84a8ae2 100644 --- a/cli/src/commands/new.rs +++ b/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() diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index f9013720fd..16613d282b 100644 --- a/cli/src/commands/rebase.rs +++ b/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::, _>(|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)?; } } diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index 3b9a9a0db6..98ba6b1f7f 100644 --- a/cli/src/commands/resolve.rs +++ b/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(()) diff --git a/cli/src/commands/restore.rs b/cli/src/commands/restore.rs index d486fac4cc..b3a599096e 100644 --- a/cli/src/commands/restore.rs +++ b/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()))?; } diff --git a/cli/src/commands/split.rs b/cli/src/commands/split.rs index fbb47300a2..01465f5ac5 100644 --- a/cli/src/commands/split.rs +++ b/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(()) } diff --git a/cli/src/commands/workspace.rs b/cli/src/commands/workspace.rs index 81c67c63b0..6c609b3de8 100644 --- a/cli/src/commands/workspace.rs +++ b/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)?; } } diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index bc74d5cb5f..6642470cef 100644 --- a/cli/src/git_util.rs +++ b/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() )?; diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 14fde6b940..e6e6a77ac2 100644 --- a/cli/src/ui.rs +++ b/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 { - self.stderr_formatter() + /// Writer to print an update that's not part of the command's main output. + pub fn status(&self) -> Box { + 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> { + 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, &'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, &'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. From 4b62284343ed7f49dc38fd54581e05b09dc0257a Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 1 Apr 2024 08:39:02 -0700 Subject: [PATCH 4/6] ui: make `hint_*()` methods return `Option` Same reasoning as the previous patch: we can avoid doing work when `--quiet` is passed. --- cli/src/cli_util.rs | 52 +++++++++++++++++++++----------------- cli/src/commands/branch.rs | 14 +++++----- cli/src/commands/git.rs | 18 ++++++------- cli/src/commands/util.rs | 10 ++++---- cli/src/git_util.rs | 10 +++++--- cli/src/merge_tools/mod.rs | 14 +++++----- cli/src/ui.rs | 14 ++++++---- 7 files changed, 74 insertions(+), 58 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index c1f3541067..034bf6673e 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1614,13 +1614,15 @@ pub fn print_checkout_stats( working copy.", stats.skipped_files )?; - writeln!( - ui.hint_default(), - "Inspect the changes compared to the intended target with `jj diff --from {}`. + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + "Inspect the changes compared to the intended target with `jj diff --from {}`. Discard the conflicting changes with `jj restore --from {}`.", - short_commit_hash(new_commit.id()), - short_commit_hash(new_commit.id()) - )?; + short_commit_hash(new_commit.id()), + short_commit_hash(new_commit.id()) + )?; + } } Ok(()) } @@ -1641,21 +1643,25 @@ pub fn print_trackable_remote_branches(ui: &Ui, view: &View) -> io::Result<()> { return Ok(()); } - writeln!( - ui.hint_default(), - "The following remote branches aren't associated with the existing local branches:" - )?; + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + "The following remote branches aren't associated with the existing local branches:" + )?; + } if let Some(mut formatter) = ui.status_formatter() { for full_name in &remote_branch_names { write!(formatter, " ")?; writeln!(formatter.labeled("branch"), "{full_name}")?; } } - writeln!( - ui.hint_default(), - "Run `jj branch track {names}` to keep local branches updated on future pulls.", - names = remote_branch_names.join(" "), - )?; + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + "Run `jj branch track {names}` to keep local branches updated on future pulls.", + names = remote_branch_names.join(" "), + )?; + } Ok(()) } @@ -2197,14 +2203,14 @@ fn resolve_default_command( if matches.subcommand_name().is_none() { let args = get_string_or_array(config, "ui.default-command").optional()?; if args.is_none() { - writeln!( - ui.hint_default(), - "Use `jj -h` for a list of available commands." - )?; - writeln!( - ui.hint_no_heading(), - "Run `jj config set --user ui.default-command log` to disable this message." - )?; + if let Some(mut writer) = ui.hint_default() { + writeln!(writer, "Use `jj -h` for a list of available commands.")?; + writeln!( + writer, + "Run `jj config set --user ui.default-command log` to disable this \ + message." + )?; + } } let default_command = args.unwrap_or_else(|| vec!["log".to_string()]); diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index 4f74384600..48cb3464ed 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -321,12 +321,14 @@ fn cmd_branch_rename( ui.warning_default(), "Branch {old_branch} has tracking remote branches which were not renamed." )?; - writeln!( - ui.hint_default(), - "to rename the branch on the remote, you can `jj git push --branch {old_branch}` \ - first (to delete it on the remote), and then `jj git push --branch {new_branch}`. \ - `jj git push --all` would also be sufficient." - )?; + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + "to rename the branch on the remote, you can `jj git push --branch {old_branch}` \ + first (to delete it on the remote), and then `jj git push --branch \ + {new_branch}`. `jj git push --all` would also be sufficient." + )?; + } } Ok(()) diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 2cc975f9ca..d9853c5493 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -595,10 +595,9 @@ fn get_default_fetch_remotes( } else if let Some(remote) = get_single_remote(git_repo)? { // if nothing was explicitly configured, try to guess if remote != DEFAULT_REMOTE { - writeln!( - ui.hint_default(), - "Fetching from the only existing remote: {remote}" - )?; + if let Some(mut writer) = ui.hint_default() { + writeln!(writer, "Fetching from the only existing remote: {remote}")?; + } } Ok(vec![remote]) } else { @@ -1038,10 +1037,9 @@ fn get_default_push_remote( } else if let Some(remote) = get_single_remote(git_repo)? { // similar to get_default_fetch_remotes if remote != DEFAULT_REMOTE { - writeln!( - ui.hint_default(), - "Pushing to the only existing remote: {remote}" - )?; + if let Some(mut writer) = ui.hint_default() { + writeln!(writer, "Pushing to the only existing remote: {remote}")?; + } } Ok(remote) } else { @@ -1059,7 +1057,9 @@ impl RejectedBranchUpdateReason { fn print(&self, ui: &Ui) -> io::Result<()> { writeln!(ui.warning_default(), "{}", self.message)?; if let Some(hint) = &self.hint { - writeln!(ui.hint_default(), "{hint}")?; + if let Some(mut writer) = ui.hint_default() { + writeln!(writer, "{hint}")?; + } } Ok(()) } diff --git a/cli/src/commands/util.rs b/cli/src/commands/util.rs index 44bda4fee1..49b0a04e05 100644 --- a/cli/src/commands/util.rs +++ b/cli/src/commands/util.rs @@ -128,16 +128,16 @@ fn cmd_util_completion( args: &UtilCompletionArgs, ) -> Result<(), CommandError> { let mut app = command.app().clone(); - let warn = |shell| { + let warn = |shell| -> std::io::Result<()> { writeln!( ui.warning_default(), "`jj util completion --{shell}` will be removed in a future version, and this will be \ a hard error" )?; - writeln!( - ui.hint_default(), - "Use `jj util completion {shell}` instead" - ) + if let Some(mut writer) = ui.hint_default() { + writeln!(writer, "Use `jj util completion {shell}` instead")?; + } + Ok(()) }; let shell = match (args.shell, args.fish, args.zsh, args.bash) { (Some(s), false, false, false) => s, diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 6642470cef..05f4ba6192 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -409,12 +409,14 @@ pub fn print_failed_git_export( .iter() .any(|failed| matches!(failed.reason, FailedRefExportReason::FailedToSet(_))) { - writeln!( - ui.hint_default(), - r#"Git doesn't allow a branch name that looks like a parent directory of + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + r#"Git doesn't allow a branch name that looks like a parent directory of another (e.g. `foo` and `foo/bar`). Try to rename the branches that failed to export or their "parent" branches."#, - )?; + )?; + } } } Ok(()) diff --git a/cli/src/merge_tools/mod.rs b/cli/src/merge_tools/mod.rs index 19baec5f4e..95140cd133 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -123,12 +123,14 @@ fn editor_args_from_settings( Ok(args) } else { let default_editor = BUILTIN_EDITOR_NAME; - writeln!( - ui.hint_default(), - "Using default editor '{default_editor}'; run `jj config set --user {key} :builtin` \ - to disable this message." - ) - .ok(); + if let Some(mut writer) = ui.hint_default() { + writeln!( + writer, + "Using default editor '{default_editor}'; run `jj config set --user {key} \ + :builtin` to disable this message." + ) + .ok(); + } Ok(default_editor.into()) } } diff --git a/cli/src/ui.rs b/cli/src/ui.rs index e6e6a77ac2..51de85c3bf 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -388,21 +388,25 @@ impl Ui { /// Writer to print hint with the default "Hint: " heading. pub fn hint_default( &self, - ) -> HeadingLabeledWriter, &'static str, &'static str> { + ) -> Option, &'static str, &'static str>> { self.hint_with_heading("Hint: ") } /// Writer to print hint without the "Hint: " heading. - pub fn hint_no_heading(&self) -> LabeledWriter, &'static str> { - LabeledWriter::new(self.stderr_formatter(), "hint") + pub fn hint_no_heading(&self) -> Option, &'static str>> { + Some(LabeledWriter::new(self.stderr_formatter(), "hint")) } /// Writer to print hint with the given heading. pub fn hint_with_heading( &self, heading: H, - ) -> HeadingLabeledWriter, &'static str, H> { - HeadingLabeledWriter::new(self.stderr_formatter(), "hint", heading) + ) -> Option, &'static str, H>> { + Some(HeadingLabeledWriter::new( + self.stderr_formatter(), + "hint", + heading, + )) } /// Writer to print warning with the default "Warning: " heading. From 937dbbdadb7ee364dcffbcda6c9607387df0e400 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 30 Mar 2024 10:18:56 -0700 Subject: [PATCH 5/6] cli: add a global `--quiet` flag, which silences status messages Note that `jj resolve` already had its own `--quiet` flag. The output with `--quiet` for that command got a lot quieter with the global `--quiet` also taking effect. That seems reasonable to me. --- CHANGELOG.md | 2 ++ cli/src/cli_util.rs | 13 +++++++++++++ cli/src/ui.rs | 25 ++++++++++++++++--------- cli/tests/cli-reference@.md.snap | 4 ++++ cli/tests/test_global_opts.rs | 14 ++++++++++++++ cli/tests/test_resolve_command.rs | 15 +-------------- 6 files changed, 50 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d93e165f5..f8b4e7fa8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `--all` is now named `--all-remotes` for `jj branch list` +* There is a new global `--quiet` flag to silence commands' non-primary output. + ### Fixed bugs ## [0.15.1] - 2024-03-06 diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 034bf6673e..5723beeb27 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -2137,6 +2137,16 @@ pub struct EarlyArgs { /// When to colorize output (always, never, auto) #[arg(long, value_name = "WHEN", global = true)] pub color: Option, + /// Silence non-primary command output + /// + /// For example, `jj files` will still list files, but it won't tell you if + /// the working copy was snapshotted or if descendants were rebased. + /// + /// Warnings and errors will still be printed. + #[arg(long, global = true, action = ArgAction::SetTrue)] + // Parsing with ignore_errors will crash if this is bool, so use + // Option. + pub quiet: Option, /// Disable the pager #[arg(long, value_name = "WHEN", global = true, action = ArgAction::SetTrue)] // Parsing with ignore_errors will crash if this is bool, so use @@ -2306,6 +2316,9 @@ fn handle_early_args( if let Some(choice) = args.color { args.config_toml.push(format!(r#"ui.color="{choice}""#)); } + if args.quiet.unwrap_or_default() { + args.config_toml.push(r#"ui.quiet=true"#.to_string()); + } if args.no_pager.unwrap_or_default() { args.config_toml.push(r#"ui.paginate="never""#.to_owned()); } diff --git a/cli/src/ui.rs b/cli/src/ui.rs index 51de85c3bf..df480849ce 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -163,6 +163,7 @@ impl Write for UiStderr<'_> { pub struct Ui { color: bool, + quiet: bool, pager_cmd: CommandNameAndArgs, paginate: PaginationChoice, progress_indicator: bool, @@ -222,6 +223,10 @@ fn use_color(choice: ColorChoice) -> bool { } } +fn be_quiet(config: &config::Config) -> bool { + config.get_bool("ui.quiet").unwrap_or_default() +} + #[derive(Clone, Copy, Debug, Default, Eq, PartialEq, serde::Deserialize)] #[serde(rename_all(deserialize = "kebab-case"))] pub enum PaginationChoice { @@ -245,6 +250,7 @@ fn pager_setting(config: &config::Config) -> Result Result { let color = use_color(color_setting(config)); + let quiet = be_quiet(config); // Sanitize ANSI escape codes if we're printing to a terminal. Doesn't affect // ANSI escape codes that originate from the formatter itself. let sanitize = io::stdout().is_terminal(); @@ -252,6 +258,7 @@ impl Ui { let progress_indicator = progress_indicator_setting(config); Ok(Ui { color, + quiet, formatter_factory, pager_cmd: pager_setting(config)?, paginate: pagination_setting(config)?, @@ -262,6 +269,7 @@ impl Ui { pub fn reset(&mut self, config: &config::Config) -> Result<(), CommandError> { self.color = use_color(color_setting(config)); + self.quiet = be_quiet(config); self.paginate = pagination_setting(config)?; self.pager_cmd = pager_setting(config)?; self.progress_indicator = progress_indicator_setting(config); @@ -375,14 +383,17 @@ impl Ui { /// Writer to print an update that's not part of the command's main output. pub fn status(&self) -> Box { - Box::new(self.stderr()) + if self.quiet { + Box::new(std::io::sink()) + } else { + 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> { - Some(self.stderr_formatter()) + (!self.quiet).then(|| self.stderr_formatter()) } /// Writer to print hint with the default "Hint: " heading. @@ -394,7 +405,7 @@ impl Ui { /// Writer to print hint without the "Hint: " heading. pub fn hint_no_heading(&self) -> Option, &'static str>> { - Some(LabeledWriter::new(self.stderr_formatter(), "hint")) + (!self.quiet).then(|| LabeledWriter::new(self.stderr_formatter(), "hint")) } /// Writer to print hint with the given heading. @@ -402,11 +413,7 @@ impl Ui { &self, heading: H, ) -> Option, &'static str, H>> { - Some(HeadingLabeledWriter::new( - self.stderr_formatter(), - "hint", - heading, - )) + (!self.quiet).then(|| HeadingLabeledWriter::new(self.stderr_formatter(), "hint", heading)) } /// Writer to print warning with the default "Warning: " heading. diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 8ff377a2f0..85b4b4ace1 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -159,6 +159,10 @@ To get started, see the tutorial at https://github.com/martinvonz/jj/blob/main/d Possible values: `true`, `false` * `--color ` — When to colorize output (always, never, auto) +* `--quiet` — Silence non-primary command output + + Possible values: `true`, `false` + * `--no-pager` — Disable the pager Possible values: `true`, `false` diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index 0ad86b7709..cf358b9b90 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -441,6 +441,19 @@ fn test_color_ui_messages() { "###); } +#[test] +fn test_quiet() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + + // Can skip message about new working copy with `--quiet` + std::fs::write(repo_path.join("file1"), "contents").unwrap(); + let (_stdout, stderr) = + test_env.jj_cmd_ok(&repo_path, &["--quiet", "describe", "-m=new description"]); + insta::assert_snapshot!(stderr, @""); +} + #[test] fn test_early_args() { // Test that help output parses early args @@ -535,6 +548,7 @@ fn test_help() { --at-operation Operation to load the repo at [default: @] [aliases: at-op] --debug Enable debug logging --color When to colorize output (always, never, auto) + --quiet Silence non-primary command output --no-pager Disable the pager --config-toml Additional configuration options (can be repeated) "###); diff --git a/cli/tests/test_resolve_command.rs b/cli/tests/test_resolve_command.rs index 057b76164f..1338138106 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -728,20 +728,7 @@ fn test_multiple_conflicts() { std::fs::write(&editor_script, "expect\n\0write\nresolution another_file\n").unwrap(); let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["resolve", "--quiet", "another_file"]); insta::assert_snapshot!(stdout, @""); - insta::assert_snapshot!(stderr, @r###" - Resolving conflicts in: another_file - New conflicts appeared in these commits: - vruxwmqv 3c438f88 conflict | (conflict) conflict - To resolve the conflicts, start by updating to it: - jj new vruxwmqvtpmx - Then use `jj resolve`, or edit the conflict markers in the file directly. - Once the conflicts are resolved, you may want inspect the result with `jj diff`. - Then run `jj squash` to move the resolution into the conflicted commit. - Working copy now at: vruxwmqv 3c438f88 conflict | (conflict) conflict - Parent commit : zsuskuln de7553ef a | a - Parent commit : royxmykx f68bc2f0 b | b - Added 0 files, modified 1 files, removed 0 files - "###); + insta::assert_snapshot!(stderr, @""); // For the rest of the test, we call `jj resolve` several times in a row to // resolve each conflict in the order it chooses. From 6b3c7a0a6ec360d31a06a3f26f650833f2133d1c Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 31 Mar 2024 22:32:43 -0700 Subject: [PATCH 6/6] resolve: remove `--quiet` flag, rely on global one --- cli/src/commands/resolve.rs | 27 ++++++++++----------------- cli/tests/cli-reference@.md.snap | 4 ---- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/cli/src/commands/resolve.rs b/cli/src/commands/resolve.rs index 98ba6b1f7f..5f622fe7b5 100644 --- a/cli/src/commands/resolve.rs +++ b/cli/src/commands/resolve.rs @@ -51,10 +51,6 @@ pub(crate) struct ResolveArgs { // `diff --summary`, but should be more verbose. #[arg(long, short)] list: bool, - /// Do not print the list of remaining conflicts (if any) after resolving a - /// conflict - #[arg(long, short, conflicts_with = "list")] - quiet: bool, /// Specify 3-way merge tool to be used #[arg(long, conflicts_with = "list", value_name = "NAME")] tool: Option, @@ -115,20 +111,17 @@ pub(crate) fn cmd_resolve( format!("Resolve conflicts in commit {}", commit.id().hex()), )?; - // TODO: Delete local `--quiet` - if !args.quiet { - 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)?; - } + 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(()) } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 85b4b4ace1..f73c3fea4c 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1523,10 +1523,6 @@ Note that conflicts can also be resolved without using this command. You may edi Possible values: `true`, `false` -* `-q`, `--quiet` — Do not print the list of remaining conflicts (if any) after resolving a conflict - - Possible values: `true`, `false` - * `--tool ` — Specify 3-way merge tool to be used