From 4962d9af3be7a4c9d39c772f629013892dbec699 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 1 Apr 2024 08:39:02 -0700 Subject: [PATCH] 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 c1f354106..034bf6673 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 4f7438460..48cb3464e 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 2cc975f9c..d9853c549 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 44bda4fee..49b0a04e0 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 6642470ce..05f4ba619 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 19baec5f4..95140cd13 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 e6e6a77ac..51de85c3b 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.