cli: use StringPattern to find branches to delete/forget

The error message for unmatched patterns is adjusted so that it can be applied
to both exact and glob patterns.

Cargo.lock

@@ -1003,7 +1003,6 @@ dependencies = [
  "esl01-renderdag",
  "futures 0.3.28",
  "git2",
- "glob",
  "hex",
  "indexmap",
  "insta",

cli/Cargo.toml

@@ -41,7 +41,6 @@ dirs = { workspace = true }
 esl01-renderdag = { workspace = true }
 futures = { workspace = true }
 git2 = { workspace = true }
-glob = { workspace = true }
 hex = { workspace = true }
 indexmap = { workspace = true }
 itertools = { workspace = true }

cli/src/cli_util.rs

@@ -379,12 +379,6 @@ impl From<FsPathParseError> for CommandError {
     }
 }
 
-impl From<glob::PatternError> for CommandError {
-    fn from(err: glob::PatternError) -> Self {
-        user_error(format!("Failed to compile glob: {err}"))
-    }
-}
-
 impl From<clap::Error> for CommandError {
     fn from(err: clap::Error) -> Self {
         CommandError::ClapCliError(Arc::new(err))

cli/src/commands/branch.rs

@@ -1,4 +1,4 @@
-use std::collections::{BTreeSet, HashSet};
+use std::collections::HashSet;
 use std::fmt;
 use std::io::Write as _;
 use std::str::FromStr;
@@ -59,8 +59,8 @@ pub struct BranchDeleteArgs {
     names: Vec<String>,
 
     /// A glob pattern indicating branches to delete.
-    #[arg(long)]
+    #[arg(long, value_parser = StringPattern::glob)]
-    pub glob: Vec<String>,
+    pub glob: Vec<StringPattern>,
 }
 
 /// List branches and their targets
@@ -100,8 +100,8 @@ pub struct BranchForgetArgs {
     pub names: Vec<String>,
 
     /// A glob pattern indicating branches to forget.
-    #[arg(long)]
+    #[arg(long, value_parser = StringPattern::glob)]
-    pub glob: Vec<String>,
+    pub glob: Vec<StringPattern>,
 }
 
 /// Update a given branch to point to a certain commit.
@@ -276,20 +276,18 @@ fn cmd_branch_set(
     Ok(())
 }
 
-/// This function may return the same branch more than once
+fn find_branches(
-fn find_globs(
     view: &View,
-    globs: &[String],
+    name_patterns: &[StringPattern],
     allow_deleted: bool,
 ) -> Result<Vec<String>, CommandError> {
     let mut matching_branches: Vec<String> = vec![];
-    let mut failed_globs = vec![];
+    let mut unmatched_patterns = vec![];
-    for glob_str in globs {
+    for pattern in name_patterns {
-        let glob = glob::Pattern::new(glob_str)?;
         let names = view
             .branches()
             .filter_map(|(branch_name, branch_target)| {
-                if glob.matches(branch_name)
+                if pattern.matches(branch_name)
                     && (allow_deleted || branch_target.local_target.is_present())
                 {
                     Some(branch_name.to_owned())
@@ -299,25 +297,22 @@ fn find_globs(
             })
             .collect_vec();
         if names.is_empty() {
-            failed_globs.push(glob);
+            unmatched_patterns.push(pattern);
         }
         matching_branches.extend(names);
     }
-    match &failed_globs[..] {
+    match &unmatched_patterns[..] {
-        [] => { /* No problem */ }
+        [] => {
-        [glob] => {
+            matching_branches.sort_unstable();
-            return Err(user_error(format!(
+            matching_branches.dedup();
-                "The provided glob '{glob}' did not match any branches"
+            Ok(matching_branches)
-            )))
         }
-        globs => {
+        [pattern] if pattern.is_exact() => Err(user_error(format!("No such branch: {pattern}"))),
-            return Err(user_error(format!(
+        patterns => Err(user_error(format!(
-                "The provided globs '{}' did not match any branches",
+            "No matching branches for patterns: {}",
-                globs.iter().join("', '")
+            patterns.iter().join(", ")
-            )))
+        ))),
-        }
+    }
-    };
-    Ok(matching_branches)
 }
 
 fn cmd_branch_delete(
@@ -327,20 +322,14 @@ fn cmd_branch_delete(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let view = workspace_command.repo().view();
-    for branch_name in &args.names {
+    let name_patterns = itertools::chain(
-        if workspace_command
+        args.names.iter().map(StringPattern::exact),
-            .repo()
+        args.glob.iter().cloned(),
-            .view()
+    )
-            .get_local_branch(branch_name)
+    .collect_vec();
-            .is_absent()
+    let names = find_branches(view, &name_patterns, false)?;
-        {
+    let mut tx =
-            return Err(user_error(format!("No such branch: {branch_name}")));
+        workspace_command.start_transaction(&format!("delete {}", make_branch_term(&names)));
-        }
-    }
-    let globbed_names = find_globs(view, &args.glob, false)?;
-    let names: BTreeSet<String> = args.names.iter().cloned().chain(globbed_names).collect();
-    let branch_term = make_branch_term(names.iter().collect_vec().as_slice());
-    let mut tx = workspace_command.start_transaction(&format!("delete {branch_term}"));
     for branch_name in names.iter() {
         tx.mut_repo()
             .set_local_branch_target(branch_name, RefTarget::absent());
@@ -359,13 +348,14 @@ fn cmd_branch_forget(
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
     let view = workspace_command.repo().view();
-    if let Some(branch_name) = args.names.iter().find(|name| !view.has_branch(name)) {
+    let name_patterns = itertools::chain(
-        return Err(user_error(format!("No such branch: {branch_name}")));
+        args.names.iter().map(StringPattern::exact),
-    }
+        args.glob.iter().cloned(),
-    let globbed_names = find_globs(view, &args.glob, true)?;
+    )
-    let names: BTreeSet<String> = args.names.iter().cloned().chain(globbed_names).collect();
+    .collect_vec();
-    let branch_term = make_branch_term(names.iter().collect_vec().as_slice());
+    let names = find_branches(view, &name_patterns, true)?;
-    let mut tx = workspace_command.start_transaction(&format!("forget {branch_term}"));
+    let mut tx =
+        workspace_command.start_transaction(&format!("forget {}", make_branch_term(&names)));
     for branch_name in names.iter() {
         tx.mut_repo().remove_branch(branch_name);
     }

cli/tests/test_branch_command.rs

@@ -122,9 +122,11 @@ fn test_branch_forget_glob() {
     "###);
 
     // Malformed glob
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "forget", "--glob", "foo-[1-3"]);
+    let stderr = test_env.jj_cmd_cli_error(&repo_path, &["branch", "forget", "--glob", "foo-[1-3"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Failed to compile glob: Pattern syntax error near position 4: invalid range pattern
+    error: invalid value 'foo-[1-3' for '--glob <GLOB>': Pattern syntax error near position 4: invalid range pattern
+
+    For more information, try '--help'.
     "###);
 
     // We get an error if none of the globs match anything
@@ -139,7 +141,7 @@ fn test_branch_forget_glob() {
         ],
     );
     insta::assert_snapshot!(stderr, @r###"
-    Error: The provided globs 'baz*', 'boom*' did not match any branches
+    Error: No matching branches for patterns: baz*, boom*
     "###);
 }
 
@@ -188,7 +190,7 @@ fn test_branch_delete_glob() {
     // forget`, it's not allowed to delete already deleted branches.
     let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "delete", "--glob=foo-[1-3]"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: The provided glob 'foo-[1-3]' did not match any branches
+    Error: No matching branches for patterns: foo-[1-3]
     "###);
 
     // Deleting a branch via both explicit name and glob pattern, or with
@@ -225,9 +227,11 @@ fn test_branch_delete_glob() {
     "###);
 
     // Malformed glob
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["branch", "delete", "--glob", "foo-[1-3"]);
+    let stderr = test_env.jj_cmd_cli_error(&repo_path, &["branch", "delete", "--glob", "foo-[1-3"]);
     insta::assert_snapshot!(stderr, @r###"
-    Error: Failed to compile glob: Pattern syntax error near position 4: invalid range pattern
+    error: invalid value 'foo-[1-3' for '--glob <GLOB>': Pattern syntax error near position 4: invalid range pattern
+
+    For more information, try '--help'.
     "###);
 }