From f5e12071977e66edcfd010887b20b4a1938e9a48 Mon Sep 17 00:00:00 2001 From: Mattias Granlund Date: Thu, 26 Feb 2026 11:24:10 +0100 Subject: [PATCH] Handle hunk-based operations for file additions/removals Allow hunk-based uncommit operations to work on files that were added or removed by treating a missing "before" entry as an empty blob and None state. This lets unified-patch computation proceed for additions, applies hunks against an empty before state, and correctly handles the case where removing all hunks from an added file should delete the file from the tree. Also preserve the correct file mode when the resulting contents match the before or after state and clean up builder upsert/remove logic accordingly. --- .../create_tree_without_diff.rs | 58 ++++++++++++------- 1 file changed, 37 insertions(+), 21 deletions(-) diff --git a/crates/but-workspace/src/tree_manipulation/create_tree_without_diff.rs b/crates/but-workspace/src/tree_manipulation/create_tree_without_diff.rs index 6f241641d3b..d57f8a5d5b8 100644 --- a/crates/but-workspace/src/tree_manipulation/create_tree_without_diff.rs +++ b/crates/but-workspace/src/tree_manipulation/create_tree_without_diff.rs @@ -131,11 +131,21 @@ pub fn create_tree_without_diff( if change.hunk_headers.is_empty() { revert_file_to_before_state(&before_entry, &mut builder, &change)?; } else { - let Some(before_entry) = before_entry else { - anyhow::bail!( - "Deletions or additions aren't well-defined for hunk-based operations - use the whole-file mode instead" - ); - }; + // For file additions (no before_entry), use None/empty as + // the "before" state so the hunk subtraction still works. + let (before_state, before_data): (Option, Vec) = + if let Some(ref be) = before_entry { + let blob = be.object()?.into_blob(); + ( + Some(ChangeState { + id: be.id().detach(), + kind: be.mode().kind(), + }), + blob.data.clone(), + ) + } else { + (None, Vec::new()) + }; let diff = but_core::UnifiedPatch::compute( repository, @@ -145,10 +155,7 @@ pub fn create_tree_without_diff( id: after_entry.id().detach(), kind: after_entry.mode().kind(), }, - ChangeState { - id: before_entry.id().detach(), - kind: before_entry.mode().kind(), - }, + before_state, context_lines, )? .context( @@ -185,28 +192,37 @@ pub fn create_tree_without_diff( } // TODO: Validate that the hunks correspond with actual changes? - let before_blob = before_entry.object()?.into_blob(); - let new_hunks = new_hunks_after_removals( diff_hunks.into_iter().map(Into::into).collect(), good_hunk_headers, )?; let new_after_contents = but_core::apply_hunks( - before_blob.data.as_bstr(), + before_data.as_bstr(), after_blob.data.as_bstr(), &new_hunks, )?; - let mode = if new_after_contents == before_blob.data { - before_entry.mode().kind() + + if before_entry.is_none() && new_after_contents == before_data { + // All changes removed from a file addition - remove + // the file from the tree entirely. + builder.remove(change.path.as_bstr())?; } else { - after_entry.mode().kind() - }; - let new_after_contents = repository.write_blob(&new_after_contents)?; + let mode = if new_after_contents == before_data { + before_entry + .as_ref() + .expect("before_entry.is_none() case handled above") + .mode() + .kind() + } else { + after_entry.mode().kind() + }; + let new_after_contents = repository.write_blob(&new_after_contents)?; - // Keep the mode of the after state. We _should_ at some - // point introduce the mode specifically as part of the - // DiscardSpec, but for now, we can just use the after state. - builder.upsert(change.path.as_bstr(), mode, new_after_contents)?; + // Keep the mode of the after state. We _should_ at some + // point introduce the mode specifically as part of the + // DiscardSpec, but for now, we can just use the after state. + builder.upsert(change.path.as_bstr(), mode, new_after_contents)?; + } } } _ => {