From c04f418e678885253c9beb55b7916459be2992fb Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 17 Apr 2021 15:25:52 -0700 Subject: [PATCH] revsets: add difference operator --- lib/src/revset.pest | 12 +++-- lib/src/revset.rs | 103 +++++++++++++++++++++++++++++++++++---- lib/tests/test_revset.rs | 83 +++++++++++++++++++++++++++++++ 3 files changed, 185 insertions(+), 13 deletions(-) diff --git a/lib/src/revset.pest b/lib/src/revset.pest index df370fb74..1c77a79f8 100644 --- a/lib/src/revset.pest +++ b/lib/src/revset.pest @@ -19,6 +19,9 @@ parents = { ":" } ancestors = { "*:" } prefix_operator = _{ parents | ancestors } +difference = { "-" } +infix_operator = _{ difference } + function_name = @{ (ASCII_ALPHANUMERIC | "_")+ } // The grammar accepts a string literal or an expression for function // arguments. We then decide when walking the parse tree if we @@ -36,7 +39,6 @@ function_arguments = { | "" } - primary = { function_name ~ "(" ~ function_arguments ~ ")" | "(" ~ expression ~ ")" @@ -45,6 +47,10 @@ primary = { prefix_expression = { prefix_operator* ~ primary } -expression = { - prefix_expression +infix_expression = { + prefix_expression ~ (infix_operator ~ prefix_expression)* +} + +expression = { + infix_expression } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 70af6d4cf..3307ff318 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -12,8 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::cmp::Reverse; +use std::cmp::{Ordering, Reverse}; use std::collections::HashSet; +use std::iter::Peekable; use pest::iterators::Pairs; use pest::Parser; @@ -106,12 +107,13 @@ pub enum RevsetExpression { needle: String, base_expression: Box, }, + Difference(Box, Box), } fn parse_expression_rule(mut pairs: Pairs) -> Result { let first = pairs.next().unwrap(); match first.as_rule() { - Rule::prefix_expression => parse_prefix_expression_rule(first.into_inner()), + Rule::infix_expression => parse_infix_expression_rule(first.into_inner()), _ => { panic!( "unxpected revset parse rule {:?} in: {:?}", @@ -122,6 +124,28 @@ fn parse_expression_rule(mut pairs: Pairs) -> Result, +) -> Result { + let mut expression1 = parse_prefix_expression_rule(pairs.next().unwrap().into_inner())?; + while let Some(operator) = pairs.next() { + let expression2 = parse_prefix_expression_rule(pairs.next().unwrap().into_inner())?; + match operator.as_rule() { + Rule::difference => { + expression1 = + RevsetExpression::Difference(Box::new(expression1), Box::new(expression2)) + } + _ => { + panic!( + "unxpected revset infix operator rule {:?}", + operator.as_rule() + ); + } + } + } + Ok(expression1) +} + fn parse_prefix_expression_rule( mut pairs: Pairs, ) -> Result { @@ -294,12 +318,15 @@ fn parse_function_argument_to_string( } Rule::expression => { let first = first.into_inner().next().unwrap(); - if first.as_rule() == Rule::prefix_expression { + if first.as_rule() == Rule::infix_expression { let first = first.into_inner().next().unwrap(); - if first.as_rule() == Rule::primary { + if first.as_rule() == Rule::prefix_expression { let first = first.into_inner().next().unwrap(); - if first.as_rule() == Rule::symbol { - return Ok(first.as_str().to_owned()); + if first.as_rule() == Rule::primary { + let first = first.into_inner().next().unwrap(); + if first.as_rule() == Rule::symbol { + return Ok(first.as_str().to_owned()); + } } } } @@ -331,6 +358,7 @@ pub fn parse(revset_str: &str) -> Result { } pub trait Revset<'repo> { + // All revsets currently iterate in order of descending index position fn iter<'revset>(&'revset self) -> Box> + 'revset>; } @@ -368,10 +396,60 @@ impl<'repo> Iterator for RevWalkRevsetIterator<'repo> { } } -pub fn evaluate_expression<'repo>( +struct DifferenceRevset<'revset, 'repo: 'revset> { + // The minuend (what to subtract from) + set1: Box + 'revset>, + // The subtrahend (what to subtract) + set2: Box + 'revset>, +} + +impl<'repo> Revset<'repo> for DifferenceRevset<'_, 'repo> { + fn iter<'revset>(&'revset self) -> Box> + 'revset> { + Box::new(DifferenceRevsetIterator { + iter1: self.set1.iter().peekable(), + iter2: self.set2.iter().peekable(), + }) + } +} + +struct DifferenceRevsetIterator<'revset, 'repo> { + iter1: Peekable> + 'revset>>, + iter2: Peekable> + 'revset>>, +} + +impl<'revset, 'repo> Iterator for DifferenceRevsetIterator<'revset, 'repo> { + type Item = IndexEntry<'repo>; + + fn next(&mut self) -> Option { + loop { + match (self.iter1.peek(), self.iter2.peek()) { + (None, _) => { + return None; + } + (_, None) => { + return self.iter1.next(); + } + (Some(entry1), Some(entry2)) => match entry1.position().cmp(&entry2.position()) { + Ordering::Less => { + self.iter2.next(); + } + Ordering::Equal => { + self.iter2.next(); + self.iter1.next(); + } + Ordering::Greater => { + return self.iter1.next(); + } + }, + } + } + } +} + +pub fn evaluate_expression<'revset, 'repo: 'revset>( repo: RepoRef<'repo>, expression: &RevsetExpression, -) -> Result + 'repo>, RevsetError> { +) -> Result + 'revset>, RevsetError> { match expression { RevsetExpression::Symbol(symbol) => { let commit_id = resolve_symbol(repo, &symbol)?.id().clone(); @@ -432,13 +510,18 @@ pub fn evaluate_expression<'repo>( index_entries.sort_by_key(|b| Reverse(b.position())); Ok(Box::new(EagerRevset { index_entries })) } + RevsetExpression::Difference(expression1, expression2) => { + let set1 = evaluate_expression(repo, expression1.as_ref())?; + let set2 = evaluate_expression(repo, expression2.as_ref())?; + Ok(Box::new(DifferenceRevset { set1, set2 })) + } } } -fn non_obsolete_heads<'repo>( +fn non_obsolete_heads<'revset, 'repo: 'revset>( repo: RepoRef<'repo>, heads: Box + 'repo>, -) -> Box + 'repo> { +) -> Box + 'revset> { let mut commit_ids = HashSet::new(); let mut work: Vec<_> = heads.iter().collect(); let evolution = repo.evolution(); diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index a1002aa11..c5b56a3be 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -560,3 +560,86 @@ fn test_evaluate_expression_description(use_git: bool) { tx.discard(); } + +#[test_case(false ; "local store")] +#[test_case(true ; "git store")] +fn test_evaluate_expression_difference(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 root_commit = repo.store().root_commit(); + let commit1 = testutils::create_random_commit(&settings, &repo).write_to_repo(mut_repo); + let commit2 = testutils::create_random_commit(&settings, &repo) + .set_parents(vec![commit1.id().clone()]) + .write_to_repo(mut_repo); + let commit3 = testutils::create_random_commit(&settings, &repo) + .set_parents(vec![commit2.id().clone()]) + .write_to_repo(mut_repo); + let commit4 = testutils::create_random_commit(&settings, &repo) + .set_parents(vec![commit3.id().clone()]) + .write_to_repo(mut_repo); + let commit5 = testutils::create_random_commit(&settings, &repo) + .set_parents(vec![commit2.id().clone()]) + .write_to_repo(mut_repo); + + // Difference between ancestors + assert_eq!( + resolve_commit_ids( + mut_repo.as_repo_ref(), + &format!("*:{}-*:{}", commit4.id().hex(), commit5.id().hex()) + ), + vec![commit4.id().clone(), commit3.id().clone()] + ); + assert_eq!( + resolve_commit_ids( + mut_repo.as_repo_ref(), + &format!("*:{}-*:{}", commit5.id().hex(), commit4.id().hex()) + ), + vec![commit5.id().clone()] + ); + assert_eq!( + resolve_commit_ids( + mut_repo.as_repo_ref(), + &format!("*:{}-*:{}", commit4.id().hex(), commit2.id().hex()) + ), + vec![commit4.id().clone(), commit3.id().clone()] + ); + + // Associativity + assert_eq!( + resolve_commit_ids( + mut_repo.as_repo_ref(), + &format!( + "*:{}-{}-{}", + commit4.id().hex(), + commit2.id().hex(), + commit3.id().hex() + ) + ), + vec![ + commit4.id().clone(), + commit1.id().clone(), + root_commit.id().clone(), + ] + ); + + // Subtracting a difference does not add back any commits + assert_eq!( + resolve_commit_ids( + mut_repo.as_repo_ref(), + &format!( + "(*:{}-*:{})-(*:{}-*:{})", + commit4.id().hex(), + commit1.id().hex(), + commit3.id().hex(), + commit1.id().hex(), + ) + ), + vec![commit4.id().clone()] + ); + + tx.discard(); +}