cli: provide hint for typo of revset function name

This is similar to what Mercurial does. The similarity threshold is copied
from clap, but we might want to adjust it later.
This commit is contained in:
Yuya Nishihara 2023-03-23 18:03:01 +09:00
parent d3d8afc77b
commit ddeb645d7f
5 changed files with 96 additions and 5 deletions

1
Cargo.lock generated
View file

@ -825,6 +825,7 @@ dependencies = [
"rpassword",
"serde",
"slab",
"strsim",
"tempfile",
"testutils",
"textwrap 0.16.0",

View file

@ -55,6 +55,7 @@ regex = "1.7.2"
rpassword = "7.2.0"
serde = { version = "1.0", features = ["derive"] }
slab = "0.4.8"
strsim = "0.10.0"
tempfile = "3.4.0"
textwrap = "0.16.0"
timeago = { version = "0.4.1", default-features = false }

View file

@ -74,8 +74,11 @@ pub enum RevsetParseErrorKind {
similar_op: String,
description: String,
},
#[error("Revset function \"{0}\" doesn't exist")]
NoSuchFunction(String),
#[error(r#"Revset function "{name}" doesn't exist"#)]
NoSuchFunction {
name: String,
candidates: Vec<String>,
},
#[error("Invalid arguments to revset function \"{name}\": {message}")]
InvalidFunctionArguments { name: String, message: String },
#[error("Invalid file pattern: {0}")]
@ -715,12 +718,26 @@ fn parse_function_expression(
func(name, arguments_pair, state)
} else {
Err(RevsetParseError::with_span(
RevsetParseErrorKind::NoSuchFunction(name.to_owned()),
RevsetParseErrorKind::NoSuchFunction {
name: name.to_owned(),
candidates: collect_function_names(state.aliases_map),
},
name_pair.as_span(),
))
}
}
fn collect_function_names(aliases_map: &RevsetAliasesMap) -> Vec<String> {
let mut names = BUILTIN_FUNCTION_MAP
.keys()
.map(|&n| n.to_owned())
.collect_vec();
names.extend(aliases_map.function_aliases.keys().map(|n| n.to_owned()));
names.sort_unstable();
names.dedup();
names
}
type RevsetFunction =
fn(&str, Pair<Rule>, ParseState) -> Result<Rc<RevsetExpression>, RevsetParseError>;

View file

@ -45,7 +45,7 @@ use jujutsu_lib::repo::{
use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};
use jujutsu_lib::revset::{
resolve_symbols, Revset, RevsetAliasesMap, RevsetError, RevsetExpression, RevsetIteratorExt,
RevsetParseError, RevsetWorkspaceContext,
RevsetParseError, RevsetParseErrorKind, RevsetWorkspaceContext,
};
use jujutsu_lib::settings::UserSettings;
use jujutsu_lib::transaction::Transaction;
@ -97,6 +97,29 @@ pub fn user_error_with_hint(message: impl Into<String>, hint: impl Into<String>)
}
}
fn collect_similar<'a, S: AsRef<str>>(name: &str, candidates: &'a [S]) -> Vec<&'a S> {
candidates
.iter()
.filter_map(|cand| {
// The parameter is borrowed from clap f5540d26
(strsim::jaro(name, cand.as_ref()) > 0.7).then_some(cand)
})
.collect_vec()
}
fn format_similarity_hint<S: AsRef<str>>(candidates: &[S]) -> Option<String> {
match candidates {
[] => None,
names => {
let quoted_names = names
.iter()
.map(|s| format!(r#""{}""#, s.as_ref()))
.join(", ");
Some(format!("Did you mean {quoted_names}?"))
}
}
}
impl From<std::io::Error> for CommandError {
fn from(err: std::io::Error) -> Self {
if err.kind() == std::io::ErrorKind::BrokenPipe {
@ -229,7 +252,17 @@ impl From<GitExportError> for CommandError {
impl From<RevsetParseError> for CommandError {
fn from(err: RevsetParseError) -> Self {
let message = iter::successors(Some(&err), |e| e.origin()).join("\n");
user_error(format!("Failed to parse revset: {message}"))
// Only for the top-level error as we can't attach hint to inner errors
let hint = match err.kind() {
RevsetParseErrorKind::NoSuchFunction { name, candidates } => {
format_similarity_hint(&collect_similar(name, candidates))
}
_ => None,
};
CommandError::UserError {
message: format!("Failed to parse revset: {message}"),
hint,
}
}
}

View file

@ -184,6 +184,45 @@ fn test_bad_function_call() {
"###);
}
#[test]
fn test_function_name_hint() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_success(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
let evaluate_err = |expr| test_env.jj_cmd_failure(&repo_path, &["log", "-r", expr]);
test_env.add_config(
r###"
[revset-aliases]
'branches(x)' = 'x' # override builtin function
'my_author(x)' = 'author(x)' # similar name to builtin function
'author_sym' = 'x' # not a function alias
"###,
);
// The suggestion "branches" shouldn't be duplicated
insta::assert_snapshot!(evaluate_err("branch()"), @r###"
Error: Failed to parse revset: --> 1:1
|
1 | branch()
| ^----^
|
= Revset function "branch" doesn't exist
Hint: Did you mean "branches"?
"###);
// Both builtin function and function alias should be suggested
insta::assert_snapshot!(evaluate_err("author_()"), @r###"
Error: Failed to parse revset: --> 1:1
|
1 | author_()
| ^-----^
|
= Revset function "author_" doesn't exist
Hint: Did you mean "author", "my_author"?
"###);
}
#[test]
fn test_alias() {
let test_env = TestEnvironment::default();