From 62f07789422760e83879f6192baf16d0a6013958 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Fri, 16 Apr 2021 10:24:22 -0700 Subject: [PATCH] revsets: add a description() revset The revset is currently eagerly evaluated, which is clearly bad. We'll need to fix that later. --- lib/src/revset.rs | 88 ++++++++++++++++++++++++++++++++++++++++ lib/tests/test_revset.rs | 61 ++++++++++++++++++++++++++++ 2 files changed, 149 insertions(+) diff --git a/lib/src/revset.rs b/lib/src/revset.rs index bb2b94099..cad25fbe3 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -102,6 +102,10 @@ pub enum RevsetExpression { Ancestors(Box), AllHeads, NonObsoleteHeads(Box), + Description { + needle: String, + base_expression: Box, + }, } fn parse_expression_rule(mut pairs: Pairs) -> Result { @@ -196,6 +200,32 @@ fn parse_function_expression( }) } } + "description" => { + if !(1..=2).contains(&arg_count) { + return Err(RevsetParseError::InvalidFunctionArguments { + name, + message: "Expected 1 or 2 arguments".to_string(), + }); + } + let needle = parse_function_argument_to_string( + &name, + argument_pairs.next().unwrap().into_inner(), + )?; + let base_expression = if arg_count == 1 { + RevsetExpression::Ancestors(Box::new(RevsetExpression::NonObsoleteHeads(Box::new( + RevsetExpression::AllHeads, + )))) + } else { + parse_function_argument_to_expression( + &name, + argument_pairs.next().unwrap().into_inner(), + )? + }; + Ok(RevsetExpression::Description { + needle, + base_expression: Box::new(base_expression), + }) + } _ => Err(RevsetParseError::NoSuchFunction(name)), } } @@ -220,6 +250,42 @@ fn parse_function_argument_to_expression( } } +fn parse_function_argument_to_string( + name: &str, + mut pairs: Pairs, +) -> Result { + // Make a clone of the pairs for error messages + let pairs_clone = pairs.clone(); + let first = pairs.next().unwrap(); + assert!(pairs.next().is_none()); + match first.as_rule() { + Rule::literal_string => { + return Ok(first + .as_str() + .strip_prefix('"') + .unwrap() + .strip_suffix('"') + .unwrap() + .to_owned()) + } + Rule::expression => { + let mut expression_pairs = first.into_inner(); + let first = expression_pairs.next().unwrap(); + if first.as_rule() == Rule::symbol { + return Ok(first.as_str().to_owned()); + } + } + _ => {} + } + Err(RevsetParseError::InvalidFunctionArguments { + name: name.to_string(), + message: format!( + "Expected function argument of type string, found: {}", + pairs_clone.as_str() + ), + }) +} + pub fn parse(revset_str: &str) -> Result { let mut pairs: Pairs = RevsetParser::parse(Rule::expression, revset_str).unwrap(); let first = pairs.next().unwrap(); @@ -315,6 +381,28 @@ pub fn evaluate_expression<'repo>( let base_set = evaluate_expression(repo, base_expression.as_ref())?; Ok(non_obsolete_heads(repo, base_set)) } + RevsetExpression::Description { + needle, + base_expression, + } => { + // TODO: Definitely make this lazy. We should have a common way of defining + // revsets that simply filter a base revset. + let base_set = evaluate_expression(repo, base_expression.as_ref())?; + let mut commit_ids = vec![]; + for entry in base_set.iter() { + let commit = repo.store().get_commit(&entry.commit_id()).unwrap(); + if commit.description().contains(needle.as_str()) { + commit_ids.push(entry.commit_id()); + } + } + let index = repo.index(); + let mut index_entries: Vec<_> = commit_ids + .iter() + .map(|id| index.entry_by_id(id).unwrap()) + .collect(); + index_entries.sort_by_key(|b| Reverse(b.position())); + Ok(Box::new(EagerRevset { index_entries })) + } } } diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index e2840ba50..35f608bda 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -281,6 +281,20 @@ fn test_parse_revset_function() { message: "Expected 1 argument".to_string() }) ); + assert_eq!( + parse("description(foo,bar)"), + Ok(RevsetExpression::Description { + needle: "foo".to_string(), + base_expression: Box::new(RevsetExpression::Symbol("bar".to_string())) + }) + ); + assert_eq!( + parse("description(foo(),bar)"), + Err(RevsetParseError::InvalidFunctionArguments { + name: "description".to_string(), + message: "Expected function argument of type string, found: foo()".to_string() + }) + ); } fn resolve_commit_ids(repo: RepoRef, revset_str: &str) -> Vec { @@ -469,3 +483,50 @@ fn test_evaluate_expression_obsolete(use_git: bool) { tx.discard(); } + +#[test_case(false ; "local store")] +#[test_case(true ; "git store")] +fn test_evaluate_expression_description(use_git: bool) { + let settings = testutils::user_settings(); + let (_temp_dir, repo) = testutils::init_repo(&settings, use_git); + + let mut tx = repo.start_transaction("test"); + let mut_repo = tx.mut_repo(); + + let commit1 = testutils::create_random_commit(&settings, &repo) + .set_description("commit 1".to_string()) + .write_to_repo(mut_repo); + let commit2 = testutils::create_random_commit(&settings, &repo) + .set_parents(vec![commit1.id().clone()]) + .set_description("commit 2".to_string()) + .write_to_repo(mut_repo); + let commit3 = testutils::create_random_commit(&settings, &repo) + .set_parents(vec![commit2.id().clone()]) + .set_description("commit 3".to_string()) + .write_to_repo(mut_repo); + + // Can find multiple matches + assert_eq!( + resolve_commit_ids(mut_repo.as_repo_ref(), "description(commit)"), + vec![ + commit3.id().clone(), + commit2.id().clone(), + commit1.id().clone() + ] + ); + // Can find a unique match + assert_eq!( + resolve_commit_ids(mut_repo.as_repo_ref(), "description(\"commit 2\")"), + vec![commit2.id().clone()] + ); + // Searches only in given base set if specified + assert_eq!( + resolve_commit_ids( + mut_repo.as_repo_ref(), + "description(\"commit 2\",all_heads())" + ), + vec![] + ); + + tx.discard(); +}