Merge pull request #4415 from gitbutlerapp/update-svelte

Fix up branches list

app/src/lib/branches/service.ts

@@ -34,31 +34,19 @@ export class BranchService {
 }
 
 function mergeBranchesAndPrs(
-	vbranches: VirtualBranch[] | undefined,
+	_vbranches: VirtualBranch[] | undefined,
 	pullRequests: PullRequest[] | undefined,
 	remoteBranches: Branch[] | undefined
 ): CombinedBranch[] {
 	const contributions: CombinedBranch[] = [];
 
-	// First we add everything with a virtual branch
-	if (vbranches) {
-		contributions.push(
-			...vbranches.map((vb) => {
-				const pr = pullRequests?.find((pr) => pr.sourceBranch === vb.upstreamName);
-				return new CombinedBranch({ vbranch: vb, remoteBranch: vb.upstream, pr });
-			})
-		);
-	}
-
 	// Then remote branches that have no virtual branch, combined with pull requests if present
 	if (remoteBranches) {
 		contributions.push(
-			...remoteBranches
-				.filter((rb) => !contributions.some((cb) => rb.sha === cb.upstreamSha))
-				.map((rb) => {
-					const pr = pullRequests?.find((pr) => pr.sha === rb.sha);
-					return new CombinedBranch({ remoteBranch: rb, pr });
-				})
+			...remoteBranches.map((rb) => {
+				const pr = pullRequests?.find((pr) => pr.sha === rb.sha);
+				return new CombinedBranch({ remoteBranch: rb, pr });
+			})
 		);
 	}
 
@@ -74,10 +62,8 @@ function mergeBranchesAndPrs(
 	}
 
 	// This should be everything considered a branch in one list
-	const filtered = contributions
-		.filter((b) => !b.vbranch)
-		.sort((a, b) => {
-			return (a.modifiedAt || new Date(0)) < (b.modifiedAt || new Date(0)) ? 1 : -1;
-		});
+	const filtered = contributions.sort((a, b) => {
+		return (a.modifiedAt || new Date(0)) < (b.modifiedAt || new Date(0)) ? 1 : -1;
+	});
 	return filtered;
 }

app/src/lib/components/FilterBranchesButton.svelte

@@ -12,7 +12,6 @@
 
 	export let includePrs: Writable<boolean | undefined>;
 	export let includeRemote: Writable<boolean | undefined>;
-	export let includeStashed: Writable<boolean | undefined>;
 	export let hideBots: Writable<boolean | undefined>;
 	export let hideInactive: Writable<boolean | undefined>;
 
@@ -41,13 +40,9 @@
 					<Checkbox small bind:checked={$includePrs} slot="control" />
 				</ContextMenuItem>
 			{/if}
-			<ContextMenuItem label="Remote" on:click={() => ($includeRemote = !$includeRemote)}>
+			<ContextMenuItem label="Branches" on:click={() => ($includeRemote = !$includeRemote)}>
 				<Checkbox small bind:checked={$includeRemote} slot="control" />
 			</ContextMenuItem>
-
-			<ContextMenuItem label="Unapplied" on:click={() => ($includeStashed = !$includeStashed)}>
-				<Checkbox small bind:checked={$includeStashed} slot="control" />
-			</ContextMenuItem>
 		</ContextMenuSection>
 
 		<ContextMenuSection>

app/src/lib/navigation/Branches.svelte

@@ -23,34 +23,24 @@
 	// let contextMenu: ContextMenuActions;
 	let includePrs = persisted(true, 'includePrs_' + projectId);
 	let includeRemote = persisted(true, 'includeRemote_' + projectId);
