From 145731ec740756b46cbb9b3fdc0435fec222141e Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 21 Apr 2021 23:34:20 -0700 Subject: [PATCH] revsets: change operators around a bit to prepare for infix DAG range operator I really liked the idea of having the operators for parents and ancestors (etc.) look similar, but that turned out to be problematic when we want to add an infix operator for a DAG range (hg's `::` revset operator and git's `--ancestry-path` flag). Let's say we chose `:*:` as the operator. Part of the problem is how to parse `foo:*:bar` without eagerly parsing the `foo:`. It would also be nicer to use exactly the same operator as prefix, postfix, and infix. Since the "parents" operator can be repeated, we can't have it be just `:` and the "ancestors" operator be `::`. We could make the "ancestors" operator be something like `*:*` (or anything symmetric with the `:` symbol on the inside). However, at that point, the operator is getting ugly and hard to type. Another option would be to use `:` for ancestors and `::` for parents, but that is counterintuitive and get annoying if you want to repeat it. So it seems that the best option is to simply pick different symbols for parents/children and ancestors/descendants/range. This patch changes the ancestors/descendants operators to both be `,,`. I'm not at all attached to that particular symbol. I suspect we'll change it later. --- lib/src/revset.pest | 6 +++--- lib/src/revset.rs | 16 ++++++++-------- lib/tests/test_revset.rs | 35 +++++++++++++++++++++-------------- src/commands.rs | 2 +- 4 files changed, 33 insertions(+), 26 deletions(-) diff --git a/lib/src/revset.pest b/lib/src/revset.pest index c0703c4f9..601fe8dc3 100644 --- a/lib/src/revset.pest +++ b/lib/src/revset.pest @@ -21,12 +21,12 @@ literal_string = { "\"" ~ (!"\"" ~ ANY)+ ~ "\"" } whitespace = _{ " " } parents_op = { ":" } -ancestors_op = { "*:" } +ancestors_op = { ",," } prefix_op = _{ parents_op | ancestors_op } children_op = { ":" } -descendants_op = { ":*" } -postfix_op = _{ descendants_op | children_op } +descendants_op = { ",," } +postfix_op = _{ children_op | descendants_op } union_op = { "|" } intersection_op = { "&" } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 0136c8584..7cb5a260c 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -835,14 +835,14 @@ mod tests { ); // Parse the "ancestors" operator assert_eq!( - parse("*:@"), + parse(",,@"), Ok(RevsetExpression::Ancestors(Box::new( RevsetExpression::Symbol("@".to_string()) ))) ); // Parse the "descendants" operator assert_eq!( - parse("@:*"), + parse("@,,"), Ok(RevsetExpression::Descendants { base_expression: Box::new(RevsetExpression::Symbol("@".to_string())), candidates: RevsetExpression::non_obsolete_commits(), @@ -881,13 +881,13 @@ mod tests { ); // Space is allowed around expressions assert_eq!( - parse(" *:@ "), + parse(" ,,@ "), Ok(RevsetExpression::Ancestors(Box::new( RevsetExpression::Symbol("@".to_string()) ))) ); // Space is not allowed around prefix operators - assert_matches!(parse(" *: @ "), Err(RevsetParseError::SyntaxError(_))); + assert_matches!(parse(" ,, @ "), Err(RevsetParseError::SyntaxError(_))); // Incomplete parse assert_matches!(parse("foo | :"), Err(RevsetParseError::IncompleteParse(_))); // Space is allowed around infix operators and function arguments @@ -934,7 +934,7 @@ mod tests { }) ); // Parse combinations of prefix and postfix operators. They all currently have - // the same precedence, so "*:foo:" means "(*:foo):" and not "*:(foo:)". + // the same precedence, so ",,foo:" means "(,,foo):" and not ",,(foo:)". assert_eq!( parse(":foo:"), Ok(RevsetExpression::Children { @@ -945,7 +945,7 @@ mod tests { }) ); assert_eq!( - parse("*:foo:*"), + parse(",,foo,,"), Ok(RevsetExpression::Descendants { base_expression: Box::new(RevsetExpression::Ancestors(Box::new( RevsetExpression::Symbol("foo".to_string()) @@ -954,7 +954,7 @@ mod tests { }) ); assert_eq!( - parse(":foo:*"), + parse(":foo,,"), Ok(RevsetExpression::Descendants { base_expression: Box::new(RevsetExpression::Parents(Box::new( RevsetExpression::Symbol("foo".to_string()) @@ -963,7 +963,7 @@ mod tests { }) ); assert_eq!( - parse("*:foo:"), + parse(",,foo:"), Ok(RevsetExpression::Children { base_expression: Box::new(RevsetExpression::Ancestors(Box::new( RevsetExpression::Symbol("foo".to_string()) diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index f6c6490d8..cacb3a0d1 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -312,7 +312,7 @@ fn test_evaluate_expression_parents(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!(":*:{}", commit2.id().hex()) + &format!(":,,{}", commit2.id().hex()) ), vec![commit1.id().clone(), root_commit.id().clone()] ); @@ -419,14 +419,14 @@ fn test_evaluate_expression_ancestors(use_git: bool) { // The ancestors of the root commit is just the root commit itself assert_eq!( - resolve_commit_ids(mut_repo.as_repo_ref(), "*:root"), + resolve_commit_ids(mut_repo.as_repo_ref(), ",,root"), vec![root_commit.id().clone()] ); // Can find ancestors of a specific commit. Commits reachable via multiple paths // are not repeated. assert_eq!( - resolve_commit_ids(mut_repo.as_repo_ref(), &format!("*:{}", commit4.id().hex())), + resolve_commit_ids(mut_repo.as_repo_ref(), &format!(",,{}", commit4.id().hex())), vec![ commit4.id().clone(), commit3.id().clone(), @@ -472,7 +472,7 @@ fn test_evaluate_expression_descendants(use_git: bool) { // The descendants of the root commit is all the non-hidden commits in the repo // (commit2 is excluded) assert_eq!( - resolve_commit_ids(mut_repo.as_repo_ref(), "root:*"), + resolve_commit_ids(mut_repo.as_repo_ref(), "root,,"), vec![ commit6.id().clone(), commit5.id().clone(), @@ -486,7 +486,14 @@ fn test_evaluate_expression_descendants(use_git: bool) { // Can find ancestors of a specific commit assert_eq!( - resolve_commit_ids(mut_repo.as_repo_ref(), &format!("{}:*", commit3.id().hex())), + resolve_commit_ids( + mut_repo.as_repo_ref(), + &format!( + "{},,\ + ", + commit3.id().hex() + ) + ), vec![ commit6.id().clone(), commit4.id().clone(), @@ -676,7 +683,7 @@ fn test_evaluate_expression_union(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!("*:{} | *:{}", commit4.id().hex(), commit5.id().hex()) + &format!(",,{} | ,,{}", commit4.id().hex(), commit5.id().hex()) ), vec![ commit5.id().clone(), @@ -693,7 +700,7 @@ fn test_evaluate_expression_union(use_git: bool) { resolve_commit_ids( mut_repo.as_repo_ref(), &format!( - "(*:{} - *:{}) | *:{}", + "(,,{} - ,,{}) | ,,{}", commit4.id().hex(), commit2.id().hex(), commit5.id().hex() @@ -714,7 +721,7 @@ fn test_evaluate_expression_union(use_git: bool) { resolve_commit_ids( mut_repo.as_repo_ref(), &format!( - "(*:{} - *:{}) | {}", + "(,,{} - ,,{}) | {}", commit4.id().hex(), commit2.id().hex(), commit5.id().hex(), @@ -758,7 +765,7 @@ fn test_evaluate_expression_intersection(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!("*:{} & *:{}", commit4.id().hex(), commit5.id().hex()) + &format!(",,{} & ,,{}", commit4.id().hex(), commit5.id().hex()) ), vec![ commit2.id().clone(), @@ -807,21 +814,21 @@ fn test_evaluate_expression_difference(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!("*:{} - *:{}", commit4.id().hex(), commit5.id().hex()) + &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()) + &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()) + &format!(",,{} - ,,{}", commit4.id().hex(), commit2.id().hex()) ), vec![commit4.id().clone(), commit3.id().clone()] ); @@ -831,7 +838,7 @@ fn test_evaluate_expression_difference(use_git: bool) { resolve_commit_ids( mut_repo.as_repo_ref(), &format!( - "*:{} - {} - {}", + ",,{} - {} - {}", commit4.id().hex(), commit2.id().hex(), commit3.id().hex() @@ -849,7 +856,7 @@ fn test_evaluate_expression_difference(use_git: bool) { resolve_commit_ids( mut_repo.as_repo_ref(), &format!( - "(*:{} - *:{}) - (*:{} - *:{})", + "(,,{} - ,,{}) - (,,{} - ,,{})", commit4.id().hex(), commit1.id().hex(), commit3.id().hex(), diff --git a/src/commands.rs b/src/commands.rs index d6f1cc3e5..ab685de4e 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -342,7 +342,7 @@ fn get_app<'a, 'b>() -> App<'a, 'b> { .long("revisions") .short("r") .takes_value(true) - .default_value("*:non_obsolete_heads()"), + .default_value(",,non_obsolete_heads()"), ) .arg(Arg::with_name("no-graph").long("no-graph")); let obslog_command = SubCommand::with_name("obslog")