Skip to content

Commit

Permalink
cli: add convenient methods to print hint or warning with default hea…
Browse files Browse the repository at this point in the history
…dings

The lowercase "warning: " is unified to "Warning: " as it is the jj's
convention afaik.

The _default() suffix could be dropped from these methods, but it's probably
better to break the existing codebase for the moment. Otherwise, the caller
might do writeln!(ui.warning(), "Warning: ..").
  • Loading branch information
yuja committed Mar 25, 2024
1 parent e03a274 commit 3351101
Show file tree
Hide file tree
Showing 22 changed files with 90 additions and 76 deletions.
6 changes: 3 additions & 3 deletions cli/src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1602,7 +1602,7 @@ pub fn print_checkout_stats(
stats.skipped_files
)?;
writeln!(
ui.hint_with_heading("Hint: "),
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()),
Expand Down Expand Up @@ -1639,7 +1639,7 @@ pub fn print_trackable_remote_branches(ui: &Ui, view: &View) -> io::Result<()> {
}
drop(formatter);
writeln!(
ui.hint_with_heading("Hint: "),
ui.hint_default(),
"Run `jj branch track {names}` to keep local branches updated on future pulls.",
names = remote_branch_names.join(" "),
)?;
Expand Down Expand Up @@ -2207,7 +2207,7 @@ fn resolve_default_command(
let args = get_string_or_array(config, "ui.default-command").optional()?;
if args.is_none() {
writeln!(
ui.hint_with_heading("Hint: "),
ui.hint_default(),
"Use `jj -h` for a list of available commands."
)?;
writeln!(
Expand Down
4 changes: 2 additions & 2 deletions cli/src/command_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,7 +534,7 @@ fn print_error(ui: &Ui, heading: &str, err: &dyn error::Error, hints: &[String])
writeln!(ui.error_with_heading(heading), "{err}")?;
print_error_sources(ui, err.source())?;
for hint in hints {
writeln!(ui.hint_with_heading("Hint: "), "{hint}")?;
writeln!(ui.hint_default(), "{hint}")?;
}
Ok(())
}
Expand Down Expand Up @@ -577,7 +577,7 @@ fn handle_clap_error(ui: &mut Ui, err: &clap::Error, hints: &[String]) -> io::Re
}
write!(ui.stderr(), "{clap_str}")?;
for hint in hints {
writeln!(ui.hint_with_heading("Hint: "), "{hint}")?;
writeln!(ui.hint_default(), "{hint}")?;
}
Ok(ExitCode::from(2))
}
8 changes: 4 additions & 4 deletions cli/src/commands/branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ fn cmd_branch_create(

if branch_names.len() > 1 {
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"Creating multiple branches: {}",
branch_names.join(", "),
)?;
Expand Down Expand Up @@ -321,11 +321,11 @@ fn cmd_branch_rename(
.any(|(_, remote_ref)| remote_ref.is_tracking())
{
writeln!(
ui.warning_with_heading("Warning: "),
ui.warning_default(),
"Branch {old_branch} has tracking remote branches which were not renamed."
)?;
writeln!(
ui.hint_with_heading("Hint: "),
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."
Expand Down Expand Up @@ -370,7 +370,7 @@ fn cmd_branch_set(

if branch_names.len() > 1 {
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"Updating multiple branches: {}",
branch_names.join(", "),
)?;
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ pub(crate) fn cmd_checkout(
args: &CheckoutArgs,
) -> Result<(), CommandError> {
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"`jj checkout` is deprecated; use `jj new` instead, which is equivalent"
)?;
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"`jj checkout` will be removed in a future version, and this will be a hard error"
)?;
let mut workspace_command = command.workspace_helper(ui)?;
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1060,7 +1060,7 @@ impl RejectedBranchUpdateReason {
fn print(&self, ui: &Ui) -> io::Result<()> {
writeln!(ui.warning_no_heading(), "{}", self.message)?;
if let Some(hint) = &self.hint {
writeln!(ui.hint_with_heading("Hint: "), "{hint}")?;
writeln!(ui.hint_default(), "{hint}")?;
}
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub(crate) fn cmd_init(
if args.git || args.git_repo.is_some() {
git::git_init(ui, command, &wc_path, colocate, args.git_repo.as_deref())?;
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"`--git` and `--git-repo` are deprecated.
Use `jj git init` instead"
)?;
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ pub(crate) fn cmd_log(
if only_path == "." && workspace_command.parse_file_path(only_path)?.is_root() {
// For users of e.g. Mercurial, where `.` indicates the current commit.
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"The argument {only_path:?} is being interpreted as a path, but this is often not \
useful because all non-empty commits touch '.'. If you meant to show the \
working copy commit, pass -r '@' instead."
Expand All @@ -258,7 +258,7 @@ pub(crate) fn cmd_log(
&& revset::parse(only_path, &workspace_command.revset_parse_context()).is_ok()
{
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"The argument {only_path:?} is being interpreted as a path. To specify a revset, \
pass -r {only_path:?} instead."
)?;
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/merge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,11 @@ pub(crate) fn cmd_merge(
args: &new::NewArgs,
) -> Result<(), CommandError> {
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"`jj merge` is deprecated; use `jj new` instead, which is equivalent"
)?;
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"`jj merge` will be removed in a future version, and this will be a hard error"
)?;
if args.revisions.len() < 2 {
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/move.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ pub(crate) fn cmd_move(
args: &MoveArgs,
) -> Result<(), CommandError> {
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"`jj move` is deprecated; use `jj squash` instead, which is equivalent"
)?;
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"`jj move` will be removed in a future version, and this will be a hard error"
)?;
let mut workspace_command = command.workspace_helper(ui)?;
Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/squash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ from the source will be moved into the destination.
.is_ok()
{
writeln!(
ui.warning_with_heading("warning: "),
ui.warning_default(),
"The argument {only_path:?} is being interpreted as a path. To specify a \
revset, pass -r {only_path:?} instead."
)?;
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,12 @@ fn cmd_util_completion(
let mut app = command.app().clone();
let warn = |shell| {
writeln!(
ui.warning_with_heading("Warning: "),
ui.warning_default(),
"`jj util completion --{shell}` will be removed in a future version, and this will be \
a hard error"
)?;
writeln!(
ui.hint_with_heading("Hint: "),
ui.hint_default(),
"Use `jj util completion {shell}` instead"
)
};
Expand Down
2 changes: 1 addition & 1 deletion cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ pub fn print_failed_git_export(
.any(|failed| matches!(failed.reason, FailedRefExportReason::FailedToSet(_)))
{
writeln!(
ui.hint_with_heading("Hint: "),
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."#,
Expand Down
14 changes: 14 additions & 0 deletions cli/src/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,13 @@ impl Ui {
})
}

/// Writer to print hint with the default "Hint: " heading.
pub fn hint_default(
&self,
) -> 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) -> LabeledWriter<Box<dyn Formatter + '_>, &'static str> {
LabeledWriter::new(self.stderr_formatter(), "hint")
Expand All @@ -386,6 +393,13 @@ impl Ui {
HeadingLabeledWriter::new(self.stderr_formatter(), "hint", heading)
}

/// Writer to print warning with the default "Warning: " heading.
pub fn warning_default(
&self,
) -> HeadingLabeledWriter<Box<dyn Formatter + '_>, &'static str, &'static str> {
self.warning_with_heading("Warning: ")
}

/// Writer to print warning without the "Warning: " heading.
pub fn warning_no_heading(&self) -> LabeledWriter<Box<dyn Formatter + '_>, &'static str> {
LabeledWriter::new(self.stderr_formatter(), "warning")
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/test_branch_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn test_branch_multiple_names() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "create", "foo", "bar"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
warning: Creating multiple branches: foo, bar
Warning: Creating multiple branches: foo, bar
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ bar foo 230dd059e1b0
Expand All @@ -36,7 +36,7 @@ fn test_branch_multiple_names() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "set", "foo", "bar"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
warning: Updating multiple branches: foo, bar
Warning: Updating multiple branches: foo, bar
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ bar foo 8bb159bc30a9
Expand Down
20 changes: 10 additions & 10 deletions cli/tests/test_checkout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ fn test_checkout() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["checkout", "@"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
warning: `jj checkout` will be removed in a future version, and this will be a hard error
Warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
Warning: `jj checkout` will be removed in a future version, and this will be a hard error
Working copy now at: zsuskuln 05ce7118 (empty) (no description set)
Parent commit : rlvkpnrz 5c52832c (empty) second
"###);
Expand Down Expand Up @@ -66,8 +66,8 @@ fn test_checkout_not_single_rev() {

let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "root()..@"]);
insta::assert_snapshot!(stderr, @r###"
warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
warning: `jj checkout` will be removed in a future version, and this will be a hard error
Warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
Warning: `jj checkout` will be removed in a future version, and this will be a hard error
Error: Revset "root()..@" resolved to more than one revision
Hint: The revset "root()..@" resolved to these revisions:
royxmykx 2f859371 (empty) (no description set)
Expand All @@ -80,8 +80,8 @@ fn test_checkout_not_single_rev() {

let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "root()..@-"]);
insta::assert_snapshot!(stderr, @r###"
warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
warning: `jj checkout` will be removed in a future version, and this will be a hard error
Warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
Warning: `jj checkout` will be removed in a future version, and this will be a hard error
Error: Revset "root()..@-" resolved to more than one revision
Hint: The revset "root()..@-" resolved to these revisions:
mzvwutvl 5c1afd8b (empty) fifth
Expand All @@ -93,8 +93,8 @@ fn test_checkout_not_single_rev() {

let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "@-|@--"]);
insta::assert_snapshot!(stderr, @r###"
warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
warning: `jj checkout` will be removed in a future version, and this will be a hard error
Warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
Warning: `jj checkout` will be removed in a future version, and this will be a hard error
Error: Revset "@-|@--" resolved to more than one revision
Hint: The revset "@-|@--" resolved to these revisions:
mzvwutvl 5c1afd8b (empty) fifth
Expand All @@ -103,8 +103,8 @@ fn test_checkout_not_single_rev() {

let stderr = test_env.jj_cmd_failure(&repo_path, &["checkout", "none()"]);
insta::assert_snapshot!(stderr, @r###"
warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
warning: `jj checkout` will be removed in a future version, and this will be a hard error
Warning: `jj checkout` is deprecated; use `jj new` instead, which is equivalent
Warning: `jj checkout` will be removed in a future version, and this will be a hard error
Error: Revset "none()" didn't resolve to any revisions
"###);
}
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_global_opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ fn test_color_ui_messages() {
// warning
let (_stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["log", "@"]);
insta::assert_snapshot!(stderr, @r###"
[1m[38;5;3mwarning: [0m[38;5;3mThe argument "@" is being interpreted as a path. To specify a revset, pass -r "@" instead.[39m
[1m[38;5;3mWarning: [0m[38;5;3mThe argument "@" is being interpreted as a path. To specify a revset, pass -r "@" instead.[39m
"###);

// error inlined in template output
Expand Down
8 changes: 4 additions & 4 deletions cli/tests/test_immutable_commits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,16 +135,16 @@ fn test_rewrite_immutable_commands() {
// move --from
let stderr = test_env.jj_cmd_failure(&repo_path, &["move", "--from=main"]);
insta::assert_snapshot!(stderr, @r###"
warning: `jj move` is deprecated; use `jj squash` instead, which is equivalent
warning: `jj move` will be removed in a future version, and this will be a hard error
Warning: `jj move` is deprecated; use `jj squash` instead, which is equivalent
Warning: `jj move` will be removed in a future version, and this will be a hard error
Error: Commit 3d14df18607e is immutable
Hint: Configure the set of immutable commits via `revset-aliases.immutable_heads()`.
"###);
// move --to
let stderr = test_env.jj_cmd_failure(&repo_path, &["move", "--to=main"]);
insta::assert_snapshot!(stderr, @r###"
warning: `jj move` is deprecated; use `jj squash` instead, which is equivalent
warning: `jj move` will be removed in a future version, and this will be a hard error
Warning: `jj move` is deprecated; use `jj squash` instead, which is equivalent
Warning: `jj move` will be removed in a future version, and this will be a hard error
Error: Commit 3d14df18607e is immutable
Hint: Configure the set of immutable commits via `revset-aliases.immutable_heads()`.
"###);
Expand Down
Loading

0 comments on commit 3351101

Please sign in to comment.