-	let includeStashed = persisted(true, 'includeStashed_' + projectId);
 	let hideBots = persisted(false, 'hideBots_' + projectId);
 	let hideInactive = persisted(false, 'hideInactive_' + projectId);
 
 	let filtersActive = derived(
-		[includePrs, includeRemote, includeStashed, hideBots, hideInactive],
-		([prs, remote, stashed, bots, inactive]) => {
-			return !prs || !remote || !stashed || bots || inactive;
+		[includePrs, includeRemote, hideBots, hideInactive],
+		([prs, remote, bots, inactive]) => {
+			return !prs || !remote || bots || inactive;
 		}
 	);
 
 	$: branches = $branchService?.branches || readable([]);
 	$: filteredBranches = branches
 		? derived(
-				[branches, textFilter, includePrs, includeRemote, includeStashed, hideBots, hideInactive],
-				([
-					branches,
-					textFilter,
-					includePrs,
-					includeRemote,
-					includeStashed,
-					hideBots,
-					hideInactive
-				]) => {
+				[branches, textFilter, includePrs, includeRemote, hideBots, hideInactive],
+				([branches, textFilter, includePrs, includeRemote, hideBots, hideInactive]) => {
 					const filteredByType = filterByType(branches, {
 						includePrs,
 						includeRemote,
-						includeStashed,
 						hideBots
 					});
 					const filteredBySearch = filterByText(filteredByType, textFilter);
@@ -67,16 +57,17 @@
 		params: {
 			includePrs: boolean;
 			includeRemote: boolean;
-			includeStashed: boolean;
 			hideBots: boolean;
 		}
 	): CombinedBranch[] {
 		return branches.filter((b) => {
 			if (params.includePrs && b.pr) {
 				return !params.hideBots || !b.pr.author?.isBot;
+			} else {
+				if (b.pr) return false;
 			}
+
 			if (params.includeRemote && b.remoteBranch) return true;
-			if (params.includeStashed && b.vbranch) return true;
 			return false;
 		});
 	}
@@ -118,7 +109,6 @@
 				{filtersActive}
 				{includePrs}
 				{includeRemote}
-				{includeStashed}
 				{hideBots}
 				{hideInactive}
 				showPrCheckbox={!!$gitHost}

app/src/lib/vbranches/types.ts

@@ -314,6 +314,7 @@ export class Branch {
 	upstream?: string;
 	lastCommitTimestampMs?: number | undefined;
 	lastCommitAuthor?: string | undefined;
+	givenName!: string;
 
 	get displayName(): string {
 		return this.name.replace('refs/remotes/', '').replace('refs/heads/', '');

app/src/routes/[projectId]/remote/[...name]/+page.svelte

@@ -9,8 +9,8 @@
 	import { page } from '$app/stores';
 
 	const branchService = getBranchServiceStore();
-	const branches = $branchService?.branches;
-	const error = $branchService?.error;
+	const branches = $derived($branchService?.branches);
+	const error = $derived($branchService?.error);
 	const branch = $derived(
 		$branches?.find((cb) => cb.remoteBranch?.displayName === $page.params.name)
 	);

crates/gitbutler-branch-actions/src/remote.rs

@@ -27,6 +27,7 @@ pub struct RemoteBranch {
     pub sha: git2::Oid,
     pub name: Refname,
     pub upstream: Option<RemoteRefname>,
+    pub given_name: String,
     pub last_commit_timestamp_ms: Option<u128>,
     pub last_commit_author: Option<String>,
 }
@@ -69,7 +70,7 @@ pub fn list_remote_branches(project_repository: &ProjectRepository) -> Result<Ve
         .context("failed to list remote branches")?
         .flatten()
     {
-        let branch = branch_to_remote_branch(&branch);
+        let branch = branch_to_remote_branch(project_repository, &branch);
 
         if let Some(branch) = branch {
             let branch_is_trunk = branch.name.branch() == Some(default_target.branch.branch())
@@ -87,21 +88,24 @@ pub fn list_remote_branches(project_repository: &ProjectRepository) -> Result<Ve
 }
 
 pub(crate) fn get_branch_data(
-    project_repository: &ProjectRepository,
+    ctx: &ProjectRepository,
     refname: &Refname,
 ) -> Result<RemoteBranchData> {
-    let default_target = default_target(&project_repository.project().gb_dir())?;
+    let default_target = default_target(&ctx.project().gb_dir())?;
 
-    let branch = project_repository
+    let branch = ctx
         .repo()
         .find_branch_by_refname(refname)?
         .ok_or(anyhow::anyhow!("failed to find branch {}", refname))?;
 
-    branch_to_remote_branch_data(project_repository, &branch, default_target.sha)?
+    branch_to_remote_branch_data(ctx, &branch, default_target.sha)?
         .context("failed to get branch data")
 }
 
-pub(crate) fn branch_to_remote_branch(branch: &git2::Branch) -> Option<RemoteBranch> {
+pub(crate) fn branch_to_remote_branch(
+    ctx: &ProjectRepository,
+    branch: &git2::Branch,
+) -> Option<RemoteBranch> {
     let commit = match branch.get().peel_to_commit() {
         Ok(c) => c,
         Err(err) => {
@@ -116,6 +120,9 @@ pub(crate) fn branch_to_remote_branch(branch: &git2::Branch) -> Option<RemoteBra
     let name = Refname::try_from(branch)
         .context("could not get branch name")
         .ok()?;
+
+    let given_name = ctx.given_name_for_branch(branch).ok()?;
+
     branch.get().target().map(|sha| RemoteBranch {
         sha,
         upstream: if let Refname::Local(local_name) = &name {
@@ -124,6 +131,7 @@ pub(crate) fn branch_to_remote_branch(branch: &git2::Branch) -> Option<RemoteBra
             None
         },
         name,
+        given_name,
         last_commit_timestamp_ms: commit
             .time()
             .seconds()

crates/gitbutler-branch-actions/src/virtual.rs

@@ -472,8 +472,8 @@ pub fn list_virtual_branches(
             .context("failed to find merge base")?;
         let base_current = true;
 
-        let upstream =
-            upstream_branch.and_then(|upstream_branch| branch_to_remote_branch(&upstream_branch));
+        let upstream = upstream_branch
+            .and_then(|upstream_branch| branch_to_remote_branch(ctx, &upstream_branch));
 
         let mut files = diffs_into_virtual_files(ctx, files);
 

crates/gitbutler-command-context/Cargo.toml

@@ -10,3 +10,4 @@ anyhow = "1.0.86"
 git2.workspace = true
 tracing = "0.1.40"
 gitbutler-project.workspace = true
+itertools = "0.13"

crates/gitbutler-command-context/src/repository.rs

@@ -1,5 +1,6 @@
-use anyhow::Result;
+use anyhow::{Context, Result};
 use gitbutler_project::Project;
+use itertools::Itertools;
 
 pub struct ProjectRepository {
     git_repository: git2::Repository,
@@ -70,4 +71,50 @@ impl ProjectRepository {
     pub fn repo(&self) -> &git2::Repository {
         &self.git_repository
     }
+
+    /// Fetches a branches name without the remote name attached
+    ///
+    /// 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)
+    ///
+    /// An ideal implementation wouldn't require us to list all the references,
+    /// but there doesn't seem to be a libgit2 solution to this.
+    pub fn given_name_for_branch(&self, branch: &git2::Branch) -> Result<String> {
+        let reference = branch.get();
+        let repo = self.repo();
+
+        if reference.is_remote() {
+            let shorthand_name = reference
+                .shorthand()
+                .ok_or(anyhow::anyhow!("Branch name was not utf-8"))?;
+
+            let remotes = repo.remotes().context("Failed to get remotes")?;
+
+            let longest_remote = remotes
+                .iter()
+                .flatten()
+                .sorted_by_key(|remote_name| -(remote_name.len() as i32))
+                .find(|reference_name| shorthand_name.starts_with(reference_name))
+                .ok_or(anyhow::anyhow!(
+                    "Failed to find remote branch's corresponding remote"
+                ))?;
+
+            let shorthand_name = shorthand_name
+                .strip_prefix(longest_remote)
+                .and_then(|str| str.strip_prefix("/"))
+                .ok_or(anyhow::anyhow!(
+                    "Failed to cut remote name {} off of shorthand name {}",
+                    longest_remote,
+                    shorthand_name
+                ))?;
+
+            Ok(shorthand_name.to_string())
+        } else {
+            reference
+                .shorthand()
+                .ok_or(anyhow::anyhow!("Branch name was not utf-8"))
+                .map(String::from)
+        }
+    }
 }