ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

revsets: remove heads arguments from builder API functions

Browse source 
Now that we no longer have to be careful whether we mean "all heads"
or "non-obsolete heads", there's no need to pass them as
arguments. It's still possible to get a DAG range to a hidden commit
by using `RevsetExpression::dag_range_to()`, as long as the hidden
commit is indexed.
This commit is contained in:
Martin von Zweigbergk 2021-10-06 12:26:42 -07:00
parent 9ec27645cf
commit 41779551fb
3 changed files with 34 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

82
lib/src/revset.rs
View file

@ -173,10 +173,7 @@ pub enum RevsetExpression {
Commits(Vec<CommitId>), Commits(Vec<CommitId>),
Symbol(String), Symbol(String),
Parents(Rc<RevsetExpression>), Parents(Rc<RevsetExpression>),
Children { Children(Rc<RevsetExpression>),
roots: Rc<RevsetExpression>,
heads: Rc<RevsetExpression>,
},
Ancestors(Rc<RevsetExpression>), Ancestors(Rc<RevsetExpression>),
// Commits that are ancestors of "heads" but not ancestors of "roots" // Commits that are ancestors of "heads" but not ancestors of "roots"
Range { Range {
@ -264,19 +261,18 @@ impl RevsetExpression {
} }
/// Children of `self`. /// Children of `self`.
pub fn children( pub fn children(self: &Rc<RevsetExpression>) -> Rc<RevsetExpression> {
self: &Rc<RevsetExpression>, Rc::new(RevsetExpression::Children(self.clone()))
heads: &Rc<RevsetExpression>, }
) -> Rc<RevsetExpression> {
Rc::new(RevsetExpression::Children { /// Descendants of `self`.
roots: self.clone(), pub fn descendants(self: &Rc<RevsetExpression>) -> Rc<RevsetExpression> {
heads: heads.clone(), self.dag_range_to(&RevsetExpression::heads())
})
} }
/// Commits that are descendants of `self` and ancestors of `heads`, both /// Commits that are descendants of `self` and ancestors of `heads`, both
/// inclusive. /// inclusive.
pub fn descendants( pub fn dag_range_to(
self: &Rc<RevsetExpression>, self: &Rc<RevsetExpression>,
heads: &Rc<RevsetExpression>, heads: &Rc<RevsetExpression>,
) -> Rc<RevsetExpression> { ) -> Rc<RevsetExpression> {
@ -404,12 +400,12 @@ fn parse_range_expression_rule(
if let Some(next) = pairs.next() { if let Some(next) = pairs.next() {
match next.as_rule() { match next.as_rule() {
Rule::descendants_op => { Rule::descendants_op => {
expression = expression.descendants(&RevsetExpression::heads()); expression = expression.descendants();
} }
Rule::dag_range_op => { Rule::dag_range_op => {
let heads_expression = let heads_expression =
parse_children_expression_rule(pairs.next().unwrap().into_inner())?; parse_children_expression_rule(pairs.next().unwrap().into_inner())?;
expression = expression.descendants(&heads_expression); expression = expression.dag_range_to(&heads_expression);
} }
Rule::range_op => { Rule::range_op => {
let expression2 = let expression2 =
@ -431,7 +427,7 @@ fn parse_children_expression_rule(
for operator in pairs { for operator in pairs {
match operator.as_rule() { match operator.as_rule() {
Rule::children_op => { Rule::children_op => {
expression = expression.children(&RevsetExpression::heads()); expression = expression.children();
} }
_ => { _ => {
panic!( panic!(
@ -517,7 +513,7 @@ fn parse_function_expression(
if arg_count == 1 { if arg_count == 1 {
let expression = let expression =
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?; parse_expression_rule(argument_pairs.next().unwrap().into_inner())?;
Ok(expression.children(&RevsetExpression::heads())) Ok(expression.children())
} else { } else {
Err(RevsetParseError::InvalidFunctionArguments { Err(RevsetParseError::InvalidFunctionArguments {
name, name,
@ -539,7 +535,7 @@ fn parse_function_expression(
if arg_count == 1 { if arg_count == 1 {
let expression = let expression =
parse_expression_rule(argument_pairs.next().unwrap().into_inner())?; parse_expression_rule(argument_pairs.next().unwrap().into_inner())?;
Ok(expression.descendants(&RevsetExpression::heads())) Ok(expression.descendants())
} else { } else {
Err(RevsetParseError::InvalidFunctionArguments { Err(RevsetParseError::InvalidFunctionArguments {
name, name,
@ -975,9 +971,9 @@ pub fn evaluate_expression<'repo>(
index_entries: parent_entries, index_entries: parent_entries,
})) }))
} }
RevsetExpression::Children { roots, heads } => { RevsetExpression::Children(roots) => {
let root_set = roots.evaluate(repo)?; let root_set = roots.evaluate(repo)?;
let candidates_expression = heads.ancestors(); let candidates_expression = roots.descendants();
let candidate_set = candidates_expression.evaluate(repo)?; let candidate_set = candidates_expression.evaluate(repo)?;
Ok(Box::new(ChildrenRevset { Ok(Box::new(ChildrenRevset {
root_set, root_set,
@ -1198,17 +1194,21 @@ mod tests {
Rc::new(RevsetExpression::Ancestors(checkout_symbol.clone())) Rc::new(RevsetExpression::Ancestors(checkout_symbol.clone()))
); );
assert_eq!( assert_eq!(
foo_symbol.children(&checkout_symbol), foo_symbol.children(),
Rc::new(RevsetExpression::Children { Rc::new(RevsetExpression::Children(foo_symbol.clone()))
);
assert_eq!(
foo_symbol.descendants(),
Rc::new(RevsetExpression::DagRange {
roots: foo_symbol.clone(), roots: foo_symbol.clone(),
heads: checkout_symbol.clone() heads: RevsetExpression::heads(),
}) })
); );
assert_eq!( assert_eq!(
foo_symbol.descendants(&checkout_symbol), foo_symbol.dag_range_to(&checkout_symbol),
Rc::new(RevsetExpression::DagRange { Rc::new(RevsetExpression::DagRange {
roots: foo_symbol.clone(), roots: foo_symbol.clone(),
heads: checkout_symbol.clone() heads: checkout_symbol.clone(),
}) })
); );
assert_eq!( assert_eq!(
@ -1271,19 +1271,13 @@ mod tests {
// Parse the "parents" operator // Parse the "parents" operator
assert_eq!(parse(":@"), Ok(checkout_symbol.parents())); assert_eq!(parse(":@"), Ok(checkout_symbol.parents()));
// Parse the "children" operator // Parse the "children" operator
assert_eq!( assert_eq!(parse("@:"), Ok(checkout_symbol.children()));
parse("@:"),
Ok(checkout_symbol.children(&RevsetExpression::heads()))
);
// Parse the "ancestors" operator // Parse the "ancestors" operator
assert_eq!(parse(",,@"), Ok(checkout_symbol.ancestors())); assert_eq!(parse(",,@"), Ok(checkout_symbol.ancestors()));
// Parse the "descendants" operator // Parse the "descendants" operator
assert_eq!( assert_eq!(parse("@,,"), Ok(checkout_symbol.descendants()));
parse("@,,"),
Ok(checkout_symbol.descendants(&RevsetExpression::heads()))
);
// Parse the "dag range" operator // Parse the "dag range" operator
assert_eq!(parse("foo,,bar"), Ok(foo_symbol.descendants(&bar_symbol))); assert_eq!(parse("foo,,bar"), Ok(foo_symbol.dag_range_to(&bar_symbol)));
// Parse the "intersection" operator // Parse the "intersection" operator
assert_eq!(parse("foo & bar"), Ok(foo_symbol.intersection(&bar_symbol))); assert_eq!(parse("foo & bar"), Ok(foo_symbol.intersection(&bar_symbol)));
// Parse the "union" operator // Parse the "union" operator
@ -1319,10 +1313,7 @@ mod tests {
// Parse repeated "children" operator // Parse repeated "children" operator
assert_eq!( assert_eq!(
parse("foo:::"), parse("foo:::"),
Ok(foo_symbol Ok(foo_symbol.children().children().children())
.children(&RevsetExpression::heads())
.children(&RevsetExpression::heads())
.children(&RevsetExpression::heads()))
); );
// Parse repeated "ancestors"/"descendants"/"dag range" operators // Parse repeated "ancestors"/"descendants"/"dag range" operators
assert_matches!(parse(",,foo,,"), Err(RevsetParseError::SyntaxError(_))); assert_matches!(parse(",,foo,,"), Err(RevsetParseError::SyntaxError(_)));
@ -1333,18 +1324,9 @@ mod tests {
assert_matches!(parse("foo,,bar,,"), Err(RevsetParseError::SyntaxError(_))); assert_matches!(parse("foo,,bar,,"), Err(RevsetParseError::SyntaxError(_)));
// Parse combinations of "parents"/"children" operators and the range operators. // Parse combinations of "parents"/"children" operators and the range operators.
// The former bind more strongly. // The former bind more strongly.
assert_eq!( assert_eq!(parse(":foo:"), Ok(foo_symbol.parents().children()));
parse(":foo:"), assert_eq!(parse(":foo,,"), Ok(foo_symbol.parents().descendants()));
Ok(foo_symbol.parents().children(&RevsetExpression::heads())) assert_eq!(parse(",,foo:"), Ok(foo_symbol.children().ancestors()));
);
assert_eq!(
parse(":foo,,"),
Ok(foo_symbol.parents().descendants(&RevsetExpression::heads()))
);
assert_eq!(
parse(",,foo:"),
Ok(foo_symbol.children(&RevsetExpression::heads()).ancestors())
);
} }
#[test] #[test]

2
lib/src/rewrite.rs
View file

@ -138,7 +138,7 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> {
let new_commits_expression = let new_commits_expression =
RevsetExpression::commits(rewritten.values().flatten().cloned().collect()); RevsetExpression::commits(rewritten.values().flatten().cloned().collect());
let to_visit_expression = old_commits_expression.descendants(&RevsetExpression::heads()); let to_visit_expression = old_commits_expression.descendants();
let to_visit_revset = to_visit_expression let to_visit_revset = to_visit_expression
.evaluate(mut_repo.as_repo_ref()) .evaluate(mut_repo.as_repo_ref())
.unwrap(); .unwrap();

3
src/commands.rs
View file

@ -2543,8 +2543,7 @@ fn cmd_rebase(
// Manually rebase children because we don't want to rebase them onto the // Manually rebase children because we don't want to rebase them onto the
// rewritten commit. (But we still want to record the commit as rewritten so // rewritten commit. (But we still want to record the commit as rewritten so
// branches and the working copy get updated to the rewritten commit.) // branches and the working copy get updated to the rewritten commit.)
let children_expression = let children_expression = RevsetExpression::commit(old_commit.id().clone()).children();
RevsetExpression::commit(old_commit.id().clone()).children(&RevsetExpression::heads());
let mut num_rebased_descendants = 0; let mut num_rebased_descendants = 0;
let store = repo_command.repo.store(); let store = repo_command.repo.store();
for child_index_entry in children_expression for child_index_entry in children_expression