From 7a6c7472a4c7a653b6d88695a5d2b18eec6be8f3 Mon Sep 17 00:00:00 2001 From: Caleb Owens Date: Thu, 25 Jul 2024 15:47:10 +0200 Subject: [PATCH] Always overwrite local references --- .../lib/branch/BranchLaneContextMenu.svelte | 137 +----------------- app/src/lib/vbranches/branchController.ts | 10 +- app/src/lib/vbranches/types.ts | 14 -- .../gitbutler-branch-actions/src/actions.rs | 7 +- crates/gitbutler-branch-actions/src/base.rs | 4 +- .../src/branch_manager/branch_creation.rs | 2 +- .../src/branch_manager/branch_removal.rs | 48 +----- .../gitbutler-branch-actions/src/virtual.rs | 13 +- .../tests/extra/mod.rs | 33 +---- .../virtual_branches/apply_virtual_branch.rs | 4 +- .../convert_to_real_branch.rs | 8 +- .../virtual_branches/selected_for_changes.rs | 6 +- .../gitbutler-tauri/src/virtual_branches.rs | 5 +- 13 files changed, 36 insertions(+), 255 deletions(-) diff --git a/app/src/lib/branch/BranchLaneContextMenu.svelte b/app/src/lib/branch/BranchLaneContextMenu.svelte index 8e6f6c378..f951ba775 100644 --- a/app/src/lib/branch/BranchLaneContextMenu.svelte +++ b/app/src/lib/branch/BranchLaneContextMenu.svelte @@ -6,8 +6,6 @@ import ContextMenuItem from '$lib/components/contextmenu/ContextMenuItem.svelte'; import ContextMenuSection from '$lib/components/contextmenu/ContextMenuSection.svelte'; import { projectAiGenEnabled } from '$lib/config/config'; - import Select from '$lib/select/Select.svelte'; - import SelectItem from '$lib/select/SelectItem.svelte'; import Button from '$lib/shared/Button.svelte'; import Modal from '$lib/shared/Modal.svelte'; import TextBox from '$lib/shared/TextBox.svelte'; @@ -15,7 +13,7 @@ import { User } from '$lib/stores/user'; import { getContext, getContextStore } from '$lib/utils/context'; import { BranchController } from '$lib/vbranches/branchController'; - import { VirtualBranch, type NameConflictResolution } from '$lib/vbranches/types'; + import { VirtualBranch } from '$lib/vbranches/types'; export let contextMenuEl: ContextMenu; export let target: HTMLElement; @@ -49,67 +47,8 @@ aiConfigurationValid = await aiService.validateConfiguration(user?.access_token); } - 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(normalizedBranchName))) { - unapplyBranchModal.show(); - } else { - // No resolution required - branchController.convertToRealBranch(branch.id); - } - } - - function setButtonCoppy() { - switch (selectedResolution) { - case 'overwrite': - return 'Overwrite and unapply'; - case 'suffix': - return 'Suffix and unapply'; - case 'rename': - return 'Rename and unapply'; - } + function unapplyBranch() { + branchController.convertToRealBranch(branch.id); } let normalizedBranchName: string; @@ -126,56 +65,6 @@ } - -
-
-

- "{normalizedBranchName}" branch already exists -

- -

- A branch with the same name already exists. -
- Please select a resolution: -

-
- - - - {#if selectedResolution === 'rename'} - - {/if} -
- {#snippet controls()} - - - {/snippet} - - { - tryUnapplyBranch(); + unapplyBranch(); contextMenuEl.close(); }} /> @@ -301,21 +190,3 @@ {/snippet} - - diff --git a/app/src/lib/vbranches/branchController.ts b/app/src/lib/vbranches/branchController.ts index 118d15873..2bf56c5fa 100644 --- a/app/src/lib/vbranches/branchController.ts +++ b/app/src/lib/vbranches/branchController.ts @@ -4,7 +4,7 @@ import * as toasts from '$lib/utils/toasts'; import posthog from 'posthog-js'; import type { BaseBranchService } from '$lib/baseBranch/baseBranchService'; import type { RemoteBranchService } from '$lib/stores/remoteBranches'; -import type { VirtualBranch, Hunk, LocalFile, NameConflictResolution } from './types'; +import type { VirtualBranch, Hunk, LocalFile } from './types'; import type { VirtualBranchService } from './virtualBranch'; export class BranchController { @@ -180,15 +180,11 @@ export class BranchController { } } - async convertToRealBranch( - branchId: string, - nameConflictResolution: NameConflictResolution = { type: 'suffix', value: undefined } - ) { + async convertToRealBranch(branchId: string) { try { await invoke('convert_to_real_branch', { projectId: this.projectId, - branch: branchId, - nameConflictResolution + branch: branchId }); this.remoteBranchService.refresh(); } catch (err) { diff --git a/app/src/lib/vbranches/types.ts b/app/src/lib/vbranches/types.ts index cf39f0a5a..ab602b9e6 100644 --- a/app/src/lib/vbranches/types.ts +++ b/app/src/lib/vbranches/types.ts @@ -356,17 +356,3 @@ export class BranchData { return this.name.replace('refs/remotes/', '').replace('origin/', '').replace('refs/heads/', ''); } } - -export type NameConflictResolution = - | { - type: 'suffix'; - value: undefined; - } - | { - type: 'overwrite'; - value: undefined; - } - | { - type: 'rename'; - value: string; - }; diff --git a/crates/gitbutler-branch-actions/src/actions.rs b/crates/gitbutler-branch-actions/src/actions.rs index 9669e7954..65afa8268 100644 --- a/crates/gitbutler-branch-actions/src/actions.rs +++ b/crates/gitbutler-branch-actions/src/actions.rs @@ -336,7 +336,6 @@ impl VirtualBranchActions { &self, project: &Project, branch_id: BranchId, - name_conflict_resolution: branch::NameConflictResolution, ) -> Result { let project_repository = open_with_verify(project)?; let mut guard = project.exclusive_worktree_access(); @@ -344,11 +343,7 @@ impl VirtualBranchActions { .project() .prepare_snapshot(guard.read_permission()); let branch_manager = project_repository.branch_manager(); - let result = branch_manager.convert_to_real_branch( - branch_id, - name_conflict_resolution, - guard.write_permission(), - ); + let result = branch_manager.convert_to_real_branch(branch_id, guard.write_permission()); let _ = snapshot_tree.and_then(|snapshot_tree| { project_repository.project().snapshot_branch_unapplied( diff --git a/crates/gitbutler-branch-actions/src/base.rs b/crates/gitbutler-branch-actions/src/base.rs index 862f4bf99..6d9194476 100644 --- a/crates/gitbutler-branch-actions/src/base.rs +++ b/crates/gitbutler-branch-actions/src/base.rs @@ -423,7 +423,7 @@ pub(crate) fn update_base_branch( // branch tree conflicts with new target, unapply branch for now. we'll handle it later, when user applies it back. let branch_manager = project_repository.branch_manager(); let unapplied_real_branch = - branch_manager.convert_to_real_branch(branch.id, Default::default(), perm)?; + branch_manager.convert_to_real_branch(branch.id, perm)?; unapplied_branch_names.push(unapplied_real_branch); @@ -457,7 +457,7 @@ pub(crate) fn update_base_branch( // unapplied. conflicts witll be dealt with when applying it back. let branch_manager = project_repository.branch_manager(); let unapplied_real_branch = - branch_manager.convert_to_real_branch(branch.id, Default::default(), perm)?; + branch_manager.convert_to_real_branch(branch.id, perm)?; unapplied_branch_names.push(unapplied_real_branch); return Ok(None); diff --git a/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs b/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs index 7e68cd82d..3b469bcb9 100644 --- a/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs +++ b/crates/gitbutler-branch-actions/src/branch_manager/branch_creation.rs @@ -333,7 +333,7 @@ impl BranchManager<'_> { .iter() .filter(|branch| branch.id != branch_id) { - self.convert_to_real_branch(branch.id, Default::default(), perm)?; + self.convert_to_real_branch(branch.id, perm)?; } // apply the branch diff --git a/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs b/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs index 36528b010..3bd893fe7 100644 --- a/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs +++ b/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs @@ -5,9 +5,9 @@ use crate::{ ensure_selected_for_changes, get_applied_status, hunk::VirtualBranchHunk, integration::get_integration_commiter, - NameConflictResolution, VirtualBranchesExt, + VirtualBranchesExt, }; -use anyhow::{anyhow, Context, Result}; +use anyhow::{Context, Result}; use gitbutler_branch::{Branch, BranchExt, BranchId}; use gitbutler_commit::commit_headers::CommitHeadersV2; use gitbutler_oplog::SnapshotExt; @@ -23,7 +23,6 @@ impl BranchManager<'_> { pub fn convert_to_real_branch( &self, branch_id: BranchId, - name_conflict_resolution: NameConflictResolution, perm: &mut WorktreeWritePermission, ) -> Result { let vb_state = self.project_repository.project().virtual_branches(); @@ -31,7 +30,7 @@ impl BranchManager<'_> { let mut target_branch = vb_state.get_branch(branch_id)?; // Convert the vbranch to a real branch - let real_branch = self.build_real_branch(&mut target_branch, name_conflict_resolution)?; + let real_branch = self.build_real_branch(&mut target_branch)?; self.delete_branch(branch_id, perm)?; @@ -129,51 +128,12 @@ impl BranchManager<'_> { } impl BranchManager<'_> { - fn build_real_branch( - &self, - vbranch: &mut Branch, - name_conflict_resolution: NameConflictResolution, - ) -> Result> { + fn build_real_branch(&self, vbranch: &mut Branch) -> Result> { let repo = self.project_repository.repo(); let target_commit = repo.find_commit(vbranch.head)?; let branch_name = vbranch.name.clone(); let branch_name = normalize_branch_name(&branch_name); - // Is there a name conflict? - let branch_name = if repo - .find_branch(branch_name.as_str(), git2::BranchType::Local) - .is_ok() - { - match name_conflict_resolution { - NameConflictResolution::Suffix => { - let mut suffix = 1; - loop { - let new_branch_name = format!("{}-{}", branch_name, suffix); - if repo - .find_branch(new_branch_name.as_str(), git2::BranchType::Local) - .is_err() - { - break new_branch_name; - } - suffix += 1; - } - } - NameConflictResolution::Rename(new_name) => { - if repo - .find_branch(new_name.as_str(), git2::BranchType::Local) - .is_ok() - { - Err(anyhow!("Branch with name {} already exists", new_name))? - } else { - new_name - } - } - NameConflictResolution::Overwrite => branch_name, - } - } else { - branch_name - }; - let vb_state = self.project_repository.project().virtual_branches(); let branch = repo.branch(&branch_name, &target_commit, true)?; vbranch.source_refname = Some(Refname::try_from(&branch)?); diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index ddf75732c..b99c36229 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -20,7 +20,7 @@ use std::{ use anyhow::{anyhow, bail, Context, Result}; use bstr::ByteSlice; use git2_hooks::HookResult; -use serde::{Deserialize, Serialize}; +use serde::Serialize; use crate::branch_manager::BranchManagerExt; use crate::commit::{commit_to_vbranch_commit, VirtualBranchCommit}; @@ -82,15 +82,6 @@ pub struct VirtualBranches { pub skipped_files: Vec, } -#[derive(Default, Debug, Serialize, Deserialize)] -#[serde(rename_all = "camelCase", tag = "type", content = "value")] -pub enum NameConflictResolution { - #[default] - Suffix, - Rename(String), - Overwrite, -} - pub fn unapply_ownership( project_repository: &ProjectRepository, ownership: &BranchOwnershipClaims, @@ -256,7 +247,7 @@ fn resolve_old_applied_state( for mut branch in branches { if branch.is_old_unapplied() { - branch_manager.convert_to_real_branch(branch.id, Default::default(), perm)?; + branch_manager.convert_to_real_branch(branch.id, perm)?; } else { branch.applied = branch.in_workspace; vb_state.set_branch(branch)?; diff --git a/crates/gitbutler-branch-actions/tests/extra/mod.rs b/crates/gitbutler-branch-actions/tests/extra/mod.rs index 5e9ac6a8c..e39226bb6 100644 --- a/crates/gitbutler-branch-actions/tests/extra/mod.rs +++ b/crates/gitbutler-branch-actions/tests/extra/mod.rs @@ -1160,11 +1160,8 @@ fn unapply_branch() -> Result<()> { assert!(branch.active); let branch_manager = project_repository.branch_manager(); - let real_branch = branch_manager.convert_to_real_branch( - branch1_id, - Default::default(), - guard.write_permission(), - )?; + let real_branch = + branch_manager.convert_to_real_branch(branch1_id, guard.write_permission())?; let contents = std::fs::read(Path::new(&project.path).join(file_path))?; assert_eq!("line1\nline2\nline3\nline4\n", String::from_utf8(contents)?); @@ -1251,21 +1248,15 @@ fn apply_unapply_added_deleted_files() -> Result<()> { list_virtual_branches(project_repository, guard.write_permission()).unwrap(); let branch_manager = project_repository.branch_manager(); - let real_branch_2 = branch_manager.convert_to_real_branch( - branch2_id, - Default::default(), - guard.write_permission(), - )?; + let real_branch_2 = + branch_manager.convert_to_real_branch(branch2_id, guard.write_permission())?; // check that file2 is back let contents = std::fs::read(Path::new(&project.path).join(file_path2))?; assert_eq!("file2\n", String::from_utf8(contents)?); - let real_branch_3 = branch_manager.convert_to_real_branch( - branch3_id, - Default::default(), - guard.write_permission(), - )?; + let real_branch_3 = + branch_manager.convert_to_real_branch(branch3_id, guard.write_permission())?; // check that file3 is gone assert!(!Path::new(&project.path).join(file_path3).exists()); @@ -1345,16 +1336,8 @@ fn detect_mergeable_branch() -> Result<()> { // unapply both branches and create some conflicting ones let branch_manager = project_repository.branch_manager(); - branch_manager.convert_to_real_branch( - branch1_id, - Default::default(), - guard.write_permission(), - )?; - branch_manager.convert_to_real_branch( - branch2_id, - Default::default(), - guard.write_permission(), - )?; + branch_manager.convert_to_real_branch(branch1_id, guard.write_permission())?; + branch_manager.convert_to_real_branch(branch2_id, guard.write_permission())?; project_repository.repo().set_head("refs/heads/master")?; project_repository diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/apply_virtual_branch.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/apply_virtual_branch.rs index fe0d08ce3..f7710f249 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/apply_virtual_branch.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/apply_virtual_branch.rs @@ -54,7 +54,7 @@ async fn rebase_commit() { let unapplied_branch = { // unapply first vbranch let unapplied_branch = controller - .convert_to_real_branch(project, branch1_id, Default::default()) + .convert_to_real_branch(project, branch1_id) .await .unwrap(); @@ -163,7 +163,7 @@ async fn rebase_work() { let unapplied_branch = { // unapply first vbranch let unapplied_branch = controller - .convert_to_real_branch(project, branch1_id, Default::default()) + .convert_to_real_branch(project, branch1_id) .await .unwrap(); diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs index fcc8f596c..5446f48f6 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/convert_to_real_branch.rs @@ -23,7 +23,7 @@ async fn unapply_with_data() { assert_eq!(branches.len(), 1); controller - .convert_to_real_branch(project, branches[0].id, Default::default()) + .convert_to_real_branch(project, branches[0].id) .await .unwrap(); @@ -72,7 +72,7 @@ async fn conflicting() { ); let unapplied_branch = controller - .convert_to_real_branch(project, branches[0].id, Default::default()) + .convert_to_real_branch(project, branches[0].id) .await .unwrap(); @@ -118,7 +118,7 @@ async fn conflicting() { { // Converting the branch to a real branch should put us back in an unconflicted state controller - .convert_to_real_branch(project, branch_id, Default::default()) + .convert_to_real_branch(project, branch_id) .await .unwrap(); @@ -151,7 +151,7 @@ async fn delete_if_empty() { assert_eq!(branches.len(), 1); controller - .convert_to_real_branch(project, branches[0].id, Default::default()) + .convert_to_real_branch(project, branches[0].id) .await .unwrap(); diff --git a/crates/gitbutler-branch-actions/tests/virtual_branches/selected_for_changes.rs b/crates/gitbutler-branch-actions/tests/virtual_branches/selected_for_changes.rs index 7f0db5f03..86cc1e4cc 100644 --- a/crates/gitbutler-branch-actions/tests/virtual_branches/selected_for_changes.rs +++ b/crates/gitbutler-branch-actions/tests/virtual_branches/selected_for_changes.rs @@ -39,7 +39,7 @@ async fn unapplying_selected_branch_selects_anther() { assert!(!b2.selected_for_changes); controller - .convert_to_real_branch(project, b_id, Default::default()) + .convert_to_real_branch(project, b_id) .await .unwrap(); @@ -303,7 +303,7 @@ async fn unapply_virtual_branch_should_reset_selected_for_changes() { assert!(!b2.selected_for_changes); controller - .convert_to_real_branch(project, b1_id, Default::default()) + .convert_to_real_branch(project, b1_id) .await .unwrap(); @@ -371,7 +371,7 @@ async fn applying_first_branch() { assert_eq!(branches.len(), 1); let unapplied_branch = controller - .convert_to_real_branch(project, branches[0].id, Default::default()) + .convert_to_real_branch(project, branches[0].id) .await .unwrap(); let unapplied_branch = Refname::from_str(&unapplied_branch).unwrap(); diff --git a/crates/gitbutler-tauri/src/virtual_branches.rs b/crates/gitbutler-tauri/src/virtual_branches.rs index 9ff49a462..59ddac8cc 100644 --- a/crates/gitbutler-tauri/src/virtual_branches.rs +++ b/crates/gitbutler-tauri/src/virtual_branches.rs @@ -5,8 +5,8 @@ pub mod commands { use gitbutler_branch::{BranchCreateRequest, BranchId, BranchUpdateRequest}; use gitbutler_branch_actions::BaseBranch; use gitbutler_branch_actions::RemoteBranchFile; - use gitbutler_branch_actions::{NameConflictResolution, VirtualBranchActions, VirtualBranches}; use gitbutler_branch_actions::{RemoteBranch, RemoteBranchData}; + use gitbutler_branch_actions::{VirtualBranchActions, VirtualBranches}; use gitbutler_error::error::Code; use gitbutler_project as projects; use gitbutler_project::{FetchResult, ProjectId}; @@ -203,11 +203,10 @@ pub mod commands { projects: State<'_, projects::Controller>, project_id: ProjectId, branch: BranchId, - name_conflict_resolution: NameConflictResolution, ) -> Result<(), Error> { let project = projects.get(project_id)?; VirtualBranchActions - .convert_to_real_branch(&project, branch, name_conflict_resolution) + .convert_to_real_branch(&project, branch) .await?; emit_vbranches(&windows, project_id).await; Ok(())