From 8e5671975ca9320cd8c98b34455d2516a0917af2 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Mon, 17 Jun 2024 12:30:57 +0900 Subject: [PATCH] ui: remove Option<_> wrapping from ui.hint_() helpers It's cumbersome to unwrap the Option just to print a short hint message. Let's send the message to null output instead. --- cli/src/cli_util.rs | 32 ++++++++++++++------------------ cli/src/commands/branch.rs | 38 ++++++++++++++++---------------------- cli/src/commands/git.rs | 18 +++++++++--------- cli/src/commands/util.rs | 7 ++++--- cli/src/git_util.rs | 10 ++++------ cli/src/merge_tools/mod.rs | 14 ++++++-------- cli/src/ui.rs | 18 +++++++++++------- 7 files changed, 64 insertions(+), 73 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 46afc76b2..784d3ddb4 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -1937,15 +1937,13 @@ pub fn print_checkout_stats( working copy.", stats.skipped_files )?; - if let Some(mut writer) = ui.hint_default() { - writeln!( - writer, - "Inspect the changes compared to the intended target with `jj diff --from {}`. + writeln!( + ui.hint_default(), + "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(()) } @@ -2483,16 +2481,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() { - if let Some(mut writer) = ui.hint_default() { - writeln!(writer, "Use `jj -h` for a list of available commands.")?; - } - if let Some(mut writer) = ui.hint_no_heading() { - writeln!( - writer, - "Run `jj config set --user ui.default-command log` to disable this \ - message." - )?; - } + 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." + )?; } 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 84974057f..933e23a8e 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -329,14 +329,12 @@ fn cmd_branch_rename( ui.warning_default(), "Branch {old_branch} has tracking remote branches which were not renamed." )?; - 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." - )?; - } + 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." + )?; } Ok(()) @@ -770,21 +768,17 @@ fn cmd_branch_list( // Print only one of these hints. It's not important to mention unexported // branches, but user might wonder why deleted branches are still listed. if found_deleted_tracking_local_branch { - if let Some(mut writer) = ui.hint_default() { - writeln!( - writer, - "Branches marked as deleted will be *deleted permanently* on the remote on the \ - next `jj git push`. Use `jj branch forget` to prevent this." - )?; - } + writeln!( + ui.hint_default(), + "Branches marked as deleted will be *deleted permanently* on the remote on the next \ + `jj git push`. Use `jj branch forget` to prevent this." + )?; } else if found_deleted_local_branch { - if let Some(mut writer) = ui.hint_default() { - writeln!( - writer, - "Branches marked as deleted will be deleted from the underlying Git repo on the \ - next `jj git export`." - )?; - } + writeln!( + ui.hint_default(), + "Branches marked as deleted will be deleted from the underlying Git repo on the next \ + `jj git export`." + )?; } Ok(()) diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 66d288286..4b3888646 100644 --- a/cli/src/commands/git.rs +++ b/cli/src/commands/git.rs @@ -608,9 +608,10 @@ 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 { - if let Some(mut writer) = ui.hint_default() { - writeln!(writer, "Fetching from the only existing remote: {remote}")?; - } + writeln!( + ui.hint_default(), + "Fetching from the only existing remote: {remote}" + )?; } Ok(vec![remote]) } else { @@ -1073,9 +1074,10 @@ 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 { - if let Some(mut writer) = ui.hint_default() { - writeln!(writer, "Pushing to the only existing remote: {remote}")?; - } + writeln!( + ui.hint_default(), + "Pushing to the only existing remote: {remote}" + )?; } Ok(remote) } else { @@ -1093,9 +1095,7 @@ impl RejectedBranchUpdateReason { fn print(&self, ui: &Ui) -> io::Result<()> { writeln!(ui.warning_default(), "{}", self.message)?; if let Some(hint) = &self.hint { - if let Some(mut writer) = ui.hint_default() { - writeln!(writer, "{hint}")?; - } + writeln!(ui.hint_default(), "{hint}")?; } Ok(()) } diff --git a/cli/src/commands/util.rs b/cli/src/commands/util.rs index 8ddffc880..54feeb7f8 100644 --- a/cli/src/commands/util.rs +++ b/cli/src/commands/util.rs @@ -134,9 +134,10 @@ fn cmd_util_completion( "`jj util completion --{shell}` will be removed in a future version, and this will be \ a hard error" )?; - if let Some(mut writer) = ui.hint_default() { - writeln!(writer, "Use `jj util completion {shell}` instead")?; - } + writeln!( + ui.hint_default(), + "Use `jj util completion {shell}` instead" + )?; Ok(()) }; let shell = match (args.shell, args.fish, args.zsh, args.bash) { diff --git a/cli/src/git_util.rs b/cli/src/git_util.rs index 05f4ba619..6642470ce 100644 --- a/cli/src/git_util.rs +++ b/cli/src/git_util.rs @@ -409,14 +409,12 @@ pub fn print_failed_git_export( .iter() .any(|failed| matches!(failed.reason, FailedRefExportReason::FailedToSet(_))) { - 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 + writeln!( + ui.hint_default(), + 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 9d738a03c..091989e95 100644 --- a/cli/src/merge_tools/mod.rs +++ b/cli/src/merge_tools/mod.rs @@ -120,14 +120,12 @@ fn editor_args_from_settings( Ok(args) } else { let default_editor = BUILTIN_EDITOR_NAME; - 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(); - } + writeln!( + ui.hint_default(), + "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 8b89fadbe..38dcaef57 100644 --- a/cli/src/ui.rs +++ b/cli/src/ui.rs @@ -23,7 +23,9 @@ use tracing::instrument; use crate::command_error::{config_error_with_message, CommandError}; use crate::config::CommandNameAndArgs; -use crate::formatter::{Formatter, FormatterFactory, HeadingLabeledWriter, LabeledWriter}; +use crate::formatter::{ + Formatter, FormatterFactory, HeadingLabeledWriter, LabeledWriter, PlainTextFormatter, +}; const BUILTIN_PAGER_NAME: &str = ":builtin"; @@ -410,22 +412,24 @@ impl Ui { /// Writer to print hint with the default "Hint: " heading. pub fn hint_default( &self, - ) -> Option, &'static str, &'static str>> { + ) -> HeadingLabeledWriter, &'static str, &'static str> { self.hint_with_heading("Hint: ") } /// Writer to print hint without the "Hint: " heading. - pub fn hint_no_heading(&self) -> Option, &'static str>> { - (!self.quiet).then(|| LabeledWriter::new(self.stderr_formatter(), "hint")) + pub fn hint_no_heading(&self) -> LabeledWriter, &'static str> { + let formatter = self + .status_formatter() + .unwrap_or_else(|| Box::new(PlainTextFormatter::new(io::sink()))); + LabeledWriter::new(formatter, "hint") } /// Writer to print hint with the given heading. pub fn hint_with_heading( &self, heading: H, - ) -> Option, &'static str, H>> { - self.hint_no_heading() - .map(|writer| writer.with_heading(heading)) + ) -> HeadingLabeledWriter, &'static str, H> { + self.hint_no_heading().with_heading(heading) } /// Writer to print warning with the default "Warning: " heading.