revset: remove filter_by_diff(), have caller intersect expression

To be able to make e.g. `jj log some/path` perform well on cloud-based
repos, a custom revset engine needs to be able to see the paths to
filter by. That way it is able pass those to a server-side index. This
commit helps with that by effectively converting `jj log -r foo
some/path` into `jj log -r 'foo & file(some/path)'`.

lib/src/revset.rs

@@ -12,7 +12,6 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::borrow::Borrow;
 use std::cmp::{Ordering, Reverse};
 use std::collections::{HashMap, HashSet};
 use std::iter::Peekable;
@@ -2212,17 +2211,6 @@ fn build_predicate_fn<'index>(
     }
 }
 
-pub fn filter_by_diff<'index>(
-    repo: &'index dyn Repo,
-    matcher: impl Borrow<dyn Matcher + 'index> + 'index,
-    candidates: Box<dyn Revset<'index> + 'index>,
-) -> Box<dyn Revset<'index> + 'index> {
-    Box::new(FilterRevset::<PurePredicateFn> {
-        candidates,
-        predicate: Box::new(move |entry| has_diff_from_parent(repo, entry, matcher.borrow())),
-    })
-}
-
 fn has_diff_from_parent(repo: &dyn Repo, entry: &IndexEntry<'_>, matcher: &dyn Matcher) -> bool {
     let commit = repo.store().get_commit(&entry.commit_id()).unwrap();
     let parents = commit.parents();

lib/tests/test_revset.rs

@@ -17,13 +17,12 @@ use std::path::Path;
 use assert_matches::assert_matches;
 use jujutsu_lib::backend::{CommitId, MillisSinceEpoch, ObjectId, Signature, Timestamp};
 use jujutsu_lib::git;
-use jujutsu_lib::matchers::{FilesMatcher, Matcher};
 use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
 use jujutsu_lib::repo::Repo;
 use jujutsu_lib::repo_path::RepoPath;
 use jujutsu_lib::revset::{
-    self, optimize, parse, resolve_symbol, RevsetAliasesMap, RevsetError, RevsetExpression,
-    RevsetIteratorExt, RevsetWorkspaceContext,
+    optimize, parse, resolve_symbol, RevsetAliasesMap, RevsetError, RevsetExpression,
+    RevsetFilterPredicate, RevsetIteratorExt, RevsetWorkspaceContext,
 };
 use jujutsu_lib::settings::GitSettings;
 use jujutsu_lib::workspace::Workspace;
@@ -1906,7 +1905,7 @@ fn test_evaluate_expression_difference(use_git: bool) {
 
 #[test_case(false ; "local backend")]
 #[test_case(true ; "git backend")]
-fn test_filter_by_diff(use_git: bool) {
+fn test_filter_by_file(use_git: bool) {
     let settings = testutils::user_settings();
     let test_workspace = TestWorkspace::init(&settings, use_git);
     let repo = &test_workspace.repo;
@@ -1962,15 +1961,12 @@ fn test_filter_by_diff(use_git: bool) {
         .write()
         .unwrap();
 
-    // matcher API:
     let resolve = |file_path: &RepoPath| -> Vec<CommitId> {
         let mut_repo = &*mut_repo;
-        let matcher = FilesMatcher::new(&[file_path.clone()]);
-        let candidates = RevsetExpression::all().evaluate(mut_repo, None).unwrap();
-        let commit_ids = revset::filter_by_diff(mut_repo, &matcher as &dyn Matcher, candidates)
-            .iter()
-            .commit_ids()
-            .collect();
+        let expression =
+            RevsetExpression::filter(RevsetFilterPredicate::File(Some(vec![file_path.clone()])));
+        let revset = expression.evaluate(mut_repo, None).unwrap();
+        let commit_ids = revset.iter().commit_ids().collect();
         commit_ids
     };
 

src/commands/mod.rs

@@ -35,7 +35,9 @@ use jujutsu_lib::matchers::EverythingMatcher;
 use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
 use jujutsu_lib::repo::{ReadonlyRepo, Repo};
 use jujutsu_lib::repo_path::RepoPath;
-use jujutsu_lib::revset::{RevsetAliasesMap, RevsetExpression, RevsetIteratorExt};
+use jujutsu_lib::revset::{
+    RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt,
+};
 use jujutsu_lib::revset_graph_iterator::{
     RevsetGraphEdge, RevsetGraphEdgeType, RevsetGraphIterator,
 };
@@ -1411,13 +1413,20 @@ fn cmd_log(ui: &mut Ui, command: &CommandHelper, args: &LogArgs) -> Result<(), C
         workspace_command.parse_revset(args.revisions.as_deref().unwrap_or(&default_revset))?;
     let repo = workspace_command.repo();
     let wc_commit_id = workspace_command.get_wc_commit_id();
-    let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let revset = workspace_command.evaluate_revset(&revset_expression)?;
-    let revset = if !args.paths.is_empty() {
-        revset::filter_by_diff(repo, matcher.as_ref(), revset)
+    let revset_expression = if !args.paths.is_empty() {
+        let repo_paths: Vec<_> = args
+            .paths
+            .iter()
+            .map(|path_arg| workspace_command.parse_file_path(path_arg))
+            .try_collect()?;
+        revset_expression.intersection(&RevsetExpression::filter(RevsetFilterPredicate::File(
+            Some(repo_paths),
+        )))
     } else {
-        revset
+        revset_expression
     };
+    let matcher = workspace_command.matcher_from_values(&args.paths)?;
+    let revset = workspace_command.evaluate_revset(&revset_expression)?;
 
     let store = repo.store();
     let diff_formats =