From e1c57338a1cbaac969c7345315948684153987d5 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 27 Mar 2023 23:13:31 -0700 Subject: [PATCH] revset: split out no-args `head()` to `visible_heads()` The `heads()` revset function with one argument is the counterpart to `roots()`. Without arguments, it returns the visible heads in the repo, i.e. `heads(all())`. The two use cases are quite different, and I think it would be good to clarify that the no-arg form returns the visible heads, so let's split that out to a new `visible_heads()` function. --- CHANGELOG.md | 4 ++++ docs/revsets.md | 5 ++--- lib/src/default_revset_engine.rs | 2 +- lib/src/revset.rs | 18 +++++++++--------- lib/tests/test_revset.rs | 12 ++++++------ tests/test_revset_output.rs | 2 +- 6 files changed, 23 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 591db0d97..589760825 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The minimum supported Rust version (MSRV) is now 1.64.0. +* The `heads()` revset function was split up into two functions. `heads()` + without arguments is now called `visible_heads()`. `heads()` with one argument + is unchanged. + ### New features * `jj git push --deleted` will remove all locally deleted branches from the remote. diff --git a/docs/revsets.md b/docs/revsets.md index 496dc9232..9892b92c4 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -104,9 +104,8 @@ revsets (expressions) as arguments. * `git_refs()`: All Git ref targets as of the last import. If a Git ref is in a conflicted state, all its possible targets are included. * `git_head()`: The Git `HEAD` target as of the last import. -* `heads([x])`: Commits in `x` that are not ancestors of other commits in `x`. - If `x` was not specified, it selects all visible heads (as if you had said - `heads(all())`). +* `visible_heads()`: All visible heads (same as `heads(all())`). +* `heads(x)`: Commits in `x` that are not ancestors of other commits in `x`. * `roots(x)`: Commits in `x` that are not descendants of other commits in `x`. * `latest(x[, count])`: Latest `count` commits in `x`, based on committer timestamp. The default `count` is 1. diff --git a/lib/src/default_revset_engine.rs b/lib/src/default_revset_engine.rs index 485836740..0c54086a1 100644 --- a/lib/src/default_revset_engine.rs +++ b/lib/src/default_revset_engine.rs @@ -627,7 +627,7 @@ impl<'index, 'heads> EvaluationContext<'index, 'heads> { // but if it does, the heads set could be extended to include the commits // (and `remote_branches()`) specified in the revset expression. Alternatively, // some optimization rules could be removed, but that means `author(_) & x` - // would have to test `:heads() & x`. + // would have to test `:visble_heads() & x`. let walk = self.composite_index.walk_revs(self.visible_heads, &[]); Ok(Box::new(RevWalkRevset { walk })) } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 2cbc258d4..10933ed0f 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -798,19 +798,19 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: Ok(RevsetExpression::all()) }); map.insert("heads", |name, arguments_pair, state| { - let ([], [opt_arg]) = expect_arguments(name, arguments_pair)?; - if let Some(arg) = opt_arg { - let candidates = parse_expression_rule(arg.into_inner(), state)?; - Ok(candidates.heads()) - } else { - Ok(RevsetExpression::visible_heads()) - } + let arg = expect_one_argument(name, arguments_pair)?; + let candidates = parse_expression_rule(arg.into_inner(), state)?; + Ok(candidates.heads()) }); map.insert("roots", |name, arguments_pair, state| { let arg = expect_one_argument(name, arguments_pair)?; let candidates = parse_expression_rule(arg.into_inner(), state)?; Ok(candidates.roots()) }); + map.insert("visible_heads", |name, arguments_pair, _state| { + expect_no_arguments(name, arguments_pair)?; + Ok(RevsetExpression::visible_heads()) + }); map.insert("branches", |name, arguments_pair, state| { let ([], [opt_arg]) = expect_arguments(name, arguments_pair)?; let needle = if let Some(arg) = opt_arg { @@ -2046,7 +2046,7 @@ mod tests { assert_eq!(parse("foo | -"), Err(RevsetParseErrorKind::SyntaxError)); // Space is allowed around infix operators and function arguments assert_eq!( - parse(" description( arg1 ) ~ file( arg1 , arg2 ) ~ heads( ) "), + parse(" description( arg1 ) ~ file( arg1 , arg2 ) ~ visible_heads( ) "), Ok( RevsetExpression::filter(RevsetFilterPredicate::Description("arg1".to_string())) .minus(&RevsetExpression::filter(RevsetFilterPredicate::File( @@ -2205,7 +2205,7 @@ mod tests { )) ); assert_eq!( - parse("description(heads())"), + parse("description(visible_heads())"), Err(RevsetParseErrorKind::InvalidFunctionArguments { name: "description".to_string(), message: "Expected function argument of type string".to_string() diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 9f7d2ca4d..fdf91952a 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -600,10 +600,10 @@ fn test_evaluate_expression_heads(use_git: bool) { vec![commit3.id().clone()] ); - // Heads of all commits is the set of heads in the repo + // Heads of all commits is the set of visible heads in the repo assert_eq!( resolve_commit_ids(mut_repo, "heads(all())"), - resolve_commit_ids(mut_repo, "heads()") + resolve_commit_ids(mut_repo, "visible_heads()") ); } @@ -1187,7 +1187,7 @@ fn test_evaluate_expression_visible_heads(use_git: bool) { let commit3 = graph_builder.commit_with_parents(&[&commit1]); assert_eq!( - resolve_commit_ids(mut_repo, "heads()"), + resolve_commit_ids(mut_repo, "visible_heads()"), vec![commit3.id().clone(), commit2.id().clone()] ); } @@ -1626,7 +1626,7 @@ fn test_evaluate_expression_description(use_git: bool) { ); // Searches only among candidates if specified assert_eq!( - resolve_commit_ids(mut_repo, "heads() & description(\"commit 2\")"), + resolve_commit_ids(mut_repo, "visible_heads() & description(\"commit 2\")"), vec![] ); } @@ -1692,7 +1692,7 @@ fn test_evaluate_expression_author(use_git: bool) { ); // Searches only among candidates if specified assert_eq!( - resolve_commit_ids(mut_repo, "heads() & author(\"name2\")"), + resolve_commit_ids(mut_repo, "visible_heads() & author(\"name2\")"), vec![] ); // Filter by union of pure predicate and set @@ -1766,7 +1766,7 @@ fn test_evaluate_expression_committer(use_git: bool) { ); // Searches only among candidates if specified assert_eq!( - resolve_commit_ids(mut_repo, "heads() & committer(\"name2\")"), + resolve_commit_ids(mut_repo, "visible_heads() & committer(\"name2\")"), vec![] ); } diff --git a/tests/test_revset_output.rs b/tests/test_revset_output.rs index 7c706159c..0da5fae02 100644 --- a/tests/test_revset_output.rs +++ b/tests/test_revset_output.rs @@ -98,7 +98,7 @@ fn test_bad_function_call() { 1 | heads(foo, bar) | ^------^ | - = Invalid arguments to revset function "heads": Expected 0 to 1 arguments + = Invalid arguments to revset function "heads": Expected 1 arguments "###); let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "latest(a, not_an_integer)"]);