diff --git a/CHANGELOG.md b/CHANGELOG.md index a37b8caa..020202a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ This is was a immediate followup patch release to `0.20` see [release notes](htt - support inspecting annotation of tag ([#1076](https://github.com/extrawurst/gitui/issues/1076)) - support deleting tag on remote ([#1074](https://github.com/extrawurst/gitui/issues/1074)) - support git credentials helper (https) ([#800](https://github.com/extrawurst/gitui/issues/800)) +- Correct diff of renamed files [[@Mifom](https://github.com/Mifom)] ([#1038](https://github.com/extrawurst/gitui/issues/1038)) ### Fixed - Keep commit message when pre-commit hook fails ([#1035](https://github.com/extrawurst/gitui/issues/1035)) diff --git a/asyncgit/src/diff.rs b/asyncgit/src/diff.rs index a2685b4f..b6d4b5cc 100644 --- a/asyncgit/src/diff.rs +++ b/asyncgit/src/diff.rs @@ -29,8 +29,10 @@ pub enum DiffType { /// #[derive(Debug, Hash, Clone, PartialEq)] pub struct DiffParams { + /// + pub src_path: String, /// path to the file to diff - pub path: String, + pub dst_path: String, /// what kind of diff pub diff_type: DiffType, /// diff options @@ -161,26 +163,28 @@ impl AsyncDiff { let res = match params.diff_type { DiffType::Stage => sync::diff::get_diff( repo_path, - ¶ms.path, + ¶ms.src_path, + ¶ms.dst_path, true, Some(params.options), )?, DiffType::WorkDir => sync::diff::get_diff( repo_path, - ¶ms.path, + ¶ms.src_path, + ¶ms.dst_path, false, Some(params.options), )?, DiffType::Commit(id) => sync::diff::get_diff_commit( repo_path, id, - params.path.clone(), + params.dst_path.clone(), Some(params.options), )?, DiffType::Commits(ids) => sync::diff::get_diff_commits( repo_path, ids, - params.path.clone(), + params.dst_path.clone(), Some(params.options), )?, }; diff --git a/asyncgit/src/sync/commit_files.rs b/asyncgit/src/sync/commit_files.rs index d7fbf7fc..e4f5acb0 100644 --- a/asyncgit/src/sync/commit_files.rs +++ b/asyncgit/src/sync/commit_files.rs @@ -26,21 +26,24 @@ pub fn get_commit_files( get_commit_diff(repo_path, &repo, id, None, None)? }; - let res = diff - .deltas() - .map(|delta| { - let status = StatusItemType::from(delta.status()); + let res = + diff.deltas() + .map(|delta| { + let status = StatusItemType::from(delta.status()); - StatusItem { - path: delta - .new_file() - .path() - .map(|p| p.to_str().unwrap_or("").to_string()) - .unwrap_or_default(), - status, - } - }) - .collect::>(); + StatusItem { + old_path: delta.old_file().path().map(|p| { + p.to_str().unwrap_or("").to_string() + }), + new_path: delta + .new_file() + .path() + .map(|p| p.to_str().unwrap_or("").to_string()) + .unwrap_or_default(), + status, + } + }) + .collect::>(); Ok(res) } diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index f42252c0..d5634b2b 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -10,7 +10,8 @@ use crate::{ }; use easy_cast::Conv; use git2::{ - Delta, Diff, DiffDelta, DiffFormat, DiffHunk, Patch, Repository, + Delta, Diff, DiffDelta, DiffFindOptions, DiffFormat, DiffHunk, + Patch, Repository, }; use scopetime::scope_time; use std::{cell::RefCell, fs, path::Path, rc::Rc}; @@ -149,7 +150,8 @@ impl Default for DiffOptions { pub(crate) fn get_diff_raw<'a>( repo: &'a Repository, - p: &str, + src: &str, + dst: &str, stage: bool, reverse: bool, options: Option, @@ -162,10 +164,11 @@ pub(crate) fn get_diff_raw<'a>( opt.ignore_whitespace(options.ignore_whitespace); opt.interhunk_lines(options.interhunk_lines); } - opt.pathspec(p); + opt.pathspec(src); + opt.pathspec(dst); opt.reverse(reverse); - let diff = if stage { + let mut diff = if stage { // diff against head if let Ok(id) = get_head_repo(repo) { let parent = repo.find_commit(id.into())?; @@ -189,13 +192,17 @@ pub(crate) fn get_diff_raw<'a>( repo.diff_index_to_workdir(None, Some(&mut opt))? }; + diff.find_similar(Some( + DiffFindOptions::new().renames(true).for_untracked(true), + ))?; Ok(diff) } /// returns diff of a specific file either in `stage` or workdir pub fn get_diff( repo_path: &RepoPath, - p: &str, + src: &str, + dst: &str, stage: bool, options: Option, ) -> Result { @@ -203,7 +210,7 @@ pub fn get_diff( let repo = repo(repo_path)?; let work_dir = work_dir(&repo)?; - let diff = get_diff_raw(&repo, p, stage, false, options)?; + let diff = get_diff_raw(&repo, src, dst, stage, false, options)?; raw_diff_to_file_diff(&diff, work_dir) } @@ -250,8 +257,8 @@ pub fn get_diff_commits( /// //TODO: refactor into helper type with the inline closures as dedicated functions #[allow(clippy::too_many_lines)] -fn raw_diff_to_file_diff<'a>( - diff: &'a Diff, +fn raw_diff_to_file_diff( + diff: &Diff, work_dir: &Path, ) -> Result { let res = Rc::new(RefCell::new(FileDiff::default())); @@ -422,7 +429,7 @@ mod tests { }, }; use std::{ - fs::{self, File}, + fs::{self, remove_file, File}, io::Write, path::Path, }; @@ -444,8 +451,14 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - let diff = - get_diff(repo_path, "foo/bar.txt", false, None).unwrap(); + let diff = get_diff( + repo_path, + "foo/bar.txt", + "foo/bar.txt", + false, + None, + ) + .unwrap(); assert_eq!(diff.hunks.len(), 1); assert_eq!(&*diff.hunks[0].lines[1].content, "test"); @@ -475,6 +488,7 @@ mod tests { let diff = get_diff( repo_path, file_path.to_str().unwrap(), + file_path.to_str().unwrap(), true, None, ) @@ -530,7 +544,8 @@ mod tests { let res = get_status(repo_path, StatusType::WorkingDir, None) .unwrap(); assert_eq!(res.len(), 1); - assert_eq!(res[0].path, "bar.txt"); + assert_eq!(res[0].new_path, "bar.txt"); + assert_eq!(res[0].old_path, Some("bar.txt".to_string())); stage_add_file(repo_path, Path::new("bar.txt")).unwrap(); assert_eq!(get_statuses(repo_path), (0, 1)); @@ -546,7 +561,8 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 1)); let res = - get_diff(repo_path, "bar.txt", false, None).unwrap(); + get_diff(repo_path, "bar.txt", "bar.txt", false, None) + .unwrap(); assert_eq!(res.hunks.len(), 2) } @@ -568,6 +584,7 @@ mod tests { let diff = get_diff( &sub_path.to_str().unwrap().into(), file_path.to_str().unwrap(), + file_path.to_str().unwrap(), false, None, ) @@ -596,6 +613,7 @@ mod tests { let diff = get_diff( repo_path, file_path.to_str().unwrap(), + file_path.to_str().unwrap(), false, None, ) @@ -622,6 +640,7 @@ mod tests { let diff = get_diff( repo_path, file_path.to_str().unwrap(), + file_path.to_str().unwrap(), false, None, ) @@ -665,4 +684,50 @@ mod tests { Ok(()) } + + #[test] + fn test_rename() { + let (_td, repo) = repo_init().unwrap(); + let root = repo.path().parent().unwrap(); + let repo_path: &RepoPath = + &root.as_os_str().to_str().unwrap().into(); + + assert_eq!(get_statuses(repo_path), (0, 0)); + + let file_path = root.join("bar.txt"); + + { + File::create(&file_path) + .unwrap() + .write_all(HUNK_A.as_bytes()) + .unwrap(); + } + + let res = get_status(repo_path, StatusType::WorkingDir, None) + .unwrap(); + assert_eq!(res.len(), 1); + assert_eq!(res[0].new_path, "bar.txt"); + assert_eq!(res[0].old_path, Some("bar.txt".to_string())); + + stage_add_file(repo_path, Path::new("bar.txt")).unwrap(); + assert_eq!(get_statuses(repo_path), (0, 1)); + + // Move file + let other_file_path = root.join("baz.txt"); + { + File::create(&other_file_path) + .unwrap() + .write_all(HUNK_A.as_bytes()) + .unwrap(); + remove_file(&file_path).unwrap(); + } + + assert_eq!(get_statuses(repo_path), (1, 1)); + + let res = + get_diff(repo_path, "bar.txt", "baz.txt", false, None) + .unwrap(); + + assert_eq!(res.hunks.len(), 0) + } } diff --git a/asyncgit/src/sync/hunks.rs b/asyncgit/src/sync/hunks.rs index 786c1c90..c77653bb 100644 --- a/asyncgit/src/sync/hunks.rs +++ b/asyncgit/src/sync/hunks.rs @@ -13,14 +13,22 @@ use scopetime::scope_time; /// pub fn stage_hunk( repo_path: &RepoPath, - file_path: &str, + src_file_path: &str, + dst_file_path: &str, hunk_hash: u64, ) -> Result<()> { scope_time!("stage_hunk"); let repo = repo(repo_path)?; - let diff = get_diff_raw(&repo, file_path, false, false, None)?; + let diff = get_diff_raw( + &repo, + src_file_path, + dst_file_path, + false, + false, + None, + )?; let mut opt = ApplyOptions::new(); opt.hunk_callback(|hunk| { @@ -45,7 +53,9 @@ pub fn reset_hunk( let repo = repo(repo_path)?; - let diff = get_diff_raw(&repo, file_path, false, false, None)?; + let diff = get_diff_raw( + &repo, file_path, file_path, false, false, None, + )?; let hunk_index = find_hunk_index(&diff, hunk_hash); if let Some(hunk_index) = hunk_index { @@ -57,7 +67,9 @@ pub fn reset_hunk( res }); - let diff = get_diff_raw(&repo, file_path, false, true, None)?; + let diff = get_diff_raw( + &repo, file_path, file_path, false, true, None, + )?; repo.apply(&diff, ApplyLocation::WorkDir, Some(&mut opt))?; @@ -96,14 +108,22 @@ fn find_hunk_index(diff: &Diff, hunk_hash: u64) -> Option { /// pub fn unstage_hunk( repo_path: &RepoPath, - file_path: &str, + src_file_path: &str, + dst_file_path: &str, hunk_hash: u64, ) -> Result { scope_time!("revert_hunk"); let repo = repo(repo_path)?; - let diff = get_diff_raw(&repo, file_path, true, false, None)?; + let diff = get_diff_raw( + &repo, + src_file_path, + dst_file_path, + true, + false, + None, + )?; let diff_count_positive = diff.deltas().len(); let hunk_index = find_hunk_index(&diff, hunk_hash); @@ -112,7 +132,14 @@ pub fn unstage_hunk( Ok, )?; - let diff = get_diff_raw(&repo, file_path, true, true, None)?; + let diff = get_diff_raw( + &repo, + src_file_path, + dst_file_path, + true, + true, + None, + )?; if diff.deltas().len() != diff_count_positive { return Err(Error::Generic(format!( @@ -174,6 +201,7 @@ mod tests { let diff = get_diff( sub_path, file_path.to_str().unwrap(), + file_path.to_str().unwrap(), false, None, )?; diff --git a/asyncgit/src/sync/merge.rs b/asyncgit/src/sync/merge.rs index e495100f..21fffeec 100644 --- a/asyncgit/src/sync/merge.rs +++ b/asyncgit/src/sync/merge.rs @@ -42,7 +42,7 @@ pub fn abort_pending_state(repo_path: &RepoPath) -> Result<()> { let repo = repo(repo_path)?; reset_stage(repo_path, "*")?; - reset_workdir(repo_path, "*")?; + reset_workdir(repo_path, None, "*")?; repo.cleanup_state()?; diff --git a/asyncgit/src/sync/patches.rs b/asyncgit/src/sync/patches.rs index dae51160..4506c7a6 100644 --- a/asyncgit/src/sync/patches.rs +++ b/asyncgit/src/sync/patches.rs @@ -18,6 +18,7 @@ pub(crate) fn get_file_diff_patch_and_hunklines<'a>( let diff = get_diff_raw( repo, file, + file, is_staged, reverse, Some(DiffOptions { diff --git a/asyncgit/src/sync/remotes/push.rs b/asyncgit/src/sync/remotes/push.rs index 12413526..39e2342a 100644 --- a/asyncgit/src/sync/remotes/push.rs +++ b/asyncgit/src/sync/remotes/push.rs @@ -335,7 +335,7 @@ mod tests { None ) .unwrap()[0] - .path, + .new_path, String::from("temp_file.txt") ); diff --git a/asyncgit/src/sync/reset.rs b/asyncgit/src/sync/reset.rs index f057f231..3c4ba3c1 100644 --- a/asyncgit/src/sync/reset.rs +++ b/asyncgit/src/sync/reset.rs @@ -22,7 +22,11 @@ pub fn reset_stage(repo_path: &RepoPath, path: &str) -> Result<()> { } /// -pub fn reset_workdir(repo_path: &RepoPath, path: &str) -> Result<()> { +pub fn reset_workdir( + repo_path: &RepoPath, + old_path: Option<&str>, + new_path: &str, +) -> Result<()> { scope_time!("reset_workdir"); let repo = repo(repo_path)?; @@ -32,7 +36,11 @@ pub fn reset_workdir(repo_path: &RepoPath, path: &str) -> Result<()> { .update_index(true) // windows: needs this to be true WTF?! .remove_untracked(true) .force() - .path(path); + .path(new_path); + + if let Some(path) = old_path { + checkout_opts.path(path); + } repo.checkout_index(None, Some(&mut checkout_opts))?; Ok(()) @@ -121,7 +129,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 1)); - reset_workdir(repo_path, "bar.txt").unwrap(); + reset_workdir(repo_path, None, "bar.txt").unwrap(); debug_cmd_print(repo_path, "git status"); @@ -147,7 +155,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - reset_workdir(repo_path, "foo/bar.txt").unwrap(); + reset_workdir(repo_path, None, "foo/bar.txt").unwrap(); debug_cmd_print(repo_path, "git status"); @@ -193,7 +201,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (4, 1)); - reset_workdir(repo_path, "foo").unwrap(); + reset_workdir(repo_path, None, "foo").unwrap(); assert_eq!(get_statuses(repo_path), (1, 1)); @@ -233,7 +241,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 1)); - reset_workdir(repo_path, file).unwrap(); + reset_workdir(repo_path, None, file).unwrap(); debug_cmd_print(repo_path, "git status"); @@ -285,6 +293,7 @@ mod tests { reset_workdir( &root.join("foo").as_os_str().to_str().unwrap().into(), + None, "foo/bar.txt", ) .unwrap(); @@ -313,7 +322,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - reset_workdir(repo_path, "foo/bar").unwrap(); + reset_workdir(repo_path, None, "foo/bar").unwrap(); debug_cmd_print(repo_path, "git status"); diff --git a/asyncgit/src/sync/staging/stage_tracked.rs b/asyncgit/src/sync/staging/stage_tracked.rs index 5f9861a1..82a5753b 100644 --- a/asyncgit/src/sync/staging/stage_tracked.rs +++ b/asyncgit/src/sync/staging/stage_tracked.rs @@ -98,7 +98,8 @@ mod test { ) .unwrap(); - let diff = get_diff(path, "test.txt", true, None).unwrap(); + let diff = get_diff(path, "test.txt", "test.txt", true, None) + .unwrap(); assert_eq!(diff.lines, 3); assert_eq!(&*diff.hunks[0].lines[0].content, "@@ -1 +1,2 @@"); @@ -137,7 +138,8 @@ c = 4"; ) .unwrap(); - let diff = get_diff(path, "test.txt", true, None).unwrap(); + let diff = get_diff(path, "test.txt", "test.txt", true, None) + .unwrap(); assert_eq!(diff.lines, 5); assert_eq!(&*diff.hunks[0].lines[0].content, "@@ -1,2 +1 @@"); @@ -168,7 +170,8 @@ c = 4"; assert_eq!(get_statuses(path), (0, 1)); let diff_before = - get_diff(path, "test.txt", true, None).unwrap(); + get_diff(path, "test.txt", "test.txt", true, None) + .unwrap(); assert_eq!(diff_before.lines, 5); @@ -185,7 +188,8 @@ c = 4"; assert_eq!(get_statuses(path), (1, 1)); - let diff = get_diff(path, "test.txt", true, None).unwrap(); + let diff = get_diff(path, "test.txt", "test.txt", true, None) + .unwrap(); assert_eq!(diff.lines, 4); } diff --git a/asyncgit/src/sync/status.rs b/asyncgit/src/sync/status.rs index 345a64b2..2104a911 100644 --- a/asyncgit/src/sync/status.rs +++ b/asyncgit/src/sync/status.rs @@ -5,7 +5,9 @@ use crate::{ error::Result, sync::{config::untracked_files_config_repo, repository::repo}, }; -use git2::{Delta, Status, StatusOptions, StatusShow}; +use git2::{ + Delta, DiffDelta, Status, StatusEntry, StatusOptions, StatusShow, +}; use scopetime::scope_time; use std::path::Path; @@ -62,7 +64,9 @@ impl From for StatusItemType { #[derive(Clone, Hash, PartialEq, Debug)] pub struct StatusItem { /// - pub path: String, + pub old_path: Option, + /// + pub new_path: String, /// pub status: StatusItemType, } @@ -94,6 +98,18 @@ impl From for StatusShow { } } +fn get_diff<'a>( + status_type: StatusType, + status_entry: &'a StatusEntry, +) -> Option> { + (status_type != StatusType::WorkingDir) + .then(|| status_entry.head_to_index()) + .or_else(|| { + (status_type != StatusType::Stage) + .then(|| status_entry.index_to_workdir()) + }) + .flatten() +} /// pub fn is_workdir_clean( repo_path: &RepoPath, @@ -151,7 +167,8 @@ pub fn get_status( .show(status_type.into()) .update_index(true) .include_untracked(show_untracked.include_untracked()) - .renames_head_to_index(true) + .renames_head_to_index(status_type != StatusType::WorkingDir) + .renames_index_to_workdir(status_type != StatusType::Stage) .recurse_untracked_dirs( show_untracked.recurse_untracked_dirs(), ); @@ -163,7 +180,7 @@ pub fn get_status( for e in statuses.iter() { let status: Status = e.status(); - let path = match e.head_to_index() { + let new_path = match get_diff(status_type, &e) { Some(diff) => diff .new_file() .path() @@ -182,16 +199,93 @@ pub fn get_status( ) })?, }; + let old_path = get_diff(status_type, &e) + .and_then(|diff| diff.old_file().path()) + .map(|path| { + path.to_str().map(String::from).ok_or_else(|| { + Error::Generic( + "failed to get path to diff's new file." + .to_string(), + ) + }) + }) + .transpose()?; res.push(StatusItem { - path, + old_path, + new_path, status: StatusItemType::from(status), }); } res.sort_by(|a, b| { - Path::new(a.path.as_str()).cmp(Path::new(b.path.as_str())) + Path::new(a.new_path.as_str()) + .cmp(Path::new(b.new_path.as_str())) }); Ok(res) } + +#[cfg(test)] +mod tests { + use crate::{ + error::Result, + sync::{ + commit, stage_add_file, stage_addremoved, + status::{get_status, StatusItemType, StatusType}, + tests::repo_init_empty, + RepoPath, + }, + }; + use std::{ + fs::{self, File}, + io::Write, + path::Path, + }; + + #[test] + fn test_rename_file() -> Result<()> { + let bar = Path::new("bar"); + let foo = Path::new("foo"); + let (_td, repo) = repo_init_empty()?; + let root = repo.path().parent().unwrap(); + let repo_path: &RepoPath = + &root.as_os_str().to_str().unwrap().into(); + + let mut file = File::create(&root.join(bar))?; + file.write_all(b"\x00")?; + + let statuses = + get_status(repo_path, StatusType::Stage, None).unwrap(); + for diff in statuses.iter() { + assert_eq!(diff.status, StatusItemType::New); + assert_eq!(diff.old_path, Some("bar".to_string())); + assert_eq!(diff.new_path, "bar".to_string()); + } + + stage_add_file(repo_path, bar)?; + let _id = commit(repo_path, "")?; + + fs::rename(&root.join(bar), &root.join(foo))?; + stage_add_file(repo_path, foo)?; + stage_addremoved(repo_path, bar)?; + let statuses = + get_status(repo_path, StatusType::Stage, None).unwrap(); + for diff in statuses.iter() { + assert_eq!(diff.status, StatusItemType::Renamed); + assert_eq!(diff.old_path, Some("bar".to_string())); + assert_eq!(diff.new_path, "foo".to_string()); + } + + fs::remove_file(&root.join(foo))?; + stage_addremoved(repo_path, foo)?; + let statuses = + get_status(repo_path, StatusType::Stage, None).unwrap(); + for diff in statuses.iter() { + assert_eq!(diff.status, StatusItemType::Deleted); + assert_eq!(diff.old_path, Some("bar".to_string())); + assert_eq!(diff.new_path, "bar".to_string()); + } + Ok(()) + } +} diff --git a/asyncgit/src/sync/utils.rs b/asyncgit/src/sync/utils.rs index 58042a9d..e4a28702 100644 --- a/asyncgit/src/sync/utils.rs +++ b/asyncgit/src/sync/utils.rs @@ -335,7 +335,8 @@ mod tests { // And that file is test.txt let diff = - get_diff(repo_path, "test.txt", true, None).unwrap(); + get_diff(repo_path, "test.txt", "test.txt", true, None) + .unwrap(); assert_eq!(&*diff.hunks[0].lines[0].content, "@@ -1 +1 @@"); } diff --git a/src/components/changes.rs b/src/components/changes.rs index 5c7e5d79..b4ca2778 100644 --- a/src/components/changes.rs +++ b/src/components/changes.rs @@ -90,18 +90,35 @@ impl ChangesComponent { fn index_add_remove(&mut self) -> Result { if let Some(tree_item) = self.selection() { if self.is_working_dir { - if let FileTreeItemKind::File(i) = tree_item.kind { - let path = Path::new(i.path.as_str()); + if let FileTreeItemKind::File(ref i) = tree_item.kind + { + let new_path = Path::new(i.new_path.as_str()); + let old_path = i + .old_path + .as_ref() + .map(|path| Path::new(path.as_str())); match i.status { StatusItemType::Deleted => { sync::stage_addremoved( &self.repo.borrow(), - path, + new_path, + )?; + } + StatusItemType::Renamed => { + if let Some(old_path) = old_path { + sync::stage_addremoved( + &self.repo.borrow(), + old_path, + )?; + } + sync::stage_add_file( + &self.repo.borrow(), + new_path, )?; } _ => sync::stage_add_file( &self.repo.borrow(), - path, + new_path, )?, }; } else { @@ -132,6 +149,25 @@ impl ChangesComponent { sync::reset_stage(&self.repo.borrow(), path)?; } + if let FileTreeItemKind::File(i) = tree_item.kind { + if i.status == StatusItemType::Renamed { + i.old_path + .map(|path| { + sync::reset_stage( + &self.repo.borrow(), + path.as_str(), + ) + }) + .transpose()?; + } + sync::reset_stage( + &self.repo.borrow(), + i.new_path.as_str(), + )?; + } + + let path = tree_item.info.full_path.as_str(); + sync::reset_stage(&self.repo.borrow(), path)?; return Ok(true); } @@ -160,9 +196,14 @@ impl ChangesComponent { if let Some(tree_item) = self.selection() { let is_folder = matches!(tree_item.kind, FileTreeItemKind::Path(_)); + let old_path = match tree_item.kind { + FileTreeItemKind::Path(_) => None, + FileTreeItemKind::File(status) => status.old_path, + }; self.queue.push(InternalEvent::ConfirmAction( Action::Reset(ResetItem { - path: tree_item.info.full_path, + old_path, + new_path: tree_item.info.full_path, is_folder, }), )); diff --git a/src/components/compare_commits.rs b/src/components/compare_commits.rs index 7944c4b5..a160b94e 100644 --- a/src/components/compare_commits.rs +++ b/src/components/compare_commits.rs @@ -251,7 +251,11 @@ impl CompareCommitsComponent { if let Some(f) = self.details.files().selection_file() { let diff_params = DiffParams { - path: f.path.clone(), + src_path: f + .old_path + .clone() + .unwrap_or_else(|| f.new_path.clone()), + dst_path: f.new_path.clone(), diff_type: DiffType::Commits(ids), options: DiffOptions::default(), }; @@ -260,7 +264,10 @@ impl CompareCommitsComponent { self.git_diff.last()? { if params == diff_params { - self.diff.update(f.path, false, last); + self.diff.update( + f.old_path, f.new_path, f.status, + false, last, + ); return Ok(()); } } diff --git a/src/components/diff.rs b/src/components/diff.rs index a4854213..5c4a731e 100644 --- a/src/components/diff.rs +++ b/src/components/diff.rs @@ -14,7 +14,7 @@ use anyhow::Result; use asyncgit::{ hash, sync::{self, diff::DiffLinePosition, RepoPathRef}, - DiffLine, DiffLineType, FileDiff, + DiffLine, DiffLineType, Error, FileDiff, StatusItemType, }; use bytesize::ByteSize; use crossterm::event::Event; @@ -30,7 +30,8 @@ use tui::{ #[derive(Default)] struct Current { - path: String, + old_path: Option, + new_path: String, is_stage: bool, hash: u64, } @@ -148,8 +149,12 @@ impl DiffComponent { .unwrap_or_default() } /// - pub fn current(&self) -> (String, bool) { - (self.current.path.clone(), self.current.is_stage) + pub fn current(&self) -> (Option, String, bool) { + ( + self.current.old_path.clone(), + self.current.new_path.clone(), + self.current.is_stage, + ) } /// pub fn clear(&mut self, pending: bool) { @@ -163,7 +168,9 @@ impl DiffComponent { /// pub fn update( &mut self, - path: String, + old_path: Option, + new_path: String, + status: StatusItemType, is_stage: bool, diff: FileDiff, ) { @@ -172,10 +179,15 @@ impl DiffComponent { let hash = hash(&diff); if self.current.hash != hash { - let reset_selection = self.current.path != path; + let reset_selection = self.current.new_path != new_path; self.current = Current { - path, + old_path: if status == StatusItemType::Renamed { + old_path + } else { + None + }, + new_path, is_stage, hash, }; @@ -463,7 +475,11 @@ impl DiffComponent { let hash = diff.hunks[hunk].header_hash; sync::unstage_hunk( &self.repo.borrow(), - &self.current.path, + self.current + .old_path + .as_ref() + .unwrap_or(&self.current.new_path), + &self.current.new_path, hash, )?; self.queue_update(); @@ -479,13 +495,17 @@ impl DiffComponent { if diff.untracked { sync::stage_add_file( &self.repo.borrow(), - Path::new(&self.current.path), + Path::new(&self.current.new_path), )?; } else { let hash = diff.hunks[hunk].header_hash; sync::stage_hunk( &self.repo.borrow(), - &self.current.path, + self.current + .old_path + .as_ref() + .unwrap_or(&self.current.new_path), + &self.current.new_path, hash, )?; } @@ -501,25 +521,32 @@ impl DiffComponent { self.queue.push(InternalEvent::Update(NeedsUpdate::ALL)); } - fn reset_hunk(&self) { + fn reset_hunk(&self) -> Result<()> { + if self.current.old_path.is_some() { + return Err(Error::Generic( + "Cannot reset in renamed files".to_string(), + ) + .into()); + } if let Some(diff) = &self.diff { if let Some(hunk) = self.selected_hunk { let hash = diff.hunks[hunk].header_hash; self.queue.push(InternalEvent::ConfirmAction( Action::ResetHunk( - self.current.path.clone(), + self.current.new_path.clone(), hash, ), )); } } + Ok(()) } fn reset_lines(&self) { self.queue.push(InternalEvent::ConfirmAction( Action::ResetLines( - self.current.path.clone(), + self.current.new_path.clone(), self.selected_lines(), ), )); @@ -536,7 +563,7 @@ impl DiffComponent { "(un)stage lines:", sync::stage_lines( &self.repo.borrow(), - &self.current.path, + &self.current.new_path, self.is_stage(), &selected_lines, ) @@ -575,7 +602,8 @@ impl DiffComponent { fn reset_untracked(&self) { self.queue.push(InternalEvent::ConfirmAction(Action::Reset( ResetItem { - path: self.current.path.clone(), + old_path: self.current.old_path.clone(), + new_path: self.current.new_path.clone(), is_folder: false, }, ))); @@ -616,9 +644,14 @@ impl DrawableComponent for DiffComponent { ); let title = format!( - "{}{}", + "{}{}{}", strings::title_diff(&self.key_config), - self.current.path + self.current + .old_path + .as_ref() + .map(|path| format!("{} -> ", path)) + .unwrap_or_default(), + self.current.new_path ); let txt = if self.pending { @@ -767,7 +800,11 @@ impl Component for DiffComponent { if diff.untracked { self.reset_untracked(); } else { - self.reset_hunk(); + try_or_popup!( + self, + "hunk error: ", + self.reset_hunk() + ); } } Ok(EventState::Consumed) diff --git a/src/components/file_revlog.rs b/src/components/file_revlog.rs index da383865..41df582d 100644 --- a/src/components/file_revlog.rs +++ b/src/components/file_revlog.rs @@ -188,7 +188,8 @@ impl FileRevlogComponent { if let Some(commit_id) = self.selected_commit() { if let Some(open_request) = &self.open_request { let diff_params = DiffParams { - path: open_request.file_path.clone(), + src_path: open_request.file_path.clone(), + dst_path: open_request.file_path.clone(), diff_type: DiffType::Commit(commit_id), options: self.options.borrow().diff, }; @@ -198,7 +199,9 @@ impl FileRevlogComponent { { if params == diff_params { self.diff.update( - open_request.file_path.to_string(), + Some(params.src_path), + params.dst_path, + asyncgit::StatusItemType::Modified, false, last, ); diff --git a/src/components/inspect_commit.rs b/src/components/inspect_commit.rs index b8286209..76b9dda0 100644 --- a/src/components/inspect_commit.rs +++ b/src/components/inspect_commit.rs @@ -278,7 +278,11 @@ impl InspectCommitComponent { if let Some(f) = self.details.files().selection_file() { let diff_params = DiffParams { - path: f.path.clone(), + src_path: f + .old_path + .clone() + .unwrap_or_else(|| f.new_path.clone()), + dst_path: f.new_path.clone(), diff_type: DiffType::Commit( request.commit_id, ), @@ -289,7 +293,10 @@ impl InspectCommitComponent { self.git_diff.last()? { if params == diff_params { - self.diff.update(f.path, false, last); + self.diff.update( + f.old_path, f.new_path, f.status, + false, last, + ); return Ok(()); } } diff --git a/src/components/status_tree.rs b/src/components/status_tree.rs index 41bb8f25..7e42fee7 100644 --- a/src/components/status_tree.rs +++ b/src/components/status_tree.rs @@ -172,21 +172,47 @@ impl StatusTreeComponent { FileTreeItemKind::File(status_item) => { let status_char = Self::item_status_char(status_item.status); - let file = Path::new(&status_item.path) + let file = Path::new(&status_item.new_path) .file_name() .and_then(std::ffi::OsStr::to_str) .expect("invalid path."); + let old_file_prefix = if status_item.status + == StatusItemType::Renamed + { + status_item + .old_path + .as_ref() + .map(|path| { + Path::new(path) + .file_name() + .and_then(std::ffi::OsStr::to_str) + .map(|old_file| { + format!("{} -> ", old_file) + }) + .expect("invalid path.") + }) + .unwrap_or_default() + } else { + String::from("") + }; let txt = if selected { format!( - "{} {}{:w$}", + "{} {}{}{:w$}", status_char, indent_str, + old_file_prefix, file, w = width as usize ) } else { - format!("{} {}{}", status_char, indent_str, file) + format!( + "{} {}{}{}", + status_char, + indent_str, + old_file_prefix, + file + ) }; Some(Span::styled( @@ -429,7 +455,8 @@ impl Component for StatusTreeComponent { queue.push(InternalEvent::OpenPopup( StackablePopupOpen::BlameFile( BlameFileOpen { - file_path: status_item.path, + file_path: status_item + .new_path, commit_id: None, selection: None, }, @@ -445,7 +472,7 @@ impl Component for StatusTreeComponent { queue.push(InternalEvent::OpenPopup( StackablePopupOpen::FileRevlog( FileRevOpen::new( - status_item.path, + status_item.new_path, ), ), )); @@ -516,7 +543,8 @@ mod tests { items .iter() .map(|a| StatusItem { - path: String::from(*a), + old_path: None, + new_path: String::from(*a), status: StatusItemType::Modified, }) .collect::>() diff --git a/src/components/utils/filetree.rs b/src/components/utils/filetree.rs index 4b26825e..9ef63f96 100644 --- a/src/components/utils/filetree.rs +++ b/src/components/utils/filetree.rs @@ -58,7 +58,7 @@ pub struct FileTreeItem { impl FileTreeItem { fn new_file(item: &StatusItem) -> Result { - let item_path = Path::new(&item.path); + let item_path = Path::new(&item.new_path); let indent = u8::try_from( item_path.ancestors().count().saturating_sub(2), )?; @@ -73,7 +73,7 @@ impl FileTreeItem { info: TreeItemInfo::new( indent, path, - item.path.clone(), + item.new_path.clone(), ), kind: FileTreeItemKind::File(item.clone()), }), @@ -148,7 +148,7 @@ impl FileTreeItems { for e in list { { - let item_path = Path::new(&e.path); + let item_path = Path::new(&e.new_path); Self::push_dirs( item_path, @@ -267,7 +267,8 @@ mod tests { items .iter() .map(|a| StatusItem { - path: String::from(*a), + old_path: None, + new_path: String::from(*a), status: StatusItemType::Modified, }) .collect::>() @@ -286,8 +287,8 @@ mod tests { res.items, vec![FileTreeItem { info: TreeItemInfo { - path: items[0].path.clone(), - full_path: items[0].path.clone(), + path: items[0].new_path.clone(), + full_path: items[0].new_path.clone(), indent: 0, visible: true, }, @@ -304,7 +305,7 @@ mod tests { FileTreeItems::new(&items, &BTreeSet::new()).unwrap(); assert_eq!(res.items.len(), 2); - assert_eq!(res.items[1].info.path, items[1].path); + assert_eq!(res.items[1].info.path, items[1].new_path); } #[test] @@ -322,7 +323,7 @@ mod tests { assert_eq!( res, - vec![String::from("a"), items[0].path.clone(),] + vec![String::from("a"), items[0].new_path.clone(),] ); } @@ -381,8 +382,8 @@ mod tests { res, vec![ String::from("a"), - items[0].path.clone(), - items[1].path.clone() + items[0].new_path.clone(), + items[1].new_path.clone() ] ); } diff --git a/src/components/utils/statustree.rs b/src/components/utils/statustree.rs index 1edc7ce6..36bd0ea3 100644 --- a/src/components/utils/statustree.rs +++ b/src/components/utils/statustree.rs @@ -438,7 +438,8 @@ mod tests { items .iter() .map(|a| StatusItem { - path: String::from(*a), + old_path: None, + new_path: String::from(*a), status: StatusItemType::Modified, }) .collect::>() diff --git a/src/queue.rs b/src/queue.rs index 3c404de8..7f013558 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -30,8 +30,10 @@ bitflags! { /// data of item that is supposed to be reset pub struct ResetItem { - /// path to the item (folder/file) - pub path: String, + /// old path to the item (folder/file) + pub old_path: Option, + /// new path to the item (folder/file) + pub new_path: String, /// are talking about a folder here? otherwise it's a single file pub is_folder: bool, } diff --git a/src/tabs/status.rs b/src/tabs/status.rs index 65497f12..bd45d7d8 100644 --- a/src/tabs/status.rs +++ b/src/tabs/status.rs @@ -19,7 +19,7 @@ use asyncgit::{ }, sync::{BranchCompare, CommitId}, AsyncDiff, AsyncGitNotification, AsyncStatus, DiffParams, - DiffType, PushType, StatusParams, + DiffType, PushType, StatusItemType, StatusParams, }; use crossbeam_channel::Sender; use crossterm::event::Event; @@ -377,7 +377,7 @@ impl Status { self.index.focus_select(is_stage); } - pub fn selected_path(&self) -> Option<(String, bool)> { + pub fn selected_data(&self) -> Option { let (idx, is_stage) = match self.diff_target { DiffTarget::Stage => (&self.index, true), DiffTarget::WorkingDir => (&self.index_wd, false), @@ -385,7 +385,12 @@ impl Status { if let Some(item) = idx.selection() { if let FileTreeItemKind::File(i) = item.kind { - return Some((i.path, is_stage)); + return Some(DiffData { + old_path: i.old_path, + new_path: i.new_path, + status: i.status, + is_stage, + }); } } None @@ -480,7 +485,13 @@ impl Status { /// pub fn update_diff(&mut self) -> Result<()> { - if let Some((path, is_stage)) = self.selected_path() { + if let Some(DiffData { + old_path, + new_path, + status, + is_stage, + }) = self.selected_data() + { let diff_type = if is_stage { DiffType::Stage } else { @@ -488,30 +499,46 @@ impl Status { }; let diff_params = DiffParams { - path: path.clone(), + src_path: old_path + .clone() + .unwrap_or_else(|| new_path.clone()), + dst_path: new_path.clone(), diff_type, options: self.options.borrow().diff, }; - if self.diff.current() == (path.clone(), is_stage) { + if self.diff.current() + == (old_path.clone(), new_path.clone(), is_stage) + { // we are already showing a diff of the right file // maybe the diff changed (outside file change) if let Some((params, last)) = self.git_diff.last()? { if params == diff_params { // all params match, so we might need to update - self.diff.update(path, is_stage, last); + self.diff.update( + old_path, new_path, status, is_stage, + last, + ); } else { // params changed, we need to request the right diff self.request_diff( diff_params, - path, + old_path, + new_path, + status, is_stage, )?; } } } else { // we dont show the right diff right now, so we need to request - self.request_diff(diff_params, path, is_stage)?; + self.request_diff( + diff_params, + old_path, + new_path, + status, + is_stage, + )?; } } else { self.diff.clear(false); @@ -523,11 +550,14 @@ impl Status { fn request_diff( &mut self, diff_params: DiffParams, - path: String, + old_path: Option, + new_path: String, + status: StatusItemType, is_stage: bool, ) -> Result<(), anyhow::Error> { if let Some(diff) = self.git_diff.request(diff_params)? { - self.diff.update(path, is_stage, diff); + self.diff + .update(old_path, new_path, status, is_stage, diff); } else { self.diff.clear(true); } @@ -539,7 +569,8 @@ impl Status { pub fn reset(&mut self, item: &ResetItem) -> bool { if let Err(e) = sync::reset_workdir( &self.repo.borrow(), - item.path.as_str(), + item.old_path.as_deref(), + item.new_path.as_str(), ) { self.queue.push(InternalEvent::ShowErrorMsg(format!( "reset failed:\n{}", @@ -701,6 +732,13 @@ impl Status { } } +pub struct DiffData { + old_path: Option, + new_path: String, + status: StatusItemType, + is_stage: bool, +} + impl Component for Status { fn commands( &self, @@ -818,10 +856,10 @@ impl Component for Status { && (self.can_focus_diff() || self.is_focus_on_diff()) { - if let Some((path, _)) = self.selected_path() { + if let Some(diff_data) = self.selected_data() { self.queue.push( InternalEvent::OpenExternalEditor(Some( - path, + diff_data.new_path, )), ); }