From f89f2f9e7dbb1ebc9d76339c78801d95156f83b9 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 1 Nov 2023 12:01:17 +0900 Subject: [PATCH] cli: add "branch list [NAMES]..." filter Like "jj log PATHS...", unmatched name isn't an error. I don't think "jj branch list glob:'push-*'" should fail just because there are no in-flight PR branches. --- CHANGELOG.md | 6 +-- cli/src/commands/branch.rs | 66 ++++++++++++++++++++------------ cli/tests/test_branch_command.rs | 51 +++++++++++++++++++----- 3 files changed, 87 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77abdb96d..9adb5f105 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,9 +61,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `branches()`/`remote_branches()`/`author()`/`committer()`/`description()` revsets now support glob matching. -* `jj branch delete`/`forget`, and `jj git push --branch` now support [string - pattern syntax](docs/revsets.md#string-patterns). The `--glob` option is - deprecated in favor of `glob:` pattern. +* `jj branch delete`/`forget`/`list`, and `jj git push --branch` now support + [string pattern syntax](docs/revsets.md#string-patterns). The `--glob` option + is deprecated in favor of `glob:` pattern. * The `branches`/`tags`/`git_refs`/`git_head` template keywords now return a list of `RefName`s. They were previously pre-formatted strings. diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index 5641b96c8..52c37fb76 100644 --- a/cli/src/commands/branch.rs +++ b/cli/src/commands/branch.rs @@ -84,9 +84,17 @@ pub struct BranchDeleteArgs { pub struct BranchListArgs { /// Show all tracking and non-tracking remote branches including the ones /// whose targets are synchronized with the local branches. - #[arg(long, short, conflicts_with = "revisions")] + #[arg(long, short, conflicts_with_all = ["names", "revisions"])] all: bool, + /// Show branches whose local name matches + /// + /// By default, the specified name matches exactly. Use `glob:` prefix to + /// select branches by wildcard pattern. For details, see + /// https://github.com/martinvonz/jj/blob/main/docs/revsets.md#string-patterns. + #[arg(value_parser = parse_string_pattern)] + pub names: Vec, + /// Show branches whose local targets are in the given revisions. /// /// Note that `-r deleted_branch` will not work since `deleted_branch` @@ -547,29 +555,39 @@ fn cmd_branch_list( let workspace_command = command.workspace_helper(ui)?; let repo = workspace_command.repo(); let view = repo.view(); - let branch_names_to_list: Option> = if !args.revisions.is_empty() { - // Match against local targets only, which is consistent with "jj git push". - let filter_expressions: Vec<_> = args - .revisions - .iter() - .map(|revision_str| workspace_command.parse_revset(revision_str, Some(ui))) - .try_collect()?; - let filter_expression = RevsetExpression::union_all(&filter_expressions); - // Intersects with the set of local branch targets to minimize the lookup space. - let revset_expression = RevsetExpression::branches(StringPattern::everything()) - .intersection(&filter_expression); - let revset_expression = revset::optimize(revset_expression); - let revset = workspace_command.evaluate_revset(revset_expression)?; - let filtered_targets: HashSet = revset.iter().collect(); - // TODO: Suppose we have name-based filter like --glob, should these filters - // be AND-ed or OR-ed? Maybe OR as "jj git push" would do. Perhaps, we - // can consider these options as producers of branch names, not filters - // of different kind (which are typically intersected.) - let branch_names = view - .local_branches() - .filter(|(_, target)| target.added_ids().any(|id| filtered_targets.contains(id))) - .map(|(name, _)| name) - .collect(); + + // Like cmd_git_push(), names and revisions are OR-ed. + let branch_names_to_list = if !args.names.is_empty() || !args.revisions.is_empty() { + let mut branch_names: HashSet<&str> = HashSet::new(); + if !args.names.is_empty() { + branch_names.extend( + view.branches() + .filter(|&(name, _)| args.names.iter().any(|pattern| pattern.matches(name))) + .map(|(name, _)| name), + ); + } + if !args.revisions.is_empty() { + // Match against local targets only, which is consistent with "jj git push". + let filter_expressions: Vec<_> = args + .revisions + .iter() + .map(|revision_str| workspace_command.parse_revset(revision_str, Some(ui))) + .try_collect()?; + let filter_expression = RevsetExpression::union_all(&filter_expressions); + // Intersects with the set of local branch targets to minimize the lookup space. + let revset_expression = RevsetExpression::branches(StringPattern::everything()) + .intersection(&filter_expression); + let revset_expression = revset::optimize(revset_expression); + let revset = workspace_command.evaluate_revset(revset_expression)?; + let filtered_targets: HashSet = revset.iter().collect(); + branch_names.extend( + view.local_branches() + .filter(|(_, target)| { + target.added_ids().any(|id| filtered_targets.contains(id)) + }) + .map(|(name, _)| name), + ); + } Some(branch_names) } else { None diff --git a/cli/tests/test_branch_command.rs b/cli/tests/test_branch_command.rs index 44a51efd1..33f94732d 100644 --- a/cli/tests/test_branch_command.rs +++ b/cli/tests/test_branch_command.rs @@ -861,7 +861,7 @@ fn test_branch_list() { } #[test] -fn test_branch_list_filtered_by_revset() { +fn test_branch_list_filtered() { let test_env = TestEnvironment::default(); test_env.add_config(r#"revset-aliases."immutable_heads()" = "none()""#); @@ -921,13 +921,14 @@ fn test_branch_list_filtered_by_revset() { @origin (ahead by 1 commits, behind by 1 commits): xyxluytn hidden 3e9a5af6 (empty) remote-rewrite "###); - let query = |revset| test_env.jj_cmd_success(&local_path, &["branch", "list", "-r", revset]); + let query = + |args: &[&str]| test_env.jj_cmd_success(&local_path, &[&["branch", "list"], args].concat()); let query_error = - |revset| test_env.jj_cmd_failure(&local_path, &["branch", "list", "-r", revset]); + |args: &[&str]| test_env.jj_cmd_failure(&local_path, &[&["branch", "list"], args].concat()); // "all()" doesn't include deleted branches since they have no local targets. // So "all()" is identical to "branches()". - insta::assert_snapshot!(query("all()"), @r###" + insta::assert_snapshot!(query(&["-rall()"]), @r###" local-keep: kpqxywon c7b4c09c (empty) local-keep remote-keep: nlwprzpn 911e9120 (empty) remote-keep remote-rewrite: xyxluytn e31634b6 (empty) rewritten @@ -936,7 +937,7 @@ fn test_branch_list_filtered_by_revset() { // Exclude remote-only branches. "remote-rewrite@origin" is included since // local "remote-rewrite" target matches. - insta::assert_snapshot!(query("branches()"), @r###" + insta::assert_snapshot!(query(&["-rbranches()"]), @r###" local-keep: kpqxywon c7b4c09c (empty) local-keep remote-keep: nlwprzpn 911e9120 (empty) remote-keep remote-rewrite: xyxluytn e31634b6 (empty) rewritten @@ -944,18 +945,50 @@ fn test_branch_list_filtered_by_revset() { "###); // Select branches by name. - insta::assert_snapshot!(query("branches(remote-rewrite)"), @r###" + insta::assert_snapshot!(query(&["remote-rewrite"]), @r###" + remote-rewrite: xyxluytn e31634b6 (empty) rewritten + @origin (ahead by 1 commits, behind by 1 commits): xyxluytn hidden 3e9a5af6 (empty) remote-rewrite + "###); + insta::assert_snapshot!(query(&["-rbranches(remote-rewrite)"]), @r###" remote-rewrite: xyxluytn e31634b6 (empty) rewritten @origin (ahead by 1 commits, behind by 1 commits): xyxluytn hidden 3e9a5af6 (empty) remote-rewrite "###); - // Can't select deleted branch. - insta::assert_snapshot!(query("branches(remote-delete)"), @r###" + // Can select deleted branch by name pattern, but not by revset. + insta::assert_snapshot!(query(&["remote-delete"]), @r###" + remote-delete (deleted) + @origin: yxusvupt dad5f298 (empty) remote-delete + (this branch will be *deleted permanently* on the remote on the + next `jj git push`. Use `jj branch forget` to prevent this) "###); - insta::assert_snapshot!(query_error("remote-delete"), @r###" + insta::assert_snapshot!(query(&["-rbranches(remote-delete)"]), @r###" + "###); + insta::assert_snapshot!(query_error(&["-rremote-delete"]), @r###" Error: Revision "remote-delete" doesn't exist Hint: Did you mean "remote-delete@origin", "remote-keep", "remote-rewrite", "remote-rewrite@origin"? "###); + + // Name patterns are OR-ed. + insta::assert_snapshot!(query(&["glob:*-keep", "remote-delete"]), @r###" + local-keep: kpqxywon c7b4c09c (empty) local-keep + remote-delete (deleted) + @origin: yxusvupt dad5f298 (empty) remote-delete + (this branch will be *deleted permanently* on the remote on the + next `jj git push`. Use `jj branch forget` to prevent this) + remote-keep: nlwprzpn 911e9120 (empty) remote-keep + "###); + + // Unmatched name pattern shouldn't be an error. A warning can be added later. + insta::assert_snapshot!(query(&["local-keep", "glob:push-*"]), @r###" + local-keep: kpqxywon c7b4c09c (empty) local-keep + "###); + + // Name pattern and revset are OR-ed. + insta::assert_snapshot!(query(&["local-keep", "-rbranches(remote-rewrite)"]), @r###" + local-keep: kpqxywon c7b4c09c (empty) local-keep + remote-rewrite: xyxluytn e31634b6 (empty) rewritten + @origin (ahead by 1 commits, behind by 1 commits): xyxluytn hidden 3e9a5af6 (empty) remote-rewrite + "###); } fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {