feat(revset): suggest similar branch names

This commit is contained in:
Waleed Khan 2023-06-04 23:35:34 -05:00
parent f61cbae022
commit 23a4124d20
3 changed files with 98 additions and 21 deletions

View file

@ -41,8 +41,11 @@ use crate::store::Store;
/// Error occurred during symbol resolution.
#[derive(Debug, Error)]
pub enum RevsetResolutionError {
#[error("Revision \"{0}\" doesn't exist")]
NoSuchRevision(String),
#[error("Revision \"{name}\" doesn't exist")]
NoSuchRevision {
name: String,
candidates: Vec<String>,
},
#[error("An empty string is not a valid revision")]
EmptyString,
#[error("Commit ID prefix \"{0}\" is ambiguous")]
@ -1670,10 +1673,13 @@ pub struct FailingSymbolResolver;
impl SymbolResolver for FailingSymbolResolver {
fn resolve_symbol(&self, symbol: &str) -> Result<Vec<CommitId>, RevsetResolutionError> {
Err(RevsetResolutionError::NoSuchRevision(format!(
"Won't resolve symbol {symbol:?}. When creating revsets programmatically, avoid using \
RevsetExpression::symbol(); use RevsetExpression::commits() instead."
)))
Err(RevsetResolutionError::NoSuchRevision {
name: format!(
"Won't resolve symbol {symbol:?}. When creating revsets programmatically, avoid \
using RevsetExpression::symbol(); use RevsetExpression::commits() instead."
),
candidates: Default::default(),
})
}
}
@ -1722,7 +1728,10 @@ impl SymbolResolver for DefaultSymbolResolver<'_> {
if let Some(workspace_id) = self.workspace_id {
workspace_id.clone()
} else {
return Err(RevsetResolutionError::NoSuchRevision(symbol.to_owned()));
return Err(RevsetResolutionError::NoSuchRevision {
name: symbol.to_owned(),
candidates: Default::default(),
});
}
} else {
WorkspaceId::new(symbol.strip_suffix('@').unwrap().to_string())
@ -1730,7 +1739,10 @@ impl SymbolResolver for DefaultSymbolResolver<'_> {
if let Some(commit_id) = self.repo.view().get_wc_commit_id(&target_workspace) {
Ok(vec![commit_id.clone()])
} else {
Err(RevsetResolutionError::NoSuchRevision(symbol.to_owned()))
Err(RevsetResolutionError::NoSuchRevision {
name: symbol.to_owned(),
candidates: Default::default(),
})
}
} else if symbol == "root" {
Ok(vec![self.repo.store().root_commit_id().clone()])
@ -1791,7 +1803,13 @@ impl SymbolResolver for DefaultSymbolResolver<'_> {
}
}
Err(RevsetResolutionError::NoSuchRevision(symbol.to_owned()))
Err(RevsetResolutionError::NoSuchRevision {
name: symbol.to_owned(),
candidates: {
let branch_names = self.repo.view().branches().keys().collect_vec();
collect_similar(symbol, &branch_names)
},
})
}
}
}
@ -1869,7 +1887,9 @@ fn resolve_symbols(
RevsetExpression::Present(candidates) => {
resolve_symbols(repo, candidates.clone(), symbol_resolver)
.or_else(|err| match err {
RevsetResolutionError::NoSuchRevision(_) => Ok(RevsetExpression::none()),
RevsetResolutionError::NoSuchRevision { .. } => {
Ok(RevsetExpression::none())
}
RevsetResolutionError::EmptyString
| RevsetResolutionError::AmbiguousCommitIdPrefix(_)
| RevsetResolutionError::AmbiguousChangeIdPrefix(_)

View file

@ -21,7 +21,7 @@ use jujutsu_lib::commit::Commit;
use jujutsu_lib::git;
use jujutsu_lib::git_backend::GitBackend;
use jujutsu_lib::index::{HexPrefix, PrefixResolution};
use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
use jujutsu_lib::op_store::{BranchTarget, RefTarget, WorkspaceId};
use jujutsu_lib::repo::Repo;
use jujutsu_lib::repo_path::RepoPath;
use jujutsu_lib::revset::{
@ -174,13 +174,13 @@ fn test_resolve_symbol_commit_id() {
);
assert_matches!(
resolve_symbol(repo.as_ref(), "040", None),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "040"
Err(RevsetResolutionError::NoSuchRevision{name, candidates}) if name == "040" && candidates.is_empty()
);
// Test non-hex string
assert_matches!(
resolve_symbol(repo.as_ref(), "foo", None),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "foo"
Err(RevsetResolutionError::NoSuchRevision{name, candidates}) if name == "foo" && candidates.is_empty()
);
// Test present() suppresses only NoSuchRevision error
@ -313,7 +313,7 @@ fn test_resolve_symbol_change_id(readonly: bool) {
);
assert_matches!(
resolve_symbol(repo, "zvlyw", None),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "zvlyw"
Err(RevsetResolutionError::NoSuchRevision{name, candidates}) if name == "zvlyw" && candidates.is_empty()
);
// Test that commit and changed id don't conflict ("040" and "zvz" are the
@ -334,7 +334,10 @@ fn test_resolve_symbol_change_id(readonly: bool) {
// Test non-hex string
assert_matches!(
resolve_symbol(repo, "foo", None),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "foo"
Err(RevsetResolutionError::NoSuchRevision{
name,
candidates
}) if name == "foo" && candidates.is_empty()
);
}
@ -357,15 +360,24 @@ fn test_resolve_symbol_checkout(use_git: bool) {
// With no workspaces, no variation can be resolved
assert_matches!(
resolve_symbol(mut_repo, "@", None),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "@"
Err(RevsetResolutionError::NoSuchRevision{
name,
candidates,
}) if name == "@" && candidates.is_empty()
);
assert_matches!(
resolve_symbol(mut_repo, "@", Some(&ws1)),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "@"
Err(RevsetResolutionError::NoSuchRevision{
name,
candidates,
}) if name == "@" && candidates.is_empty()
);
assert_matches!(
resolve_symbol(mut_repo, "ws1@", Some(&ws1)),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "ws1@"
Err(RevsetResolutionError::NoSuchRevision{
name,
candidates,
}) if name == "ws1@" && candidates.is_empty()
);
// Add some workspaces
@ -376,7 +388,10 @@ fn test_resolve_symbol_checkout(use_git: bool) {
// @ cannot be resolved without a default workspace ID
assert_matches!(
resolve_symbol(mut_repo, "@", None),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "@"
Err(RevsetResolutionError::NoSuchRevision{
name,
candidates,
}) if name == "@" && candidates.is_empty()
);
// Can resolve "@" shorthand with a default workspace ID
assert_eq!(
@ -432,7 +447,7 @@ fn test_resolve_symbol_git_refs() {
// Nonexistent ref
assert_matches!(
resolve_symbol(mut_repo, "nonexistent", None),
Err(RevsetResolutionError::NoSuchRevision(s)) if s == "nonexistent"
Err(RevsetResolutionError::NoSuchRevision{name, candidates}) if name == "nonexistent" && candidates.is_empty()
);
// Full ref
@ -2484,3 +2499,31 @@ fn test_change_id_index() {
);
assert_eq!(resolve_prefix("a"), PrefixResolution::AmbiguousMatch);
}
#[test]
fn test_no_such_revision_suggestion() {
let settings = testutils::user_settings();
let test_repo = TestRepo::init(true);
let repo = &test_repo.repo;
let mut tx = repo.start_transaction(&settings, "test");
let mut_repo = tx.mut_repo();
let commit = write_random_commit(mut_repo, &settings);
for branch_name in ["foo", "bar", "baz"] {
mut_repo.set_branch(
branch_name.to_string(),
BranchTarget {
local_target: Some(RefTarget::Normal(commit.id().clone())),
remote_targets: Default::default(),
},
);
}
assert_matches!(resolve_symbol(mut_repo, "bar", None), Ok(_));
assert_matches!(
resolve_symbol(mut_repo, "bax", None),
Err(RevsetResolutionError::NoSuchRevision { name, candidates })
if name == "bax" && candidates == vec!["bar".to_string(), "baz".to_string()]
);
}

View file

@ -279,7 +279,21 @@ impl From<RevsetParseError> for CommandError {
impl From<RevsetResolutionError> for CommandError {
fn from(err: RevsetResolutionError) -> Self {
user_error(format!("{err}"))
let hint = match &err {
RevsetResolutionError::NoSuchRevision {
name: _,
candidates,
} => format_similarity_hint(candidates),
RevsetResolutionError::EmptyString
| RevsetResolutionError::AmbiguousCommitIdPrefix(_)
| RevsetResolutionError::AmbiguousChangeIdPrefix(_)
| RevsetResolutionError::StoreError(_) => None,
};
CommandError::UserError {
message: format!("{err}"),
hint,
}
}
}