cli: print hints using "hint" style, not "error" style

Several lines of red text can be overwhelming, and makes it harder to
tell the hint from the error. Let's separate the hint from the error
instead. This matches what hg does. Having the hints separated out
also means that we could have a single config to turn them off.
This commit is contained in:
Martin von Zweigbergk 2022-11-12 15:28:32 -08:00 committed by Martin von Zweigbergk
parent 2a17bb76e9
commit 6a15cc02bf
4 changed files with 44 additions and 22 deletions

View file

@ -54,7 +54,10 @@ use crate::templater::TemplateFormatter;
use crate::ui::{ColorChoice, Ui};
pub enum CommandError {
UserError(String),
UserError {
message: String,
hint: Option<String>,
},
ConfigError(String),
/// Invalid command line
CliError(String),
@ -65,7 +68,16 @@ pub enum CommandError {
}
pub fn user_error(message: impl Into<String>) -> CommandError {
CommandError::UserError(message.into())
CommandError::UserError {
message: message.into(),
hint: None,
}
}
pub fn user_error_with_hint(message: impl Into<String>, hint: impl Into<String>) -> CommandError {
CommandError::UserError {
message: message.into(),
hint: Some(hint.into()),
}
}
impl From<std::io::Error> for CommandError {
@ -228,16 +240,18 @@ impl CommandHelper {
Workspace::load(ui.settings(), &wc_path, &self.backend_factories).map_err(|err| {
match err {
WorkspaceLoadError::NoWorkspaceHere(wc_path) => {
let mut message = format!("There is no jj repo in \"{}\"", wc_path_str);
let message = format!("There is no jj repo in \"{}\"", wc_path_str);
let git_dir = wc_path.join(".git");
if git_dir.is_dir() {
// TODO: Make this hint separate from the error, so the caller can
// format it differently.
message += "
It looks like this is a git repo. You can create a jj repo backed by it by running this:
jj init --git-repo=.";
user_error_with_hint(
message,
"It looks like this is a git repo. You can create a jj repo \
backed by it by running this:
jj init --git-repo=.",
)
} else {
user_error(message)
}
user_error(message)
}
WorkspaceLoadError::RepoDoesNotExist(repo_dir) => user_error(format!(
"The repository directory at {} is missing. Was it moved?",
@ -355,14 +369,15 @@ impl WorkspaceCommandHelper {
fn check_working_copy_writable(&self) -> Result<(), CommandError> {
if self.may_update_working_copy {
Ok(())
} else if self.global_args.no_commit_working_copy {
Err(user_error(
"This command must be able to update the working copy (don't use \
--no-commit-working-copy).",
))
} else {
Err(user_error(
"This command must be able to update the working copy (don't use --at-op).",
let hint = if self.global_args.no_commit_working_copy {
"Don't use --no-commit-working-copy."
} else {
"Don't use --at-op."
};
Err(user_error_with_hint(
"This command must be able to update the working copy.",
hint,
))
}
}
@ -1329,8 +1344,11 @@ pub fn parse_args(
pub fn handle_command_result(ui: &mut Ui, result: Result<(), CommandError>) -> i32 {
match result {
Ok(()) => 0,
Err(CommandError::UserError(message)) => {
Err(CommandError::UserError { message, hint }) => {
ui.write_error(&format!("Error: {}\n", message)).unwrap();
if let Some(hint) = hint {
ui.write_hint(&format!("Hint: {}\n", hint)).unwrap();
}
1
}
Err(CommandError::ConfigError(message)) => {

View file

@ -54,7 +54,8 @@ use pest::Parser;
use crate::cli_util::{
print_checkout_stats, resolve_base_revs, short_commit_description, short_commit_hash,
user_error, write_commit_summary, Args, CommandError, CommandHelper, WorkspaceCommandHelper,
user_error, user_error_with_hint, write_commit_summary, Args, CommandError, CommandHelper,
WorkspaceCommandHelper,
};
use crate::formatter::{Formatter, PlainTextFormatter};
use crate::graphlog::{AsciiGraphDrawer, Edge};
@ -1087,8 +1088,9 @@ fn cmd_init(ui: &mut Ui, command: &CommandHelper, args: &InitArgs) -> Result<(),
Workspace::init_internal_git(ui.settings(), &wc_path)?;
} else {
if !ui.settings().allow_native_backend() {
return Err(user_error(
"The native backend is disallowed by default. Did you mean to pass `--git`?
return Err(user_error_with_hint(
"The native backend is disallowed by default.",
"Did you mean to pass `--git`?
Set `ui.allow-init-native` to allow initializing a repo with the native backend.",
));
}

View file

@ -186,7 +186,8 @@ fn test_init_local_disallowed() {
let test_env = TestEnvironment::default();
let stdout = test_env.jj_cmd_failure(test_env.env_root(), &["init", "repo"]);
insta::assert_snapshot!(stdout, @r###"
Error: The native backend is disallowed by default. Did you mean to pass `--git`?
Error: The native backend is disallowed by default.
Hint: Did you mean to pass `--git`?
Set `ui.allow-init-native` to allow initializing a repo with the native backend.
"###);
}

View file

@ -46,7 +46,8 @@ fn test_untrack() {
// Errors out when not run at the head operation
let stderr = test_env.jj_cmd_failure(&repo_path, &["untrack", "file1", "--at-op", "@-"]);
insta::assert_snapshot!(stderr.replace("jj.exe", "jj"), @r###"
Error: This command must be able to update the working copy (don't use --at-op).
Error: This command must be able to update the working copy.
Hint: Don't use --at-op.
"###);
// Errors out when no path is specified
test_env.jj_cmd_cli_error(&repo_path, &["untrack"]);