revset: allow tags() to take a pattern for an argument

This makes it more consistent with `bookmarks()`.

Co-authored-by: Austin Seipp <[email protected]>

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 <TABLE-NAME>` no longer removes a table (such as `[ui]`.)

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.

lib/src/revset.rs

@@ -137,7 +137,7 @@ pub enum RevsetCommitRef {
         remote_pattern: StringPattern,
         remote_ref_state: Option<RemoteRefState>,
     },
-    Tags,
+    Tags(StringPattern),
     GitRefs,
     GitHead,
 }
@@ -348,8 +348,8 @@ impl<St: ExpressionState<CommitRef = RevsetCommitRef>> RevsetExpression<St> {
         }))
     }
 
-    pub fn tags() -> Rc<Self> {
-        Rc::new(Self::CommitRef(RevsetCommitRef::Tags))
+    pub fn tags(pattern: StringPattern) -> Rc<Self> {
+        Rc::new(Self::CommitRef(RevsetCommitRef::Tags(pattern)))
     }
 
     pub fn git_refs() -> Rc<Self> {
@@ -785,9 +785,14 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = 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,15 +3464,15 @@ 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,
         }
         "###);
         insta::assert_debug_snapshot!(
             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(_))
         );
     }
 

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<Item = (&'a str, &'a RefTarget)> + '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) {

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