discard changes: hunk selections

crates/but-cli/src/args.rs

@@ -57,11 +57,13 @@ pub enum Subcommands {
         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')]
+        #[clap(long)]
         hunk_indices: Vec<usize>,
+        /// The 1-based pairs of 4 numbers equivalent to '(old_start,old_lines,new_start,new_lines)'
+        #[clap(long, num_args = 4, conflicts_with = "hunk_indices", value_names = ["old-start", "old-lines", "new-start", "new-lines"])]
+        hunk_headers: Vec<u32>,
         /// 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

@@ -138,43 +138,59 @@ pub mod stacks {
     }
 }
 
+pub(crate) mod discard_change {
+    pub enum IndicesOrHeaders<'a> {
+        Indices(&'a [usize]),
+        Headers(&'a [u32]),
+    }
+}
 pub(crate) fn discard_change(
     cwd: &Path,
     current_rela_path: &Path,
     previous_rela_path: Option<&Path>,
-    hunk_indices: &[usize],
+    indices_or_headers: Option<discard_change::IndicesOrHeaders>,
 ) -> 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()
-                    )
-                })
+    let hunk_headers = match indices_or_headers {
+        None => vec![],
+        Some(discard_change::IndicesOrHeaders::Headers(headers)) => headers
+            .windows(4)
+            .map(|n| HunkHeader {
+                old_start: n[0],
+                old_lines: n[1],
+                new_start: n[2],
+                new_lines: n[3],
             })
-            .collect::<Result<Vec<HunkHeader>, _>>()?
+            .collect(),
+        Some(discard_change::IndicesOrHeaders::Indices(hunk_indices)) => {
+            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,

crates/but-cli/src/main.rs

@@ -18,13 +18,24 @@ fn main() -> Result<()> {
     match &args.cmd {
         args::Subcommands::DiscardChange {
             hunk_indices,
+            hunk_headers,
             current_path,
             previous_path,
         } => command::discard_change(
             &args.current_dir,
             current_path,
             previous_path.as_deref(),
-            hunk_indices,
+            if !hunk_indices.is_empty() {
+                Some(command::discard_change::IndicesOrHeaders::Indices(
+                    hunk_indices,
+                ))
+            } else if !hunk_headers.is_empty() {
+                Some(command::discard_change::IndicesOrHeaders::Headers(
+                    hunk_headers,
+                ))
+            } else {
+                None
+            },
         ),
         args::Subcommands::Commit {
             message,

crates/but-testsupport/src/lib.rs

@@ -6,6 +6,24 @@ use gix::config::tree::Key;
 pub use gix_testtools;
 use std::path::Path;
 
+/// Choose a slightly more obvious, yet easy to type syntax than a function with 4 parameters.
+/// i.e. `hunk_header("-1,10", "+1,10")`.
+/// Returns `( (old_start, old_lines), (new_start, new_lines) )`.
+pub fn hunk_header(old: &str, new: &str) -> ((u32, u32), (u32, u32)) {
+    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)
+    }
+    (parse_header(old), parse_header(new))
+}
+
 /// While `gix` can't (or can't conveniently) do everything, let's make using `git` easier.
 pub fn git(repo: &gix::Repository) -> std::process::Command {
     let mut cmd = std::process::Command::new(gix::path::env::exe_invocation());

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

@@ -1,4 +1,4 @@
-use crate::commit_engine::HunkHeader;
+use crate::commit_engine::{HunkHeader, HunkRange};
 use anyhow::Context;
 use bstr::{BStr, BString, ByteSlice};
 
@@ -30,8 +30,8 @@ pub fn apply_hunks(
             .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);
+        for old_line in catchup_base_lines {
+            result_image.extend_from_slice(old_line);
         }
         let _consume_old_hunk_to_replace_with_new = old_iter
             .by_ref()
@@ -42,12 +42,16 @@ pub fn apply_hunks(
         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(new_skips)
-            .take(selected_hunk.new_lines as usize);
-        for line in new_hunk_lines {
-            result_image.extend_from_slice(line.as_bstr());
+        if selected_hunk.new_lines == 0 {
+            let _explicit_skips = new_iter.by_ref().take(new_skips).count();
+        } else {
+            let new_hunk_lines = new_iter
+                .by_ref()
+                .skip(new_skips)
+                .take(selected_hunk.new_lines as usize);
+            for new_line in new_hunk_lines {
+                result_image.extend_from_slice(new_line);
+            }
         }
         new_cursor += new_skips + selected_hunk.new_lines as usize;
     }
@@ -57,3 +61,50 @@ pub fn apply_hunks(
     }
     Ok(result_image)
 }
+
+// TODO: one day make `HunkHeader` use this type instead of loose fields.
+impl HunkHeader {
+    /// Return our old-range as self-contained structure.
+    pub fn old_range(&self) -> HunkRange {
+        HunkRange {
+            start: self.old_start,
+            lines: self.old_lines,
+        }
+    }
+
+    /// Return our new-range as self-contained structure.
+    pub fn new_range(&self) -> HunkRange {
+        HunkRange {
+            start: self.new_start,
+            lines: self.new_lines,
+        }
+    }
+}
+
+impl std::fmt::Debug for HunkHeader {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(
+            f,
+            r#"HunkHeader("-{},{}", "+{},{}")"#,
+            self.old_start, self.old_lines, self.new_start, self.new_lines
+        )
+    }
+}
+
+impl HunkRange {
+    /// Calculate the line number that is one past of what we include, i.e. the first excluded line number.
+    pub fn end(&self) -> u32 {
+        self.start + self.lines
+    }
+    /// Calculate line number of the last line.
+    pub fn last_line(&self) -> u32 {
+        if self.lines == 0 {
+            return self.start;
+        }
+        self.start + self.lines - 1
+    }
+    /// Return `true` if a hunk with `start` and `lines` is fully contained in this hunk.
+    pub fn contains(self, other: HunkRange) -> bool {
+        other.start >= self.start && other.end() <= self.end()
+    }
+}

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

@@ -94,7 +94,7 @@ pub struct DiffSpec {
 }
 
 /// The header of a hunk that represents a change to a file.
-#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)]
+#[derive(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.
@@ -107,6 +107,17 @@ pub struct HunkHeader {
     pub new_lines: u32,
 }
 
+/// The range of a hunk as denoted by a 1-based starting line, and the amount of lines from there.
+#[derive(Debug, Copy, Clone, Eq, PartialEq)]
+pub struct HunkRange {
+    /// The number of the first line in the hunk, 1 based.
+    pub start: u32,
+    /// The amount of lines in the range.
+    ///
+    /// If `0`, this is an empty hunk.
+    pub lines: u32,
+}
+
 impl From<but_core::unified_diff::DiffHunk> for HunkHeader {
     fn from(
         DiffHunk {

crates/but-workspace/src/discard/function.rs

@@ -20,6 +20,22 @@ use but_core::{ChangeState, TreeStatus};
 /// In practice, this is like a selective 'inverse-checkout', as such it must have a lot of the capabilities of checkout, but focussed
 /// on just a couple of paths, and with special handling for renamed files, something that `checkout` can't naturally handle
 /// as it's only dealing with single file-paths.
+///
+/// ### Hunk-based discarding
+///
+/// When an instance in `changes` contains hunks, these are the hunks to be discarded. If they match a whole hunk in the worktree changes,
+/// it will be discarded entirely, simply by not applying it.
+///
+/// ### Sub-Hunk discarding
+///
+/// It's possible to specify ranges of hunks to discard. To do that, they need an *anchor*. The *anchor* is the pair of
+/// `(line_number, line_count)` that should not be changed, paired with the *other* pair with the new `(line_number, line_count)`
+/// to discard.
+///
+/// For instance, when there is a single patch `-1,10 +1,10` and we want to bring back the removed 5th line *and* the added 5th line,
+/// we'd specify *just* two selections, one in the old via `-5,1 +1,10` and one in the new via `-1,10 +5,1`.
+/// This works because internally, it will always match the hunks (and sub-hunks) with their respective pairs obtained through a
+/// worktree status.
 pub fn discard_workspace_changes(
     repo: &gix::Repository,
     changes: impl IntoIterator<Item = DiscardSpec>,