From ea4f47da1bc181602de4848add1f310e59c9e32a Mon Sep 17 00:00:00 2001 From: Kiril Videlov Date: Sun, 21 Jul 2024 20:12:54 +0200 Subject: [PATCH] get_applied_status returns VirtualBranchFiles instead of a tuple of path and hunks --- .../src/branch_manager/branch_removal.rs | 10 ++- crates/gitbutler-branch-actions/src/file.rs | 9 +++ crates/gitbutler-branch-actions/src/lib.rs | 1 + crates/gitbutler-branch-actions/src/status.rs | 13 +++- .../gitbutler-branch-actions/src/virtual.rs | 66 +++++++++++-------- .../tests/extra/mod.rs | 49 +++++++++++--- 6 files changed, 106 insertions(+), 42 deletions(-) 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 b0c9089f6..039052bb7 100644 --- a/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs +++ b/crates/gitbutler-branch-actions/src/branch_manager/branch_removal.rs @@ -1,6 +1,9 @@ +use std::path::PathBuf; + use crate::{ conflicts::{self}, ensure_selected_for_changes, get_applied_status, + hunk::VirtualBranchHunk, integration::get_integration_commiter, NameConflictResolution, VirtualBranchesExt, }; @@ -87,10 +90,15 @@ impl BranchManager<'_> { |final_tree, status| { let final_tree = final_tree?; let branch = status.0; + let files = status + .1 + .into_iter() + .map(|file| (file.path, file.hunks)) + .collect::)>>(); let tree_oid = gitbutler_diff::write::hunks_onto_oid( self.project_repository, &branch.head, - status.1, + files, )?; let branch_tree = repo.find_tree(tree_oid)?; let mut result = diff --git a/crates/gitbutler-branch-actions/src/file.rs b/crates/gitbutler-branch-actions/src/file.rs index 167b3a78d..f691799ff 100644 --- a/crates/gitbutler-branch-actions/src/file.rs +++ b/crates/gitbutler-branch-actions/src/file.rs @@ -74,6 +74,15 @@ pub struct VirtualBranchFile { pub large: bool, } +pub trait Get { + fn get(&self, path: &Path) -> Option; +} +impl Get for Vec { + fn get(&self, path: &Path) -> Option { + self.iter().find(|f| f.path == path).cloned() + } +} + pub(crate) fn list_virtual_commit_files( project_repository: &ProjectRepository, commit: &git2::Commit, diff --git a/crates/gitbutler-branch-actions/src/lib.rs b/crates/gitbutler-branch-actions/src/lib.rs index 2c3fc4627..606822bcd 100644 --- a/crates/gitbutler-branch-actions/src/lib.rs +++ b/crates/gitbutler-branch-actions/src/lib.rs @@ -15,6 +15,7 @@ mod integration; pub use integration::{update_gitbutler_integration, verify_branch}; mod file; +pub use file::Get; pub use file::RemoteBranchFile; mod remote; diff --git a/crates/gitbutler-branch-actions/src/status.rs b/crates/gitbutler-branch-actions/src/status.rs index 45a719123..6e1122edd 100644 --- a/crates/gitbutler-branch-actions/src/status.rs +++ b/crates/gitbutler-branch-actions/src/status.rs @@ -8,6 +8,7 @@ use gitbutler_project::access::WorktreeWritePermission; use gitbutler_repo::RepositoryExt; use std::{collections::HashMap, path::PathBuf, vec}; +use crate::file::{virtual_hunks_into_virtual_files, VirtualBranchFile}; use crate::hunk::file_hunks_from_diffs; use crate::{ conflicts::RepoConflictsExt, @@ -19,7 +20,7 @@ use crate::{ /// Represents the uncommitted status of the applied virtual branches in the workspace. pub struct VirtualBranchesStatus { /// A collection of branches and their associated uncommitted file changes. - pub branches: Vec<(Branch, HashMap>)>, + pub branches: Vec<(Branch, Vec)>, /// A collection of files that were skipped during the diffing process (due to being very large and unprocessable). pub skipped_files: Vec, } @@ -208,8 +209,16 @@ pub fn get_applied_status( }) .collect(); + let files_by_branch: Vec<(Branch, Vec)> = hunks_by_branch + .iter() + .map(|(branch, hunks)| { + let files = virtual_hunks_into_virtual_files(project_repository, hunks.clone()); + (branch.clone(), files) + }) + .collect(); + Ok(VirtualBranchesStatus { - branches: hunks_by_branch, + branches: files_by_branch, skipped_files, }) } diff --git a/crates/gitbutler-branch-actions/src/virtual.rs b/crates/gitbutler-branch-actions/src/virtual.rs index 45e619b43..ddf75732c 100644 --- a/crates/gitbutler-branch-actions/src/virtual.rs +++ b/crates/gitbutler-branch-actions/src/virtual.rs @@ -25,11 +25,12 @@ use serde::{Deserialize, Serialize}; use crate::branch_manager::BranchManagerExt; use crate::commit::{commit_to_vbranch_commit, VirtualBranchCommit}; use crate::conflicts::{self, RepoConflictsExt}; -use crate::file::{virtual_hunks_into_virtual_files, VirtualBranchFile}; +use crate::file::VirtualBranchFile; use crate::hunk::VirtualBranchHunk; use crate::integration::get_workspace_head; use crate::remote::{branch_to_remote_branch, RemoteBranch}; use crate::status::get_applied_status; +use crate::Get; use crate::VirtualBranchesExt; use gitbutler_error::error::Code; use gitbutler_error::error::Marker; @@ -110,17 +111,17 @@ pub fn unapply_ownership( .map( |(_branch, branch_files)| -> Result> { let mut hunks_to_unapply: Vec<(PathBuf, GitHunk)> = Vec::new(); - for (path, hunks) in branch_files { + for file in branch_files { let ownership_hunks: Vec<&Hunk> = ownership .claims .iter() - .filter(|o| o.file_path == *path) + .filter(|o| o.file_path == file.path) .flat_map(|f| &f.hunks) .collect(); - for hunk in hunks { + for hunk in &file.hunks { let hunk: GitHunk = hunk.clone().into(); if ownership_hunks.contains(&&Hunk::from(&hunk)) { - hunks_to_unapply.push((path.clone(), hunk)); + hunks_to_unapply.push((file.path.clone(), hunk)); } } } @@ -155,10 +156,15 @@ pub fn unapply_ownership( target_commit.tree().context("failed to get target tree"), |final_tree, status| { let final_tree = final_tree?; + let files = status + .1 + .into_iter() + .map(|file| (file.path, file.hunks)) + .collect::)>>(); let tree_oid = gitbutler_diff::write::hunks_onto_oid( project_repository, &integration_commit_id, - status.1, + files, )?; let branch_tree = repo.find_tree(tree_oid)?; let mut result = repo.merge_trees(&base_tree, &final_tree, &branch_tree, None)?; @@ -276,7 +282,6 @@ pub fn list_virtual_branches( .get_default_target() .context("failed to get default target")?; - // let (statuses, skipped_files, locks) = get_applied_status(ctx, Some(perm))?; let status = get_applied_status(ctx, Some(perm))?; let max_selected_for_changes = status .branches @@ -285,9 +290,9 @@ pub fn list_virtual_branches( .max() .unwrap_or(-1); - for (branch, files) in status.branches { + for (branch, mut files) in status.branches { let repo = ctx.repo(); - update_conflict_markers(ctx, &files)?; + update_conflict_markers(ctx, files.clone())?; let upstream_branch = match branch.clone().upstream { Some(upstream) => repo.find_branch_by_refname(&Refname::from(upstream))?, @@ -350,8 +355,6 @@ pub fn list_virtual_branches( let upstream = upstream_branch .and_then(|upstream_branch| branch_to_remote_branch(ctx, &upstream_branch)); - let mut files = virtual_hunks_into_virtual_files(ctx, files); - let path_claim_positions: HashMap<&PathBuf, usize> = branch .ownership .claims @@ -932,7 +935,7 @@ pub fn commit( .find(|(branch, _)| branch.id == branch_id) .with_context(|| format!("branch {branch_id} not found"))?; - update_conflict_markers(project_repository, &files) + update_conflict_markers(project_repository, files.clone()) .context(Code::CommitMergeConflictFailure)?; project_repository @@ -940,15 +943,16 @@ pub fn commit( .context(Code::CommitMergeConflictFailure)?; let tree_oid = if let Some(ownership) = ownership { - let files = files.into_iter().filter_map(|(filepath, hunks)| { - let hunks = hunks + let files = files.into_iter().filter_map(|file| { + let hunks = file + .hunks .into_iter() .filter(|hunk| { let hunk: GitHunk = hunk.clone().into(); ownership .claims .iter() - .find(|f| f.file_path.eq(&filepath)) + .find(|f| f.file_path.eq(&file.path)) .map_or(false, |f| { f.hunks.iter().any(|h| { h.start == hunk.new_start @@ -960,11 +964,15 @@ pub fn commit( if hunks.is_empty() { None } else { - Some((filepath, hunks)) + Some((file.path, hunks)) } }); gitbutler_diff::write::hunks_onto_commit(project_repository, branch.head, files)? } else { + let files = files + .into_iter() + .map(|file| (file.path, file.hunks)) + .collect::)>>(); gitbutler_diff::write::hunks_onto_commit(project_repository, branch.head, files)? }; @@ -1504,8 +1512,8 @@ pub(crate) fn amend( .filter_map(|file_ownership| { let hunks = target_status .get(&file_ownership.file_path) - .map(|hunks| { - hunks + .map(|file| { + file.hunks .iter() .filter(|hunk| { let hunk: GitHunk = (*hunk).clone().into(); @@ -1985,11 +1993,11 @@ pub(crate) fn move_commit( let branch_head_diff: HashMap<_, _> = gitbutler_diff::diff_files_into_hunks(branch_head_diff).collect(); - let is_source_locked = source_branch_non_comitted_files - .iter() - .any(|(path, hunks)| { - branch_head_diff.get(path).map_or(false, |head_diff_hunks| { - hunks.iter().any(|hunk| { + let is_source_locked = source_branch_non_comitted_files.iter().any(|file| { + branch_head_diff + .get(&file.path) + .map_or(false, |head_diff_hunks| { + file.hunks.iter().any(|hunk| { let hunk: GitHunk = hunk.clone().into(); head_diff_hunks.iter().any(|head_hunk| { joined( @@ -2001,7 +2009,7 @@ pub(crate) fn move_commit( }) }) }) - }); + }); if is_source_locked { bail!("the source branch contains hunks locked to the target commit") @@ -2073,14 +2081,14 @@ pub(crate) fn move_commit( // conflicts file. fn update_conflict_markers( project_repository: &ProjectRepository, - files: &HashMap + Clone>>, + files: Vec, ) -> Result<()> { let conflicting_files = conflicts::conflicting_files(project_repository)?; - for (file_path, non_commited_hunks) in files { + for file in files { let mut conflicted = false; - if conflicting_files.contains(file_path) { + if conflicting_files.contains(&file.path) { // check file for conflict markers, resolve the file if there are none in any hunk - for hunk in non_commited_hunks { + for hunk in file.hunks { let hunk: GitHunk = hunk.clone().into(); if hunk.diff_lines.contains_str(b"<<<<<<< ours") { conflicted = true; @@ -2090,7 +2098,7 @@ fn update_conflict_markers( } } if !conflicted { - conflicts::resolve(project_repository, file_path).unwrap(); + conflicts::resolve(project_repository, file.path).unwrap(); } } } diff --git a/crates/gitbutler-branch-actions/tests/extra/mod.rs b/crates/gitbutler-branch-actions/tests/extra/mod.rs index d1dccdc5d..cc4d80515 100644 --- a/crates/gitbutler-branch-actions/tests/extra/mod.rs +++ b/crates/gitbutler-branch-actions/tests/extra/mod.rs @@ -17,6 +17,7 @@ use gitbutler_branch::{ BranchOwnershipClaims, Target, {BranchCreateRequest, BranchUpdateRequest}, }; use gitbutler_branch_actions::BranchManagerExt; +use gitbutler_branch_actions::Get; use gitbutler_branch_actions::{ commit, get_applied_status, integrate_upstream_commits, is_remote_branch_mergeable, list_virtual_branches, unapply_ownership, update_branch, update_gitbutler_integration, @@ -549,15 +550,27 @@ fn move_hunks_multiple_sources() -> Result<()> { assert_eq!(files_by_branch_id[&branch2_id].len(), 0); assert_eq!(files_by_branch_id[&branch3_id].len(), 1); assert_eq!( - files_by_branch_id[&branch3_id][Path::new("test.txt")].len(), + files_by_branch_id[&branch3_id] + .get(Path::new("test.txt")) + .unwrap() + .hunks + .len(), 2 ); assert_eq!( - files_by_branch_id[&branch3_id][Path::new("test.txt")][0].diff, + files_by_branch_id[&branch3_id] + .get(Path::new("test.txt")) + .unwrap() + .hunks[0] + .diff, "@@ -1,3 +1,4 @@\n+line0\n line1\n line2\n line3\n" ); assert_eq!( - files_by_branch_id[&branch3_id][Path::new("test.txt")][1].diff, + files_by_branch_id[&branch3_id] + .get(Path::new("test.txt")) + .unwrap() + .hunks[1] + .diff, "@@ -10,3 +11,4 @@ line9\n line10\n line11\n line12\n+line13\n" ); Ok(()) @@ -628,21 +641,37 @@ fn move_hunks_partial_explicitly() -> Result<()> { assert_eq!(files_by_branch_id.len(), 2); assert_eq!(files_by_branch_id[&branch1_id].len(), 1); assert_eq!( - files_by_branch_id[&branch1_id][Path::new("test.txt")].len(), + files_by_branch_id[&branch1_id] + .get(Path::new("test.txt")) + .unwrap() + .hunks + .len(), 1 ); assert_eq!( - files_by_branch_id[&branch1_id][Path::new("test.txt")][0].diff, + files_by_branch_id[&branch1_id] + .get(Path::new("test.txt")) + .unwrap() + .hunks[0] + .diff, "@@ -11,3 +12,4 @@ line10\n line11\n line12\n line13\n+line14\n" ); assert_eq!(files_by_branch_id[&branch2_id].len(), 1); assert_eq!( - files_by_branch_id[&branch2_id][Path::new("test.txt")].len(), + files_by_branch_id[&branch2_id] + .get(Path::new("test.txt")) + .unwrap() + .hunks + .len(), 1 ); assert_eq!( - files_by_branch_id[&branch2_id][Path::new("test.txt")][0].diff, + files_by_branch_id[&branch2_id] + .get(Path::new("test.txt")) + .unwrap() + .hunks[0] + .diff, "@@ -1,3 +1,4 @@\n+line0\n line1\n line2\n line3\n" ); @@ -678,7 +707,7 @@ fn add_new_hunk_to_the_end() -> Result<()> { .expect("failed to get status") .branches; assert_eq!( - statuses[0].1[Path::new("test.txt")][0].diff, + statuses[0].1.get(Path::new("test.txt")).unwrap().hunks[0].diff, "@@ -11,5 +11,5 @@ line10\n line11\n line12\n line13\n-line13\n line14\n+line15\n" ); @@ -692,11 +721,11 @@ fn add_new_hunk_to_the_end() -> Result<()> { .branches; assert_eq!( - statuses[0].1[Path::new("test.txt")][0].diff, + statuses[0].1.get(Path::new("test.txt")).unwrap().hunks[0].diff, "@@ -11,5 +12,5 @@ line10\n line11\n line12\n line13\n-line13\n line14\n+line15\n" ); assert_eq!( - statuses[0].1[Path::new("test.txt")][1].diff, + statuses[0].1.get(Path::new("test.txt")).unwrap().hunks[1].diff, "@@ -1,3 +1,4 @@\n+line0\n line1\n line2\n line3\n" );