diff --git a/crates/gitbutler-branch-actions/src/branch.rs b/crates/gitbutler-branch-actions/src/branch.rs index ab4424fb2..b80d69261 100644 --- a/crates/gitbutler-branch-actions/src/branch.rs +++ b/crates/gitbutler-branch-actions/src/branch.rs @@ -2,7 +2,9 @@ use crate::VirtualBranchesExt; use anyhow::{Context, Result}; use bstr::{BStr, BString, ByteSlice}; use core::fmt; -use gitbutler_branch::{Branch as GitButlerBranch, BranchId, ReferenceExtGix, Target}; +use gitbutler_branch::{ + Branch as GitButlerBranch, BranchId, BranchIdentity, ReferenceExtGix, Target, +}; use gitbutler_command_context::CommandContext; use gitbutler_reference::normalize_branch_name; use gix::prelude::ObjectIdExt; @@ -189,9 +191,13 @@ fn branch_group_to_branch( }; if virtual_branch.is_none() - && local_branches - .iter() - .any(|b| b.name().given_name(remotes).as_deref().ok() == Some(target.branch.branch())) + && local_branches.iter().any(|b| { + b.name() + .identity(remotes) + .as_deref() + .ok() + .map_or(false, |identity| identity == target.branch.branch()) + }) { return Ok(None); } @@ -292,7 +298,7 @@ impl GroupBranch<'_> { fn identity(&self, remotes: &BTreeSet>) -> Option { match self { GroupBranch::Local(branch) | GroupBranch::Remote(branch) => { - branch.name().given_name(remotes).ok() + branch.name().identity(remotes).ok() } // The identity of a Virtual branch is derived from the source refname, upstream or the branch given name, in that order GroupBranch::Virtual(branch) => { @@ -301,10 +307,10 @@ impl GroupBranch<'_> { let rich_name = branch.name.clone(); let rich_name = normalize_branch_name(&rich_name).ok()?; let identity = name_from_source.unwrap_or(name_from_upstream.unwrap_or(&rich_name)); - Some(identity.to_string()) + Some(identity.into()) } } - .map(BranchIdentity) + .map(BranchIdentity::from) } } @@ -312,14 +318,13 @@ impl GroupBranch<'_> { /// This excludes the target branch as well as gitbutler specific branches. fn should_list_git_branch(identity: &BranchIdentity) -> bool { // Exclude gitbutler technical branches (not useful for the user) - let is_technical = [ - "gitbutler/integration", - "gitbutler/target", - "gitbutler/oplog", - "HEAD", - ] - .contains(&&*identity.0); - !is_technical + const TECHNICAL_IDENTITIES: &[&[u8]] = &[ + b"gitbutler/integration", + b"gitbutler/target", + b"gitbutler/oplog", + b"HEAD", + ]; + !TECHNICAL_IDENTITIES.contains(&identity.0.as_bytes()) } /// A filter that can be applied to the branch listing @@ -375,32 +380,6 @@ pub struct Author { pub email: Option, } -/// The identity of a branch as to allow to group similar branches together. -/// -/// * For *local* branches, it is what's left without the standard prefix, like `refs/heads`, e.g. `main` -/// for `refs/heads/main` or `feat/one` for `refs/heads/feat/one`. -/// * For *remote* branches, it is what's without the prefix and remote name, like `main` for `refs/remotes/origin/main`. -/// or `feat/one` for `refs/remotes/my/special/remote/feat/one`. -/// * For virtual branches, it's either the above if there is a `source_refname` or an `upstream`, or it's the normalized -/// name of the virtual branch. -#[derive(Debug, Clone, Serialize, PartialEq, Eq, Hash, Ord, PartialOrd)] -pub struct BranchIdentity(String); - -/// Facilitate obtaining this type from the UI - otherwise it would be better not to have it as it should be -/// a particular thing, not any string. -impl From for BranchIdentity { - fn from(value: String) -> Self { - BranchIdentity(value) - } -} - -/// Also not for testing. -impl From<&str> for BranchIdentity { - fn from(value: &str) -> Self { - BranchIdentity(value.into()) - } -} - impl From> for Author { fn from(value: git2::Signature) -> Self { let name = value.name().map(str::to_string).map(Into::into); diff --git a/crates/gitbutler-branch-actions/src/lib.rs b/crates/gitbutler-branch-actions/src/lib.rs index ff65038f9..8ffae0891 100644 --- a/crates/gitbutler-branch-actions/src/lib.rs +++ b/crates/gitbutler-branch-actions/src/lib.rs @@ -41,6 +41,6 @@ mod commit; mod hunk; pub use branch::{ - get_branch_listing_details, list_branches, Author, BranchIdentity, BranchListing, - BranchListingDetails, BranchListingFilter, + get_branch_listing_details, list_branches, Author, BranchListing, BranchListingDetails, + BranchListingFilter, }; diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/list.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/list.rs index 48f060f19..c8a294c71 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/list.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/list.rs @@ -137,7 +137,8 @@ fn one_branch_on_integration_multiple_remotes() -> Result<()> { mod util { use anyhow::Result; - use gitbutler_branch_actions::{BranchIdentity, BranchListing, BranchListingFilter}; + use gitbutler_branch::BranchIdentity; + use gitbutler_branch_actions::{BranchListing, BranchListingFilter}; use gitbutler_command_context::CommandContext; /// A flattened and simplified mirror of `BranchListing` for comparing the actual and expected data. diff --git a/crates/gitbutler-branch/src/branch.rs b/crates/gitbutler-branch/src/branch.rs index f0ad1a57a..b94bbc1f0 100644 --- a/crates/gitbutler-branch/src/branch.rs +++ b/crates/gitbutler-branch/src/branch.rs @@ -1,7 +1,9 @@ use anyhow::Result; +use bstr::{BStr, BString, ByteSlice}; use gitbutler_id::id::Id; use gitbutler_reference::{normalize_branch_name, Refname, RemoteRefname, VirtualRefname}; use serde::{Deserialize, Serialize}; +use std::ops::Deref; use crate::ownership::BranchOwnershipClaims; @@ -135,3 +137,41 @@ pub struct BranchCreateRequest { pub order: Option, pub selected_for_changes: Option, } + +/// The identity of a branch as to allow to group similar branches together. +/// +/// * For *local* branches, it is what's left without the standard prefix, like `refs/heads`, e.g. `main` +/// for `refs/heads/main` or `feat/one` for `refs/heads/feat/one`. +/// * For *remote* branches, it is what's without the prefix and remote name, like `main` for `refs/remotes/origin/main`. +/// or `feat/one` for `refs/remotes/my/special/remote/feat/one`. +/// * For virtual branches, it's either the above if there is a `source_refname` or an `upstream`, or it's the normalized +/// name of the virtual branch. +#[derive(Debug, Clone, Serialize, PartialEq, Eq, Hash, Ord, PartialOrd)] +pub struct BranchIdentity( + /// The identity is always a valid reference name, full or partial. + // TODO(ST): make this a partial reference name + pub BString, +); + +impl Deref for BranchIdentity { + type Target = BStr; + + fn deref(&self) -> &Self::Target { + self.0.as_bstr() + } +} + +/// Facilitate obtaining this type from the UI - otherwise it would be better not to have it as it should be +/// a particular thing, not any string. +impl From for BranchIdentity { + fn from(value: String) -> Self { + BranchIdentity(value.into()) + } +} + +/// Also not for testing. +impl From<&str> for BranchIdentity { + fn from(value: &str) -> Self { + BranchIdentity(value.into()) + } +} diff --git a/crates/gitbutler-branch/src/lib.rs b/crates/gitbutler-branch/src/lib.rs index 04cf5a144..773036604 100644 --- a/crates/gitbutler-branch/src/lib.rs +++ b/crates/gitbutler-branch/src/lib.rs @@ -1,7 +1,7 @@ mod branch; use anyhow::Context; -pub use branch::{Branch, BranchCreateRequest, BranchId, BranchUpdateRequest}; +pub use branch::{Branch, BranchCreateRequest, BranchId, BranchIdentity, BranchUpdateRequest}; use bstr::ByteSlice; mod branch_ext; pub use branch_ext::BranchExt; diff --git a/crates/gitbutler-branch/src/reference_ext.rs b/crates/gitbutler-branch/src/reference_ext.rs index f7e693cf1..16066b97f 100644 --- a/crates/gitbutler-branch/src/reference_ext.rs +++ b/crates/gitbutler-branch/src/reference_ext.rs @@ -1,3 +1,4 @@ +use crate::BranchIdentity; use anyhow::{Context, Result}; use bstr::{BStr, ByteSlice}; use gix::refs::Category; @@ -19,15 +20,15 @@ pub trait ReferenceExt { // TODO(ST): replace the original with this one. pub trait ReferenceExtGix { - /// Fetches a branches name without the remote name attached + /// Produces a name by removing all prefixes, leaving only its actual name. All known + /// `remotes` are needed to be able to strip remote names. /// - /// refs/heads/my-branch -> my-branch - /// refs/remotes/origin/my-branch -> my-branch - /// refs/remotes/Byron/gitbutler/my-branch -> my-branch (where the remote is Byron/gitbutler) + /// Here are some examples: /// - /// An ideal implementation wouldn't require us to list all the references, - /// but there doesn't seem to be a libgit2 solution to this. - fn given_name(&self, remotes: &BTreeSet>) -> Result; + /// `refs/heads/my-branch` -> `my-branch` + /// `refs/remotes/origin/my-branch` -> `my-branch` + /// `refs/remotes/Byron/gitbutler/my-branch` -> `my-branch` (where the remote is `Byron/gitbutler`) + fn identity(&self, remotes: &BTreeSet>) -> Result; } impl<'repo> ReferenceExt for git2::Reference<'repo> { @@ -65,13 +66,12 @@ impl<'repo> ReferenceExt for git2::Reference<'repo> { } impl ReferenceExtGix for &gix::refs::FullNameRef { - // TODO: make this `identity()`, and use `BranchIdentity` type. - fn given_name(&self, remotes: &BTreeSet>) -> Result { + fn identity(&self, remotes: &BTreeSet>) -> Result { let (category, shorthand_name) = self .category_and_short_name() .context("Branch could not be categorized")?; if !matches!(category, Category::RemoteBranch) { - return Ok(shorthand_name.to_string()); + return Ok(BranchIdentity(shorthand_name.into())); } let longest_remote = remotes @@ -91,7 +91,6 @@ impl ReferenceExtGix for &gix::refs::FullNameRef { ))? .into(); - // TODO(correctness): this should be `BString`, but can't change it yet. - Ok(shorthand_name.to_string()) + Ok(BranchIdentity(shorthand_name.into())) } } diff --git a/crates/gitbutler-reference/src/refname/remote.rs b/crates/gitbutler-reference/src/refname/remote.rs index 1197619d9..1f119b85f 100644 --- a/crates/gitbutler-reference/src/refname/remote.rs +++ b/crates/gitbutler-reference/src/refname/remote.rs @@ -6,9 +6,11 @@ use super::error::Error; #[derive(Debug, Clone, Hash, PartialEq, Eq)] pub struct Refname { - // contains name of the remote, e.x. "origin" or "upstream" + /// contains name of the remote, e.x. "origin" or "upstream" remote: String, - // contains name of the branch, e.x. "master" or "main" + /// contains name of the branch, e.x. "master" or "main" + // TODO(ST): use `BString` for this, or maybe figure out if there could + // be better abstractions for `Refname`, or a better name for the type. branch: String, }