From 63c90c04c836102fee2c339059e84faede9a300c Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sat, 11 Dec 2021 11:23:29 -0800 Subject: [PATCH] revsets: change parent/children operators to `foo~`/`foo+` (#46) --- docs/git-comparison.md | 2 +- docs/tutorial.md | 12 ++++----- lib/src/revset.pest | 20 +++++++-------- lib/src/revset.rs | 55 +++++++++++++++------------------------- lib/tests/test_revset.rs | 18 ++++++------- src/commands.rs | 10 ++++---- 6 files changed, 51 insertions(+), 66 deletions(-) diff --git a/docs/git-comparison.md b/docs/git-comparison.md index 17bd86166..7513cc33f 100644 --- a/docs/git-comparison.md +++ b/docs/git-comparison.md @@ -107,7 +107,7 @@ parent. Edit description (commit message) of the previous change - jj describe :@ + jj describe @~ git commit --amend (first make sure that nothing is staged) diff --git a/docs/tutorial.md b/docs/tutorial.md index 213861f8d..c651053eb 100644 --- a/docs/tutorial.md +++ b/docs/tutorial.md @@ -167,7 +167,7 @@ o 000000000000 000000000000 1970-01-01 00:00:00.000 +00:00 (The `000000000000` commit is a virtual commit that's called the "root commit". It's the root commit of every repo. The `root` symbol in the revset matches it.) -There are also operators for getting the parents (`:foo`), children `foo:`, +There are also operators for getting the parents (`foo~`), children (`foo+`), ancestors (`,,foo`), descendants (`foo,,`), DAG range (`foo,,bar`, like `git log --ancestry-path`), range (`foo,,,bar`, like Git's `foo..bar`). There are also a few more functions, such as `heads()`, which filters out @@ -195,7 +195,7 @@ Now let's see how Jujutsu deals with merge conflicts. We'll start by making some commits: ```shell script # Check out the grandparent of the working copy -$ jj co ::@ +$ jj co @~~ Working copy now at: 9164f1d6a011 Added 0 files, modified 1 files, removed 0 files $ echo a > file1; jj close -m A @@ -382,7 +382,7 @@ third commit. Remember that `jj squash` moves all the changes from one commit into its parent. `jj squash -i` moves only part of the changes into its parent. Now try that: ```shell script -$ jj squash -i -r :@ +$ jj squash -i -r @~ Rebased 1 descendant commits Working copy now at: 4b4c714b36aa ``` @@ -391,7 +391,7 @@ the right side of the diff to have the desired end state in "ABC" by removing the "D" line. Then close Meld. If we look the diff of the second commit, we now see that all three lines got capitalized: ```shell script -$ jj diff -r ::@ +$ jj diff -r @~~ Modified regular file file: 1 1: aA 2 2: bB @@ -407,7 +407,7 @@ Let's try one final command for changing the contents of an exiting commit. That command is `jj edit`, which lets you edit the contents of a commit without checking it out. ```shell script -$ jj edit -r ::@ +$ jj edit -r @~~ Created 2423c134ea70 ABC Rebased 2 descendant commits Working copy now at: d31c52e8ca41 @@ -415,7 +415,7 @@ Added 0 files, modified 1 files, removed 0 files ``` When Meld starts, edit the right side by e.g. adding something to the first line. Then close Meld. You can now inspect the rewritten commit with -`jj diff -r ::@` again and you should see your addition to the first line. +`jj diff -r @~~` again and you should see your addition to the first line. Unlike `jj squash -i`, which left the content state of the commit unchanged, `jj edit` (typically) results in a different state, which means that descendant commits may have conflicts. diff --git a/lib/src/revset.pest b/lib/src/revset.pest index eaf771353..aabd5b6ee 100644 --- a/lib/src/revset.pest +++ b/lib/src/revset.pest @@ -20,11 +20,11 @@ symbol = { literal_string = { "\"" ~ (!"\"" ~ ANY)+ ~ "\"" } whitespace = _{ " " } +parents_op = { "~" } +children_op = { "+" } + // We could use the same rule name for the shared operators if we can // think of a good name. -parents_op = { ":" } -children_op = { ":" } - ancestors_op = { ",," } descendants_op = { ",," } dag_range_op = { ",," } @@ -47,16 +47,14 @@ primary = { | symbol } -parents_expression = { parents_op* ~ primary } - -children_expression = { parents_expression ~ children_op* } +neighbors_expression = { primary ~ (parents_op | children_op)* } range_expression = { - children_expression ~ dag_range_op ~ children_expression - | children_expression ~ range_op ~ children_expression - | ancestors_op ~ children_expression - | children_expression ~ descendants_op - | children_expression + neighbors_expression ~ dag_range_op ~ neighbors_expression + | neighbors_expression ~ range_op ~ neighbors_expression + | ancestors_op ~ neighbors_expression + | neighbors_expression ~ descendants_op + | neighbors_expression } infix_expression = { diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 8da30ed45..5ad192435 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -408,17 +408,17 @@ fn parse_range_expression_rule( match first.as_rule() { Rule::ancestors_op => { return Ok( - parse_children_expression_rule(pairs.next().unwrap().into_inner())?.ancestors(), + parse_neighbors_expression_rule(pairs.next().unwrap().into_inner())?.ancestors(), ); } - Rule::children_expression => { + Rule::neighbors_expression => { // Fall through } _ => { panic!("unxpected revset range operator rule {:?}", first.as_rule()); } } - let mut expression = parse_children_expression_rule(first.into_inner())?; + let mut expression = parse_neighbors_expression_rule(first.into_inner())?; if let Some(next) = pairs.next() { match next.as_rule() { Rule::descendants_op => { @@ -426,12 +426,12 @@ fn parse_range_expression_rule( } Rule::dag_range_op => { let heads_expression = - parse_children_expression_rule(pairs.next().unwrap().into_inner())?; + parse_neighbors_expression_rule(pairs.next().unwrap().into_inner())?; expression = expression.dag_range_to(&heads_expression); } Rule::range_op => { let expression2 = - parse_children_expression_rule(pairs.next().unwrap().into_inner())?; + parse_neighbors_expression_rule(pairs.next().unwrap().into_inner())?; expression = expression.range(&expression2); } _ => { @@ -442,18 +442,21 @@ fn parse_range_expression_rule( Ok(expression) } -fn parse_children_expression_rule( +fn parse_neighbors_expression_rule( mut pairs: Pairs, ) -> Result, RevsetParseError> { - let mut expression = parse_parents_expression_rule(pairs.next().unwrap().into_inner())?; + let mut expression = parse_primary_rule(pairs.next().unwrap().into_inner())?; for operator in pairs { match operator.as_rule() { + Rule::parents_op => { + expression = expression.parents(); + } Rule::children_op => { expression = expression.children(); } _ => { panic!( - "unxpected revset children operator rule {:?}", + "unxpected revset neighbors operator rule {:?}", operator.as_rule() ); } @@ -462,22 +465,6 @@ fn parse_children_expression_rule( Ok(expression) } -fn parse_parents_expression_rule( - mut pairs: Pairs, -) -> Result, RevsetParseError> { - let first = pairs.next().unwrap(); - match first.as_rule() { - Rule::primary => parse_primary_rule(first.into_inner()), - Rule::parents_op => Ok(parse_parents_expression_rule(pairs)?.parents()), - _ => { - panic!( - "unxpected revset parents operator rule {:?}", - first.as_rule() - ); - } - } -} - fn parse_primary_rule(mut pairs: Pairs) -> Result, RevsetParseError> { let first = pairs.next().unwrap(); match first.as_rule() { @@ -1337,9 +1324,9 @@ mod tests { // Parse a quoted symbol assert_eq!(parse("\"foo\""), Ok(foo_symbol.clone())); // Parse the "parents" operator - assert_eq!(parse(":@"), Ok(checkout_symbol.parents())); + assert_eq!(parse("@~"), Ok(checkout_symbol.parents())); // Parse the "children" operator - assert_eq!(parse("@:"), Ok(checkout_symbol.children())); + assert_eq!(parse("@+"), Ok(checkout_symbol.children())); // Parse the "ancestors" operator assert_eq!(parse(",,@"), Ok(checkout_symbol.ancestors())); // Parse the "descendants" operator @@ -1352,14 +1339,14 @@ mod tests { assert_eq!(parse("foo | bar"), Ok(foo_symbol.union(&bar_symbol))); // Parse the "difference" operator assert_eq!(parse("foo - bar"), Ok(foo_symbol.minus(&bar_symbol))); - // Parentheses are allowed after prefix operators - assert_eq!(parse(":(@)"), Ok(checkout_symbol.parents())); + // Parentheses are allowed before suffix operators + assert_eq!(parse("(@)~"), Ok(checkout_symbol.parents())); // Space is allowed around expressions assert_eq!(parse(" ,,@ "), Ok(checkout_symbol.ancestors())); // Space is not allowed around prefix operators assert_matches!(parse(" ,, @ "), Err(RevsetParseError::SyntaxError(_))); // Incomplete parse - assert_matches!(parse("foo | :"), Err(RevsetParseError::SyntaxError(_))); + assert_matches!(parse("foo | ~"), Err(RevsetParseError::SyntaxError(_))); // Space is allowed around infix operators and function arguments assert_eq!( parse(" description( arg1 , arg2 ) - parents( arg1 ) - heads( ) "), @@ -1375,12 +1362,12 @@ mod tests { let foo_symbol = RevsetExpression::symbol("foo".to_string()); // Parse repeated "parents" operator assert_eq!( - parse(":::foo"), + parse("foo~~~"), Ok(foo_symbol.parents().parents().parents()) ); // Parse repeated "children" operator assert_eq!( - parse("foo:::"), + parse("foo+++"), Ok(foo_symbol.children().children().children()) ); // Parse repeated "ancestors"/"descendants"/"dag range" operators @@ -1392,9 +1379,9 @@ mod tests { assert_matches!(parse("foo,,bar,,"), Err(RevsetParseError::SyntaxError(_))); // Parse combinations of "parents"/"children" operators and the range operators. // The former bind more strongly. - assert_eq!(parse(":foo:"), Ok(foo_symbol.parents().children())); - assert_eq!(parse(":foo,,"), Ok(foo_symbol.parents().descendants())); - assert_eq!(parse(",,foo:"), Ok(foo_symbol.children().ancestors())); + assert_eq!(parse("foo~+"), Ok(foo_symbol.parents().children())); + assert_eq!(parse("foo~,,"), Ok(foo_symbol.parents().descendants())); + assert_eq!(parse(",,foo+"), Ok(foo_symbol.children().ancestors())); } #[test] diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index ba8a47811..37b1338e1 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -499,18 +499,18 @@ fn test_evaluate_expression_parents(use_git: bool) { let commit5 = graph_builder.commit_with_parents(&[&commit2]); // The root commit has no parents - assert_eq!(resolve_commit_ids(mut_repo.as_repo_ref(), ":root"), vec![]); + assert_eq!(resolve_commit_ids(mut_repo.as_repo_ref(), "root~"), vec![]); // Can find parents of the current checkout mut_repo.set_checkout(commit2.id().clone()); assert_eq!( - resolve_commit_ids(mut_repo.as_repo_ref(), ":@"), + resolve_commit_ids(mut_repo.as_repo_ref(), "@~"), vec![commit1.id().clone()] ); // Can find parents of a merge commit 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![commit3.id().clone(), commit2.id().clone()] ); @@ -518,7 +518,7 @@ fn test_evaluate_expression_parents(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!(":({} | {})", commit2.id().hex(), commit3.id().hex()) + &format!("({} | {})~", commit2.id().hex(), commit3.id().hex()) ), vec![commit1.id().clone(), root_commit.id().clone()] ); @@ -527,7 +527,7 @@ fn test_evaluate_expression_parents(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!(":({} | {})", commit1.id().hex(), commit2.id().hex()) + &format!("({} | {})~", commit1.id().hex(), commit2.id().hex()) ), vec![commit1.id().clone(), root_commit.id().clone()] ); @@ -536,7 +536,7 @@ fn test_evaluate_expression_parents(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![commit3.id().clone(), commit2.id().clone()] ); @@ -569,7 +569,7 @@ fn test_evaluate_expression_children(use_git: bool) { // Can find children of the root commit assert_eq!( - resolve_commit_ids(mut_repo.as_repo_ref(), "root:"), + resolve_commit_ids(mut_repo.as_repo_ref(), "root+"), vec![commit1.id().clone(), checkout_id] ); @@ -578,7 +578,7 @@ fn test_evaluate_expression_children(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!("({} | {}):", commit1.id().hex(), commit2.id().hex()) + &format!("({} | {})+", commit1.id().hex(), commit2.id().hex()) ), vec![ commit4.id().clone(), @@ -591,7 +591,7 @@ fn test_evaluate_expression_children(use_git: bool) { assert_eq!( resolve_commit_ids( mut_repo.as_repo_ref(), - &format!("({} | {}):", commit3.id().hex(), commit4.id().hex()) + &format!("({} | {})+", commit3.id().hex(), commit4.id().hex()) ), vec![commit5.id().clone()] ); diff --git a/src/commands.rs b/src/commands.rs index f5993519b..3ebb1696c 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -410,8 +410,8 @@ impl WorkspaceCommandHelper { // it's another symbol, then we don't. If it's more complex, then we do // (just to be safe). TODO: Maybe make this smarter. How do we generally // figure out if a revset needs to commit the working copy? For example, - // ":@" should perhaps not result in a new working copy commit, but - // "::@" should. "foo::" is probably also should, since we would + // "@~" should perhaps not result in a new working copy commit, but + // "@~~" should. "foo++" is probably also should, since we would // otherwise need to evaluate the revset and see if "foo::" includes the // parent of the current checkout. Other interesting cases include some kind of // reference pointing to the working copy commit. If it's a @@ -947,7 +947,7 @@ With the `--from` and/or `--to` options, shows the difference from/to the given "Create a new, empty change. This may be useful if you want to make some changes \ you're unsure of on top of the working copy. If the changes turned out to useful, \ you can `jj squash` them into the previous working copy. If they turned out to be \ - unsuccessful, you can `jj abandon` them and `jj co :@` the previous working copy.", + unsuccessful, you can `jj abandon` them and `jj co @~` the previous working copy.", ) .arg( Arg::with_name("revision") @@ -1005,7 +1005,7 @@ With the `--from` and/or `--to` options, shows the difference from/to the given Arg::with_name("from") .long("from") .takes_value(true) - .default_value(":@") + .default_value("@~") .help("Revision to restore from (source)"), ) .arg( @@ -1119,7 +1119,7 @@ A A", .multiple(true) .help("The revision to rebase onto"), ); - // TODO: It seems better to default the destination to `:@`. Maybe the working + // TODO: It seems better to default the destination to `@~`. Maybe the working // copy should be rebased on top? let backout_command = SubCommand::with_name("backout") .about("Apply the reverse of a revision on top of another revision")