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

@@ -41,6 +41,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]`.)

cli/tests/test_tag_command.rs

@@ -117,3 +117,92 @@ fn test_tag_list() {
     added_targets: commit2
     "###);
 }
+
+#[test]
+fn test_tag_patterns() {
+    let test_env = TestEnvironment::default();
+    test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]);
+    let repo_path = test_env.env_root().join("repo");
+    let git_repo = {
+        let mut git_repo_path = repo_path.clone();
+        git_repo_path.extend([".jj", "repo", "store", "git"]);
+        git2::Repository::open(git_repo_path).unwrap()
+    };
+
+    let copy_ref = |src_name: &str, dest_name: &str| {
+        let src = git_repo.find_reference(src_name).unwrap();
+        let oid = src.target().unwrap();
+        git_repo.reference(dest_name, oid, true, "").unwrap();
+    };
+
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-mcommit1"]);
+    test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "bookmark1"]);
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-mcommit2"]);
+    test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "bookmark2"]);
+    test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-mcommit3"]);
+    test_env.jj_cmd_ok(&repo_path, &["bookmark", "create", "bookmark3"]);
+    test_env.jj_cmd_ok(&repo_path, &["git", "export"]);
+
+    copy_ref("refs/heads/bookmark1", "refs/tags/test_tag");
+    copy_ref("refs/heads/bookmark2", "refs/tags/test_tag2");
+    copy_ref("refs/heads/bookmark3", "refs/tags/test_tag3");
+    copy_ref("refs/heads/bookmark1", "refs/tags/conflicted_tag");
+    test_env.jj_cmd_ok(&repo_path, &["git", "import"]);
+    copy_ref("refs/heads/bookmark2", "refs/tags/conflicted_tag");
+    test_env.jj_cmd_ok(&repo_path, &["git", "import"]);
+    copy_ref("refs/heads/bookmark3", "refs/tags/conflicted_tag");
+    test_env.jj_cmd_ok(&repo_path, &["git", "import", "--at-op=@-"]);
+    test_env.jj_cmd_ok(&repo_path, &["status"]); // resolve concurrent ops
+
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["log", "-r", "tags()"]),
+        @r###"
+    ◆  royxmykx [email protected] 2001-02-03 08:05:12 bookmark3 conflicted_tag?? test_tag3 68d950ce
+    │  (empty) commit3
+    ~
+
+    ◆  zsuskuln [email protected] 2001-02-03 08:05:10 bookmark2 conflicted_tag?? test_tag2 3db783e0
+    │  (empty) commit2
+    ~
+
+    ◆  rlvkpnrz [email protected] 2001-02-03 08:05:08 bookmark1 test_tag caf975d0
+    │  (empty) commit1
+    ~
+    "###);
+
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["log", "-r", "tags(test_tag)"]),
+        @r###"
+    ◆  royxmykx [email protected] 2001-02-03 08:05:12 bookmark3 conflicted_tag?? test_tag3 68d950ce
+    │  (empty) commit3
+    ~
+
+    ◆  zsuskuln [email protected] 2001-02-03 08:05:10 bookmark2 conflicted_tag?? test_tag2 3db783e0
+    │  (empty) commit2
+    ~
+
+    ◆  rlvkpnrz [email protected] 2001-02-03 08:05:08 bookmark1 test_tag caf975d0
+    │  (empty) commit1
+    ~
+    "###);
+
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["log", "-r", "tags("]);
+    insta::assert_snapshot!(stderr,
+        @r###"
+    Error: Failed to parse revset: Syntax error
+    Caused by:  --> 1:6
+      |
+    1 | tags(
+      |      ^---
+      |
+      = expected <identifier> or <expression>
+    "###);
+
+    insta::assert_snapshot!(
+        test_env.jj_cmd_success(&repo_path, &["log", "-r", "tags(exact:test_tag)"]),
+        @r"
+    ◆  rlvkpnrz [email protected] 2001-02-03 08:05:08 bookmark1 test_tag caf975d0
+    │  (empty) commit1
+    ~
+    ");
+}

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()
+                .local_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 local tag `(name, target)`s matching the given pattern. Entries
+    /// are sorted by `name`.
+    pub fn local_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) {