From 9054c004cf489cb021ea75c1377e54d997fef78c Mon Sep 17 00:00:00 2001 From: Nikita Galaiko Date: Fri, 14 Jul 2023 10:56:38 +0200 Subject: [PATCH] extract diff to a separate mod --- src-tauri/src/bin/butler.rs | 4 +- src-tauri/src/project_repository/diff.rs | 146 +++++++++++++ src-tauri/src/project_repository/mod.rs | 1 + .../src/project_repository/repository.rs | 21 +- .../virtual_branches/branch/file_ownership.rs | 14 +- .../src/virtual_branches/branch/writer.rs | 2 +- src-tauri/src/virtual_branches/mod.rs | 200 +++++------------- .../src/virtual_branches/target/reader.rs | 2 +- .../src/virtual_branches/target/writer.rs | 2 +- 9 files changed, 216 insertions(+), 176 deletions(-) create mode 100644 src-tauri/src/project_repository/diff.rs diff --git a/src-tauri/src/bin/butler.rs b/src-tauri/src/bin/butler.rs index fb5064879..618e44fe5 100644 --- a/src-tauri/src/bin/butler.rs +++ b/src-tauri/src/bin/butler.rs @@ -144,7 +144,7 @@ fn run_branches(butler: ButlerCli) { println!("{}", branch.id.red()); println!("{}", branch.name.red()); for file in branch.files { - println!(" {}", file.path.blue()); + println!(" {}", file.path.display().to_string().blue()); for hunk in file.hunks { println!("--"); println!(" {}", hunk.diff.green()); @@ -333,7 +333,7 @@ fn run_status(butler: ButlerCli) { println!("applied: {}", branch.applied.to_string().green()); println!(" files:"); for file in files { - println!(" {}", file.path.yellow()); + println!(" {}", file.path.display().to_string().yellow()); for hunk in file.hunks { println!(" {}", hunk.id); } diff --git a/src-tauri/src/project_repository/diff.rs b/src-tauri/src/project_repository/diff.rs new file mode 100644 index 000000000..65f86def6 --- /dev/null +++ b/src-tauri/src/project_repository/diff.rs @@ -0,0 +1,146 @@ +use std::{collections::HashMap, path}; + +use anyhow::{Context, Result}; + +use super::Repository; + +pub struct Hunk { + pub old_start: usize, + pub old_lines: usize, + pub new_start: usize, + pub new_lines: usize, + pub diff: String, +} + +pub fn workdir( + repository: &Repository, + commit_oid: &git2::Oid, +) -> Result>> { + let commit = repository + .git_repository + .find_commit(*commit_oid) + .context("failed to find commit")?; + let tree = commit.tree().context("failed to find tree")?; + + let mut opts = git2::DiffOptions::new(); + opts.recurse_untracked_dirs(true) + .include_untracked(true) + .show_untracked_content(true); + + let diff = repository + .git_repository + .diff_tree_to_workdir(Some(&tree), Some(&mut opts))?; + + hunks_by_filepath(&diff) +} + +pub fn trees( + repository: &Repository, + old_tree: &git2::Tree, + new_tree: &git2::Tree, +) -> Result>> { + let mut opts = git2::DiffOptions::new(); + opts.recurse_untracked_dirs(true) + .include_untracked(true) + .show_untracked_content(true); + + let diff = + repository + .git_repository + .diff_tree_to_tree(Some(&old_tree), Some(&new_tree), None)?; + + hunks_by_filepath(&diff) +} + +fn hunks_by_filepath(diff: &git2::Diff) -> Result>> { + // find all the hunks + let mut hunks_by_filepath: HashMap> = HashMap::new(); + let mut current_diff = String::new(); + + let mut current_file_path: Option = None; + let mut current_hunk_id: Option = None; + let mut current_new_start: Option = None; + let mut current_new_lines: Option = None; + let mut current_old_start: Option = None; + let mut current_old_lines: Option = None; + + diff.print(git2::DiffFormat::Patch, |delta, hunk, line| { + let file_path = delta.new_file().path().unwrap_or_else(|| { + delta + .old_file() + .path() + .expect("failed to get file name from diff") + }); + + let (hunk_id, new_start, new_lines, old_start, old_lines) = if let Some(hunk) = hunk { + ( + format!( + "{}-{} {}-{}", + hunk.new_start(), + hunk.new_lines(), + hunk.old_start(), + hunk.old_lines(), + ), + hunk.new_start(), + hunk.new_lines(), + hunk.old_start(), + hunk.old_lines(), + ) + } else { + return true; + }; + + let is_path_changed = if current_file_path.is_none() { + false + } else { + !file_path.eq(current_file_path.as_ref().unwrap()) + }; + + let is_hunk_changed = if current_hunk_id.is_none() { + false + } else { + !hunk_id.eq(current_hunk_id.as_ref().unwrap()) + }; + + if is_hunk_changed || is_path_changed { + let file_path = current_file_path.as_ref().unwrap().to_path_buf(); + hunks_by_filepath.entry(file_path).or_default().push(Hunk { + old_start: current_old_start.unwrap(), + old_lines: current_old_lines.unwrap(), + new_start: current_new_start.unwrap(), + new_lines: current_new_lines.unwrap(), + diff: current_diff.clone(), + }); + current_diff = String::new(); + } + + match line.origin() { + '+' | '-' | ' ' => current_diff.push_str(&format!("{}", line.origin())), + _ => {} + } + + current_diff.push_str(std::str::from_utf8(line.content()).unwrap()); + current_file_path = Some(file_path.to_path_buf()); + current_hunk_id = Some(hunk_id); + current_new_start = Some(new_start as usize); + current_new_lines = Some(new_lines as usize); + current_old_start = Some(old_start as usize); + current_old_lines = Some(old_lines as usize); + + true + }) + .context("failed to print diff")?; + + // push the last hunk + if let Some(file_path) = current_file_path { + hunks_by_filepath.entry(file_path).or_default().push(Hunk { + old_start: current_old_start.unwrap(), + old_lines: current_old_lines.unwrap(), + new_start: current_new_start.unwrap(), + new_lines: current_new_lines.unwrap(), + diff: current_diff, + }); + } + + Ok(hunks_by_filepath) +} diff --git a/src-tauri/src/project_repository/mod.rs b/src-tauri/src/project_repository/mod.rs index e1a93c210..a02eeafd9 100644 --- a/src-tauri/src/project_repository/mod.rs +++ b/src-tauri/src/project_repository/mod.rs @@ -1,5 +1,6 @@ pub mod activity; pub mod conflicts; +pub mod diff; mod repository; pub use repository::{Error, FileStatus, Repository}; diff --git a/src-tauri/src/project_repository/repository.rs b/src-tauri/src/project_repository/repository.rs index 2ea606b8b..ead318c68 100644 --- a/src-tauri/src/project_repository/repository.rs +++ b/src-tauri/src/project_repository/repository.rs @@ -2,7 +2,7 @@ use core::fmt; use std::{cell::Cell, collections::HashMap, env, path, process::Command}; use anyhow::{Context, Result}; -use git2::{CredentialType, Diff}; +use git2::CredentialType; use serde::Serialize; use walkdir::WalkDir; @@ -33,7 +33,7 @@ pub struct Repository<'repository> { impl<'repository> Repository<'repository> { pub fn path(&self) -> &path::Path { - &path::Path::new(&self.project.path) + path::Path::new(&self.project.path) } pub fn open(project: &'repository projects::Project) -> Result { @@ -173,23 +173,6 @@ impl<'repository> Repository<'repository> { Ok(statuses) } - pub fn workdir_diff(&self, commit_oid: &git2::Oid) -> Result { - let commit = self - .git_repository - .find_commit(*commit_oid) - .context("failed to find commit")?; - let tree = commit.tree().context("failed to find tree")?; - - let mut opts = git2::DiffOptions::new(); - opts.recurse_untracked_dirs(true) - .include_untracked(true) - .show_untracked_content(true); - let diff = self - .git_repository - .diff_tree_to_workdir(Some(&tree), Some(&mut opts))?; - Ok(diff) - } - pub fn git_wd_diff(&self, context_lines: usize) -> Result> { let head = self.git_repository.head()?; let tree = head.peel_to_tree()?; diff --git a/src-tauri/src/virtual_branches/branch/file_ownership.rs b/src-tauri/src/virtual_branches/branch/file_ownership.rs index a3fa2a907..0a371b7d5 100644 --- a/src-tauri/src/virtual_branches/branch/file_ownership.rs +++ b/src-tauri/src/virtual_branches/branch/file_ownership.rs @@ -1,4 +1,4 @@ -use std::{fmt, vec}; +use std::{fmt, path, vec}; use anyhow::{Context, Result}; @@ -6,7 +6,7 @@ use super::hunk::Hunk; #[derive(Debug, PartialEq, Eq, Clone)] pub struct FileOwnership { - pub file_path: String, + pub file_path: path::PathBuf, pub hunks: Vec, } @@ -35,7 +35,7 @@ impl TryFrom<&str> for FileOwnership { Err(anyhow::anyhow!("ownership ranges cannot be empty"))? } else { Ok(Self { - file_path: file_path.to_string(), + file_path: file_path.into(), hunks: ranges, }) } @@ -146,12 +146,12 @@ impl FileOwnership { impl fmt::Display for FileOwnership { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { if self.hunks.is_empty() { - write!(f, "{}", self.file_path) + write!(f, "{}", self.file_path.display()) } else { write!( f, "{}:{}", - self.file_path, + self.file_path.display(), self.hunks .iter() .map(|r| r.to_string()) @@ -172,7 +172,7 @@ mod tests { assert_eq!( ownership, FileOwnership { - file_path: "foo/bar.rs".to_string(), + file_path: "foo/bar.rs".into(), hunks: vec![(1..=2).into(), (4..=5).into()] } ); @@ -186,7 +186,7 @@ mod tests { #[test] fn ownership_to_from_string() { let ownership = FileOwnership { - file_path: "foo/bar.rs".to_string(), + file_path: "foo/bar.rs".into(), hunks: vec![(1..=2).into(), (4..=5).into()], }; assert_eq!(ownership.to_string(), "foo/bar.rs:1-2,4-5".to_string()); diff --git a/src-tauri/src/virtual_branches/branch/writer.rs b/src-tauri/src/virtual_branches/branch/writer.rs index c85fa7050..2054e0aa9 100644 --- a/src-tauri/src/virtual_branches/branch/writer.rs +++ b/src-tauri/src/virtual_branches/branch/writer.rs @@ -137,7 +137,7 @@ mod tests { .unwrap(), ownership: branch::Ownership { files: vec![branch::FileOwnership { - file_path: format!("file/{}", unsafe { TEST_INDEX }), + file_path: format!("file/{}", unsafe { TEST_INDEX }).into(), hunks: vec![], }], }, diff --git a/src-tauri/src/virtual_branches/mod.rs b/src-tauri/src/virtual_branches/mod.rs index b53a638fc..6d4fcc6c6 100644 --- a/src-tauri/src/virtual_branches/mod.rs +++ b/src-tauri/src/virtual_branches/mod.rs @@ -21,7 +21,7 @@ use uuid::Uuid; use crate::{ gb_repository, - project_repository::{self, conflicts}, + project_repository::{self, conflicts, diff}, reader, sessions, }; @@ -81,7 +81,7 @@ pub struct VirtualBranchCommit { #[serde(rename_all = "camelCase")] pub struct VirtualBranchFile { pub id: String, - pub path: String, + pub path: path::PathBuf, pub hunks: Vec, pub modified_at: u128, pub conflicted: bool, @@ -102,7 +102,7 @@ pub struct VirtualBranchHunk { pub name: String, pub diff: String, pub modified_at: u128, - pub file_path: String, + pub file_path: path::PathBuf, pub hash: String, pub start: usize, pub end: usize, @@ -719,18 +719,22 @@ pub fn list_virtual_branches( let vtree_tree = repo.find_tree(vtree)?; // do a diff between branch.head and the tree we _would_ commit - let diff = repo.diff_tree_to_tree(Some(&tree_old), Some(&vtree_tree), None)?; - let hunks_by_filepath = diff_to_hunks_by_filepath(diff, project_repository)?; + let diff = diff::trees(project_repository, &tree_old, &vtree_tree) + .context("failed to diff trees")?; + let hunks_by_filepath = hunks_by_filepath(project_repository, &diff); vfiles = hunks_by_filepath - .iter() + .into_iter() .map(|(file_path, hunks)| VirtualBranchFile { - id: file_path.clone(), - path: file_path.to_string(), + id: file_path.display().to_string(), + path: file_path.clone(), hunks: hunks.clone(), modified_at: hunks.iter().map(|h| h.modified_at).max().unwrap_or(0), - conflicted: conflicts::is_conflicting(project_repository, Some(file_path)) - .unwrap_or(false), + conflicted: conflicts::is_conflicting( + project_repository, + Some(&file_path.display().to_string()), + ) + .unwrap_or(false), }) .collect::>(); } else { @@ -947,14 +951,15 @@ pub fn create_virtual_branch_from_branch( } // do a diff between the head of this branch and the target base - let diff = repo.diff_tree_to_tree(Some(&merge_tree), Some(&tree), None)?; - let hunks_by_filepath = diff_to_hunks_by_filepath(diff, project_repository)?; + let diff = + diff::trees(project_repository, &merge_tree, &tree).context("failed to diff trees")?; + let hunks_by_filepath = hunks_by_filepath(project_repository, &diff); // assign ownership to the branch for hunk in hunks_by_filepath.values().flatten() { - branch - .ownership - .put(&FileOwnership::try_from(format!("{}:{}", hunk.file_path, hunk.id)).unwrap()); + branch.ownership.put( + &FileOwnership::try_from(format!("{}:{}", hunk.file_path.display(), hunk.id)).unwrap(), + ); } let writer = branch::Writer::new(gb_repository); @@ -1177,125 +1182,29 @@ fn diff_hash(diff: &str) -> String { format!("{:x}", md5::compute(addition)) } -fn diff_to_hunks_by_filepath( - diff: git2::Diff, +fn hunks_by_filepath( project_repository: &project_repository::Repository, -) -> Result>> { - // find all the hunks - let mut hunks_by_filepath: HashMap> = HashMap::new(); - let mut current_diff = String::new(); - - let mut current_file_path: Option = None; - let mut current_hunk_id: Option = None; - let mut current_start: Option = None; - let mut current_end: Option = None; + diff: &HashMap>, +) -> HashMap> { let mut mtimes: HashMap = HashMap::new(); - - diff.print(git2::DiffFormat::Patch, |delta, hunk, line| { - let file_path = delta.new_file().path().unwrap_or_else(|| { - delta - .old_file() - .path() - .expect("failed to get file name from diff") - }); - - let (hunk_id, hunk_start, hunk_end) = if let Some(hunk) = hunk { - ( - format!( - "{}-{}", - hunk.new_start(), - hunk.new_start() + hunk.new_lines() - ), - hunk.new_start(), - hunk.new_start() + hunk.new_lines(), - ) - } else { - return true; - }; - - let is_path_changed = if current_file_path.is_none() { - false - } else { - !file_path.eq(current_file_path.as_ref().unwrap()) - }; - - let is_hunk_changed = if current_hunk_id.is_none() { - false - } else { - !hunk_id.eq(current_hunk_id.as_ref().unwrap()) - }; - - let mtime = get_mtime( - &mut mtimes, - &project_repository - .git_repository - .workdir() - .unwrap() - .join(file_path), - ); - if is_hunk_changed || is_path_changed { - let file_path = current_file_path - .as_ref() - .unwrap() - .to_str() - .unwrap() - .to_string(); - hunks_by_filepath - .entry(file_path.clone()) - .or_default() - .push(VirtualBranchHunk { - id: current_hunk_id.as_ref().unwrap().to_string(), + diff.iter() + .map(|(file_path, hunks)| { + let hunks = hunks + .iter() + .map(|hunk| VirtualBranchHunk { + id: format!("{}-{}", hunk.new_start, hunk.new_start + hunk.new_lines), name: "".to_string(), - diff: current_diff.clone(), - modified_at: mtime, - file_path, - start: current_start.unwrap(), - end: current_end.unwrap(), - hash: diff_hash(current_diff.as_str()), - }); - current_diff = String::new(); - } - - match line.origin() { - '+' | '-' | ' ' => current_diff.push_str(&format!("{}", line.origin())), - _ => {} - } - - current_diff.push_str(std::str::from_utf8(line.content()).unwrap()); - current_file_path = Some(file_path.to_path_buf()); - current_hunk_id = Some(hunk_id); - current_start = Some(hunk_start as usize); - current_end = Some(hunk_end as usize); - - true - }) - .context("failed to print diff")?; - - if let Some(file_path) = current_file_path { - let mtime = get_mtime( - &mut mtimes, - &project_repository - .git_repository - .workdir() - .unwrap() - .join(&file_path), - ); - let file_path = file_path.to_str().unwrap().to_string(); - hunks_by_filepath - .entry(file_path.clone()) - .or_default() - .push(VirtualBranchHunk { - id: current_hunk_id.as_ref().unwrap().to_string(), - name: "".to_string(), - modified_at: mtime, - file_path, - start: current_start.unwrap(), - end: current_end.unwrap(), - hash: diff_hash(current_diff.as_str()), - diff: current_diff, - }); - } - Ok(hunks_by_filepath) + modified_at: get_mtime(&mut mtimes, &project_repository.path().join(file_path)), + file_path: file_path.clone(), + diff: hunk.diff.clone(), + start: hunk.new_start, + end: hunk.new_start + hunk.new_lines, + hash: diff_hash(&hunk.diff), + }) + .collect::>(); + (file_path.clone(), hunks) + }) + .collect::>() } // list the virtual branches and their file statuses (statusi?) @@ -1318,14 +1227,8 @@ pub fn get_status_by_branch( } .context("failed to read default target")?; - let diff = project_repository - .workdir_diff(&default_target.sha) - .context(format!( - "failed to get diff workdir with {}", - default_target.sha - ))?; - - let mut hunks_by_filepath = diff_to_hunks_by_filepath(diff, project_repository)?; + let diff = diff::workdir(project_repository, &default_target.sha).context("failed to diff")?; + let mut hunks_by_filepath = hunks_by_filepath(project_repository, &diff); let mut virtual_branches = Iterator::new(¤t_session_reader) .context("failed to create branch iterator")? @@ -1461,7 +1364,11 @@ pub fn get_status_by_branch( virtual_branches[0].ownership.put( &FileOwnership::try_from(format!( "{}:{}-{}-{}-{}", - hunk.file_path, hunk.start, hunk.end, hunk.hash, hunk.modified_at, + hunk.file_path.display(), + hunk.start, + hunk.end, + hunk.hash, + hunk.modified_at, )) .unwrap(), ); @@ -1489,7 +1396,7 @@ pub fn get_status_by_branch( let mut files = hunks .iter() - .fold(HashMap::>::new(), |mut acc, hunk| { + .fold(HashMap::>::new(), |mut acc, hunk| { acc.entry(hunk.file_path.clone()) .or_default() .push(hunk.clone()); @@ -1497,12 +1404,15 @@ pub fn get_status_by_branch( }) .into_iter() .map(|(file_path, hunks)| VirtualBranchFile { - id: file_path.clone(), + id: file_path.display().to_string(), path: file_path.clone(), hunks: hunks.clone(), modified_at: hunks.iter().map(|h| h.modified_at).max().unwrap_or(0), - conflicted: conflicts::is_conflicting(project_repository, Some(&file_path)) - .unwrap_or(false), + conflicted: conflicts::is_conflicting( + project_repository, + Some(&file_path.display().to_string()), + ) + .unwrap_or(false), }) .collect::>(); @@ -1903,7 +1813,7 @@ pub fn update_gitbutler_integration( message.push('\n'); for file in &branch.ownership.files { message.push_str(" - "); - message.push_str(file.file_path.as_str()); + message.push_str(&file.file_path.display().to_string()); message.push('\n'); } } diff --git a/src-tauri/src/virtual_branches/target/reader.rs b/src-tauri/src/virtual_branches/target/reader.rs index a9c534873..54ac95d86 100644 --- a/src-tauri/src/virtual_branches/target/reader.rs +++ b/src-tauri/src/virtual_branches/target/reader.rs @@ -69,7 +69,7 @@ mod tests { .unwrap(), ownership: branch::Ownership { files: vec![branch::FileOwnership { - file_path: format!("file/{}", unsafe { TEST_INDEX }), + file_path: format!("file/{}", unsafe { TEST_INDEX }).into(), hunks: vec![], }], }, diff --git a/src-tauri/src/virtual_branches/target/writer.rs b/src-tauri/src/virtual_branches/target/writer.rs index 3c823a87b..94d3e4680 100644 --- a/src-tauri/src/virtual_branches/target/writer.rs +++ b/src-tauri/src/virtual_branches/target/writer.rs @@ -119,7 +119,7 @@ mod tests { .unwrap(), ownership: branch::Ownership { files: vec![branch::FileOwnership { - file_path: format!("file/{}", unsafe { TEST_INDEX }), + file_path: format!("file/{}", unsafe { TEST_INDEX }).into(), hunks: vec![], }], },