Support for hunk-selections when committing and amending

Author: Byron

crates/but-workspace/src/commit_engine/mod.rs

@@ -205,7 +205,7 @@ pub enum RejectionReason {
     /// This can happen if the target tree has an entry that isn't of the same type as the source worktree changes.
     UnsupportedTreeEntry,
     /// The DiffSpec points to an actual change, or a subset of that change using a file path and optionally hunks into that file.
-    /// However, the actual change wasn't found or didn't match up in turn of hunks.
+    /// However, at least one hunk was not fully contained..
     MissingDiffSpecAssociation,
 }
 

crates/but-workspace/src/commit_engine/tree.rs

@@ -294,6 +294,12 @@ fn apply_worktree_changes<'repo>(
             for selected_hunk in &change_request.hunk_headers {
                 if !worktree_hunks.contains(selected_hunk)
                     || has_zero_based_line_numbers(selected_hunk)
+                    || !worktree_hunks.iter().any(|wth| {
+                        (wth.old_range() == selected_hunk.old_range()
+                            && wth.new_range().contains(selected_hunk.new_range()))
+                            || (wth.new_range() == selected_hunk.new_range()
+                                && wth.old_range().contains(selected_hunk.old_range()))
+                    })
                 {
                     into_err_spec(possible_change, RejectionReason::MissingDiffSpecAssociation);
                     // TODO: only skip this one hunk, but collect skipped hunks into a new err-spec.

crates/but-workspace/src/lib.rs

@@ -21,7 +21,7 @@
 //!   - Git doesn't have a notion of such a branch.
 //! * **DiffSpec**
 //!   - A type that identifies changes, either as whole file, or as hunks in the file.
-//!   - It doesn't specify if the change is in a commit, or in the worktree, so that information must provided separately.
+//!   - It doesn't specify if the change is in a commit, or in the worktree, so that information must be provided separately.
 
 use anyhow::{Context, Result};
 use author::Author;

crates/but-workspace/tests/workspace/commit_engine/amend_commit.rs

@@ -89,7 +89,7 @@ fn new_file_and_deletion_onto_merge_commit() -> anyhow::Result<()> {
     let (repo, _tmp) = writable_scenario("merge-with-two-branches-line-offset");
     // Rewrite the entire file, which is fine as we rewrite/amend the base-commit itself.
     write_sequence(&repo, "new-file", [(10, None)])?;
-    std::fs::remove_file(repo.workdir().expect("non-bare").join("file"))?;
+    std::fs::remove_file(repo.workdir_path("file").expect("non-bare"))?;
 
     let outcome = commit_whole_files_and_all_hunks_from_workspace(
         &repo,
@@ -109,7 +109,7 @@ fn make_a_file_empty() -> anyhow::Result<()> {
 
     let (repo, _tmp) = writable_scenario("merge-with-two-branches-line-offset");
     // Empty the file
-    std::fs::write(repo.workdir().expect("non-bare").join("file"), "")?;
+    std::fs::write(repo.workdir_path("file").expect("non-bare"), "")?;
     let outcome = commit_whole_files_and_all_hunks_from_workspace(
         &repo,
         Destination::AmendCommit(repo.rev_parse_single("merge")?.detach()),
@@ -129,7 +129,7 @@ fn new_file_and_deletion_onto_merge_commit_with_hunks() -> anyhow::Result<()> {
     let (repo, _tmp) = writable_scenario("merge-with-two-branches-line-offset");
     // Rewrite the entire file, which is fine as we rewrite/amend the base-commit itself.
     write_sequence(&repo, "new-file", [(10, None)])?;
-    std::fs::remove_file(repo.workdir().expect("non-bare").join("file"))?;
+    std::fs::remove_file(repo.workdir_path("file").expect("non-bare"))?;
 
     let outcome = but_workspace::commit_engine::create_commit(
         &repo,

crates/but-workspace/tests/workspace/commit_engine/new_commit.rs

@@ -1,6 +1,6 @@
 use crate::utils::{
     CONTEXT_LINES, commit_from_outcome, commit_whole_files_and_all_hunks_from_workspace,
-    read_only_in_memory_scenario, to_change_specs_all_hunks,
+    hunk_header, read_only_in_memory_scenario, to_change_specs_all_hunks,
     to_change_specs_all_hunks_with_context_lines, to_change_specs_whole_file, visualize_tree,
     writable_scenario, writable_scenario_with_ssh_key, write_sequence,
 };
@@ -56,7 +56,7 @@ fn from_unborn_head() -> anyhow::Result<()> {
     "#);
 
     std::fs::write(
-        repo.workdir().expect("non-bare").join("new-untracked"),
+        repo.workdir_path("new-untracked").expect("non-bare"),
         "new-content",
     )?;
     let outcome = commit_whole_files_and_all_hunks_from_workspace(
@@ -91,6 +91,67 @@ fn from_unborn_head() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[test]
+fn from_unborn_head_with_selection() -> anyhow::Result<()> {
+    assure_stable_env();
+
+    let (repo, _tmp) = writable_scenario("unborn-untracked");
+    let destination = Destination::NewCommit {
+        parent_commit_id: None,
+        message: "the commit with selection".into(),
+        stack_segment: None,
+    };
+    let outcome = but_workspace::commit_engine::create_commit(
+        &repo,
+        destination,
+        None,
+        vec![DiffSpec {
+            previous_path: None,
+            path: "not-yet-tracked".into(),
+            hunk_headers: vec![hunk_header("-1,0", "+1,1")],
+        }],
+        CONTEXT_LINES,
+    )?;
+
+    let tree = visualize_tree(&repo, &outcome)?;
+    insta::assert_snapshot!(tree, @r#"
+    861d6e2
+    └── not-yet-tracked:100644:d95f3ad "content\n"
+    "#);
+
+    write_sequence(&repo, "also-untracked", [(1, 10)])?;
+    let destination = Destination::NewCommit {
+        parent_commit_id: outcome.new_commit,
+        message: "the commit with sub-selection".into(),
+        stack_segment: None,
+    };
+    let outcome = but_workspace::commit_engine::create_commit(
+        &repo,
+        destination,
+        None,
+        vec![DiffSpec {
+            previous_path: None,
+            path: "also-untracked".into(),
+            // take the first 5 lines, instead of 10
+            hunk_headers: vec![hunk_header("-1,0", "+4,3")],
+        }],
+        CONTEXT_LINES,
+    )?;
+    assert_eq!(
+        outcome.rejected_specs.len(),
+        0,
+        "hunk-ranges can also be applied"
+    );
+
+    let tree = visualize_tree(&repo, &outcome)?;
+    // TODO: figure out why this is not using the selected range, and why it's lacking the file from the previous tree.
+    insta::assert_snapshot!(tree, @r#"
+    14bffe1
+    └── also-untracked:100644:f00c965 "1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n"
+    "#);
+    Ok(())
+}
+
 #[test]
 #[cfg(unix)]
 fn from_unborn_head_all_file_types() -> anyhow::Result<()> {
@@ -626,7 +687,7 @@ fn unborn_untracked_worktree_filters_are_applied_to_whole_files() -> anyhow::Res
     "#);
 
     std::fs::write(
-        repo.workdir().expect("non-bare").join("new-untracked"),
+        repo.workdir_path("new-untracked").expect("non-bare"),
         "one\r\ntwo\r\n",
     )?;
     let outcome = commit_whole_files_and_all_hunks_from_workspace(

crates/but-workspace/tests/workspace/commit_engine/refs_update.rs

@@ -1139,7 +1139,7 @@ mod utils {
         rela_path: &str,
         content: &str,
     ) -> std::io::Result<()> {
-        let work_dir = repo.workdir().expect("non-bare");
-        std::fs::write(work_dir.join(rela_path), content)
+        let work_path = repo.workdir_path(rela_path).expect("non-bare");
+        std::fs::write(work_path, content)
     }
 }

crates/but-workspace/tests/workspace/discard/hunk.rs

@@ -1,9 +1,7 @@
-use crate::discard::hunk::util::{
-    changed_file_in_worktree_with_hunks, hunk_header, previous_change_text,
-};
+use crate::discard::hunk::util::{changed_file_in_worktree_with_hunks, previous_change_text};
 use crate::utils::{
-    CONTEXT_LINES, read_only_in_memory_scenario, to_change_specs_all_hunks, visualize_index,
-    writable_scenario,
+    CONTEXT_LINES, hunk_header, read_only_in_memory_scenario, to_change_specs_all_hunks,
+    visualize_index, writable_scenario,
 };
 use bstr::{BString, ByteSlice};
 use but_core::UnifiedDiff;
@@ -829,7 +827,6 @@ mod util {
     use bstr::BString;
     use but_core::unified_diff::DiffHunk;
     use but_core::{TreeChange, UnifiedDiff};
-    use but_workspace::commit_engine::HunkHeader;
     use gix::prelude::ObjectIdExt;
 
     pub fn previous_change_text(
@@ -849,18 +846,6 @@ mod util {
             .into())
     }
 
-    /// 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 {
-        let ((old_start, old_lines), (new_start, new_lines)) =
-            but_testsupport::hunk_header(old, new);
-        HunkHeader {
-            old_start,
-            old_lines,
-            new_start,
-            new_lines,
-        }
-    }
-
     pub fn changed_file_in_worktree_with_hunks(
         repo: &gix::Repository,
         filename: &str,

crates/but-workspace/tests/workspace/utils.rs

@@ -2,7 +2,7 @@ use bstr::ByteSlice;
 use but_core::TreeStatus;
 use but_testsupport::gix_testtools;
 use but_testsupport::gix_testtools::{Creation, tempfile};
-use but_workspace::commit_engine::{Destination, DiffSpec};
+use but_workspace::commit_engine::{Destination, DiffSpec, HunkHeader};
 use gix::prelude::ObjectIdExt;
 
 pub const CONTEXT_LINES: u32 = 0;
@@ -278,3 +278,14 @@ pub fn write_local_config(repo: &gix::Repository) -> anyhow::Result<()> {
     )?;
     Ok(())
 }
+
+/// 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 {
+    let ((old_start, old_lines), (new_start, new_lines)) = but_testsupport::hunk_header(old, new);
+    HunkHeader {
+        old_start,
+        old_lines,
+        new_start,
+        new_lines,
+    }
+}