From cefbeba776c9366afde8977efb5675700d564fc9 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 28 Jun 2023 11:53:01 +0900 Subject: [PATCH] cli: add revset filter to "branch list" Typical query would be something like -r 'mine()' or -r 'branches()' to exclude remote-only branches #1136. The query matches against local targets only. This means there's no way to select deleted/forgotten branches by -r option. If we add a default revset configuration, we'll need some way to turn the default off. --- CHANGELOG.md | 2 + src/commands/branch.rs | 56 ++++++++++++++++++++-- tests/test_branch_command.rs | 91 ++++++++++++++++++++++++++++++++++++ 3 files changed, 144 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31a3412cd..e8902457d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -119,6 +119,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * `jj sparse set` now accepts an `--edit` flag which brings up the `$EDITOR` to edit sparse patterns. +* `jj branch list` can now be filtered by revset. + ### Fixed bugs * Modify/delete conflicts now include context lines diff --git a/src/commands/branch.rs b/src/commands/branch.rs index e617ade2e..20c191a6c 100644 --- a/src/commands/branch.rs +++ b/src/commands/branch.rs @@ -1,12 +1,12 @@ -use std::collections::BTreeSet; +use std::collections::{BTreeSet, HashSet}; use clap::builder::NonEmptyStringValueParser; use itertools::Itertools; use jujutsu_lib::backend::{CommitId, ObjectId}; use jujutsu_lib::git::git_tracking_branches; -use jujutsu_lib::op_store::RefTarget; +use jujutsu_lib::op_store::{BranchTarget, RefTarget}; use jujutsu_lib::repo::Repo; -use jujutsu_lib::revset; +use jujutsu_lib::revset::{self, RevsetExpression}; use jujutsu_lib::view::View; use crate::cli_util::{user_error, user_error_with_hint, CommandError, CommandHelper, RevisionArg}; @@ -65,7 +65,14 @@ pub struct BranchDeleteArgs { /// preceded by a "+". For information about branches, see /// https://github.com/martinvonz/jj/blob/main/docs/branches.md. #[derive(clap::Args, Clone, Debug)] -pub struct BranchListArgs; +pub struct BranchListArgs { + /// Show branches whose local targets are in the given revisions. + /// + /// Note that `-r deleted_branch` will not work since `deleted_branch` + /// wouldn't have a local target. + #[arg(long, short)] + revisions: Vec, +} /// Forget everything about a branch, including its local and remote /// targets. @@ -311,7 +318,7 @@ fn cmd_branch_forget( fn cmd_branch_list( ui: &mut Ui, command: &CommandHelper, - _args: &BranchListArgs, + args: &BranchListArgs, ) -> Result<(), CommandError> { let workspace_command = command.workspace_helper(ui)?; let repo = workspace_command.repo(); @@ -338,6 +345,45 @@ fn cmd_branch_list( } } + if !args.revisions.is_empty() { + // Match against local targets only, which is consistent with "jj git push". + fn local_targets(branch_target: &BranchTarget) -> &[CommitId] { + if let Some(target) = branch_target.local_target.as_ref() { + target.adds() + } else { + &[] + } + } + + let filter_expressions: Vec<_> = args + .revisions + .iter() + .map(|revision_str| workspace_command.parse_revset(revision_str)) + .try_collect()?; + let filter_expression = RevsetExpression::union_all(&filter_expressions); + // Intersects with the set of all branch targets to minimize the lookup space. + let all_targets = all_branches + .values() + .flat_map(local_targets) + .cloned() + .collect(); + let revset_expression = + RevsetExpression::commits(all_targets).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: If we add name-based filter like --glob, this might have to be + // rewritten as a positive list or predicate function. Should they + // 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.) + all_branches.retain(|_, branch_target| { + local_targets(branch_target) + .iter() + .any(|id| filtered_targets.contains(id)) + }); + } + let print_branch_target = |formatter: &mut dyn Formatter, target: &RefTarget| -> Result<(), CommandError> { match target { diff --git a/tests/test_branch_command.rs b/tests/test_branch_command.rs index 8f6636dd1..2342202bc 100644 --- a/tests/test_branch_command.rs +++ b/tests/test_branch_command.rs @@ -436,6 +436,97 @@ fn test_branch_forget_deleted_or_nonexistent_branch() { // TODO: Test `jj branch list` with a remote named `git` +#[test] +fn test_branch_list_filtered_by_revset() { + let test_env = TestEnvironment::default(); + + // Initialize remote refs + test_env.jj_cmd_success(test_env.env_root(), &["init", "remote", "--git"]); + let remote_path = test_env.env_root().join("remote"); + for branch in ["remote-keep", "remote-delete", "remote-rewrite"] { + test_env.jj_cmd_success(&remote_path, &["new", "root", "-m", branch]); + test_env.jj_cmd_success(&remote_path, &["branch", "set", branch]); + } + test_env.jj_cmd_success(&remote_path, &["new"]); + test_env.jj_cmd_success(&remote_path, &["git", "export"]); + + // Initialize local refs + let mut remote_git_path = remote_path; + remote_git_path.extend([".jj", "repo", "store", "git"]); + test_env.jj_cmd_success( + test_env.env_root(), + &["git", "clone", remote_git_path.to_str().unwrap(), "local"], + ); + let local_path = test_env.env_root().join("local"); + test_env.jj_cmd_success(&local_path, &["new", "root", "-m", "local-keep"]); + test_env.jj_cmd_success(&local_path, &["branch", "set", "local-keep"]); + + // Mutate refs in local repository + test_env.jj_cmd_success(&local_path, &["branch", "delete", "remote-delete"]); + test_env.jj_cmd_success(&local_path, &["describe", "-mrewritten", "remote-rewrite"]); + + let template = r#"separate(" ", commit_id.short(), branches, if(hidden, "(hidden)"))"#; + insta::assert_snapshot!( + test_env.jj_cmd_success( + &local_path, + &["log", "-r:(branches() | remote_branches())", "-T", template], + ), + @r###" + ◉ e31634b64294 remote-rewrite* + │ @ c7b4c09cd77c local-keep + ├─╯ + │ ◉ 3e9a5af6ef15 remote-rewrite@origin (hidden) + ├─╯ + │ ◉ 911e912015fb remote-keep + ├─╯ + │ ◉ dad5f298ca57 remote-delete@origin + ├─╯ + ◉ 000000000000 + "###); + + // All branches are listed by default. + insta::assert_snapshot!(test_env.jj_cmd_success(&local_path, &["branch", "list"]), @r###" + local-keep: c7b4c09cd77c local-keep + remote-delete (deleted) + @origin: dad5f298ca57 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: 911e912015fb remote-keep + remote-rewrite: e31634b64294 rewritten + @origin (ahead by 1 commits, behind by 1 commits): 3e9a5af6ef15 remote-rewrite + "###); + + let query = |revset| test_env.jj_cmd_success(&local_path, &["branch", "list", "-r", revset]); + + // "all()" doesn't include deleted branches since they have no local targets. + // So "all()" is identical to "branches()". + insta::assert_snapshot!(query("all()"), @r###" + local-keep: c7b4c09cd77c local-keep + remote-keep: 911e912015fb remote-keep + remote-rewrite: e31634b64294 rewritten + @origin (ahead by 1 commits, behind by 1 commits): 3e9a5af6ef15 remote-rewrite + "###); + + // Exclude remote-only branches. "remote-rewrite@origin" is included since + // local "remote-rewrite" target matches. + insta::assert_snapshot!(query("branches()"), @r###" + local-keep: c7b4c09cd77c local-keep + remote-keep: 911e912015fb remote-keep + remote-rewrite: e31634b64294 rewritten + @origin (ahead by 1 commits, behind by 1 commits): 3e9a5af6ef15 remote-rewrite + "###); + + // Select branches by name. + insta::assert_snapshot!(query("branches(remote-rewrite)"), @r###" + remote-rewrite: e31634b64294 rewritten + @origin (ahead by 1 commits, behind by 1 commits): 3e9a5af6ef15 remote-rewrite + "###); + + // Can't select deleted branch. + insta::assert_snapshot!(query("remote-delete"), @r###" + "###); +} + fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String { let template = r#"branches ++ " " ++ commit_id.short()"#; test_env.jj_cmd_success(cwd, &["log", "-T", template])