From 6cbbf4c77b42767e9d4bac89b3447cb3c0f86117 Mon Sep 17 00:00:00 2001 From: Scott Taylor Date: Sun, 7 Jul 2024 18:38:29 -0500 Subject: [PATCH] revset: add optional `tracked` arg to `remote_branches()` Adds support for `remote_branches(tracked=false)` and `remote_branches(tracked=true)`. I think this would be especially useful for configuring `immutable_heads()` because rewriting untracked remote branches usually wouldn't be desirable (since it wouldn't update the remote branch). It also makes it easy to hide branches that you don't care about from the log, since you could hide untracked branches and then only track branches that you care about. --- CHANGELOG.md | 3 ++ cli/src/commands/git/push.rs | 1 + docs/revsets.md | 9 ++++-- lib/src/revset.rs | 59 ++++++++++++++++++++++++++++++++---- lib/src/revset_parser.rs | 11 +++++++ lib/tests/test_revset.rs | 40 ++++++++++++++++++++++-- 6 files changed, 112 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ace2489aa8..f1612e031f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). address unconditionally. Only ASCII case folding is currently implemented, but this will likely change in the future. +* `remote_branches()` revset now supports optional `tracked` argument (e.g. + `remote_branches(tracked=false)` to filter to untracked remote branches). + ### Fixed bugs ## [0.19.0] - 2024-07-03 diff --git a/cli/src/commands/git/push.rs b/cli/src/commands/git/push.rs index 7e317710b8..632a3f4e38 100644 --- a/cli/src/commands/git/push.rs +++ b/cli/src/commands/git/push.rs @@ -553,6 +553,7 @@ fn find_branches_targeted_by_revisions<'a>( let current_branches_expression = RevsetExpression::remote_branches( StringPattern::everything(), StringPattern::exact(remote_name), + None, ) .range(&RevsetExpression::commit(wc_commit_id)) .intersection(&RevsetExpression::branches(StringPattern::everything())); diff --git a/docs/revsets.md b/docs/revsets.md index 8bebbac8c9..dda01541cc 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -202,8 +202,8 @@ revsets (expressions) as arguments. branches `push-123` and `repushed` but not the branch `main`. If a branch is in a conflicted state, all its possible targets are included. -* `remote_branches([branch_pattern[, [remote=]remote_pattern]])`: All remote - branch targets across all remotes. If just the `branch_pattern` is +* `remote_branches([branch_pattern], [[remote=]remote_pattern], [[tracked=]tracked])`: + All remote branch targets across all remotes. If just the `branch_pattern` is specified, the branches whose names match the given [string pattern](#string-patterns) across all remotes are selected. If both `branch_pattern` and `remote_pattern` are specified, the selection is @@ -214,6 +214,11 @@ revsets (expressions) as arguments. `main@origin` or `main@upstream`. If a branch is in a conflicted state, all its possible targets are included. + Tracked remote branches can be selected with `tracked=true`, while untracked + remote branches can be selected with `tracked=false`. For example, + `remote_branches(remote=origin, tracked=false)` would select any untracked + branches from remotes matching `origin`. + While Git-tracking branches can be selected by `@git`, these branches aren't included in `remote_branches()`. diff --git a/lib/src/revset.rs b/lib/src/revset.rs index 357cc48a55..1a471a6c9b 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -34,7 +34,7 @@ use crate::graph::GraphEdge; use crate::hex_util::to_forward_hex; use crate::id_prefix::IdPrefixContext; use crate::object_id::{HexPrefix, PrefixResolution}; -use crate::op_store::WorkspaceId; +use crate::op_store::{RemoteRefState, WorkspaceId}; use crate::repo::Repo; use crate::repo_path::RepoPathUiConverter; pub use crate::revset_parser::{ @@ -107,6 +107,7 @@ pub enum RevsetCommitRef { RemoteBranches { branch_pattern: StringPattern, remote_pattern: StringPattern, + remote_ref_state: Option, }, Tags, GitRefs, @@ -239,11 +240,13 @@ impl RevsetExpression { pub fn remote_branches( branch_pattern: StringPattern, remote_pattern: StringPattern, + remote_ref_state: Option, ) -> Rc { Rc::new(RevsetExpression::CommitRef( RevsetCommitRef::RemoteBranches { branch_pattern, remote_pattern, + remote_ref_state, }, )) } @@ -626,8 +629,8 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: Ok(RevsetExpression::branches(pattern)) }); map.insert("remote_branches", |function, _context| { - let ([], [branch_opt_arg, remote_opt_arg]) = - function.expect_named_arguments(&["", "remote"])?; + let ([], [branch_opt_arg, remote_opt_arg, tracked_opt_arg]) = + function.expect_named_arguments(&["", "remote", "tracked"])?; let branch_pattern = if let Some(branch_arg) = branch_opt_arg { expect_string_pattern(branch_arg)? } else { @@ -638,9 +641,19 @@ static BUILTIN_FUNCTION_MAP: Lazy> = Lazy: } else { StringPattern::everything() }; + let remote_ref_state = if let Some(tracked_arg) = tracked_opt_arg { + if revset_parser::expect_bool(tracked_arg)? { + Some(RemoteRefState::Tracking) + } else { + Some(RemoteRefState::New) + } + } else { + None + }; Ok(RevsetExpression::remote_branches( branch_pattern, remote_pattern, + remote_ref_state, )) }); map.insert("tags", |function, _context| { @@ -1609,11 +1622,17 @@ fn resolve_commit_ref( RevsetCommitRef::RemoteBranches { branch_pattern, remote_pattern, + remote_ref_state, } => { // TODO: should we allow to select @git branches explicitly? let commit_ids = repo .view() .remote_branches_matching(branch_pattern, remote_pattern) + .filter(|(_, remote_ref)| { + remote_ref_state + .map(|state| remote_ref.state == state) + .unwrap_or(true) + }) .filter(|&((_, remote_name), _)| { #[cfg(feature = "git")] { @@ -2313,14 +2332,16 @@ mod tests { RemoteBranches { branch_pattern: Substring(""), remote_pattern: Substring(""), + remote_ref_state: None, }, ) "###); - insta::assert_debug_snapshot!(parse("remote_branches()").unwrap(), @r###" + insta::assert_debug_snapshot!(parse("remote_branches(foo, bar, true)").unwrap(), @r###" CommitRef( RemoteBranches { - branch_pattern: Substring(""), - remote_pattern: Substring(""), + branch_pattern: Substring("foo"), + remote_pattern: Substring("bar"), + remote_ref_state: Some(Tracking), }, ) "###); @@ -2566,6 +2587,17 @@ mod tests { RemoteBranches { branch_pattern: Substring(""), remote_pattern: Substring("foo"), + remote_ref_state: None, + }, + ) + "###); + insta::assert_debug_snapshot!( + parse("remote_branches(tracked=false, remote=foo)").unwrap(), @r###" + CommitRef( + RemoteBranches { + branch_pattern: Substring(""), + remote_pattern: Substring("foo"), + remote_ref_state: Some(New), }, ) "###); @@ -2575,9 +2607,24 @@ mod tests { RemoteBranches { branch_pattern: Substring("foo"), remote_pattern: Substring("bar"), + remote_ref_state: None, + }, + ) + "###); + insta::assert_debug_snapshot!( + parse("remote_branches(foo, remote=bar, tracked=true)").unwrap(), @r###" + CommitRef( + RemoteBranches { + branch_pattern: Substring("foo"), + remote_pattern: Substring("bar"), + remote_ref_state: Some(Tracking), }, ) "###); + insta::assert_debug_snapshot!( + parse(r#"remote_branches(tracked=foo)"#).unwrap_err().kind(), + @r###"Expression("Expected expression of type bool")"###, + ); insta::assert_debug_snapshot!( parse(r#"remote_branches(remote=foo, bar)"#).unwrap_err().kind(), @r###" diff --git a/lib/src/revset_parser.rs b/lib/src/revset_parser.rs index b860f5041b..28fabab8ee 100644 --- a/lib/src/revset_parser.rs +++ b/lib/src/revset_parser.rs @@ -802,6 +802,17 @@ pub fn expect_literal( }) } +pub fn expect_bool(node: &ExpressionNode) -> Result { + expect_literal_with(node, |node| match &node.kind { + ExpressionKind::Identifier("false") => Ok(false), + ExpressionKind::Identifier("true") => Ok(true), + _ => Err(RevsetParseError::expression( + "Expected expression of type bool", + node.span, + )), + }) +} + /// Applies the give function to the innermost `node` by unwrapping alias /// expansion nodes. fn expect_literal_with( diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index 91e091f6b6..83cebfe6f1 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -2061,7 +2061,7 @@ fn test_evaluate_expression_remote_branches() { let repo = &test_repo.repo; let remote_ref = |target| RemoteRef { target, - state: RemoteRefState::Tracking, // doesn't matter + state: RemoteRefState::Tracking, }; let normal_remote_ref = |id: &CommitId| remote_ref(RefTarget::normal(id.clone())); @@ -2076,8 +2076,16 @@ fn test_evaluate_expression_remote_branches() { // Can get branches when there are none assert_eq!(resolve_commit_ids(mut_repo, "remote_branches()"), vec![]); - // Can get a few branches - mut_repo.set_remote_branch("branch1", "origin", normal_remote_ref(commit1.id())); + // Branch 1 is untracked on remote origin + mut_repo.set_remote_branch( + "branch1", + "origin", + RemoteRef { + target: RefTarget::normal(commit1.id().clone()), + state: RemoteRefState::New, + }, + ); + // Branch 2 is tracked on remote private mut_repo.set_remote_branch("branch2", "private", normal_remote_ref(commit2.id())); // Git-tracking branches aren't included mut_repo.set_remote_branch( @@ -2085,6 +2093,7 @@ fn test_evaluate_expression_remote_branches() { git::REMOTE_NAME_FOR_LOCAL_GIT_REPO, normal_remote_ref(commit_git_remote.id()), ); + // Can get a few branches assert_eq!( resolve_commit_ids(mut_repo, "remote_branches()"), vec![commit2.id().clone(), commit1.id().clone()] @@ -2128,6 +2137,23 @@ fn test_evaluate_expression_remote_branches() { resolve_commit_ids(mut_repo, r#"remote_branches(exact:branch1, exact:origin)"#), vec![commit1.id().clone()] ); + // Can filter branches by tracked and untracked + assert_eq!( + resolve_commit_ids(mut_repo, "remote_branches(tracked=false)"), + vec![commit1.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "remote_branches(tracked=true)"), + vec![commit2.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "remote_branches(branch1, origin, false)"), + vec![commit1.id().clone()] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "remote_branches(branch2, private, true)"), + vec![commit2.id().clone()] + ); // Can silently resolve to an empty set if there's no matches assert_eq!( resolve_commit_ids(mut_repo, "remote_branches(branch3)"), @@ -2149,6 +2175,14 @@ fn test_evaluate_expression_remote_branches() { resolve_commit_ids(mut_repo, r#"remote_branches(exact:branch1, exact:orig)"#), vec![] ); + assert_eq!( + resolve_commit_ids(mut_repo, "remote_branches(branch1, tracked=true)"), + vec![] + ); + assert_eq!( + resolve_commit_ids(mut_repo, "remote_branches(branch2, tracked=false)"), + vec![] + ); // Two branches pointing to the same commit does not result in a duplicate in // the revset mut_repo.set_remote_branch("branch3", "origin", normal_remote_ref(commit2.id()));