revset: remove redundant candidates argument from filter predicates

CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Breaking changes
+
+* Dropped candidates set argument from `description(needle)`, `author(needle)`,
+  `committer(needle)` revsets. Use `x & description(needle)` instead.
+
 ### New features
 
 * The new `jj git remote rename` command allows git remotes to be renamed
@@ -14,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * `jj git push` will search `@-` for branches to push if `@` has none.
 
-* The new revset function `file(pattern[, x])` finds commits modifying the
+* The new revset function `file(pattern)` finds commits modifying the
   paths specified by the `pattern`.
 
 ### Fixed bugs

docs/revsets.md

@@ -103,18 +103,13 @@ revsets (expressions) as arguments.
 * `roots(x)`: Commits in `x` that are not descendants of other commits in `x`.
 * `merges([x])`: Merge commits within `x`. If `x` was not specified, it selects
   all visible merge commits (as if you had said `merges(all())`).
-* `description(needle[, x])`: Commits with the given string in their
+* `description(needle)`: Commits with the given string in their
-  description. If a second argument was provided, then only commits in that set
+  description.
-  are considered, otherwise all visible commits are considered.
+* `author(needle)`: Commits with the given string in the author's name or
-* `author(needle[, x])`: Commits with the given string in the author's name or
+  email.
-  email. If a second argument was provided, then only commits in that set
+* `committer(needle)`: Commits with the given string in the committer's
-  are considered, otherwise all visible commits are considered.
+  name or email.
-* `committer(needle[, x])`: Commits with the given string in the committer's
+* `file(pattern)`: Commits modifying the paths specified by the `pattern`.
-  name or email. If a second argument was provided, then only commits in that
-  set are considered, otherwise all visible commits are considered.
-* `file(pattern[, x])`: Commits modifying the paths specified by the `pattern`.
-  If a second argument was provided, then only commits in that set are
-  considered, otherwise all visible commits are considered.
 
 
 ## Examples

lib/src/revset.rs

@@ -759,21 +759,17 @@ fn parse_function_expression(
             Ok(candidates.with_parent_count(2..u32::MAX))
         }
         "description" | "author" | "committer" | "file" => {
-            if !(1..=2).contains(&arg_count) {
+            if arg_count != 1 {
                 return Err(RevsetParseError::InvalidFunctionArguments {
                     name,
-                    message: "Expected 1 or 2 arguments".to_string(),
+                    message: "Expected 1 argument".to_string(),
                 });
             }
             let needle = parse_function_argument_to_string(
                 &name,
                 argument_pairs.next().unwrap().into_inner(),
             )?;
-            let candidates = if arg_count == 1 {
+            let candidates = RevsetExpression::all();
-                RevsetExpression::all()
-            } else {
-                parse_expression_rule(argument_pairs.next().unwrap().into_inner())?
-            };
             match name.as_str() {
                 "description" => Ok(candidates.with_description(needle)),
                 "author" => Ok(candidates.with_author(needle)),
@@ -1788,9 +1784,10 @@ mod tests {
         // Incomplete parse
         assert_matches!(parse("foo | -"), Err(RevsetParseError::SyntaxError(_)));
         // Space is allowed around infix operators and function arguments
+        // TODO: test two_arg_function(  arg1 ,   arg2 ) if any
         assert_eq!(
-            parse("   description(  arg1 ,   arg2 ) ~    parents(   arg1  )  ~ heads(  )  "),
+            parse("   description(  arg1 ) ~    parents(   arg1  )  ~ heads(  )  "),
-            Ok(RevsetExpression::symbol("arg2".to_string())
+            Ok(RevsetExpression::all()
                 .with_description("arg1".to_string())
                 .minus(&RevsetExpression::symbol("arg1".to_string()).parents())
                 .minus(&RevsetExpression::visible_heads()))
@@ -1848,23 +1845,23 @@ mod tests {
             })
         );
         assert_eq!(
-            parse("description(foo,bar)"),
+            parse("description(foo)"),
-            Ok(RevsetExpression::symbol("bar".to_string()).with_description("foo".to_string()))
+            Ok(RevsetExpression::all().with_description("foo".to_string()))
         );
         assert_eq!(
-            parse("description(heads(),bar)"),
+            parse("description(heads())"),
             Err(RevsetParseError::InvalidFunctionArguments {
                 name: "description".to_string(),
                 message: "Expected function argument of type string, found: heads()".to_string()
             })
         );
         assert_eq!(
-            parse("description((foo),bar)"),
+            parse("description((foo))"),
-            Ok(RevsetExpression::symbol("bar".to_string()).with_description("foo".to_string()))
+            Ok(RevsetExpression::all().with_description("foo".to_string()))
         );
         assert_eq!(
-            parse("description(\"(foo)\",bar)"),
+            parse("description(\"(foo)\")"),
-            Ok(RevsetExpression::symbol("bar".to_string()).with_description("(foo)".to_string()))
+            Ok(RevsetExpression::all().with_description("(foo)".to_string()))
         );
     }
 

lib/tests/test_revset.rs

@@ -1432,7 +1432,10 @@ fn test_evaluate_expression_description(use_git: bool) {
     );
     // Searches only among candidates if specified
     assert_eq!(
-        resolve_commit_ids(mut_repo.as_repo_ref(), "description(\"commit 2\",heads())"),
+        resolve_commit_ids(
+            mut_repo.as_repo_ref(),
+            "heads() & description(\"commit 2\")"
+        ),
         vec![]
     );
 }
@@ -1495,7 +1498,7 @@ fn test_evaluate_expression_author(use_git: bool) {
     );
     // Searches only among candidates if specified
     assert_eq!(
-        resolve_commit_ids(mut_repo.as_repo_ref(), "author(\"name2\",heads())"),
+        resolve_commit_ids(mut_repo.as_repo_ref(), "heads() & author(\"name2\")"),
         vec![]
     );
 }
@@ -1558,7 +1561,7 @@ fn test_evaluate_expression_committer(use_git: bool) {
     );
     // Searches only among candidates if specified
     assert_eq!(
-        resolve_commit_ids(mut_repo.as_repo_ref(), "committer(\"name2\",heads())"),
+        resolve_commit_ids(mut_repo.as_repo_ref(), "heads() & committer(\"name2\")"),
         vec![]
     );
 }
@@ -1839,7 +1842,7 @@ fn test_filter_by_diff(use_git: bool) {
     assert_eq!(
         resolve_commit_ids_in_workspace(
             mut_repo.as_repo_ref(),
-            &format!(r#"file("added_modified_clean", {}:)"#, commit2.id().hex()),
+            &format!(r#"{}: & file("added_modified_clean")"#, commit2.id().hex()),
             &test_workspace.workspace,
             Some(test_workspace.workspace.workspace_root()),
         ),