Unapply to real branches (#4025)

* Don't return optional

* Rename get_integration_commiter

* Add a header to wip commit

* Stuff

* Unapply all branches

* Reorder code

* Fix one test

* Name resolution

* Fix two tests

* Fix another!

* wip

* Fix so many tests

* Fix unapply.rs tests

* Fix selected for changes tests

* Move unapplying logic to delete_branch method

* Remove unused and kinda borked cherry_commit code

* Fix the tests!!!!!

* Make apply_branch private

* Change handling of headers

* Improve order integrity

* Updated types and comments to convey more meaning

Author: Caleb-T-Owens

app/src/lib/branch/BranchLanePopupMenu.svelte

@@ -7,13 +7,15 @@
 	import { projectAiGenEnabled } from '$lib/config/config';
 	import Button from '$lib/shared/Button.svelte';
 	import Modal from '$lib/shared/Modal.svelte';
+	import Select from '$lib/shared/Select.svelte';
+	import SelectItem from '$lib/shared/SelectItem.svelte';
 	import TextBox from '$lib/shared/TextBox.svelte';
 	import Toggle from '$lib/shared/Toggle.svelte';
 	import { User } from '$lib/stores/user';
 	import { normalizeBranchName } from '$lib/utils/branch';
 	import { getContext, getContextStore } from '$lib/utils/context';
 	import { BranchController } from '$lib/vbranches/branchController';
-	import { Branch } from '$lib/vbranches/types';
+	import { Branch, type NameConflictResolution } from '$lib/vbranches/types';
 	import { createEventDispatcher } from 'svelte';
 
 	export let visible: boolean;
@@ -51,8 +53,96 @@
 	function close() {
 		visible = false;
 	}
+
+	let unapplyBranchModal: Modal;
+
+	type ResolutionVariants = NameConflictResolution['type'];
+
+	const resolutions: { value: ResolutionVariants; label: string }[] = [
+		{
+			value: 'overwrite',
+			label: 'Overwrite the existing branch'
+		},
+		{
+			value: 'suffix',
+			label: 'Suffix the branch name'
+		},
+		{
+			value: 'rename',
+			label: 'Use a new name'
+		}
+	];
+
+	let selectedResolution: ResolutionVariants = resolutions[0].value;
+	let newBranchName = '';
+
+	function unapplyBranchWithSelectedResolution() {
+		let resolution: NameConflictResolution | undefined;
+		if (selectedResolution === 'rename') {
+			resolution = {
+				type: selectedResolution,
+				value: newBranchName
+			};
+		} else {
+			resolution = {
+				type: selectedResolution,
+				value: undefined
+			};
+		}
+
+		branchController.convertToRealBranch(branch.id, resolution);
+
+		unapplyBranchModal.close();
+	}
+
+	const remoteBranches = branchController.remoteBranchService.branches$;
+
+	function tryUnapplyBranch() {
+		if ($remoteBranches.find((b) => b.name.endsWith(normalizeBranchName(branch.name)))) {
+			unapplyBranchModal.show();
+		} else {
+			// No resolution required
+			branchController.convertToRealBranch(branch.id);
+		}
+	}
 </script>
 
+<Modal bind:this={unapplyBranchModal}>
+	<div class="flow">
+		<div class="modal-copy">
+			<p class="text-base-15">There is already branch with the name</p>
+			<Button size="tag" clickable={false}>{normalizeBranchName(branch.name)}</Button>
+			<p class="text-base-15">.</p>
+			<p class="text-base-15">Please choose how you want to resolve this:</p>
+		</div>
+
+		<Select
+			items={resolutions}
+			itemId={'value'}
+			labelId={'label'}
+			bind:selectedItemId={selectedResolution}
+		>
+			<SelectItem slot="template" let:item let:selected {selected}>
+				{item.label}
+			</SelectItem>
+		</Select>
+		{#if selectedResolution === 'rename'}
+			<TextBox
+				label="New branch name"
+				id="newBranchName"
+				bind:value={newBranchName}
+				placeholder="Enter new branch name"
+			/>
+		{/if}
+	</div>
+	{#snippet controls()}
+		<Button style="ghost" outline on:click={() => unapplyBranchModal.close()}>Cancel</Button>
+		<Button style="pop" kind="solid" grow on:click={unapplyBranchWithSelectedResolution}
+			>Submit</Button
+		>
+	{/snippet}
+</Modal>
+
 {#if visible}
 	<ContextMenu>
 		<ContextMenuSection>
@@ -71,7 +161,7 @@
 				<ContextMenuItem
 					label="Unapply"
 					on:click={() => {
-						if (branch.id) branchController.unapplyBranch(branch.id);
+						tryUnapplyBranch();
 						close();
 					}}
 				/>
@@ -186,3 +276,16 @@
 		</Button>
 	{/snippet}
 </Modal>
+
+<style lang="postcss">
+	.flow {
+		display: flex;
+		flex-direction: column;
+		gap: 16px;
+	}
+	.modal-copy {
+		& > * {
+			display: inline;
+		}
+	}
+</style>

app/src/lib/vbranches/branchController.ts

@@ -4,7 +4,7 @@ import * as toasts from '$lib/utils/toasts';
 import posthog from 'posthog-js';
 import type { RemoteBranchService } from '$lib/stores/remoteBranches';
 import type { BaseBranchService } from './baseBranch';
-import type { Branch, Hunk, LocalFile } from './types';
+import type { Branch, Hunk, LocalFile, NameConflictResolution } from './types';
 import type { VirtualBranchService } from './virtualBranch';
 
 export class BranchController {
@@ -180,10 +180,17 @@ export class BranchController {
 		}
 	}
 
-	async unapplyBranch(branchId: string) {
+	async convertToRealBranch(
+		branchId: string,
+		nameConflictResolution: NameConflictResolution = { type: 'suffix', value: undefined }
+	) {
 		try {
-			// TODO: make this optimistic again.
-			await invoke<void>('unapply_branch', { projectId: this.projectId, branch: branchId });
+			await invoke<void>('convert_to_real_branch', {
+				projectId: this.projectId,
+				branch: branchId,
+				nameConflictResolution
+			});
+			this.remoteBranchService.reload();
 		} catch (err) {
 			showError('Failed to unapply branch', err);
 		}
@@ -284,25 +291,6 @@ You can find them in the 'Branches' sidebar in order to resolve conflicts.`;
 		}
 	}
 
-	async cherryPick(branchId: string, targetCommitOid: string) {
-		try {
-			await invoke<void>('cherry_pick_onto_virtual_branch', {
-				projectId: this.projectId,
-				branchId,
-				targetCommitOid
-			});
-		} catch (err: any) {
-			// TODO: Probably we wanna have error code checking in a more generic way
-			if (err.code === 'errors.commit.signing_failed') {
-				showSignError(err);
-			} else {
-				showError('Failed to cherry-pick commit', err);
-			}
-		} finally {
-			this.targetBranchService.reload();
-		}
-	}
-
 	async markResolved(path: string) {
 		try {
 			await invoke<void>('mark_resolved', { projectId: this.projectId, path });

app/src/lib/vbranches/types.ts

@@ -461,3 +461,17 @@ export class BaseBranch {
 		return this.repoBaseUrl.includes('gitlab.com');
 	}
 }
+
+export type NameConflictResolution =
+	| {
+			type: 'suffix';
+			value: undefined;
+	  }
+	| {
+			type: 'overwrite';
+			value: undefined;
+	  }
+	| {
+			type: 'rename';
+			value: string;
+	  };

app/src/lib/vbranches/virtualBranch.ts

@@ -58,10 +58,8 @@ export class VirtualBranchService {
 			tap((branches) => {
 				branches.forEach((branch) => {
 					branch.files.sort((a) => (a.conflicted ? -1 : 0));
-					branch.isMergeable = invoke<boolean>('can_apply_virtual_branch', {
-						projectId: projectId,
-						branchId: branch.id
-					});
+					// This is always true now
+					branch.isMergeable = Promise.resolve(true);
 				});
 				this.fresh$.next(); // Notification for fresh reload
 			}),

crates/gitbutler-core/Cargo.toml

@@ -5,6 +5,7 @@ edition = "2021"
 authors = ["GitButler <gitbutler@gitbutler.com>"]
 publish = false
 
+
 [dev-dependencies]
 once_cell = "1.19"
 pretty_assertions = "1.4"

crates/gitbutler-core/src/git/branch_ext.rs

@@ -0,0 +1,15 @@
+use anyhow::{Context, Result};
+
+use crate::types::ReferenceName;
+
+pub trait BranchExt {
+    fn reference_name(&self) -> Result<ReferenceName>;
+}
+
+impl<'repo> BranchExt for git2::Branch<'repo> {
+    fn reference_name(&self) -> Result<ReferenceName> {
+        let name = self.get().name().context("Failed to get branch name")?;
+
+        Ok(name.into())
+    }
+}

crates/gitbutler-core/src/git/commit_buffer.rs

@@ -0,0 +1,91 @@
+use bstr::{BStr, BString, ByteSlice, ByteVec};
+use core::str;
+
+use super::CommitHeadersV2;
+
+pub struct CommitBuffer {
+    heading: Vec<(BString, BString)>,
+    message: BString,
+}
+
+impl CommitBuffer {
+    pub fn new(buffer: &[u8]) -> Self {
+        let buffer = BStr::new(buffer);
+        if let Some((heading, message)) = buffer.split_once_str("\n\n") {
+            let heading = heading
+                .lines()
+                .filter_map(|line| line.split_once_str(" "))
+                .map(|(key, value)| (key.into(), value.into()))
+                .collect();
+
+            Self {
+                heading,
+                message: message.into(),
+            }
+        } else {
+            Self {
+                heading: vec![],
+                message: buffer.into(),
+            }
+        }
+    }
+
+    pub fn set_header(&mut self, key: &str, value: &str) {
+        let mut set_heading = false;
+        self.heading.iter_mut().for_each(|(k, v)| {
+            if k == key {
+                *v = value.into();
+                set_heading = true;
+            }
+        });
+
+        if !set_heading {
+            self.heading.push((key.into(), value.into()));
+        }
+    }
+
+    /// Defers to the CommitHeadersV2 struct about which headers should be injected.
+    /// If `commit_headers: None` is provided, a default set of headers, including a generated change-id will be used
+    pub fn set_gitbutler_headers(&mut self, commit_headers: Option<CommitHeadersV2>) {
+        if let Some(commit_headers) = commit_headers {
+            commit_headers.inject_into(self)
+        } else {
+            CommitHeadersV2::inject_default(self)
+        }
+    }
+
+    pub fn as_bstring(&self) -> BString {
+        let mut output = BString::new(vec![]);
+
+        for (key, value) in &self.heading {
+            output.push_str(key);
+            output.push_str(" ");
+            output.push_str(value);
+            output.push_str("\n");
+        }
+
+        output.push_str("\n");
+
+        output.push_str(&self.message);
+
+        output
+    }
+}
+
+impl From<git2::Buf> for CommitBuffer {
+    fn from(git2_buffer: git2::Buf) -> Self {
+        Self::new(&git2_buffer)
+    }
+}
+
+impl From<BString> for CommitBuffer {
+    fn from(s: BString) -> Self {
+        Self::new(s.as_bytes())
+    }
+}
+
+impl From<CommitBuffer> for BString {
+    fn from(buffer: CommitBuffer) -> BString {
+        buffer.as_bstring()
+    }
+}

crates/gitbutler-core/src/git/commit_ext.rs

@@ -1,6 +1,7 @@
-// use anyhow::Result;
 use bstr::BStr;
 
+use super::HasCommitHeaders;
+
 /// Extension trait for `git2::Commit`.
 ///
 /// For now, it collects useful methods from `gitbutler-core::git::Commit`
@@ -15,13 +16,9 @@ impl<'repo> CommitExt for git2::Commit<'repo> {
     fn message_bstr(&self) -> &BStr {
         self.message_bytes().as_ref()
     }
+
     fn change_id(&self) -> Option<String> {
-        let cid = self.header_field_bytes("change-id").ok()?;
-        if cid.is_empty() {
-            None
-        } else {
-            String::from_utf8(cid.to_owned()).ok()
-        }
+        self.gitbutler_headers().map(|headers| headers.change_id)
     }
     fn is_signed(&self) -> bool {
         self.header_field_bytes("gpgsig").is_ok()