From cb2fcde5607e44ae94210860894bd1ce5f972064 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 23 Oct 2022 13:14:00 +0900 Subject: [PATCH] revset: implement file(pattern[, candidates]) predicate The name "file()" is just copied from hg. I'm not sure if it's good in jj's context, but I couldn't find a better name. --- CHANGELOG.md | 3 +++ docs/revsets.md | 3 +++ lib/src/revset.rs | 46 +++++++++++++++++++++++++++++++++++++-- lib/tests/test_revset.rs | 44 +++++++++++++++++++++++++++++++++---- src/cli_util.rs | 2 ++ tests/test_log_command.rs | 18 +++++++++++++++ 6 files changed, 110 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e57feb752..dfe3282ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ 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 + paths specified by the `pattern`. + ### Fixed bugs * `jj edit root` now fails gracefully. diff --git a/docs/revsets.md b/docs/revsets.md index 62c365a1a..9242c0953 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -112,6 +112,9 @@ revsets (expressions) as arguments. * `committer(needle[, x])`: Commits with the given string in the committer's 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 diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 029c6b135..d7f83783d 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -17,6 +17,7 @@ use std::cmp::{Ordering, Reverse}; use std::collections::HashSet; use std::iter::Peekable; use std::ops::Range; +use std::path::Path; use std::rc::Rc; use std::sync::Arc; @@ -29,9 +30,10 @@ use thiserror::Error; use crate::backend::{BackendError, BackendResult, CommitId}; use crate::commit::Commit; use crate::index::{HexPrefix, IndexEntry, IndexPosition, PrefixResolution, RevWalk}; -use crate::matchers::Matcher; +use crate::matchers::{Matcher, PrefixMatcher}; use crate::op_store::WorkspaceId; use crate::repo::RepoRef; +use crate::repo_path::{FsPathParseError, RepoPath}; use crate::revset_graph_iterator::RevsetGraphIterator; use crate::rewrite; use crate::store::Store; @@ -44,6 +46,10 @@ pub enum RevsetError { AmbiguousCommitIdPrefix(String), #[error("Change id prefix \"{0}\" is ambiguous")] AmbiguousChangeIdPrefix(String), + #[error("Invalid file pattern: {0}")] + FsPathParseError(#[from] FsPathParseError), + #[error("Cannot resolve file pattern without workspace")] + FsPathWithoutWorkspace, #[error("Unexpected error from store: {0}")] StoreError(#[from] BackendError), } @@ -244,6 +250,10 @@ pub enum RevsetExpression { needle: String, candidates: Rc, }, + File { + pattern: String, + candidates: Rc, + }, Union(Rc, Rc), Intersection(Rc, Rc), Difference(Rc, Rc), @@ -392,6 +402,14 @@ impl RevsetExpression { }) } + /// Commits in `self` modifying the paths specified by the `pattern`. + pub fn with_file(self: &Rc, pattern: String) -> Rc { + Rc::new(RevsetExpression::File { + candidates: self.clone(), + pattern, + }) + } + /// Commits that are in `self` or in `other` (or both). pub fn union( self: &Rc, @@ -748,7 +766,7 @@ fn parse_function_expression( }; Ok(candidates.with_parent_count(2..u32::MAX)) } - "description" | "author" | "committer" => { + "description" | "author" | "committer" | "file" => { if !(1..=2).contains(&arg_count) { return Err(RevsetParseError::InvalidFunctionArguments { name, @@ -768,6 +786,7 @@ fn parse_function_expression( "description" => Ok(candidates.with_description(needle)), "author" => Ok(candidates.with_author(needle)), "committer" => Ok(candidates.with_committer(needle)), + "file" => Ok(candidates.with_file(needle)), _ => { panic!("unexpected function name: {}", name) } @@ -1140,7 +1159,9 @@ impl<'revset, 'repo> Iterator for DifferenceRevsetIterator<'revset, 'repo> { /// Workspace information needed to evaluate revset expression. #[derive(Clone, Debug)] pub struct RevsetWorkspaceContext<'a> { + pub cwd: &'a Path, pub workspace_id: &'a WorkspaceId, + pub workspace_root: &'a Path, } pub fn evaluate_expression<'repo>( @@ -1338,6 +1359,20 @@ pub fn evaluate_expression<'repo>( }), })) } + RevsetExpression::File { + pattern, + candidates, + } => { + if let Some(ctx) = workspace_ctx { + // TODO: Add support for globs and other formats + let path = RepoPath::parse_fs_path(ctx.cwd, ctx.workspace_root, pattern)?; + let matcher: Box = Box::new(PrefixMatcher::new(&[path])); + let candidates = candidates.evaluate(repo, workspace_ctx)?; + Ok(filter_by_diff(repo, matcher, candidates)) + } else { + Err(RevsetError::FsPathWithoutWorkspace) + } + } RevsetExpression::Union(expression1, expression2) => { let set1 = expression1.evaluate(repo, workspace_ctx)?; let set2 = expression2.evaluate(repo, workspace_ctx)?; @@ -1490,6 +1525,13 @@ mod tests { needle: "needle".to_string() }) ); + assert_eq!( + foo_symbol.with_file("pattern".to_string()), + Rc::new(RevsetExpression::File { + candidates: foo_symbol.clone(), + pattern: "pattern".to_string(), + }) + ); assert_eq!( foo_symbol.union(&wc_symbol), Rc::new(RevsetExpression::Union( diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 4558c4330..d3f73ba9a 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::path::Path; + use jujutsu_lib::backend::{CommitId, MillisSinceEpoch, Signature, Timestamp}; use jujutsu_lib::commit_builder::CommitBuilder; use jujutsu_lib::matchers::{FilesMatcher, Matcher}; @@ -434,10 +436,13 @@ fn resolve_commit_ids_in_workspace( repo: RepoRef, revset_str: &str, workspace: &Workspace, + cwd: Option<&Path>, ) -> Vec { let expression = parse(revset_str).unwrap(); let workspace_ctx = RevsetWorkspaceContext { + cwd: cwd.unwrap_or_else(|| workspace.workspace_root()), workspace_id: workspace.workspace_id(), + workspace_root: workspace.workspace_root(), }; expression .evaluate(repo, Some(&workspace_ctx)) @@ -471,7 +476,12 @@ fn test_evaluate_expression_root_and_checkout(use_git: bool) { .set_wc_commit(WorkspaceId::default(), commit1.id().clone()) .unwrap(); assert_eq!( - resolve_commit_ids_in_workspace(mut_repo.as_repo_ref(), "@", &test_workspace.workspace), + resolve_commit_ids_in_workspace( + mut_repo.as_repo_ref(), + "@", + &test_workspace.workspace, + None, + ), vec![commit1.id().clone()] ); } @@ -625,7 +635,12 @@ fn test_evaluate_expression_parents(use_git: bool) { .set_wc_commit(WorkspaceId::default(), commit2.id().clone()) .unwrap(); assert_eq!( - resolve_commit_ids_in_workspace(mut_repo.as_repo_ref(), "@-", &test_workspace.workspace), + resolve_commit_ids_in_workspace( + mut_repo.as_repo_ref(), + "@-", + &test_workspace.workspace, + None, + ), vec![commit1.id().clone()] ); @@ -1739,8 +1754,8 @@ fn test_evaluate_expression_difference(use_git: bool) { #[test_case(true ; "git backend")] fn test_filter_by_diff(use_git: bool) { let settings = testutils::user_settings(); - let test_repo = TestRepo::init(use_git); - let repo = &test_repo.repo; + let test_workspace = TestWorkspace::init(&settings, use_git); + let repo = &test_workspace.repo; let mut tx = repo.start_transaction("test"); let mut_repo = tx.mut_repo(); @@ -1785,6 +1800,7 @@ fn test_filter_by_diff(use_git: bool) { CommitBuilder::for_new_commit(&settings, vec![commit2.id().clone()], tree3.id().clone()) .write_to_repo(mut_repo); + // matcher API: let resolve = |file_path: &RepoPath| -> Vec { let repo_ref = mut_repo.as_repo_ref(); let matcher = FilesMatcher::new([file_path.clone()].into()); @@ -1809,4 +1825,24 @@ fn test_filter_by_diff(use_git: bool) { commit1.id().clone() ] ); + + // file() revset: + assert_eq!( + resolve_commit_ids_in_workspace( + mut_repo.as_repo_ref(), + r#"file("repo/added_clean_clean")"#, + &test_workspace.workspace, + Some(test_workspace.workspace.workspace_root().parent().unwrap()), + ), + vec![commit1.id().clone()] + ); + assert_eq!( + resolve_commit_ids_in_workspace( + mut_repo.as_repo_ref(), + &format!(r#"file("added_modified_clean", {}:)"#, commit2.id().hex()), + &test_workspace.workspace, + Some(test_workspace.workspace.workspace_root()), + ), + vec![commit2.id().clone()] + ); } diff --git a/src/cli_util.rs b/src/cli_util.rs index dfe514bf5..946fdfc71 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -590,7 +590,9 @@ impl WorkspaceCommandHelper { revset_expression: &RevsetExpression, ) -> Result + 'repo>, RevsetError> { let workspace_ctx = RevsetWorkspaceContext { + cwd: &self.cwd, workspace_id: self.workspace.workspace_id(), + workspace_root: self.workspace.workspace_root(), }; revset_expression.evaluate(self.repo.as_repo_ref(), Some(&workspace_ctx)) } diff --git a/tests/test_log_command.rs b/tests/test_log_command.rs index 09bdc1b7a..90ff0ba06 100644 --- a/tests/test_log_command.rs +++ b/tests/test_log_command.rs @@ -197,6 +197,24 @@ fn test_log_filtered_by_path() { second A file2 "###); + + // file() revset doesn't filter the diff. + let stdout = test_env.jj_cmd_success( + &repo_path, + &[ + "log", + "-T", + "description", + "-s", + "-rfile(file2)", + "--no-graph", + ], + ); + insta::assert_snapshot!(stdout, @r###" + second + M file1 + A file2 + "###); } #[test]