Skip to content

Commit

Permalink
ui: remove Option<_> wrapping from ui.hint_() helpers
Browse files Browse the repository at this point in the history
It's cumbersome to unwrap the Option just to print a short hint message. Let's
send the message to null output instead.
  • Loading branch information
yuja committed Jun 18, 2024
1 parent a162e4f commit 8e56719
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 73 deletions.
32 changes: 14 additions & 18 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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()]);

Expand Down
38 changes: 16 additions & 22 deletions cli/src/commands/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down Expand Up @@ -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(())
Expand Down
18 changes: 9 additions & 9 deletions cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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(())
}
Expand Down
7 changes: 4 additions & 3 deletions cli/src/commands/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 4 additions & 6 deletions cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
Expand Down
14 changes: 6 additions & 8 deletions cli/src/merge_tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Expand Down
18 changes: 11 additions & 7 deletions cli/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -410,22 +412,24 @@ impl Ui {
/// Writer to print hint with the default "Hint: " heading.
pub fn hint_default(
&self,
) -> Option<HeadingLabeledWriter<Box<dyn Formatter + '_>, &'static str, &'static str>> {
) -> HeadingLabeledWriter<Box<dyn Formatter + '_>, &'static str, &'static str> {
self.hint_with_heading("Hint: ")
}

/// Writer to print hint without the "Hint: " heading.
pub fn hint_no_heading(&self) -> Option<LabeledWriter<Box<dyn Formatter + '_>, &'static str>> {
(!self.quiet).then(|| LabeledWriter::new(self.stderr_formatter(), "hint"))
pub fn hint_no_heading(&self) -> LabeledWriter<Box<dyn Formatter + '_>, &'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<H: fmt::Display>(
&self,
heading: H,
) -> Option<HeadingLabeledWriter<Box<dyn Formatter + '_>, &'static str, H>> {
self.hint_no_heading()
.map(|writer| writer.with_heading(heading))
) -> HeadingLabeledWriter<Box<dyn Formatter + '_>, &'static str, H> {
self.hint_no_heading().with_heading(heading)
}

/// Writer to print warning with the default "Warning: " heading.
Expand Down

0 comments on commit 8e56719

Please sign in to comment.