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)'`.
This commit is contained in:
Martin von Zweigbergk 2023-02-28 09:07:38 -08:00 committed by Martin von Zweigbergk
parent 26597ba61a
commit bbd6ef0c7b
3 changed files with 22 additions and 29 deletions

View file

@ -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();

View file

@ -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
};

View file

@ -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 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_expression
};
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)
} else {
revset
};
let store = repo.store();
let diff_formats =