discard changes: hunks part 2

crates/but-cli/src/args.rs

@@ -46,14 +46,18 @@ pub enum Subcommands {
     /// List all uncommitted working tree changes.
     Status {
         /// Also compute unified diffs for each tree-change.
-        #[clap(long, short = 'c', default_value_t = 3)]
+        #[clap(long, short = 'c', default_value_t = crate::command::UI_CONTEXT_LINES)]
         context_lines: u32,
         /// Also compute unified diffs for each tree-change.
         #[clap(long, short = 'd')]
         unified_diff: bool,
     },
     /// Discard the specified worktree change.
+    #[clap(disable_help_flag(true))]
     DiscardChange {
+        /// The zero-based indices of all hunks to discard.
+        #[clap(long, short = 'h')]
+        hunk_indices: Vec<usize>,
         /// The repo-relative path to the changed file to discard.
         current_path: PathBuf,
         /// If the change is a rename, identify the repo-relative path of the source.

crates/but-cli/src/command/mod.rs

@@ -1,9 +1,11 @@
-use anyhow::{Context, bail};
+use anyhow::{Context, anyhow, bail};
+use but_core::UnifiedDiff;
+use but_workspace::commit_engine::HunkHeader;
 use gitbutler_project::Project;
-use gix::bstr::BString;
+use gix::bstr::{BString, ByteSlice};
 use std::path::Path;
 
-const UI_CONTEXT_LINES: u32 = 3;
+pub(crate) const UI_CONTEXT_LINES: u32 = 3;
 
 pub fn project_from_path(path: &Path) -> anyhow::Result<Project> {
     Project::from_path(path)
@@ -140,13 +142,44 @@ pub(crate) fn discard_change(
     cwd: &Path,
     current_rela_path: &Path,
     previous_rela_path: Option<&Path>,
+    hunk_indices: &[usize],
 ) -> anyhow::Result<()> {
     let repo = gix::discover(cwd)?;
 
+    let previous_path = previous_rela_path.map(path_to_rela_path).transpose()?;
+    let path = path_to_rela_path(current_rela_path)?;
+    let hunk_headers = if hunk_indices.is_empty() {
+        vec![]
+    } else {
+        let worktree_changes = but_core::diff::worktree_changes(&repo)?
+            .changes
+            .into_iter()
+            .find(|change| {
+                change.path == path
+                    && change.previous_path() == previous_path.as_ref().map(|p| p.as_bstr())
+            }).with_context(|| format!("Couldn't find worktree change for file at '{path}' (previous-path: {previous_path:?}"))?;
+        let UnifiedDiff::Patch { hunks } =
+            worktree_changes.unified_diff(&repo, UI_CONTEXT_LINES)?
+        else {
+            bail!("No hunks available for given '{path}'")
+        };
+
+        hunk_indices
+            .iter()
+            .map(|idx| {
+                hunks.get(*idx).cloned().map(Into::into).ok_or_else(|| {
+                    anyhow!(
+                        "There was no hunk at index {idx} in '{path}' with {} hunks",
+                        hunks.len()
+                    )
+                })
+            })
+            .collect::<Result<Vec<HunkHeader>, _>>()?
+    };
     let spec = but_workspace::commit_engine::DiffSpec {
-        previous_path: previous_rela_path.map(path_to_rela_path).transpose()?,
-        path: path_to_rela_path(current_rela_path)?,
-        hunk_headers: vec![],
+        previous_path,
+        path,
+        hunk_headers,
     };
     debug_print(but_workspace::discard_workspace_changes(
         &repo,

crates/but-cli/src/main.rs

@@ -17,9 +17,15 @@ fn main() -> Result<()> {
 
     match &args.cmd {
         args::Subcommands::DiscardChange {
+            hunk_indices,
             current_path,
             previous_path,
-        } => command::discard_change(&args.current_dir, current_path, previous_path.as_deref()),
+        } => command::discard_change(
+            &args.current_dir,
+            current_path,
+            previous_path.as_deref(),
+            hunk_indices,
+        ),
         args::Subcommands::Commit {
             message,
             amend,

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

@@ -14,9 +14,9 @@ pub fn apply_hunks(
     new_image: &BStr,
     hunks: &[HunkHeader],
 ) -> anyhow::Result<BString> {
-    let mut worktree_base_cursor = 1; /* 1-based counting */
+    let mut old_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_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();
 
@@ -26,33 +26,30 @@ pub fn apply_hunks(
     // 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")?,
-        );
+        let old_skips = (selected_hunk.old_start as usize)
+            .checked_sub(old_cursor)
+            .context("hunks must be in order from top to bottom of the file")?;
+        let catchup_base_lines = old_iter.by_ref().take(old_skips);
         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;
+        old_cursor += old_skips + selected_hunk.old_lines as usize;
 
+        let new_skips = (selected_hunk.new_start as usize)
+            .checked_sub(new_cursor)
+            .context("hunks for new lines must be in order")?;
         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")?,
-            )
+            .skip(new_skips)
             .take(selected_hunk.new_lines as usize);
-
         for line in new_hunk_lines {
-            result_image.extend_from_slice(line);
+            result_image.extend_from_slice(line.as_bstr());
         }
-        worktree_actual_cursor += selected_hunk.new_lines as usize;
+        new_cursor += new_skips + selected_hunk.new_lines as usize;
     }
 
     for line in old_iter {

crates/but-workspace/tests/fixtures/scenario/mixed-hunk-modifications.sh

@@ -1,14 +1,15 @@
 #!/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.
+# Various files, of which two are in the worktree (with changes) and in the index (with changes).
+# Another pair of files was deleted, both in the worktree and in the index.
+# Lastly, each of these have specific worktree modifications in multiple hunks, and a plain file was made
+# executable in the index, and another rename destination was made executable, too.
+# Lastly, a simple file was executable, which isn't executable anymore.
 set -eu -o pipefail
 
 git init
-seq 5 18 >file
+seq 5 18 >file && chmod +x file
 seq 5 18 >file-in-index
 seq 5 18 >file-to-be-renamed
 seq 5 18 >file-to-be-renamed-in-index
@@ -33,10 +34,17 @@ eleven
 16
 EOF
 
-cp file file-in-index && git add file-in-index
+chmod -x file
 
+cp file file-in-index && \
+  chmod +x 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
+
+seq 2 18 >file-to-be-renamed && \
+  mv file-to-be-renamed file-renamed && \
+  chmod +x file-renamed
+cp file file-to-be-renamed-in-index && \
+  git mv file-to-be-renamed-in-index file-renamed-in-index
 
 

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

@@ -969,7 +969,7 @@ R  a/b/link -> a/sibling/link
     Ok(())
 }
 
-// Copy of `all_file_types_deleted_in_worktree`, but can't be loop due to `insta`.
+// Copy of `all_file_types_deleted_in_worktree`, could also be a loop but insta::allow_duplicates!() isn't pretty.
 #[test]
 #[cfg(unix)]
 fn all_file_types_deleted_in_index() -> anyhow::Result<()> {