diff --git a/crates/but-cli/src/command/mod.rs b/crates/but-cli/src/command/mod.rs index 4973324a74..26adf7aa0d 100644 --- a/crates/but-cli/src/command/mod.rs +++ b/crates/but-cli/src/command/mod.rs @@ -151,6 +151,7 @@ pub(crate) fn discard_change( debug_print(but_workspace::discard_workspace_changes( &repo, Some(spec.into()), + UI_CONTEXT_LINES, )?) } diff --git a/crates/but-core/src/diff/mod.rs b/crates/but-core/src/diff/mod.rs index 99fb55dc45..8950d55d57 100644 --- a/crates/but-core/src/diff/mod.rs +++ b/crates/but-core/src/diff/mod.rs @@ -80,6 +80,19 @@ impl ModeFlags { } } +impl ModeFlags { + /// Returns `true` if this instance indicates a type-change. + /// The only reason this isn't the case is if the executable bit changed. + pub fn is_typechange(&self) -> bool { + match self { + ModeFlags::ExecutableBitAdded | ModeFlags::ExecutableBitRemoved => false, + ModeFlags::TypeChangeFileToLink + | ModeFlags::TypeChangeLinkToFile + | ModeFlags::TypeChange => true, + } + } +} + #[cfg(test)] mod tests { mod flags { diff --git a/crates/but-core/src/diff/worktree.rs b/crates/but-core/src/diff/worktree.rs index d7aecfba83..281c7373fd 100644 --- a/crates/but-core/src/diff/worktree.rs +++ b/crates/but-core/src/diff/worktree.rs @@ -447,11 +447,24 @@ pub fn worktree_changes(repo: &gix::Repository) -> anyhow::Result IgnoredWorktreeTreeChangeStatus::TreeIndexWorktreeChangeIneffective, - Some(merged) => { + [None, None] => IgnoredWorktreeTreeChangeStatus::TreeIndexWorktreeChangeIneffective, + [Some(merged), None] | [None, Some(merged)] => { changes.push(merged); IgnoredWorktreeTreeChangeStatus::TreeIndex } + [Some(first), Some(second)] => { + ignored_changes.push(IgnoredWorktreeChange { + path: first.path.clone(), + status: IgnoredWorktreeTreeChangeStatus::TreeIndex, + }); + changes.push(first); + ignored_changes.push(IgnoredWorktreeChange { + path: second.path.clone(), + status: IgnoredWorktreeTreeChangeStatus::TreeIndex, + }); + changes.push(second); + continue; + } }; ignored_changes.push(IgnoredWorktreeChange { path: change_path, @@ -481,7 +494,7 @@ fn cmp_prefer_overlapping(a: &TreeChange, b: &TreeChange) -> Ordering { } /// Merge changes from tree/index into changes of `index_wt` and assure the merged result isn't a no-op, -/// which is when `None` is returned. +/// which is when `[None, None]` is returned. Otherwise, `[Some(_), None]` or `[Some(_), Some(_)]` are returned. /// Note that this case is more expensive as we have to hash the worktree version to check for a no-op. /// `diff_filter` is used to obtain hashes of worktree content. fn merge_changes( @@ -490,7 +503,10 @@ fn merge_changes( filter: &mut gix::filter::Pipeline<'_>, index: &gix::index::State, path_check: &mut gix::status::plumbing::SymlinkCheck, -) -> anyhow::Result> { +) -> anyhow::Result<[Option; 2]> { + fn single(change: TreeChange) -> [Option; 2] { + [Some(change), None] + } let merged = match (&mut tree_index.status, &mut index_wt.status) { (TreeStatus::Modification { .. }, TreeStatus::Addition { .. }) | (TreeStatus::Deletion { .. }, TreeStatus::Deletion { .. }) @@ -521,11 +537,11 @@ fn merge_changes( ) => { *is_untracked = true; *state = *state_wt; - return Ok(Some(tree_index)); + return Ok(single(tree_index)); } (TreeStatus::Addition { .. }, TreeStatus::Deletion { .. }) => { // keep the most recent known state, which is from the index. - return Ok(Some(index_wt)); + return Ok(single(index_wt)); } ( TreeStatus::Addition { state, .. }, @@ -538,7 +554,7 @@ fn merge_changes( // Pretend the added file (in index) is the one that was deleted, hence the rename. *ps_wt = *state; // Can't be no-op as this is a rename - return Ok(Some(index_wt)); + return Ok(single(index_wt)); } ( TreeStatus::Deletion { previous_state, .. }, @@ -565,7 +581,7 @@ fn merge_changes( index_wt } (TreeStatus::Modification { .. }, TreeStatus::Deletion { .. }) => { - return Ok(Some(index_wt)); + return Ok(single(index_wt)); } ( TreeStatus::Modification { previous_state, .. }, @@ -577,17 +593,82 @@ fn merge_changes( *ps_wt = *previous_state; index_wt } - (TreeStatus::Rename { .. }, TreeStatus::Modification { .. }) => { - todo!("rename - mod") + ( + TreeStatus::Rename { + state: state_index, .. + }, + TreeStatus::Modification { + state: state_wt, .. + }, + ) => { + *state_index = *state_wt; + return Ok(single(tree_index)); } - (TreeStatus::Rename { .. }, TreeStatus::Rename { .. }) => { - todo!("rename - rename") + ( + TreeStatus::Rename { + previous_path, + previous_state, + .. + }, + TreeStatus::Rename { + previous_path: pp_wt, + previous_state: ps_wt, + .. + }, + ) => { + // The worktree-rename is dominating, but we can combine both + // so there is the indexed version as source, and the one in the worktree + // as destination. + *pp_wt = std::mem::take(previous_path); + *ps_wt = *previous_state; + return Ok(single(index_wt)); } - (TreeStatus::Rename { .. }, TreeStatus::Deletion { .. }) => { - todo!("rename - del") + ( + TreeStatus::Rename { + previous_path, + previous_state, + .. + }, + TreeStatus::Deletion { .. }, + ) => { + // Destination is deleted as well, so what's left is a deletion of the source. + return Ok(single(TreeChange { + path: std::mem::take(previous_path), + status: TreeStatus::Deletion { + previous_state: *previous_state, + }, + })); } - (TreeStatus::Rename { .. }, TreeStatus::Addition { .. }) => { - todo!("rename-add") + ( + TreeStatus::Rename { + state: state_index, .. + }, + TreeStatus::Addition { + state: state_wt, .. + }, + ) => { + return Ok([ + Some(TreeChange { + path: tree_index.path, + status: TreeStatus::Addition { + state: *state_index, + // It's untracked as we know the destination isn't in the tree yet. + // It's just in the index, which to us doesn't exist. + is_untracked: true, + }, + }), + Some(TreeChange { + path: index_wt.path, + status: TreeStatus::Addition { + state: *state_wt, + // In theory, this should be considered tracked even though it's object file isn't + // in the index (but in the tree) as this is the source of the rename. + // However, doing so would trip up diffing code that uses this flag to know where to + // read the initial state of a file from. + is_untracked: true, + }, + }), + ]); } }; @@ -611,9 +692,9 @@ fn merge_changes( path_check, )?; Ok(if current == previous { - None + [None, None] } else { - Some(merged) + single(merged) }) } @@ -724,7 +805,7 @@ impl TreeChange { let mut diff_filter = crate::unified_diff::filter_from_state( repo, self.status.state(), - gix::diff::blob::pipeline::Mode::ToGitUnlessBinaryToTextIsPresent, + UnifiedDiff::CONVERSION_MODE, )?; self.unified_diff_with_filter(repo, context_lines, &mut diff_filter) } diff --git a/crates/but-core/tests/core/diff/worktree_changes.rs b/crates/but-core/tests/core/diff/worktree_changes.rs index a6ae85c843..711d5d669e 100644 --- a/crates/but-core/tests/core/diff/worktree_changes.rs +++ b/crates/but-core/tests/core/diff/worktree_changes.rs @@ -1445,7 +1445,6 @@ fn modified_in_index_and_worktree_mod_del() -> Result<()> { } #[test] -#[ignore = "TBD later"] fn modified_in_index_and_worktree_rename_mod() -> Result<()> { let repo = repo("modified-in-index-and-worktree-rename-mod")?; let actual = diff::worktree_changes(&repo)?; @@ -1453,10 +1452,11 @@ fn modified_in_index_and_worktree_rename_mod() -> Result<()> { WorktreeChanges { changes: [ TreeChange { - path: "dual-modified", - status: Modification { + path: "file-renamed", + status: Rename { + previous_path: "file", previous_state: ChangeState { - id: Sha1(8ea0713f9d637081cc0098035465c365c0c32949), + id: Sha1(e79c5e8f964493290a409888d5413a737e8e5dd5), kind: Blob, }, state: ChangeState { @@ -1469,7 +1469,7 @@ fn modified_in_index_and_worktree_rename_mod() -> Result<()> { ], ignored_changes: [ IgnoredWorktreeChange { - path: "dual-modified", + path: "file-renamed", status: TreeIndex, }, ], @@ -1480,16 +1480,14 @@ fn modified_in_index_and_worktree_rename_mod() -> Result<()> { unreachable!("need hunks") }; insta::assert_snapshot!(hunks[0].diff, @r" - @@ -1,2 +1,3 @@ + @@ -1,1 +1,2 @@ initial - change - +second-change + +wt-change "); Ok(()) } #[test] -#[ignore = "TBD later"] fn modified_in_index_and_worktree_rename_rename() -> Result<()> { let repo = repo("modified-in-index-and-worktree-rename-rename")?; let actual = diff::worktree_changes(&repo)?; @@ -1497,10 +1495,11 @@ fn modified_in_index_and_worktree_rename_rename() -> Result<()> { WorktreeChanges { changes: [ TreeChange { - path: "dual-modified", - status: Modification { + path: "file-renamed-in-wt", + status: Rename { + previous_path: "file", previous_state: ChangeState { - id: Sha1(8ea0713f9d637081cc0098035465c365c0c32949), + id: Sha1(e79c5e8f964493290a409888d5413a737e8e5dd5), kind: Blob, }, state: ChangeState { @@ -1513,7 +1512,7 @@ fn modified_in_index_and_worktree_rename_rename() -> Result<()> { ], ignored_changes: [ IgnoredWorktreeChange { - path: "dual-modified", + path: "file-renamed-in-index", status: TreeIndex, }, ], @@ -1523,17 +1522,15 @@ fn modified_in_index_and_worktree_rename_rename() -> Result<()> { let [UnifiedDiff::Patch { ref hunks }] = unified_diffs(actual, &repo)?[..] else { unreachable!("need hunks") }; - insta::assert_snapshot!(hunks[0].diff, @r" - @@ -1,2 +1,3 @@ - initial - change - +second-change - "); + assert_eq!( + hunks.len(), + 0, + "This is a rename without any additional change (but still a rename" + ); Ok(()) } #[test] -#[ignore = "TBD later"] fn modified_in_index_and_worktree_rename_del() -> Result<()> { let repo = repo("modified-in-index-and-worktree-rename-del")?; let actual = diff::worktree_changes(&repo)?; @@ -1541,23 +1538,18 @@ fn modified_in_index_and_worktree_rename_del() -> Result<()> { WorktreeChanges { changes: [ TreeChange { - path: "dual-modified", - status: Modification { + path: "file", + status: Deletion { previous_state: ChangeState { - id: Sha1(8ea0713f9d637081cc0098035465c365c0c32949), - kind: Blob, - }, - state: ChangeState { - id: Sha1(0000000000000000000000000000000000000000), + id: Sha1(e79c5e8f964493290a409888d5413a737e8e5dd5), kind: Blob, }, - flags: None, }, }, ], ignored_changes: [ IgnoredWorktreeChange { - path: "dual-modified", + path: "file-renamed-in-index", status: TreeIndex, }, ], @@ -1568,10 +1560,8 @@ fn modified_in_index_and_worktree_rename_del() -> Result<()> { unreachable!("need hunks") }; insta::assert_snapshot!(hunks[0].diff, @r" - @@ -1,2 +1,3 @@ - initial - change - +second-change + @@ -1,1 +1,0 @@ + -initial "); Ok(()) } @@ -1621,7 +1611,6 @@ fn modified_in_index_and_worktree_mod_rename() -> Result<()> { } #[test] -#[ignore = "TBD later"] fn modified_in_index_and_worktree_rename_add() -> Result<()> { let repo = repo("modified-in-index-and-worktree-rename-add")?; let actual = diff::worktree_changes(&repo)?; @@ -1630,17 +1619,22 @@ fn modified_in_index_and_worktree_rename_add() -> Result<()> { changes: [ TreeChange { path: "file-renamed-in-index", - status: Rename { - previous_path: "file", - previous_state: ChangeState { - id: Sha1(0000000000000000000000000000000000000000), + status: Addition { + state: ChangeState { + id: Sha1(e79c5e8f964493290a409888d5413a737e8e5dd5), kind: Blob, }, + is_untracked: true, + }, + }, + TreeChange { + path: "file", + status: Addition { state: ChangeState { - id: Sha1(e79c5e8f964493290a409888d5413a737e8e5dd5), + id: Sha1(0000000000000000000000000000000000000000), kind: Blob, }, - flags: None, + is_untracked: true, }, }, ], @@ -1649,17 +1643,30 @@ fn modified_in_index_and_worktree_rename_add() -> Result<()> { path: "file-renamed-in-index", status: TreeIndex, }, + IgnoredWorktreeChange { + path: "file", + status: TreeIndex, + }, ], } "#); - let [UnifiedDiff::Patch { ref hunks }] = unified_diffs(actual, &repo)?[..] else { + let [ + UnifiedDiff::Patch { hunks: ref hunks1 }, + UnifiedDiff::Patch { hunks: ref hunks2 }, + ] = unified_diffs(actual, &repo)?[..] + else { unreachable!("need hunks") }; - insta::assert_snapshot!(hunks[0].diff, @r" + insta::assert_snapshot!(hunks1[0].diff, @r" @@ -1,0 +1,1 @@ +initial "); + insta::assert_snapshot!(hunks2[0].diff, @r" + @@ -1,0 +1,2 @@ + +initial + +wt-change + "); Ok(()) } diff --git a/crates/but-workspace/src/commit_engine/hunks.rs b/crates/but-workspace/src/commit_engine/hunks.rs new file mode 100644 index 0000000000..5b92cbe780 --- /dev/null +++ b/crates/but-workspace/src/commit_engine/hunks.rs @@ -0,0 +1,62 @@ +use crate::commit_engine::HunkHeader; +use anyhow::Context; +use bstr::{BStr, BString, ByteSlice}; + +/// Given an `old_image` and a `new_image`, along with `hunks` that represent selections in `new_image`, apply these +/// hunks to `old_image` and return the newly constructed image. +/// This works like an overlay where selections from `new_image` are inserted into `new_image` with `hunks` as Windows. +/// +/// Note that we assume that both images are human-readable because we assume lines to be present, +/// either with Windows or Unix newlines, and we assume that the hunks match up with these lines. +/// This constraint means that the tokens used for diffing are the same lines. +pub fn apply_hunks( + old_image: &BStr, + new_image: &BStr, + hunks: &[HunkHeader], +) -> anyhow::Result { + let mut worktree_base_cursor = 1; /* 1-based counting */ + let mut old_iter = old_image.lines_with_terminator(); + let mut worktree_actual_cursor = 1; /* 1-based counting */ + let mut new_iter = new_image.lines_with_terminator(); + let mut result_image: BString = Vec::with_capacity(old_image.len().max(new_image.len())).into(); + + // To each selected hunk, put the old-lines into a buffer. + // Skip over the old hunk in old hunk in old lines. + // Skip all new lines till the beginning of the new hunk. + // Write the new hunk. + // Repeat for each hunk, and write all remaining old lines. + for selected_hunk in hunks { + let catchup_base_lines = old_iter.by_ref().take( + (selected_hunk.old_start as usize) + .checked_sub(worktree_base_cursor) + .context("hunks must be in order from top to bottom of the file")?, + ); + for line in catchup_base_lines { + result_image.extend_from_slice(line); + } + let _consume_old_hunk_to_replace_with_new = old_iter + .by_ref() + .take(selected_hunk.old_lines as usize) + .count(); + worktree_base_cursor += selected_hunk.old_lines as usize; + + let new_hunk_lines = new_iter + .by_ref() + .skip( + (selected_hunk.new_start as usize) + .checked_sub(worktree_actual_cursor) + .context("hunks for new lines must be in order")?, + ) + .take(selected_hunk.new_lines as usize); + + for line in new_hunk_lines { + result_image.extend_from_slice(line); + } + worktree_actual_cursor += selected_hunk.new_lines as usize; + } + + for line in old_iter { + result_image.extend_from_slice(line); + } + Ok(result_image) +} diff --git a/crates/but-workspace/src/commit_engine/mod.rs b/crates/but-workspace/src/commit_engine/mod.rs index 5e6e8022a8..1b14c7945d 100644 --- a/crates/but-workspace/src/commit_engine/mod.rs +++ b/crates/but-workspace/src/commit_engine/mod.rs @@ -1,5 +1,6 @@ //! The machinery used to alter and mutate commits in various ways whilst adjusting descendant commits within a [reference frame](ReferenceFrame). +use crate::commit_engine::reference_frame::InferenceMode; use anyhow::{Context, bail}; use bstr::BString; use but_core::RepositoryExt; @@ -12,8 +13,7 @@ use gix::prelude::ObjectIdExt as _; use gix::refs::transaction::PreviousValue; use serde::{Deserialize, Serialize}; -mod tree; -use crate::commit_engine::reference_frame::InferenceMode; +pub(crate) mod tree; use tree::{CreateTreeOutcome, create_tree}; pub(crate) mod index; @@ -21,6 +21,9 @@ pub(crate) mod index; pub mod reference_frame; mod refs; +mod hunks; +pub use hunks::apply_hunks; + /// Types for use in the frontend with serialization support. pub mod ui; @@ -91,7 +94,7 @@ pub struct DiffSpec { } /// The header of a hunk that represents a change to a file. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct HunkHeader { /// The 1-based line number at which the previous version of the file started. diff --git a/crates/but-workspace/src/commit_engine/tree.rs b/crates/but-workspace/src/commit_engine/tree.rs index d1d47c6332..aa52b0bf89 100644 --- a/crates/but-workspace/src/commit_engine/tree.rs +++ b/crates/but-workspace/src/commit_engine/tree.rs @@ -1,12 +1,15 @@ -use crate::commit_engine::{Destination, DiffSpec, HunkHeader, MoveSourceCommit, RejectionReason}; -use anyhow::{Context, bail}; -use bstr::{BString, ByteSlice}; +use crate::commit_engine::{ + Destination, DiffSpec, HunkHeader, MoveSourceCommit, RejectionReason, apply_hunks, +}; +use anyhow::bail; +use bstr::{BStr, ByteSlice}; use but_core::{RepositoryExt, UnifiedDiff}; use gix::filter::plumbing::pipeline::convert::ToGitOutcome; use gix::merge::tree::TreatAsUnresolved; use gix::object::tree::EntryKind; use gix::prelude::ObjectIdExt; use std::io::Read; +use std::path::Path; /// Additional information about the outcome of a [`create_tree()`] call. #[derive(Debug)] @@ -220,6 +223,7 @@ fn apply_worktree_changes<'repo>( gix::diff::blob::pipeline::Mode::ToGitUnlessBinaryToTextIsPresent, "BUG: if this changes, the uses of worktree filters need a review" ); + // TODO(perf): avoid computing the unified diff here, we only need hunks with, usually with zero context. let UnifiedDiff::Patch { hunks } = worktree_change.unified_diff_with_filter(repo, context_lines, &mut diff_filter)? else { @@ -279,34 +283,14 @@ fn apply_worktree_changes<'repo>( continue; }; - current_worktree.clear(); - let to_git = pipeline.convert_to_git( - std::fs::File::open(path)?, - &gix::path::from_bstr(base_rela_path), + worktree_file_to_git_in_buf( + &mut current_worktree, + base_rela_path, + &path, + &mut pipeline, &index, )?; - match to_git { - ToGitOutcome::Unchanged(mut file) => { - file.read_to_end(&mut current_worktree)?; - } - ToGitOutcome::Process(mut stream) => { - stream.read_to_end(&mut current_worktree)?; - } - ToGitOutcome::Buffer(buf) => current_worktree.extend_from_slice(buf), - }; - let worktree_hunks: Vec = hunks.into_iter().map(Into::into).collect(); - let mut worktree_base_cursor = 1; /* 1-based counting */ - let mut old_iter = worktree_base.lines_with_terminator(); - let mut worktree_actual_cursor = 1; /* 1-based counting */ - let mut new_iter = current_worktree.lines_with_terminator(); - let mut base_with_patches: BString = Vec::with_capacity(md.len().try_into()?).into(); - - // To each selected hunk, put the old-lines into a buffer. - // Skip over the old hunk in old hunk in old lines. - // Skip all new lines till the beginning of the new hunk. - // Write the new hunk. - // Repeat for each hunk, and write all remaining old lines. for selected_hunk in &change_request.hunk_headers { if !worktree_hunks.contains(selected_hunk) || has_zero_based_line_numbers(selected_hunk) @@ -315,39 +299,12 @@ fn apply_worktree_changes<'repo>( // TODO: only skip this one hunk, but collect skipped hunks into a new err-spec. continue 'each_change; } - let catchup_base_lines = old_iter.by_ref().take( - (selected_hunk.old_start as usize) - .checked_sub(worktree_base_cursor) - .context("hunks must be in order from top to bottom of the file")?, - ); - for line in catchup_base_lines { - base_with_patches.extend_from_slice(line); - } - let _consume_old_hunk_to_replace_with_new = old_iter - .by_ref() - .take(selected_hunk.old_lines as usize) - .count(); - worktree_base_cursor += selected_hunk.old_lines as usize; - - let new_hunk_lines = new_iter - .by_ref() - .skip( - (selected_hunk.new_start as usize) - .checked_sub(worktree_actual_cursor) - .context("hunks for new lines must be in order")?, - ) - .take(selected_hunk.new_lines as usize); - - for line in new_hunk_lines { - base_with_patches.extend_from_slice(line); - } - worktree_actual_cursor += selected_hunk.new_lines as usize; } - - for line in old_iter { - base_with_patches.extend_from_slice(line); - } - + let base_with_patches = apply_hunks( + worktree_base.as_bstr(), + current_worktree.as_bstr(), + &change_request.hunk_headers, + )?; let blob_with_selected_patches = repo.write_blob(base_with_patches.as_slice())?; base_tree_editor.upsert( change_request.path.as_bstr(), @@ -363,3 +320,28 @@ fn apply_worktree_changes<'repo>( let maybe_new_tree = (actual_base_tree != altered_base_tree_id).then_some(altered_base_tree_id); Ok((maybe_new_tree, actual_base_tree)) } + +pub(crate) fn worktree_file_to_git_in_buf( + buf: &mut Vec, + rela_path: &BStr, + path: &Path, + pipeline: &mut gix::filter::Pipeline<'_>, + index: &gix::index::State, +) -> anyhow::Result<()> { + buf.clear(); + let to_git = pipeline.convert_to_git( + std::fs::File::open(path)?, + &gix::path::from_bstr(rela_path), + index, + )?; + match to_git { + ToGitOutcome::Unchanged(mut file) => { + file.read_to_end(buf)?; + } + ToGitOutcome::Process(mut stream) => { + stream.read_to_end(buf)?; + } + ToGitOutcome::Buffer(buf2) => buf.extend_from_slice(buf2), + }; + Ok(()) +} diff --git a/crates/but-workspace/src/discard/function.rs b/crates/but-workspace/src/discard/function.rs index 58757b5384..fc46cbd480 100644 --- a/crates/but-workspace/src/discard/function.rs +++ b/crates/but-workspace/src/discard/function.rs @@ -1,14 +1,18 @@ -use crate::discard::{DiscardSpec, file}; -use anyhow::Context; +use crate::discard::{DiscardSpec, file, hunk}; +use anyhow::{Context, bail}; use bstr::ByteSlice; use but_core::{ChangeState, TreeStatus}; /// Discard the given `changes` in the worktree of `repo`. If a change could not be matched with an actual worktree change, for -/// instance due to a race, that's not an error, instead it will be returned in the result Vec. +/// instance due to a race, that's not an error, instead it will be returned in the result Vec, along with all hunks that couldn't +/// be matched. /// The returned Vec is typically empty, meaning that all `changes` could be discarded. /// +/// `context_lines` is the amount of context lines we should assume when obtaining hunks of worktree changes to match against +/// the ones we have specified in the hunks contained within `changes`. +/// /// Discarding a change is really more of an 'undo' of a change as it will restore the previous state to the desired extent - Git -/// doesn't have a notion of this. +/// doesn't have a notion of this on a whole-file basis. /// /// Each of the `changes` will be matched against actual worktree changes to make this operation as safe as possible, after all, it /// discards changes without recovery. @@ -19,6 +23,7 @@ use but_core::{ChangeState, TreeStatus}; pub fn discard_workspace_changes( repo: &gix::Repository, changes: impl IntoIterator, + context_lines: u32, ) -> anyhow::Result> { let wt_changes = but_core::diff::worktree_changes(repo)?; let mut dropped = Vec::new(); @@ -30,7 +35,7 @@ pub fn discard_workspace_changes( let mut path_check = gix::status::plumbing::SymlinkCheck::new( repo.workdir().context("non-bare repository")?.into(), ); - for spec in changes { + for mut spec in changes { let Some(wt_change) = wt_changes.changes.iter().find(|c| { c.path == spec.path && c.previous_path() == spec.previous_path.as_ref().map(|p| p.as_bstr()) @@ -115,7 +120,43 @@ pub fn discard_workspace_changes( } } } else { - todo!("hunk-based undo") + match wt_change.status { + TreeStatus::Addition { .. } | TreeStatus::Deletion { .. } => { + bail!( + "Deletions or additions aren't well-defined for hunk-based operations - use the whole-file mode instead: '{}'", + wt_change.path + ) + } + TreeStatus::Modification { + previous_state, + flags, + .. + } + | TreeStatus::Rename { + previous_state, + flags, + .. + } => { + if flags.is_some_and(|f| f.is_typechange()) { + bail!( + "Type-changed items can't be discard by hunks - use the whole-file mode isntead" + ) + } + hunk::restore_state_to_worktree( + wt_change, + previous_state, + &mut spec.hunk_headers, + &mut path_check, + &mut pipeline, + &index, + context_lines, + )?; + if !spec.hunk_headers.is_empty() { + dropped.push(spec); + continue; + } + } + } } } diff --git a/crates/but-workspace/src/discard/hunk.rs b/crates/but-workspace/src/discard/hunk.rs new file mode 100644 index 0000000000..edf07ebef0 --- /dev/null +++ b/crates/but-workspace/src/discard/hunk.rs @@ -0,0 +1,93 @@ +use crate::commit_engine::tree::worktree_file_to_git_in_buf; +use crate::commit_engine::{HunkHeader, apply_hunks}; +use anyhow::bail; +use bstr::ByteSlice; +use but_core::{ChangeState, TreeChange, UnifiedDiff}; +use gix::filter::plumbing::driver::apply::{Delay, MaybeDelayed}; +use gix::filter::plumbing::pipeline::convert::ToWorktreeOutcome; +use gix::prelude::ObjectIdExt; + +/// Discard `hunks_to_discard` in the resource at `wt_change`, whose previous version is `previous_state` and is expected to +/// be tracked and readable from the object database. We will always read what's currently on disk as the current version +/// and overwrite it. +/// +/// The general idea is to rebuild the file, but without the `hunks_to_discard`, write it into the worktree replacing +/// the previous file. `hunks_to_discard` are hunks based on a diff of what's in Git. +/// +/// Note that an index update isn't necessary, as for the purpose of the application it might as well not exist due to the +/// way our worktree-changes function ignores the index entirely. +/// +/// Note that `hunks_to_discard` that weren't present in the actual set of worktree changes will remain, so when everything +/// worked `hunks_to_discard` will be empty. +pub fn restore_state_to_worktree( + wt_change: &TreeChange, + previous_state: ChangeState, + hunks_to_discard: &mut Vec, + path_check: &mut gix::status::plumbing::SymlinkCheck, + pipeline: &mut gix::filter::Pipeline<'_>, + index: &gix::index::State, + context_lines: u32, +) -> anyhow::Result<()> { + let repo = pipeline.repo; + let state_in_worktree = ChangeState { + id: repo.object_hash().null(), + kind: previous_state.kind, + }; + let mut diff_filter = but_core::unified_diff::filter_from_state( + repo, + Some(state_in_worktree), + UnifiedDiff::CONVERSION_MODE, + )?; + let UnifiedDiff::Patch { hunks } = + wt_change.unified_diff_with_filter(repo, context_lines, &mut diff_filter)? + else { + bail!("Couldn't obtain diff for worktree changes.") + }; + + let prev_len = hunks_to_discard.len(); + let hunks_to_keep: Vec = hunks + .into_iter() + .map(Into::into) + .filter(|hunk| { + match hunks_to_discard + .iter() + .enumerate() + .find_map(|(idx, hunk_to_discard)| (hunk_to_discard == hunk).then_some(idx)) + { + None => true, + Some(idx_to_remove) => { + hunks_to_discard.remove(idx_to_remove); + false + } + } + }) + .collect(); + if prev_len == hunks_to_discard.len() { + // We want to keep everything, so nothing has to be done. + return Ok(()); + } + + let old = previous_state.id.attach(repo).object()?.detach().data; + let mut new = repo.empty_reusable_buffer(); + let rela_path = wt_change.path.as_bstr(); + let worktree_path = path_check.verified_path_allow_nonexisting(rela_path)?; + worktree_file_to_git_in_buf(&mut new, rela_path, &worktree_path, pipeline, index)?; + + let base_with_patches = apply_hunks(old.as_bstr(), new.as_bstr(), &hunks_to_keep)?; + + let to_worktree = pipeline.convert_to_worktree(&base_with_patches, rela_path, Delay::Forbid)?; + match to_worktree { + ToWorktreeOutcome::Unchanged(buf) => { + std::fs::write(&worktree_path, buf)?; + } + ToWorktreeOutcome::Buffer(buf) => { + std::fs::write(&worktree_path, buf)?; + } + ToWorktreeOutcome::Process(MaybeDelayed::Immediate(mut stream)) => { + let mut file = std::fs::File::create(&worktree_path)?; + std::io::copy(&mut stream, &mut file)?; + } + ToWorktreeOutcome::Process(MaybeDelayed::Delayed(_)) => unreachable!("disabled"), + } + Ok(()) +} diff --git a/crates/but-workspace/src/discard/mod.rs b/crates/but-workspace/src/discard/mod.rs index 2162445274..dd76460847 100644 --- a/crates/but-workspace/src/discard/mod.rs +++ b/crates/but-workspace/src/discard/mod.rs @@ -2,7 +2,7 @@ use crate::commit_engine::DiffSpec; use gix::object::tree::EntryKind; -use std::ops::Deref; +use std::ops::{Deref, DerefMut}; use std::path::{Path, PathBuf}; /// A specification of what should be discarded, either changes to the whole file, or a portion of it. @@ -24,6 +24,12 @@ impl Deref for DiscardSpec { } } +impl DerefMut for DiscardSpec { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} + impl From for DiffSpec { fn from(value: DiscardSpec) -> Self { value.0 @@ -38,6 +44,7 @@ pub mod ui { } mod file; +mod hunk; #[cfg(unix)] fn locked_resource_at( diff --git a/crates/but-workspace/tests/fixtures/generated-archives/.gitignore b/crates/but-workspace/tests/fixtures/generated-archives/.gitignore index 36f5fbfa0f..457de995ed 100644 --- a/crates/but-workspace/tests/fixtures/generated-archives/.gitignore +++ b/crates/but-workspace/tests/fixtures/generated-archives/.gitignore @@ -21,3 +21,5 @@ /all-file-types-renamed-and-overwriting-existing.tar /move-directory-into-sibling-file.tar /merge-with-two-branches-conflict.tar +/deletion-addition-untracked.tar +/mixed-hunk-modifications.tar \ No newline at end of file diff --git a/crates/but-workspace/tests/fixtures/scenario/deletion-addition-untracked.sh b/crates/but-workspace/tests/fixtures/scenario/deletion-addition-untracked.sh new file mode 100644 index 0000000000..cf243e77f7 --- /dev/null +++ b/crates/but-workspace/tests/fixtures/scenario/deletion-addition-untracked.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash + +### Description +# Provide a deleted file, an added one, and an untracked one. +set -eu -o pipefail + +git init +echo content >to-be-deleted +echo content2 >to-be-deleted-in-index +git add . && git commit -m "init" + +echo new >added-to-index && git add added-to-index +echo untracked >untracked +rm to-be-deleted +git rm to-be-deleted-in-index + diff --git a/crates/but-workspace/tests/fixtures/scenario/mixed-hunk-modifications.sh b/crates/but-workspace/tests/fixtures/scenario/mixed-hunk-modifications.sh new file mode 100644 index 0000000000..3349aa70ea --- /dev/null +++ b/crates/but-workspace/tests/fixtures/scenario/mixed-hunk-modifications.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash + +### Description +# A single tracked file, modified in the workspace to have: +# - added lines at the beginning +# - deleted lines at the end +# - modified lines, added lines, and deleted lines directly after one another in the middle. +set -eu -o pipefail + +git init +seq 5 18 >file +seq 5 18 >file-in-index +seq 5 18 >file-to-be-renamed +seq 5 18 >file-to-be-renamed-in-index +git add . && git commit -m "init" + +cat <file +1 +2 +3 +4 +5 +6-7 +8 +9 +ten +eleven +12 +20 +21 +22 +15 +16 +EOF + +cp file file-in-index && git add file-in-index + + +seq 2 18 >file-to-be-renamed && mv file-to-be-renamed file-renamed +cp file file-to-be-renamed-in-index && git mv file-to-be-renamed-in-index file-renamed-in-index + + diff --git a/crates/but-workspace/tests/workspace/commit_engine/new_commit.rs b/crates/but-workspace/tests/workspace/commit_engine/new_commit.rs index bf3dcbe7f7..2413f34764 100644 --- a/crates/but-workspace/tests/workspace/commit_engine/new_commit.rs +++ b/crates/but-workspace/tests/workspace/commit_engine/new_commit.rs @@ -141,6 +141,7 @@ fn from_first_commit_all_file_types_changed() -> anyhow::Result<()> { stack_segment: None, }, )?; + assert_eq!(outcome.rejected_specs, []); let tree = visualize_tree(&repo, &outcome)?; insta::assert_snapshot!(tree, @r#" diff --git a/crates/but-workspace/tests/workspace/discard/file.rs b/crates/but-workspace/tests/workspace/discard/file.rs index b81ecabd39..e8a1aae359 100644 --- a/crates/but-workspace/tests/workspace/discard/file.rs +++ b/crates/but-workspace/tests/workspace/discard/file.rs @@ -1,4 +1,4 @@ -use crate::utils::{visualize_index, writable_scenario, writable_scenario_slow}; +use crate::utils::{CONTEXT_LINES, visualize_index, writable_scenario, writable_scenario_slow}; use but_testsupport::{CommandExt, git, git_status, visualize_disk_tree_skip_dot_git}; use but_workspace::discard_workspace_changes; use util::{file_to_spec, renamed_file_to_spec, worktree_changes_to_discard_specs}; @@ -12,7 +12,11 @@ fn all_file_types_from_unborn() -> anyhow::Result<()> { ?? untracked-exe "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -29,7 +33,11 @@ A untracked A untracked-exe "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -48,7 +56,11 @@ D link D submodule "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -90,7 +102,11 @@ fn replace_dir_with_file_discard_all_in_order_in_worktree() -> anyhow::Result<() ?? dir "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -137,7 +153,11 @@ D dir/link D dir/submodule "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -183,7 +203,7 @@ fn replace_dir_with_file_discard_just_the_file_in_worktree() -> anyhow::Result<( ?? dir "); - let dropped = discard_workspace_changes(&repo, Some(file_to_spec("dir")))?; + let dropped = discard_workspace_changes(&repo, Some(file_to_spec("dir")), CONTEXT_LINES)?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -224,7 +244,7 @@ fn conflicts_are_invisible() -> anyhow::Result<()> { 100644:e33f5e9 file "); - let dropped = discard_workspace_changes(&repo, Some(file_to_spec("file")))?; + let dropped = discard_workspace_changes(&repo, Some(file_to_spec("file")), CONTEXT_LINES)?; assert_eq!( dropped, vec![file_to_spec("file")], @@ -259,7 +279,7 @@ D dir/link D dir/submodule "); - let dropped = discard_workspace_changes(&repo, Some(file_to_spec("dir")))?; + let dropped = discard_workspace_changes(&repo, Some(file_to_spec("dir")), CONTEXT_LINES)?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -307,7 +327,11 @@ M soon-not-executable └── soon-not-executable:100644 "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -347,7 +371,11 @@ M soon-not-executable └── soon-not-executable:100644 "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -395,7 +423,11 @@ M submodule 160000:a047f81 submodule "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); // The embedded repository we don't currently see due to a `gix` shortcoming - it ignores embedded repos @@ -438,7 +470,11 @@ MM submodule 160000:6d5e0a5 submodule "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); // `gix status` is able to see the 'embedded-repository' if it's in the index, and we can reset it as well. @@ -474,7 +510,11 @@ fn all_file_types_renamed_and_modified_in_worktree() -> anyhow::Result<()> { └── link-renamed:120755 "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -520,7 +560,11 @@ A link-renamed └── link-renamed:120755 "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -579,7 +623,11 @@ fn all_file_types_renamed_overwriting_existing_and_modified_in_worktree() -> any └── to-be-overwritten:100644 "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -645,7 +693,11 @@ M to-be-overwritten └── to-be-overwritten:100644 "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -714,6 +766,7 @@ fn all_file_types_renamed_overwriting_existing_and_modified_in_worktree_discard_ renamed_file_to_spec("executable", "dir-to-be-file"), renamed_file_to_spec("file", "file-to-be-dir/file"), ], + CONTEXT_LINES, )?; assert_eq!(dropped, []); @@ -741,7 +794,11 @@ D file-to-be-dir // Try again with what remains, something that the user will likely do as well, not really knowing // why that is. - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @r""); @@ -797,7 +854,11 @@ D a/sibling "); // This naturally starts with `a/sibling` - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -845,6 +906,7 @@ D a/sibling renamed_file_to_spec("a/b/link", "a/sibling/link"), file_to_spec("a/sibling"), ], + CONTEXT_LINES, )?; assert!(dropped.is_empty()); @@ -880,7 +942,11 @@ R a/b/file -> a/sibling/file R a/b/link -> a/sibling/link "); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); @@ -916,7 +982,11 @@ D submodule "); git(&repo).args(["add", "."]).run(); - let dropped = discard_workspace_changes(&repo, worktree_changes_to_discard_specs(&repo))?; + let dropped = discard_workspace_changes( + &repo, + worktree_changes_to_discard_specs(&repo), + CONTEXT_LINES, + )?; assert!(dropped.is_empty()); insta::assert_snapshot!(git_status(&repo)?, @""); diff --git a/crates/but-workspace/tests/workspace/discard/hunk.rs b/crates/but-workspace/tests/workspace/discard/hunk.rs index 1b0e3d6642..646c81c1ce 100644 --- a/crates/but-workspace/tests/workspace/discard/hunk.rs +++ b/crates/but-workspace/tests/workspace/discard/hunk.rs @@ -1,17 +1,179 @@ +use crate::discard::hunk::util::hunk_header; +use crate::utils::{ + CONTEXT_LINES, read_only_in_memory_scenario, to_change_specs_all_hunks, visualize_index, + writable_scenario, +}; +use but_testsupport::git_status; +use but_workspace::commit_engine::DiffSpec; +use but_workspace::discard_workspace_changes; + #[test] -#[ignore = "TBD"] fn non_modifications_trigger_error() -> anyhow::Result<()> { + let repo = read_only_in_memory_scenario("deletion-addition-untracked")?; + insta::assert_snapshot!(git_status(&repo)?, @r" + A added-to-index + D to-be-deleted + D to-be-deleted-in-index + ?? untracked + "); + + let add_single_line = hunk_header("-1,0", "+1,1"); + let remove_single_line = hunk_header("-1,1", "+1,0"); + for (file_name, hunk) in [ + ("untracked", add_single_line), + ("added-to-index", add_single_line), + ("to-be-deleted", remove_single_line), + ("to-be-deleted-in-index", remove_single_line), + ] { + let err = discard_workspace_changes( + &repo, + Some( + DiffSpec { + previous_path: None, + path: file_name.into(), + hunk_headers: vec![hunk], + } + .into(), + ), + CONTEXT_LINES, + ) + .unwrap_err(); + assert!( + err.to_string().starts_with( + "Deletions or additions aren't well-defined for hunk-based operations - use the whole-file mode instead" + ), + ); + } Ok(()) } #[test] -#[ignore = "TBD"] -fn deletion_modification_addition_mixed_in_worktree() -> anyhow::Result<()> { +fn deletion_modification_addition_of_hunks_mixed_discard_all_in_workspace() -> anyhow::Result<()> { + let (repo, _tmp) = writable_scenario("mixed-hunk-modifications"); + // Note that one of these renames can't be detected by Git but is visible to us. + insta::assert_snapshot!(git_status(&repo)?, @r" + M file + M file-in-index + RM file-to-be-renamed-in-index -> file-renamed-in-index + D file-to-be-renamed + ?? file-renamed + "); + + // Show that we detect renames correctly, despite the rename + modification. + let wt_changes = but_core::diff::worktree_changes(&repo)?; + insta::assert_debug_snapshot!(wt_changes.changes, @r#" + [ + TreeChange { + path: "file", + status: Modification { + previous_state: ChangeState { + id: Sha1(3d3b36f021391fa57312d7dfd1ad8cf5a13dca6d), + kind: Blob, + }, + state: ChangeState { + id: Sha1(0000000000000000000000000000000000000000), + kind: Blob, + }, + flags: None, + }, + }, + TreeChange { + path: "file-in-index", + status: Modification { + previous_state: ChangeState { + id: Sha1(3d3b36f021391fa57312d7dfd1ad8cf5a13dca6d), + kind: Blob, + }, + state: ChangeState { + id: Sha1(cb89473a55c3443b5567e990e2a0293895c91a4a), + kind: Blob, + }, + flags: None, + }, + }, + TreeChange { + path: "file-renamed", + status: Rename { + previous_path: "file-to-be-renamed", + previous_state: ChangeState { + id: Sha1(3d3b36f021391fa57312d7dfd1ad8cf5a13dca6d), + kind: Blob, + }, + state: ChangeState { + id: Sha1(0000000000000000000000000000000000000000), + kind: Blob, + }, + flags: None, + }, + }, + TreeChange { + path: "file-renamed-in-index", + status: Rename { + previous_path: "file-to-be-renamed-in-index", + previous_state: ChangeState { + id: Sha1(3d3b36f021391fa57312d7dfd1ad8cf5a13dca6d), + kind: Blob, + }, + state: ChangeState { + id: Sha1(0000000000000000000000000000000000000000), + kind: Blob, + }, + flags: None, + }, + }, + ] + "#); + + let specs = to_change_specs_all_hunks(&repo, wt_changes)?; + let dropped = + discard_workspace_changes(&repo, specs.into_iter().map(Into::into), CONTEXT_LINES)?; + assert!(dropped.is_empty()); + + // TODO: this most definitely isn't correct, figure out what's going on. + // Notably, discarding all hunks leaves the renamed file in place, but without modifications. + insta::assert_snapshot!(git_status(&repo)?, @r" + MM file-in-index + R file-to-be-renamed-in-index -> file-renamed-in-index + D file-to-be-renamed + ?? file-renamed + "); + // The index still only holds what was in the index before, but is representing the changed worktree. + insta::assert_snapshot!(visualize_index(&**repo.index()?), @r" + 100644:3d3b36f file + 100644:cb89473 file-in-index + 100644:3d3b36f file-renamed-in-index + 100644:3d3b36f file-to-be-renamed + "); + + // TODO: content checks. + Ok(()) } -#[test] -#[ignore = "TBD"] -fn deletion_modification_addition_mixed_in_index() -> anyhow::Result<()> { - Ok(()) +mod util { + use but_workspace::commit_engine::HunkHeader; + + /// Choose a slightly more obvious, yet easy to type syntax than a function with 4 parameters. + pub fn hunk_header(old: &str, new: &str) -> HunkHeader { + fn parse_header(hunk_info: &str) -> (u32, u32) { + let hunk_info = hunk_info.trim_start_matches(['-', '+'].as_slice()); + let parts: Vec<_> = hunk_info.split(',').collect(); + let start = parts[0].parse().unwrap(); + let lines = if parts.len() > 1 { + parts[1].parse().unwrap() + } else { + 1 + }; + (start, lines) + } + + let (old_start, old_lines) = parse_header(old); + let (new_start, new_lines) = parse_header(new); + HunkHeader { + old_start, + old_lines, + new_start, + new_lines, + } + } } diff --git a/crates/gitbutler-tauri/src/workspace.rs b/crates/gitbutler-tauri/src/workspace.rs index c4fd97bcaf..77a02a129b 100644 --- a/crates/gitbutler-tauri/src/workspace.rs +++ b/crates/gitbutler-tauri/src/workspace.rs @@ -206,9 +206,10 @@ pub fn amend_commit_from_worktree_changes( /// /// Returns the `worktree_changes` that couldn't be applied, #[tauri::command(async)] -#[instrument(skip(projects), err(Debug))] +#[instrument(skip(projects, settings), err(Debug))] pub fn discard_worktree_changes( projects: State<'_, projects::Controller>, + settings: State<'_, AppSettingsWithDiskSync>, project_id: ProjectId, worktree_changes: Vec, ) -> Result, Error> { @@ -223,6 +224,7 @@ pub fn discard_worktree_changes( change, )) }), + settings.get()?.context_lines, )?; Ok(refused .into_iter()