From 068fa0f37e87038b4345d689b0555ce9b2255c03 Mon Sep 17 00:00:00 2001 From: Luke Randall Date: Mon, 18 Nov 2024 20:44:05 +0000 Subject: [PATCH] revset: allow `tags()` to take a pattern for an argument This makes it more consistent with `bookmarks()`. Co-authored-by: Austin Seipp --- CHANGELOG.md | 3 ++ docs/revsets.md | 7 +++- lib/src/revset.rs | 64 ++++++++++++++++++++++-------- lib/src/view.rs | 11 ++++++ lib/tests/test_revset.rs | 85 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 151 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ad6f28658..b634bb9600 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * New `fork_point()` revset function can be used to obtain the fork point of multiple commits. +* The `tags()` revset function now takes an optional `pattern` argument, + mirroring that of `bookmarks()`. + ### Fixed bugs * `jj config unset ` no longer removes a table (such as `[ui]`.) diff --git a/docs/revsets.md b/docs/revsets.md index 766f77b02c..d3ea90def2 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -228,8 +228,11 @@ revsets (expressions) as arguments. All targets of untracked remote bookmarks. Supports the same optional arguments as `remote_bookmarks()`. -* `tags()`: All tag targets. If a tag is in a conflicted state, all its - possible targets are included. +* `tags([pattern])`: All tag targets. If `pattern` is specified, + this selects the tags whose name match the given [string + pattern](#string-patterns). For example, `tags(v1)` would match the + tags `v123` and `rev1` but not the tag `v2`. If a tag is + in a conflicted state, all its possible targets are included. * `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. diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 22a235e3a6..5d1d8162ba 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -137,7 +137,7 @@ pub enum RevsetCommitRef { remote_pattern: StringPattern, remote_ref_state: Option, }, - Tags, + Tags(StringPattern), GitRefs, GitHead, } @@ -348,8 +348,8 @@ impl> RevsetExpression { })) } - pub fn tags() -> Rc { - Rc::new(Self::CommitRef(RevsetCommitRef::Tags)) + pub fn tags(pattern: StringPattern) -> Rc { + Rc::new(Self::CommitRef(RevsetCommitRef::Tags(pattern))) } pub fn git_refs() -> Rc { @@ -785,9 +785,14 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: map["untracked_remote_bookmarks"], ); - map.insert("tags", |_diagnostics, function, _context| { - function.expect_no_arguments()?; - Ok(RevsetExpression::tags()) + map.insert("tags", |diagnostics, function, _context| { + let ([], [opt_arg]) = function.expect_arguments()?; + let pattern = if let Some(arg) = opt_arg { + expect_string_pattern(diagnostics, arg)? + } else { + StringPattern::everything() + }; + Ok(RevsetExpression::tags(pattern)) }); map.insert("git_refs", |_diagnostics, function, _context| { function.expect_no_arguments()?; @@ -2172,11 +2177,13 @@ fn resolve_commit_ref( .collect(); Ok(commit_ids) } - RevsetCommitRef::Tags => { - let mut commit_ids = vec![]; - for ref_target in repo.view().tags().values() { - commit_ids.extend(ref_target.added_ids().cloned()); - } + RevsetCommitRef::Tags(pattern) => { + let commit_ids = repo + .view() + .tags_matching(pattern) + .flat_map(|(_, target)| target.added_ids()) + .cloned() + .collect(); Ok(commit_ids) } RevsetCommitRef::GitRefs => { @@ -2984,6 +2991,10 @@ mod tests { insta::assert_debug_snapshot!( parse("bookmarks()").unwrap(), @r###"CommitRef(Bookmarks(Substring("")))"###); + // Default argument for tags() is "" + insta::assert_debug_snapshot!( + parse("tags()").unwrap(), + @r###"CommitRef(Tags(Substring("")))"###); insta::assert_debug_snapshot!(parse("remote_bookmarks()").unwrap(), @r###" CommitRef( RemoteBookmarks { @@ -3152,6 +3163,25 @@ mod tests { parse(r#"bookmarks(exact:"foo"+)"#).unwrap_err().kind(), @r###"Expression("Expected expression of string pattern")"###); + insta::assert_debug_snapshot!( + parse(r#"tags("foo")"#).unwrap(), + @r###"CommitRef(Tags(Substring("foo")))"###); + insta::assert_debug_snapshot!( + parse(r#"tags(exact:"foo")"#).unwrap(), + @r###"CommitRef(Tags(Exact("foo")))"###); + insta::assert_debug_snapshot!( + parse(r#"tags(substring:"foo")"#).unwrap(), + @r###"CommitRef(Tags(Substring("foo")))"###); + insta::assert_debug_snapshot!( + parse(r#"tags(bad:"foo")"#).unwrap_err().kind(), + @r###"Expression("Invalid string pattern")"###); + insta::assert_debug_snapshot!( + parse(r#"tags(exact::"foo")"#).unwrap_err().kind(), + @r###"Expression("Expected expression of string pattern")"###); + insta::assert_debug_snapshot!( + parse(r#"tags(exact:"foo"+)"#).unwrap_err().kind(), + @r###"Expression("Expected expression of string pattern")"###); + // String pattern isn't allowed at top level. assert_matches!( parse(r#"(exact:"foo")"#).unwrap_err().kind(), @@ -3434,7 +3464,7 @@ mod tests { optimize(parse("(bookmarks() & all())..(all() & tags())").unwrap()), @r###" Range { roots: CommitRef(Bookmarks(Substring(""))), - heads: CommitRef(Tags), + heads: CommitRef(Tags(Substring(""))), generation: 0..18446744073709551615, } "###); @@ -3442,7 +3472,7 @@ mod tests { optimize(parse("(bookmarks() & all())::(all() & tags())").unwrap()), @r###" DagRange { roots: CommitRef(Bookmarks(Substring(""))), - heads: CommitRef(Tags), + heads: CommitRef(Tags(Substring(""))), } "###); @@ -3501,21 +3531,21 @@ mod tests { optimize(parse("(bookmarks() & all()) | (all() & tags())").unwrap()), @r###" Union( CommitRef(Bookmarks(Substring(""))), - CommitRef(Tags), + CommitRef(Tags(Substring(""))), ) "###); insta::assert_debug_snapshot!( optimize(parse("(bookmarks() & all()) & (all() & tags())").unwrap()), @r###" Intersection( CommitRef(Bookmarks(Substring(""))), - CommitRef(Tags), + CommitRef(Tags(Substring(""))), ) "###); insta::assert_debug_snapshot!( optimize(parse("(bookmarks() & all()) ~ (all() & tags())").unwrap()), @r###" Difference( CommitRef(Bookmarks(Substring(""))), - CommitRef(Tags), + CommitRef(Tags(Substring(""))), ) "###); } @@ -3565,7 +3595,7 @@ mod tests { )); assert_matches!( unwrap_union(&optimized).1.as_ref(), - RevsetExpression::CommitRef(RevsetCommitRef::Tags) + RevsetExpression::CommitRef(RevsetCommitRef::Tags(_)) ); } diff --git a/lib/src/view.rs b/lib/src/view.rs index 64b677bcdb..85abd5021f 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -309,6 +309,17 @@ impl View { self.data.tags.get(name).flatten() } + /// Iterates tags `(name, target)`s matching the given pattern. Entries + /// are sorted by `name`. + pub fn tags_matching<'a: 'b, 'b>( + &'a self, + pattern: &'b StringPattern, + ) -> impl Iterator + 'b { + pattern + .filter_btree_map(&self.data.tags) + .map(|(name, target)| (name.as_ref(), target)) + } + /// Sets tag to point to the given target. If the target is absent, the tag /// will be removed. pub fn set_tag_target(&mut self, name: &str, target: RefTarget) { diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 5d45a08d79..51ee358de4 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -2305,6 +2305,91 @@ fn test_evaluate_expression_remote_bookmarks() { ); } +#[test] +fn test_evaluate_expression_tags() { + let settings = testutils::user_settings(); + let test_repo = TestRepo::init(); + let repo = &test_repo.repo; + + let mut tx = repo.start_transaction(&settings); + let mut_repo = tx.repo_mut(); + + let commit1 = write_random_commit(mut_repo, &settings); + let commit2 = write_random_commit(mut_repo, &settings); + let commit3 = write_random_commit(mut_repo, &settings); + let commit4 = write_random_commit(mut_repo, &settings); + + // Can get tags when there are none + assert_eq!(resolve_commit_ids(mut_repo, "tags()"), vec![]); + // Can get a few tags + mut_repo.set_tag_target("tag1", RefTarget::normal(commit1.id().clone())); + mut_repo.set_tag_target("tag2", RefTarget::normal(commit2.id().clone())); + assert_eq!( + resolve_commit_ids(mut_repo, "tags()"), + vec![commit2.id().clone(), commit1.id().clone()] + ); + // Can get tags with matching names + assert_eq!( + resolve_commit_ids(mut_repo, "tags(tag1)"), + vec![commit1.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "tags(tag)"), + vec![commit2.id().clone(), commit1.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "tags(exact:tag1)"), + vec![commit1.id().clone()] + ); + assert_eq!(resolve_commit_ids(mut_repo, r#"tags(glob:"Tag?")"#), vec![]); + assert_eq!( + resolve_commit_ids(mut_repo, r#"tags(glob-i:"Tag?")"#), + vec![commit2.id().clone(), commit1.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "tags(regex:'ag')"), + vec![commit2.id().clone(), commit1.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "tags(regex:'^[Tt]ag1$')"), + vec![commit1.id().clone()] + ); + // Can silently resolve to an empty set if there's no matches + assert_eq!(resolve_commit_ids(mut_repo, "tags(tag3)"), vec![]); + assert_eq!(resolve_commit_ids(mut_repo, "tags(exact:ag1)"), vec![]); + // Two tags pointing to the same commit does not result in a duplicate in + // the revset + mut_repo.set_tag_target("tag3", RefTarget::normal(commit2.id().clone())); + assert_eq!( + resolve_commit_ids(mut_repo, "tags()"), + vec![commit2.id().clone(), commit1.id().clone()] + ); + // Can get tags when there are conflicted refs + mut_repo.set_tag_target( + "tag1", + RefTarget::from_legacy_form( + [commit1.id().clone()], + [commit2.id().clone(), commit3.id().clone()], + ), + ); + mut_repo.set_tag_target( + "tag2", + RefTarget::from_legacy_form( + [commit2.id().clone()], + [commit3.id().clone(), commit4.id().clone()], + ), + ); + mut_repo.set_tag_target("tag3", RefTarget::absent()); + assert_eq!( + resolve_commit_ids(mut_repo, "tags()"), + vec![ + commit4.id().clone(), + commit3.id().clone(), + commit2.id().clone() + ] + ); +} + #[test] fn test_evaluate_expression_latest() { let settings = testutils::user_settings();