diff --git a/.gitignore b/.gitignore index ea390ad..01abd7f 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,6 @@ node_modules !.env.example vite.config.js.timestamp-* vite.config.ts.timestamp-* + +# superpowers brainstorming scratch (mockups, prototypes) +.superpowers/ diff --git a/README.md b/README.md index 1c42534..77e88e3 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,9 @@ If `git` or `python3` is missing, the app shows a startup banner with an **Insta ## Build from source +Needs **Node `^20.19.0 || >=22.12.0`** (Vite 8's floor; CI builds on 24). `npm install` only warns +on an older runtime, so the failure would otherwise surface later as a confusing build error. + ```sh npm install npm run tauri dev # run the app in development diff --git a/crates/git-core/src/git_ops.rs b/crates/git-core/src/git_ops.rs index 4261a24..9980c4c 100644 --- a/crates/git-core/src/git_ops.rs +++ b/crates/git-core/src/git_ops.rs @@ -37,6 +37,39 @@ fn is_inside_worktree(path: &Path) -> bool { .unwrap_or(false) } +/// Is `path` ITSELF a Git directory? True for a bare repository's own folder — which has no +/// `.git` child, so an existence check on that never sees it — and for a normal repository's +/// `.git`. `--resolve-git-dir` answers for the path given and does NOT walk up to a parent, +/// so a plain folder that merely sits inside a repository is not mistaken for one; that case +/// belongs to `is_inside_worktree`. Callers pass an absolute path (`parent` is canonicalized +/// before the join), so the operand cannot be read as a flag. +/// Is `path` inside a repository's metadata — the Git directory itself, or anything below it? +/// +/// `is_git_dir` answers only for the exact directory, so a folder picker landing on +/// `repo/.git/hooks` or `bare.git/objects` walked straight past it, and `--is-inside-work-tree` is +/// false down there as well. `--is-inside-git-dir` is documented to be true anywhere below the +/// repository directory, which is exactly the question a candidate PARENT has to answer. +/// A path in no repository at all makes git exit non-zero; that counts as false. +fn is_inside_git_dir(path: &Path) -> bool { + Command::new("git") + .current_dir(path) + .args(["rev-parse", "--is-inside-git-dir"]) + .output() + .ok() + .filter(|output| output.status.success()) + .map(|output| String::from_utf8_lossy(&output.stdout).trim() == "true") + .unwrap_or(false) +} + +fn is_git_dir(path: &Path) -> bool { + Command::new("git") + .args(["rev-parse", "--resolve-git-dir"]) + .arg(path) + .output() + .map(|output| output.status.success()) + .unwrap_or(false) +} + /// Initialize a repository in one direct child of an existing parent folder. /// A non-empty destination is reported without mutation until the caller /// explicitly retries with `allow_non_empty`. @@ -51,6 +84,18 @@ pub fn initialize_repository( if !parent.is_dir() { return Err("The selected parent path is not a folder.".to_string()); } + // A Git directory is still a folder, and `--is-inside-work-tree` answers false inside one, so + // the nesting probe below cannot catch it either — and when the destination does not exist yet + // that probe falls back to this very path. Picking `some-repo/.git`, or a bare repo, would + // therefore create the new repository inside another repository's metadata. `is_inside_git_dir` + // rather than `is_git_dir` because a picker reaches `.git/hooks` and `bare.git/objects` just as + // easily as the top of either, and an exact-path probe does not see those. + if is_inside_git_dir(&parent) { + return Err( + "The selected parent folder is a Git repository's internal directory. Choose a different folder." + .to_string(), + ); + } let name = folder_name.trim(); if name.is_empty() @@ -91,7 +136,10 @@ pub fn initialize_repository( if destination.exists() && !destination.is_dir() { return Err("A file already exists at the requested repository path.".to_string()); } - if destination.join(".git").exists() { + // `.git` catches a normal repository; `is_git_dir` catches a bare one, which has no `.git` + // child and reports `false` for `--is-inside-work-tree`, so it slipped past both guards and + // `git init` would nest a fresh repository inside it. + if destination.join(".git").exists() || is_git_dir(&destination) { return Err("That folder is already a Git repository. Open it instead.".to_string()); } let nesting_probe = if destination.is_dir() { @@ -339,21 +387,29 @@ pub fn check_prerequisites(filter_repo_argv: &[String]) -> PrerequisiteCheck { #[cfg(test)] mod tests { use super::*; - use std::time::{SystemTime, UNIX_EPOCH}; + use std::sync::atomic::{AtomicU32, Ordering}; + + // These tests run on parallel threads within ONE process, so the pid alone does not + // make a path unique. A wall-clock stamp did not either: two threads could read the + // same nanosecond, `fs::create_dir` then failed with AlreadyExists, and the `unwrap()` + // panicked — a flake that hit a different test on each run. A process-wide counter is + // collision-proof by construction, and is the pattern the TempRepo fixtures in ops.rs + // and ops_worktree.rs already use. + static COUNTER: AtomicU32 = AtomicU32::new(0); struct TempFolder(PathBuf); impl TempFolder { fn new() -> Self { - let stamp = SystemTime::now() - .duration_since(UNIX_EPOCH) - .unwrap() - .as_nanos(); - let path = std::env::temp_dir().join(format!( - "git-it-init-test-{}-{stamp}", - std::process::id() - )); - fs::create_dir(&path).unwrap(); + let id = COUNTER.fetch_add(1, Ordering::SeqCst); + let path = std::env::temp_dir() + .join(format!("git-it-init-test-{}-{}", std::process::id(), id)); + // pid + counter is unique among LIVE processes, but not against the dead: a run + // killed before `Drop` leaves its directories behind, and once the OS recycles + // that pid a fresh process counting from zero reproduces the same path. Clear + // any such leftover first — the same guard the other TempRepo fixtures use. + let _ = fs::remove_dir_all(&path); + fs::create_dir_all(&path).unwrap(); Self(path) } } @@ -424,4 +480,80 @@ mod tests { assert!(err.contains("nested repository")); assert!(!parent.0.join("nested").exists()); } + + /// A bare repository has no `.git` child, and `rev-parse --is-inside-work-tree` answers + /// `false` inside one — so neither existing guard saw it. `allow_non_empty` here is the + /// dangerous path: the user is warned the folder is not empty, confirms, and `git init` + /// then creates a nested repository inside the bare one. + #[test] + fn initialize_repository_rejects_an_existing_bare_repository() { + let parent = TempFolder::new(); + let mut init = Command::new("git"); + init.current_dir(&parent.0).args(["init", "-q", "--bare", "shipped.git"]); + run(&mut init).unwrap(); + + let err = initialize_repository(&parent.0, "shipped.git", "main", true).unwrap_err(); + + assert!( + err.contains("already a Git repository"), + "should be refused as an existing repository, got: {err}" + ); + assert!( + !parent.0.join("shipped.git").join(".git").exists(), + "must not have initialized a nested repository inside the bare one" + ); + } + + /// The same blind spot one level up. When the destination does not exist yet the nesting probe + /// falls back to the PARENT, and `--is-inside-work-tree` is false inside a Git directory just + /// as it is inside a bare repo — so picking `some-repo/.git` (or a bare repo) as the parent + /// created the new repository inside another repository's metadata. + #[test] + fn initialize_repository_rejects_a_git_directory_as_parent() { + let outer = TempFolder::new(); + + // A normal repository's `.git`, and a bare repository, are both Git directories. + let mut init = Command::new("git"); + init.current_dir(&outer.0).args(["init", "-q", "host"]); + run(&mut init).unwrap(); + let mut bare = Command::new("git"); + bare.current_dir(&outer.0).args(["init", "-q", "--bare", "shipped.git"]); + run(&mut bare).unwrap(); + + // The Git directory itself, a bare repo, and — because an exact-path probe misses them — + // directories BELOW either one, which a folder picker reaches just as easily. + for parent in [ + outer.0.join("host").join(".git"), + outer.0.join("host").join(".git").join("hooks"), + outer.0.join("shipped.git"), + outer.0.join("shipped.git").join("objects"), + ] { + let err = initialize_repository(&parent, "proj", "main", false) + .expect_err(&format!("{} should have been refused", parent.display())); + assert!( + err.contains("Git repository"), + "{}: should be refused, got: {err}", + parent.display() + ); + assert!( + !parent.join("proj").exists(), + "{}: must not have created anything inside a Git directory", + parent.display() + ); + } + } + + /// The guard must not over-reach: a plain folder that merely sits next to a repository + /// is still a valid destination. `--resolve-git-dir` answers for the path given and does + /// not walk up, which is what keeps this case working. + #[test] + fn initialize_repository_still_accepts_a_plain_empty_folder() { + let parent = TempFolder::new(); + fs::create_dir(parent.0.join("fresh")).unwrap(); + + let result = initialize_repository(&parent.0, "fresh", "main", true).unwrap(); + + assert!(result.initialized); + assert!(parent.0.join("fresh").join(".git").is_dir()); + } } diff --git a/crates/git-core/src/ops_worktree.rs b/crates/git-core/src/ops_worktree.rs index 0d8f5b1..f5358e4 100644 --- a/crates/git-core/src/ops_worktree.rs +++ b/crates/git-core/src/ops_worktree.rs @@ -178,6 +178,13 @@ pub fn diff(repo: &Path, path: Option<&str>, staged: bool, context: u32) -> Resu c.current_dir(repo) .arg("diff") .arg("--no-color") + // Pin the path prefixes. `diff.mnemonicPrefix` rewrites them per-command (`i/`, `w/`, `c/`) + // and `diff.noprefix` removes them entirely, and this output is not just displayed — the + // hunks are fed straight back to `git apply`. Under `noprefix` the patch loses a path + // component to apply's default `-p1` and lands on the WRONG FILE; anything that reads the + // `a/`…`b/` convention out of the header is likewise wrong. The user's config governs what + // they read in a terminal, not what this reconstructs and re-applies. + .args(["--src-prefix=a/", "--dst-prefix=b/"]) .arg(format!("-U{}", context)); if staged { c.arg("--cached"); @@ -259,14 +266,96 @@ pub fn split_hunks(diff: &str) -> (String, Vec) { (header, hunks) } -/// Pipe a patch (reconstructed from git's own diff output) to `git apply --cached [--reverse]` -/// via stdin. Never uses a temp file or shell. -fn git_apply(repo: &Path, patch: &str, reverse: bool) -> Result<(), String> { +/// Parse `@@ -A[,B] +C[,D] @@[ heading]` into `(old_start, old_n, new_start, new_n, heading)`. +/// An omitted count means 1 — git writes `@@ -5 +5 @@` for a single-line range. `heading` is +/// everything after the closing `@@` (git's function-context hint), returned verbatim so it can +/// be spliced back on. None if the line is not a hunk header. +fn parse_hunk_header(at_line: &str) -> Option<(u64, u64, u64, u64, &str)> { + let (ranges, heading) = at_line.strip_prefix("@@ ")?.split_once(" @@")?; + let (old, new) = ranges.split_once(' ')?; + let range = |s: &str, sign: char| -> Option<(u64, u64)> { + let s = s.strip_prefix(sign)?; + Some(match s.split_once(',') { + Some((start, count)) => (start.parse().ok()?, count.parse().ok()?), + None => (s.parse().ok()?, 1), + }) + }; + let (old_start, old_n) = range(old, '-')?; + let (new_start, new_n) = range(new, '+')?; + Some((old_start, old_n, new_start, new_n, heading)) +} + +/// Format a hunk header, always with explicit counts and always newline-terminated. +fn hunk_header(old_start: u64, old_n: u64, new_start: u64, new_n: u64, heading: &str) -> String { + let heading = heading.strip_suffix('\n').unwrap_or(heading); + format!("@@ -{},{} +{},{} @@{}\n", old_start, old_n, new_start, new_n, heading) +} + +/// Re-anchor one hunk lifted out of a multi-hunk diff so it applies at the right line ALONE. +/// +/// `git apply` positions a hunk by the coordinate of the image it is producing: `new_start` +/// going forward, `old_start` going `--reverse` (reverse swaps the two sides). It then searches +/// for the preimage around that line, which is why an off-by-N coordinate usually goes unnoticed. +/// But at `-U0` a pure insertion (forward) or a pure deletion (reverse) has an EMPTY preimage — +/// there is nothing to search for, so git applies at exactly the line named and reports success +/// from the wrong place. +/// +/// An extracted hunk carries both coordinates from the FULL diff, where the side we are not +/// applying against is offset by every line the hunks we did NOT extract added or removed earlier +/// in the file. Only the side facing our target is trustworthy: `old_start` for a forward apply +/// (the target is the diff's old image — the index), `new_start` for a reverse one (the target is +/// its new image — the worktree, or the index when unstaging). Keep that side, derive the other. +/// +/// `prefix` is the number of unchanged lines before the hunk. git writes a zero-length range as +/// the line it sits AFTER (`-16,0` = insert after old line 16) and a non-empty range as the first +/// line it covers (`prefix + 1`) — that convention is the whole of the arithmetic below. +fn reanchor(old_start: u64, old_n: u64, new_start: u64, new_n: u64, reverse: bool) -> (u64, u64) { + if reverse { + let prefix = if new_n == 0 { new_start } else { new_start.saturating_sub(1) }; + (if old_n == 0 { prefix } else { prefix + 1 }, new_start) + } else { + let prefix = if old_n == 0 { old_start } else { old_start.saturating_sub(1) }; + (old_start, if new_n == 0 { prefix } else { prefix + 1 }) + } +} + +/// Re-anchor a whole hunk taken verbatim from git's diff (see `reanchor`). The body — and so both +/// line counts — is untouched. A header we cannot parse is handed back unchanged: git wrote it, so +/// git can read it, and refusing to guess beats emitting something we invented. +fn reanchor_hunk(hunk: &str, reverse: bool) -> String { + let mut parts = hunk.splitn(2, '\n'); + let at_line = parts.next().unwrap_or(""); + let body = parts.next().unwrap_or(""); + match parse_hunk_header(at_line) { + Some((old_start, old_n, new_start, new_n, heading)) => { + let (o, n) = reanchor(old_start, old_n, new_start, new_n, reverse); + format!("{}{}", hunk_header(o, old_n, n, new_n, heading), body) + } + None => hunk.to_string(), + } +} + +/// Pipe a patch (reconstructed from git's own diff output) to `git apply` via stdin. +/// `cached` selects the target: true → `--cached` (the index, for stage/unstage), +/// false → the WORKING TREE only (for discard). Never uses a temp file or shell. +/// +/// `zero_context` must be set when the patch came from a `-U0` diff. Without it git enforces +/// "a hunk with no trailing context must match at the end of the file", which a context-free +/// hunk always trips: reverse applies then fail outright, and a forward insertion silently lands +/// at EOF. `--unidiff-zero` lifts exactly that rule, so it is passed ONLY at `-U0` — at any real +/// context depth those checks are the safety net and stay on. +fn git_apply(repo: &Path, patch: &str, reverse: bool, cached: bool, zero_context: bool) -> Result<(), String> { let mut c = Command::new("git"); - c.current_dir(repo).arg("apply").arg("--cached"); + c.current_dir(repo).arg("apply"); + if cached { + c.arg("--cached"); + } if reverse { c.arg("--reverse"); } + if zero_context { + c.arg("--unidiff-zero"); + } c.arg("-") .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -290,35 +379,64 @@ fn git_apply(repo: &Path, patch: &str, reverse: bool) -> Result<(), String> { /// Stage one hunk (by index) of `path`'s UNSTAGED diff. /// The patch is reconstructed from a diff fetched at the caller's display `context`, /// so the supplied `hunk_index` lines up with the hunks the user is actually viewing -/// (the frontend fetches the displayed diff at the same context). The partial-hunk -/// builder recomputes the `@@` header from the emitted lines, so any context depth works. +/// (the frontend fetches the displayed diff at the same context). `reanchor_hunk` repairs +/// the coordinate the extracted hunk inherited from the hunks left behind, so any context +/// depth works — see `reanchor` for why that is not cosmetic at `-U0`. /// Hunk indices refer to the CURRENT live diff; after a successful stage/unstage the remaining /// diff re-indexes, so callers must re-fetch the diff before issuing another hunk op. pub fn stage_hunk(repo: &Path, path: &str, hunk_index: usize, context: u32) -> Result<(), String> { let d = diff(repo, Some(path), false, context)?; let (header, hunks) = split_hunks(&d); let h = hunks.get(hunk_index).ok_or("hunk index out of range")?; - git_apply(repo, &format!("{}{}", header, h), false) + git_apply(repo, &format!("{}{}", header, reanchor_hunk(h, false)), false, true, context == 0) } /// Unstage one hunk (by index) of `path`'s STAGED diff. /// The patch is reconstructed from a diff fetched at the caller's display `context`, /// so the supplied `hunk_index` lines up with the hunks the user is actually viewing -/// (the frontend fetches the displayed diff at the same context). The partial-hunk -/// builder recomputes the `@@` header from the emitted lines, so any context depth works. +/// (the frontend fetches the displayed diff at the same context). `reanchor_hunk` repairs +/// the coordinate the extracted hunk inherited from the hunks left behind, so any context +/// depth works — see `reanchor` for why that is not cosmetic at `-U0`. /// Hunk indices refer to the CURRENT live diff; after a successful stage/unstage the remaining /// diff re-indexes, so callers must re-fetch the diff before issuing another hunk op. pub fn unstage_hunk(repo: &Path, path: &str, hunk_index: usize, context: u32) -> Result<(), String> { let d = diff(repo, Some(path), true, context)?; let (header, hunks) = split_hunks(&d); let h = hunks.get(hunk_index).ok_or("hunk index out of range")?; - git_apply(repo, &format!("{}{}", header, h), true) + git_apply(repo, &format!("{}{}", header, reanchor_hunk(h, true)), true, true, context == 0) } // --------------------------------------------------------------------------- // Line-level (intra-hunk) staging // --------------------------------------------------------------------------- +/// Reject a selection whose change-line ordinals are not one contiguous run. +/// +/// Callers select a contiguous RANGE of rows, so the ordinals they produce are always +/// consecutive — context rows carry no ordinal and therefore open no gap. A gapped set +/// means the caller grouped lines that are not adjacent in the hunk. `build_partial_hunk` +/// emits in hunk source order, so such a patch applies cleanly while placing the kept and +/// restored lines in the wrong order — silent corruption with no reflog to recover from. +/// There is no position for the omitted lines that matches the caller's intent, so refusing +/// is the only safe answer. +fn require_contiguous(selected: &[usize]) -> Result<(), String> { + if selected.is_empty() { + return Ok(()); + } + let mut s: Vec = selected.to_vec(); + s.sort_unstable(); + s.dedup(); + let span = s[s.len() - 1] - s[0] + 1; + if span != s.len() { + return Err(format!( + "refusing a non-contiguous line selection {:?}: those change lines are not adjacent \ + in the hunk, and applying them would reorder the file", + s + )); + } + Ok(()) +} + /// Build a partial single-hunk patch keeping only the selected change lines. /// `selected` holds ORDINALS over the hunk's change lines (the +/- lines, counted /// in order starting at 0; context and `\ No newline` lines are NOT counted). @@ -333,29 +451,67 @@ pub fn unstage_hunk(repo: &Path, path: &str, hunk_index: usize, context: u32) -> /// - Unselected `-` lines are dropped (they are absent from the staged file). /// /// Selected `+`/`-` lines: kept as-is in both directions (mark real change). -/// Context, `\ No newline` handling, header recompute, and the any_real_change -/// guard are unchanged. -/// Returns None when the selection keeps no change line (caller should no-op). -fn build_partial_hunk(hunk: &str, selected: &std::collections::HashSet, reverse: bool) -> Option { +/// The any_real_change guard is unchanged; `\ No newline` handling is described below. +/// +/// The `@@` counts are recomputed from the emitted lines and the start lines are re-anchored +/// (see `reanchor`). Dropping lines never moves the side we anchor on: forward, the emitted old +/// side is exactly the hunk's old side (dropped `+` occupy no old line); reverse, the emitted new +/// side is exactly the hunk's new side (dropped `-` occupy no new line). +/// +/// `Ok(None)` when the selection keeps no change line (caller supplies its own message). +/// `Err` when the selection cannot be expressed as a patch at all — see the `'\\'` arm. +fn build_partial_hunk( + hunk: &str, + selected: &std::collections::HashSet, + reverse: bool, +) -> Result, String> { let mut lines = hunk.splitn(2, '\n'); let at_line = lines.next().unwrap_or(""); let body = lines.next().unwrap_or(""); - // Parse @@ -A[,B] +C[,D] @@ [heading] - // We only need A (old_start). Be tolerant of missing counts. - let old_start: u64 = { - // Find "-A" after "@@" - let after_at = at_line.trim_start_matches('@').trim_start_matches(' '); - // e.g. "-12,6 +12,7 @@ heading" or "-5 +5 @@" - let old_part = after_at.trim_start_matches('-'); - let end = old_part.find([',', ' ']).unwrap_or(old_part.len()); - old_part[..end].parse().unwrap_or(1) + let Some((old_start, _, new_start, _, heading)) = parse_hunk_header(at_line) else { + return Ok(None); + }; + + // Signs of the change lines, indexed by ordinal — the lookahead a `\ No newline` + // marker needs to tell whether the line it follows is still file-final once the + // unselected lines are dropped or demoted. + let signs: Vec = body + .split_inclusive('\n') + .filter_map(|l| match l.chars().next() { + Some(c @ ('+' | '-')) => Some(c), + _ => None, + }) + .collect(); + + // Does any change line from ordinal `from` onward still occupy `old_side` + // (or the new side) of the emitted patch? A demoted line becomes context and + // so occupies both; a dropped one occupies neither. + let occupies_from = |from: usize, old_side: bool| -> bool { + signs.iter().enumerate().skip(from).any(|(o, &s)| { + match (s, selected.contains(&o)) { + ('+', true) => !old_side, + ('-', true) => old_side, + ('+', false) => reverse, + ('-', false) => !reverse, + _ => false, + } + }) }; + // What the previous source line was actually emitted as. Paired with the first + // ordinal that comes after it — the marker arm needs both. + #[derive(Clone, Copy)] + enum Emitted { + Context, + Del, + Add, + } + let mut old_n: u64 = 0; let mut new_n: u64 = 0; let mut ord: usize = 0; - let mut last_emitted = false; + let mut last: Option<(Emitted, usize)> = None; let mut any_real_change = false; let mut out = String::new(); @@ -368,7 +524,7 @@ fn build_partial_hunk(hunk: &str, selected: &std::collections::HashSet, r out.push_str(raw_line); old_n += 1; new_n += 1; - last_emitted = true; + last = Some((Emitted::Context, ord)); } Some('+') => { let this = ord; @@ -377,7 +533,7 @@ fn build_partial_hunk(hunk: &str, selected: &std::collections::HashSet, r // Selected addition: keep as `+` in both directions. out.push_str(raw_line); new_n += 1; - last_emitted = true; + last = Some((Emitted::Add, ord)); any_real_change = true; } else if reverse { // Unstage path: this `+` line IS in the staged (new) image, @@ -387,10 +543,10 @@ fn build_partial_hunk(hunk: &str, selected: &std::collections::HashSet, r out.push_str(rest); old_n += 1; new_n += 1; - last_emitted = true; + last = Some((Emitted::Context, ord)); } else { // Stage path: unselected addition → drop it entirely. - last_emitted = false; + last = None; } } Some('-') => { @@ -400,12 +556,12 @@ fn build_partial_hunk(hunk: &str, selected: &std::collections::HashSet, r // Selected deletion: keep as `-` in both directions. out.push_str(raw_line); old_n += 1; - last_emitted = true; + last = Some((Emitted::Del, ord)); any_real_change = true; } else if reverse { // Unstage path: this `-` line is absent from the staged (new) image, // so drop it entirely (it has no presence in the staged file to anchor on). - last_emitted = false; + last = None; } else { // Stage path: unselected deletion → demote to context. let rest = &raw_line[1..]; @@ -413,15 +569,49 @@ fn build_partial_hunk(hunk: &str, selected: &std::collections::HashSet, r out.push_str(rest); old_n += 1; new_n += 1; - last_emitted = true; + last = Some((Emitted::Context, ord)); } } Some('\\') => { - // "\ No newline at end of file" — keep only if last line was emitted - if last_emitted { - out.push_str(raw_line); + // `\ No newline at end of file` claims the line before it is the LAST line + // of the image(s) that line belongs to. A partial selection can leave content + // after it on one of those sides, which makes the claim false — and `git apply` + // resolves the contradiction by CONCATENATING the two lines onto one, silently + // and with no reflog on the discard path. So the marker survives only while it + // is still true. + match last { + Some((Emitted::Del, from)) => { + if !occupies_from(from, true) { + out.push_str(raw_line); + } + } + Some((Emitted::Add, from)) => { + if !occupies_from(from, false) { + out.push_str(raw_line); + } + } + Some((Emitted::Context, from)) => { + // A context line is one line shared by both images, so it cannot end + // one of them and not the other. When the selection asks for exactly + // that — staging an addition after a demoted final line, say — no + // patch expresses it, and emitting one anyway is how the corruption + // happened. Refuse instead; the whole-hunk action still works. + let old_more = occupies_from(from, true); + let new_more = occupies_from(from, false); + if old_more != new_more { + return Err("this selection cannot be expressed as a patch: it splits \ + a change at a no-newline end of file, where the kept line \ + would have to end with a newline on one side and not the \ + other. Use the whole-hunk action instead." + .to_string()); + } + if !old_more { + out.push_str(raw_line); + } + } + None => {} } - // do not change counts or last_emitted + // do not change counts or `last` } None | Some(_) => { // bare empty line or other: treat as context @@ -435,44 +625,183 @@ fn build_partial_hunk(hunk: &str, selected: &std::collections::HashSet, r out.push_str(&content); old_n += 1; new_n += 1; - last_emitted = true; + last = Some((Emitted::Context, ord)); } } } } if !any_real_change { - return None; + return Ok(None); } - let header = format!("@@ -{},{} +{},{} @@\n", old_start, old_n, old_start, new_n); - Some(format!("{}{}", header, out)) + let (o, n) = reanchor(old_start, old_n, new_start, new_n, reverse); + Ok(Some(format!("{}{}", hunk_header(o, old_n, n, new_n, heading), out))) } /// Stage selected lines (change-line ordinals) of one hunk of `path`'s UNSTAGED diff. /// The diff is fetched at the caller's display `context` so `hunk_index` and the /// change-line ordinals line up with the diff the user is viewing. `build_partial_hunk` -/// recomputes the `@@` header from the emitted lines, so any context depth works. +/// recomputes the `@@` header counts from the emitted lines and re-anchors its start lines +/// (see `reanchor`), so any context depth works. pub fn stage_lines(repo: &Path, path: &str, hunk_index: usize, selected: &[usize], context: u32) -> Result<(), String> { + require_contiguous(selected)?; let d = diff(repo, Some(path), false, context)?; let (header, hunks) = split_hunks(&d); let h = hunks.get(hunk_index).ok_or("hunk index out of range")?; let set: std::collections::HashSet = selected.iter().copied().collect(); - let partial = build_partial_hunk(h, &set, false).ok_or("no lines selected to stage")?; - git_apply(repo, &format!("{}{}", header, partial), false) + let partial = build_partial_hunk(h, &set, false)?.ok_or("no lines selected to stage")?; + git_apply(repo, &format!("{}{}", header, partial), false, true, context == 0) } /// Unstage selected lines (change-line ordinals) of one hunk of `path`'s STAGED diff. /// The diff is fetched at the caller's display `context` so `hunk_index` and the /// change-line ordinals line up with the diff the user is viewing. `build_partial_hunk` -/// recomputes the `@@` header from the emitted lines, so any context depth works. +/// recomputes the `@@` header counts from the emitted lines and re-anchors its start lines +/// (see `reanchor`), so any context depth works. pub fn unstage_lines(repo: &Path, path: &str, hunk_index: usize, selected: &[usize], context: u32) -> Result<(), String> { + require_contiguous(selected)?; let d = diff(repo, Some(path), true, context)?; let (header, hunks) = split_hunks(&d); let h = hunks.get(hunk_index).ok_or("hunk index out of range")?; let set: std::collections::HashSet = selected.iter().copied().collect(); - let partial = build_partial_hunk(h, &set, true).ok_or("no lines selected to unstage")?; - git_apply(repo, &format!("{}{}", header, partial), true) + let partial = build_partial_hunk(h, &set, true)?.ok_or("no lines selected to unstage")?; + git_apply(repo, &format!("{}{}", header, partial), true, true, context == 0) +} + +/// Build the file header for a discard patch: git's own header, minus the parts that describe +/// the FILE rather than its contents and would otherwise be applied as unasked-for side effects. +/// +/// - `old mode` / `new mode`. `git apply` honours a mode pair, so `chmod +x` plus an edited line +/// — one diff, one header — meant "Discard 1 line" also took the executable bit off. Not in the +/// confirmation, not in the line count, not undoable. Always dropped here. Stage and unstage +/// keep the mode deliberately: there it belongs to the same index entry the caller is moving, +/// and the result is recoverable either way. +/// +/// - `new file mode` + `--- /dev/null`. True of the whole diff of an intent-to-add path +/// (`git add -N`), which `working_changes` reports as tracked-and-unstaged so the UI offers +/// line-level discard on it. But a partial selection keeps the unselected additions as CONTEXT, +/// which gives the patch an old side the header denies, and git refuses the lot with +/// "new file X depends on old contents". Rewritten to an ordinary content header exactly when +/// the emitted hunk has an old side, so a selection covering every addition still deletes the +/// file the way a whole-hunk discard does. +/// +/// The deleted-file mirror (`+++ /dev/null`) needs no such repair: reverse-apply DROPS unselected +/// `-` lines rather than demoting them, so the new side stays empty and the header stays true. +fn discard_header(header: &str, hunk: &str) -> String { + let keeps_old_side = parse_hunk_header(hunk.lines().next().unwrap_or("")) + .is_some_and(|(_, old_n, _, _, _)| old_n > 0); + // Derive the `---` side from the `+++` one rather than re-deriving the path: swapping the + // leading `b/` leaves git's quoting of exotic paths intact (the quote precedes the prefix, + // so `"b/od\td"` becomes `"a/od\td"`). + // + // Only ever swap a LEADING `b/`. `diff()` pins the prefixes, but if that ever stops being + // true a blind `replace` would hit the first `b/` inside the pathname instead — turning + // `w/lib/util.js` into `w/lia/util.js`, which git reads as a rename and applies to a file the + // user never touched. Verified: it empties the real file and rewrites the innocent one, and + // returns success. When the prefix is absent we emit nothing and git refuses the patch, which + // is the only acceptable default on an apply with no reflog behind it. + let old_side = header + .lines() + .find_map(|l| l.strip_prefix("+++ ")) + .and_then(|p| { + let (quote, rest) = match p.strip_prefix('"') { + Some(rest) => ("\"", rest), + None => ("", p), + }; + rest.strip_prefix("b/") + .map(|tail| format!("--- {quote}a/{tail}\n")) + }); + // Repair the new-file header only when there is a real path to repair it WITH. Otherwise + // leave every line of it alone: an untouched header is the behaviour that shipped before + // this repair existed, and git refuses it. Half-repairing — dropping `new file mode` while + // keeping `--- /dev/null` — is a shape nothing has verified. + let normalize_new_file = keeps_old_side && old_side.is_some(); + header + .split_inclusive('\n') + .filter_map(|line| { + let mode_pair = line.starts_with("old mode ") || line.starts_with("new mode "); + let new_file = normalize_new_file && line.starts_with("new file mode "); + if mode_pair || new_file { + None + } else if normalize_new_file && line.starts_with("--- /dev/null") { + old_side.clone() + } else { + Some(line.to_string()) + } + }) + .collect() +} + +/// Refuse a discard whose `hunk_index` / ordinals were picked against a different diff. +/// +/// `expected_diff` is the exact `diff()` text the caller displayed. The confirmation dialog in +/// front of a discard has no timeout, so an external edit — another editor, another Git It window, +/// a build step — can re-split the file between the click and the confirm. Nothing about a plain +/// integer index says which hunk it meant, so a re-fetched diff would happily reverse-apply hunk N +/// of a file the user never saw, and unlike stage/unstage there is no index or reflog to undo it. +/// +/// The comparison is deliberately whole-file rather than per-hunk. Not every edit moves the hunk +/// the user picked — one confined to a later hunk does not — but telling those apart means trusting +/// the same index arithmetic that is in question, and refusing costs one retry while guessing wrong +/// costs work that cannot be recovered. This is additive — `require_contiguous` remains the +/// independent defence against a gapped selection reordering the file. +fn require_unchanged_diff(live: &str, expected: &str) -> Result<(), String> { + if live == expected { + return Ok(()); + } + Err("the file changed since the diff you were shown — nothing was discarded. \ + Check the updated diff and try again." + .to_string()) +} + +/// Discard one hunk (by index) of `path`'s UNSTAGED diff: reverse-apply it to the +/// WORKING TREE only (no `--cached`). Because the unstaged diff's old side is the +/// INDEX, the lines revert to their staged state — staged changes to the same file +/// are untouched. DESTRUCTIVE / not undoable. +/// +/// `expected_diff` is the diff the caller showed the user; the op is refused if the live diff +/// no longer matches it (see `require_unchanged_diff`). Hunk indices refer to that diff; after a +/// successful op the remaining diff re-indexes, so callers must re-fetch before issuing another. +pub fn discard_hunk( + repo: &Path, + path: &str, + hunk_index: usize, + expected_diff: &str, + context: u32, +) -> Result<(), String> { + let d = diff(repo, Some(path), false, context)?; + require_unchanged_diff(&d, expected_diff)?; + let (header, hunks) = split_hunks(&d); + let h = hunks.get(hunk_index).ok_or("hunk index out of range")?; + let reanchored = reanchor_hunk(h, true); + git_apply(repo, &format!("{}{}", discard_header(&header, &reanchored), reanchored), true, false, context == 0) +} + +/// Discard selected change-line ordinals of one hunk of `path`'s UNSTAGED diff. +/// `build_partial_hunk(.., reverse: true)` emits a patch whose NEW side matches the +/// working file (unselected `+` become context because they ARE present there; +/// unselected `-` are dropped because they are not) — exactly what a worktree +/// reverse-apply needs. DESTRUCTIVE / not undoable. +/// +/// `expected_diff` is the diff the caller showed the user; the op is refused if the live diff +/// no longer matches it (see `require_unchanged_diff`). +pub fn discard_lines( + repo: &Path, + path: &str, + hunk_index: usize, + selected: &[usize], + expected_diff: &str, + context: u32, +) -> Result<(), String> { + require_contiguous(selected)?; + let d = diff(repo, Some(path), false, context)?; + require_unchanged_diff(&d, expected_diff)?; + let (header, hunks) = split_hunks(&d); + let h = hunks.get(hunk_index).ok_or("hunk index out of range")?; + let set: std::collections::HashSet = selected.iter().copied().collect(); + let partial = build_partial_hunk(h, &set, true)?.ok_or("no lines selected to discard")?; + git_apply(repo, &format!("{}{}", discard_header(&header, &partial), partial), true, false, context == 0) } /// Stash current changes (staged + unstaged). Message is optional. @@ -766,11 +1095,188 @@ mod tests { v.iter().copied().collect() } + const BASE_20: &str = + "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nr\ns\nt\n"; + const TWO_ADD_HUNKS: &str = + "a\nb\nB2\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nQ2\nq\nr\ns\nt\n"; + const FIRST_ADD_ONLY: &str = + "a\nb\nB2\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nr\ns\nt\n"; + const SECOND_ADD_ONLY: &str = + "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nQ2\nq\nr\ns\nt\n"; + const THREE_LINE_SHIFT_HUNKS: &str = + "a\nb\nB2a\nB2b\nB2c\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nQ2\nq\nr\ns\nt\n"; + const THREE_LINE_SHIFT_FIRST_ONLY: &str = + "a\nb\nB2a\nB2b\nB2c\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nr\ns\nt\n"; + // A 3-line insertion early on, then a PURE DELETION later: the reverse-apply twin of the + // pure-insertion case above. Restoring `q` has an empty preimage, so nothing anchors the + // patch except the header coordinate, and the earlier hunk shifts it by 3. + const SHIFTED_DELETE_HUNKS: &str = + "a\nb\nB2a\nB2b\nB2c\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nr\ns\nt\n"; + const SHIFTED_DELETE_RESTORED: &str = + "a\nb\nB2a\nB2b\nB2c\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nr\ns\nt\n"; + const MIXED_SECOND_HUNK: &str = + "a\nb\nB2\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nQ2\nr\ns\nt\n"; + const MIXED_SECOND_ONLY: &str = + "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nQ2\nr\ns\nt\n"; + const MIXED_FIRST_ONLY: &str = + "a\nb\nB2\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nr\ns\nt\n"; + const MIXED_SELECTED_DELETION_STAGED: &str = + "a\nb\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nr\ns\nt\n"; + const MIXED_SELECTED_DELETION_RESTORED: &str = + "a\nb\nB2\nc\nd\ne\nf\ng\nh\ni\nj\nk\nl\nm\nn\no\np\nq\nQ2\nr\ns\nt\n"; + + fn read_index_file(r: &TempRepo, path: &str) -> String { + let output = Command::new("git") + .current_dir(&r.path) + .arg("show") + .arg(format!(":{}", path)) + .output() + .unwrap(); + assert!( + output.status.success(), + "git show index file: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap() + } + + /// Discard against the diff as it stands right now — the freshness snapshot the UI + /// captures when the user clicks. Tests that exercise the stale-snapshot refusal pass + /// their own `expected_diff` instead. + fn discard_hunk_now(r: &TempRepo, path: &str, hunk_index: usize, context: u32) -> Result<(), String> { + let live = diff(&r.path, Some(path), false, context)?; + discard_hunk(&r.path, path, hunk_index, &live, context) + } + + fn discard_lines_now( + r: &TempRepo, + path: &str, + hunk_index: usize, + selected: &[usize], + context: u32, + ) -> Result<(), String> { + let live = diff(&r.path, Some(path), false, context)?; + discard_lines(&r.path, path, hunk_index, selected, &live, context) + } + + fn repo_with_second_hunk( + edited: &str, + staged: bool, + context: u32, + second_header_prefix: &str, + ) -> TempRepo { + let r = TempRepo::new(); + r.commit_file("matrix.txt", BASE_20, "init"); + r.write("matrix.txt", edited); + if staged { + r.git(&["add", "matrix.txt"]); + } + let d = diff(&r.path, Some("matrix.txt"), staged, context).unwrap(); + let (_header, hunks) = split_hunks(&d); + assert_eq!(hunks.len(), 2, "expected two hunks, diff was:\n{}", d); + assert!( + hunks[1].starts_with(second_header_prefix), + "second hunk must have the expected divergent starts, diff was:\n{}", + d + ); + r + } + + fn assert_operation_content( + r: &TempRepo, + result: Result<(), String>, + expected_index: &str, + expected_worktree: &str, + ) { + let actual_index = read_index_file(r, "matrix.txt"); + let actual_worktree = fs::read_to_string(r.path.join("matrix.txt")).unwrap(); + assert!( + result.is_ok() + && actual_index == expected_index + && actual_worktree == expected_worktree, + "result: {:?}\nindex expected:\n{}index actual:\n{}worktree expected:\n{}worktree actual:\n{}", + result, + expected_index, + actual_index, + expected_worktree, + actual_worktree + ); + } + + fn assert_stage_hunk_case( + edited: &str, + context: u32, + second_header_prefix: &str, + expected_index: &str, + ) { + let r = repo_with_second_hunk(edited, false, context, second_header_prefix); + let result = stage_hunk(&r.path, "matrix.txt", 1, context); + assert_operation_content(&r, result, expected_index, edited); + } + + fn assert_unstage_hunk_case( + edited: &str, + context: u32, + second_header_prefix: &str, + expected_index: &str, + ) { + let r = repo_with_second_hunk(edited, true, context, second_header_prefix); + let result = unstage_hunk(&r.path, "matrix.txt", 1, context); + assert_operation_content(&r, result, expected_index, edited); + } + + fn assert_stage_lines_case( + edited: &str, + context: u32, + second_header_prefix: &str, + selected: &[usize], + expected_index: &str, + ) { + let r = repo_with_second_hunk(edited, false, context, second_header_prefix); + let result = stage_lines(&r.path, "matrix.txt", 1, selected, context); + assert_operation_content(&r, result, expected_index, edited); + } + + fn assert_unstage_lines_case( + edited: &str, + context: u32, + second_header_prefix: &str, + selected: &[usize], + expected_index: &str, + ) { + let r = repo_with_second_hunk(edited, true, context, second_header_prefix); + let result = unstage_lines(&r.path, "matrix.txt", 1, selected, context); + assert_operation_content(&r, result, expected_index, edited); + } + + fn assert_discard_hunk_case( + edited: &str, + context: u32, + second_header_prefix: &str, + expected_worktree: &str, + ) { + let r = repo_with_second_hunk(edited, false, context, second_header_prefix); + let result = discard_hunk_now(&r, "matrix.txt", 1, context); + assert_operation_content(&r, result, BASE_20, expected_worktree); + } + + fn assert_discard_lines_case( + edited: &str, + context: u32, + second_header_prefix: &str, + selected: &[usize], + expected_worktree: &str, + ) { + let r = repo_with_second_hunk(edited, false, context, second_header_prefix); + let result = discard_lines_now(&r, "matrix.txt", 1, selected, context); + assert_operation_content(&r, result, BASE_20, expected_worktree); + } + /// Two added lines; select only ordinal 0. The second add is dropped; new_n reduced by 1. #[test] fn partial_hunk_two_adds_select_first() { let hunk = "@@ -10,3 +10,5 @@\n context\n+add0\n+add1\n context2\n"; - let result = build_partial_hunk(hunk, &set(&[0]), false).expect("should produce patch"); + let result = build_partial_hunk(hunk, &set(&[0]), false).unwrap().expect("should produce patch"); assert!(result.contains("+add0"), "selected add kept"); assert!(!result.contains("+add1"), "unselected add dropped"); // old_n: 2 context lines = 2; new_n: 2 context + 1 kept add = 3 @@ -782,7 +1288,7 @@ mod tests { fn partial_hunk_minus_kept_vs_demoted() { // hunk with two `-` lines (ordinals 0,1); select only 0 let hunk = "@@ -5,4 +5,2 @@\n ctx\n-keep\n-demote\n ctx2\n"; - let result = build_partial_hunk(hunk, &set(&[0]), false).expect("should produce patch"); + let result = build_partial_hunk(hunk, &set(&[0]), false).unwrap().expect("should produce patch"); // "keep" stays as `-keep` assert!(result.contains("-keep"), "kept minus preserved"); // "demote" becomes ` demote` (context) @@ -796,7 +1302,7 @@ mod tests { fn partial_hunk_mixed_select_plus_only() { let hunk = "@@ -12,4 +12,4 @@\n ctx1\n-removed\n+added\n ctx2\n"; // ordinal 0 = `-removed`, ordinal 1 = `+added`; select only 1 - let result = build_partial_hunk(hunk, &set(&[1]), false).expect("should produce patch"); + let result = build_partial_hunk(hunk, &set(&[1]), false).unwrap().expect("should produce patch"); assert!(result.starts_with("@@ -12,3 +12,4 @@\n"), "header: {}", &result); assert!(result.contains(" removed"), "demoted to context"); assert!(!result.contains("-removed"), "not a removal"); @@ -808,17 +1314,20 @@ mod tests { fn partial_hunk_no_newline_marker() { let hunk = "@@ -1,1 +1,1 @@\n-old\n\\ No newline at end of file\n+new\n\\ No newline at end of file\n"; // ordinal 0 = `-old`, ordinal 1 = `+new` - // select only 1 (the add); `-old` becomes context - let result_add_only = build_partial_hunk(hunk, &set(&[1]), false).expect("patch"); - // The no-newline after `-old` becomes context so last_emitted=true → marker kept - // The no-newline after `+new` which is kept → also kept - assert!(result_add_only.contains("\\ No newline"), "marker kept after emitted lines"); - - // Now select only 0 (the remove); `+new` is dropped - let result_rm_only = build_partial_hunk(hunk, &set(&[0]), false).expect("patch"); - // The no-newline after `-old` (which is kept): last_emitted=true → kept - // The no-newline after `+new` (which is dropped): last_emitted=false → dropped - // We expect marker after the kept `-old`, but not a second one after dropped `+new` + + // Select only 1 (the add), so `-old` is demoted to context. This USED to emit a + // patch keeping the marker after that context line, which asserted `old` was + // file-final while `+new` followed it. `git apply --cached` accepts that and + // resolves the contradiction by concatenating: the index became "oldnew", not + // "old\nnew" (verified against real git). One context line cannot end the old + // image and not the new one, so no patch expresses this selection — refuse it. + let err = build_partial_hunk(hunk, &set(&[1]), false).unwrap_err(); + assert!(err.contains("no-newline"), "should name the cause, got: {err}"); + + // Now select only 0 (the remove); `+new` is dropped. The old side genuinely ends + // at `old` with no newline and nothing follows it, so the marker still holds. + let result_rm_only = build_partial_hunk(hunk, &set(&[0]), false).unwrap().expect("patch"); + // Marker after the kept `-old`, but not a second one after the dropped `+new`. let count = result_rm_only.matches("\\ No newline").count(); assert_eq!(count, 1, "only one no-newline marker (after kept line), got: {}", result_rm_only); } @@ -827,14 +1336,14 @@ mod tests { #[test] fn partial_hunk_empty_selection_is_none() { let hunk = "@@ -1,2 +1,3 @@\n ctx\n+add\n ctx2\n"; - assert!(build_partial_hunk(hunk, &set(&[]), false).is_none()); + assert!(build_partial_hunk(hunk, &set(&[]), false).unwrap().is_none()); } /// Count-omitted header `@@ -5 +5 @@` parses old_start as 5. #[test] fn partial_hunk_count_omitted_header() { let hunk = "@@ -5 +5 @@\n+newline\n"; - let result = build_partial_hunk(hunk, &set(&[0]), false).expect("patch"); + let result = build_partial_hunk(hunk, &set(&[0]), false).unwrap().expect("patch"); assert!(result.starts_with("@@ -5,"), "old_start=5: {}", result); } @@ -850,7 +1359,7 @@ mod tests { let hunk = "@@ -10,14 +10,14 @@\n c1\n c2\n c3\n c4\n c5\n c6\n-removed\n+added\n c7\n c8\n c9\n c10\n c11\n c12\n"; // Select only the `+added` (ordinal 1): the unselected `-removed` is demoted to context. - let result = build_partial_hunk(hunk, &set(&[1]), false).expect("should produce patch"); + let result = build_partial_hunk(hunk, &set(&[1]), false).unwrap().expect("should produce patch"); assert!(result.contains("+added"), "selected add kept"); assert!(result.contains(" removed"), "unselected minus demoted to context"); assert!(!result.contains("-removed"), "unselected minus is not a removal"); @@ -878,7 +1387,7 @@ mod tests { assert_eq!(header, expected, "header counts must match emitted body: {}", result); // Select only the `-removed` (ordinal 0): the unselected `+added` is dropped. - let rm_only = build_partial_hunk(hunk, &set(&[0]), false).expect("should produce patch"); + let rm_only = build_partial_hunk(hunk, &set(&[0]), false).unwrap().expect("should produce patch"); assert!(rm_only.contains("-removed"), "selected minus kept as removal"); assert!(!rm_only.contains("+added"), "unselected add dropped"); } @@ -909,6 +1418,588 @@ mod tests { assert!(unstaged.contains("+line3"), "line3 still unstaged"); } + // ── context / multi-hunk operation matrix ─────────────────────────────── + + #[test] + fn context_three_second_hunk_stage_hunk_updates_index_at_exact_position() { + assert_stage_hunk_case(TWO_ADD_HUNKS, 3, "@@ -14,6 +15,7 @@", SECOND_ADD_ONLY); + } + + #[test] + fn context_three_second_hunk_unstage_hunk_updates_index_at_exact_position() { + assert_unstage_hunk_case(TWO_ADD_HUNKS, 3, "@@ -14,6 +15,7 @@", FIRST_ADD_ONLY); + } + + #[test] + fn context_three_second_hunk_stage_lines_updates_index_at_exact_position() { + assert_stage_lines_case(TWO_ADD_HUNKS, 3, "@@ -14,6 +15,7 @@", &[0], SECOND_ADD_ONLY); + } + + #[test] + fn context_three_second_hunk_unstage_lines_updates_index_at_exact_position() { + assert_unstage_lines_case(TWO_ADD_HUNKS, 3, "@@ -14,6 +15,7 @@", &[0], FIRST_ADD_ONLY); + } + + #[test] + fn context_three_second_hunk_discard_hunk_updates_worktree_at_exact_position() { + assert_discard_hunk_case(TWO_ADD_HUNKS, 3, "@@ -14,6 +15,7 @@", FIRST_ADD_ONLY); + } + + #[test] + fn context_three_second_hunk_discard_lines_updates_worktree_at_exact_position() { + assert_discard_lines_case(TWO_ADD_HUNKS, 3, "@@ -14,6 +15,7 @@", &[0], FIRST_ADD_ONLY); + } + + #[test] + fn context_zero_second_hunk_stage_hunk_updates_index_at_exact_position() { + assert_stage_hunk_case(TWO_ADD_HUNKS, 0, "@@ -16,0 +18 @@", SECOND_ADD_ONLY); + } + + #[test] + fn context_zero_second_hunk_unstage_hunk_updates_index_at_exact_position() { + assert_unstage_hunk_case(TWO_ADD_HUNKS, 0, "@@ -16,0 +18 @@", FIRST_ADD_ONLY); + } + + #[test] + fn context_zero_second_hunk_stage_lines_updates_index_at_exact_position() { + assert_stage_lines_case(TWO_ADD_HUNKS, 0, "@@ -16,0 +18 @@", &[0], SECOND_ADD_ONLY); + } + + #[test] + fn context_zero_second_hunk_unstage_lines_updates_index_at_exact_position() { + assert_unstage_lines_case(TWO_ADD_HUNKS, 0, "@@ -16,0 +18 @@", &[0], FIRST_ADD_ONLY); + } + + #[test] + fn context_zero_second_hunk_discard_hunk_updates_worktree_at_exact_position() { + assert_discard_hunk_case(TWO_ADD_HUNKS, 0, "@@ -16,0 +18 @@", FIRST_ADD_ONLY); + } + + #[test] + fn context_zero_second_hunk_discard_lines_updates_worktree_at_exact_position() { + assert_discard_lines_case(TWO_ADD_HUNKS, 0, "@@ -16,0 +18 @@", &[0], FIRST_ADD_ONLY); + } + + #[test] + fn context_zero_mixed_second_hunk_stage_hunk_updates_index_exactly() { + assert_stage_hunk_case(MIXED_SECOND_HUNK, 0, "@@ -17 +18 @@", MIXED_SECOND_ONLY); + } + + #[test] + fn context_zero_mixed_second_hunk_unstage_hunk_updates_index_exactly() { + assert_unstage_hunk_case(MIXED_SECOND_HUNK, 0, "@@ -17 +18 @@", MIXED_FIRST_ONLY); + } + + #[test] + fn context_zero_mixed_second_hunk_stage_lines_stages_only_the_deletion() { + assert_stage_lines_case( + MIXED_SECOND_HUNK, + 0, + "@@ -17 +18 @@", + &[0], + MIXED_SELECTED_DELETION_STAGED, + ); + } + + #[test] + fn context_zero_mixed_second_hunk_unstage_lines_unstages_only_the_deletion() { + assert_unstage_lines_case( + MIXED_SECOND_HUNK, + 0, + "@@ -17 +18 @@", + &[0], + MIXED_SELECTED_DELETION_RESTORED, + ); + } + + #[test] + fn context_zero_mixed_second_hunk_discard_hunk_updates_worktree_exactly() { + assert_discard_hunk_case(MIXED_SECOND_HUNK, 0, "@@ -17 +18 @@", MIXED_FIRST_ONLY); + } + + #[test] + fn context_zero_mixed_second_hunk_discard_lines_restores_only_the_deletion() { + assert_discard_lines_case( + MIXED_SECOND_HUNK, + 0, + "@@ -17 +18 @@", + &[0], + MIXED_SELECTED_DELETION_RESTORED, + ); + } + + // Reverse-direction twin of the ctx-0 pure-insertion cases: the hunk that RESTORES `q` has + // an empty preimage, so `git apply --reverse` places it purely by the header coordinate, and + // the extracted hunk inherits an old-side start that the 3-line earlier insertion has moved. + // Without `reanchor` the line comes back three rows too high — Ok(()), wrong file. + #[test] + fn context_zero_shifted_deletion_discard_hunk_restores_at_exact_position() { + assert_discard_hunk_case(SHIFTED_DELETE_HUNKS, 0, "@@ -17 +19,0 @@", SHIFTED_DELETE_RESTORED); + } + + #[test] + fn context_zero_shifted_deletion_discard_lines_restores_at_exact_position() { + assert_discard_lines_case( + SHIFTED_DELETE_HUNKS, + 0, + "@@ -17 +19,0 @@", + &[0], + SHIFTED_DELETE_RESTORED, + ); + } + + #[test] + fn context_zero_shifted_deletion_unstage_hunk_restores_at_exact_position() { + assert_unstage_hunk_case(SHIFTED_DELETE_HUNKS, 0, "@@ -17 +19,0 @@", SHIFTED_DELETE_RESTORED); + } + + #[test] + fn context_one_three_line_shift_stage_hunk_updates_index_at_exact_position() { + assert_stage_hunk_case( + THREE_LINE_SHIFT_HUNKS, + 1, + "@@ -16,2 +19,3 @@", + SECOND_ADD_ONLY, + ); + } + + #[test] + fn context_one_three_line_shift_unstage_hunk_updates_index_at_exact_position() { + assert_unstage_hunk_case( + THREE_LINE_SHIFT_HUNKS, + 1, + "@@ -16,2 +19,3 @@", + THREE_LINE_SHIFT_FIRST_ONLY, + ); + } + + #[test] + fn context_one_three_line_shift_stage_lines_updates_index_at_exact_position() { + assert_stage_lines_case( + THREE_LINE_SHIFT_HUNKS, + 1, + "@@ -16,2 +19,3 @@", + &[0], + SECOND_ADD_ONLY, + ); + } + + #[test] + fn context_one_three_line_shift_unstage_lines_updates_index_at_exact_position() { + assert_unstage_lines_case( + THREE_LINE_SHIFT_HUNKS, + 1, + "@@ -16,2 +19,3 @@", + &[0], + THREE_LINE_SHIFT_FIRST_ONLY, + ); + } + + #[test] + fn context_one_three_line_shift_discard_hunk_updates_worktree_at_exact_position() { + assert_discard_hunk_case( + THREE_LINE_SHIFT_HUNKS, + 1, + "@@ -16,2 +19,3 @@", + THREE_LINE_SHIFT_FIRST_ONLY, + ); + } + + #[test] + fn context_one_three_line_shift_discard_lines_updates_worktree_at_exact_position() { + assert_discard_lines_case( + THREE_LINE_SHIFT_HUNKS, + 1, + "@@ -16,2 +19,3 @@", + &[0], + THREE_LINE_SHIFT_FIRST_ONLY, + ); + } + + // ── discard (worktree reverse-apply) ───────────────────────────────────── + + #[test] + fn discard_hunk_reverts_only_that_hunk() { + let r = TempRepo::new(); + // 20 lines so two separated edits land in two hunks at -U3. + let base: String = (1..=20).map(|i| format!("line{}\n", i)).collect(); + r.commit_file("f.txt", &base, "init"); + + let mut edited: Vec = (1..=20).map(|i| format!("line{}\n", i)).collect(); + edited[1] = "CHANGED2\n".to_string(); + edited[17] = "CHANGED18\n".to_string(); + r.write("f.txt", &edited.concat()); + + let d = diff(&r.path, Some("f.txt"), false, 3).unwrap(); + let (_h, hunks) = split_hunks(&d); + assert_eq!(hunks.len(), 2, "expected two hunks, diff was:\n{}", d); + + discard_hunk_now(&r, "f.txt", 0, 3).unwrap(); + + let now = fs::read_to_string(r.path.join("f.txt")).unwrap(); + assert!(now.contains("line2\n"), "hunk 0 should be reverted"); + assert!(!now.contains("CHANGED2"), "hunk 0's change should be gone"); + assert!(now.contains("CHANGED18"), "hunk 1 must be untouched"); + } + + #[test] + fn discard_lines_reverts_only_selected() { + let r = TempRepo::new(); + r.commit_file("f.txt", "base\n", "init"); + r.write("f.txt", "base\nline1\nline2\nline3\n"); + // ordinals 0,1,2 == +line1,+line2,+line3 — discard only line2. + discard_lines_now(&r, "f.txt", 0, &[1], 3).unwrap(); + assert_eq!( + fs::read_to_string(r.path.join("f.txt")).unwrap(), + "base\nline1\nline3\n", + "only the selected line should be reverted" + ); + } + + /// The load-bearing one: discarding UNSTAGED lines reverts them to the INDEX + /// state, not to HEAD, so staged work on the same file survives. This is what + /// makes Discard safe to sit beside Stage. + #[test] + fn discard_lines_leaves_staged_changes_intact() { + let r = TempRepo::new(); + r.commit_file("f.txt", "base\n", "init"); + r.write("f.txt", "base\nkeep\ndrop\n"); + + // Stage only `keep` (ordinal 0); `drop` stays unstaged. + stage_lines(&r.path, "f.txt", 0, &[0], 3).unwrap(); + let staged = diff(&r.path, Some("f.txt"), true, 3).unwrap(); + assert!(staged.contains("+keep"), "precondition: keep must be staged"); + + // The unstaged diff now holds exactly one change line (`+drop`) at ordinal 0. + discard_lines_now(&r, "f.txt", 0, &[0], 3).unwrap(); + + assert_eq!( + fs::read_to_string(r.path.join("f.txt")).unwrap(), + "base\nkeep\n", + "drop reverted to the INDEX state, not HEAD" + ); + let still = diff(&r.path, Some("f.txt"), true, 3).unwrap(); + assert!(still.contains("+keep"), "staged work must survive the discard"); + } + + #[test] + fn rejects_a_non_contiguous_selection_instead_of_reordering() { + let r = TempRepo::new(); + r.commit_file("f.txt", "ctx1\nctx2\naaa\nbbb\nccc\nctx3\nctx4\n", "init"); + r.write("f.txt", "ctx1\nctx2\nXXX\nYYY\nZZZ\nctx3\nctx4\n"); + + // Ordinals: -aaa=0 -bbb=1 -ccc=2 +XXX=3 +YYY=4 +ZZZ=5. + // [2,5] is the shape split view's paired-row selection used to emit. + let err = discard_lines_now(&r, "f.txt", 0, &[2, 5], 3).unwrap_err(); + assert!(err.contains("non-contiguous"), "unexpected error: {}", err); + + // The refusal must be total — the worktree is untouched. + assert_eq!( + fs::read_to_string(r.path.join("f.txt")).unwrap(), + "ctx1\nctx2\nXXX\nYYY\nZZZ\nctx3\nctx4\n" + ); + + let err = stage_lines(&r.path, "f.txt", 0, &[0, 3], 3).unwrap_err(); + assert!(err.contains("non-contiguous"), "unexpected error: {}", err); + assert!(r.staged_paths().is_empty(), "nothing may reach the index"); + } + + #[test] + fn contiguous_selections_are_still_accepted() { + let r = TempRepo::new(); + r.commit_file("f.txt", "base\n", "init"); + r.write("f.txt", "base\nl1\nl2\nl3\n"); + // Out of order and with a duplicate — still one contiguous run once normalized. + discard_lines_now(&r, "f.txt", 0, &[2, 1, 1], 3).unwrap(); + assert_eq!( + fs::read_to_string(r.path.join("f.txt")).unwrap(), + "base\nl1\n" + ); + } + + // ── stale diff snapshot (discard TOCTOU) ───────────────────────────────── + + /// Set up the exact race the confirmation dialog opens: the user is shown a two-hunk diff and + /// picks hunk 1 (`Q2`); while the modal sits there something else edits the file, adding a + /// change EARLIER in it. Hunk 1 is still in range — it now names the new edit instead. + /// Returns (repo, the diff the user saw, the file as the user last saw it). + fn repo_with_diff_changed_under_the_dialog() -> (TempRepo, String, String) { + // 30 lines so three edits ~9 apart stay three separate hunks at -U3. + let base: String = (1..=30).map(|i| format!("line{}\n", i)).collect(); + let seen_state = base + .replace("line3\n", "B2\nline3\n") + .replace("line21\n", "Q2\nline21\n"); + + let r = TempRepo::new(); + r.commit_file("f.txt", &base, "init"); + r.write("f.txt", &seen_state); + let shown = diff(&r.path, Some("f.txt"), false, 3).unwrap(); + assert_eq!(split_hunks(&shown).1.len(), 2, "user saw two hunks:\n{}", shown); + + // …meanwhile, an external edit lands between `B2` and `Q2`. + r.write("f.txt", &seen_state.replace("line12\n", "H2\nline12\n")); + let now = diff(&r.path, Some("f.txt"), false, 3).unwrap(); + assert_eq!( + split_hunks(&now).1.len(), + 3, + "index 1 must now name a DIFFERENT hunk:\n{}", + now + ); + (r, shown, seen_state) + } + + /// Stale index 1 would reverse-apply the `H2` hunk — an edit the user never saw, with no + /// reflog to get it back. The snapshot check must refuse and leave the file alone. + #[test] + fn discard_hunk_refuses_a_diff_that_changed_under_the_dialog() { + let (r, shown, _) = repo_with_diff_changed_under_the_dialog(); + let before = read_file(&r, "f.txt"); + + let err = discard_hunk(&r.path, "f.txt", 1, &shown, 3).unwrap_err(); + assert!(err.contains("nothing was discarded"), "unexpected error: {}", err); + assert_eq!( + read_file(&r, "f.txt"), + before, + "a refused discard must not touch the worktree" + ); + } + + #[test] + fn discard_lines_refuses_a_diff_that_changed_under_the_dialog() { + let (r, shown, _) = repo_with_diff_changed_under_the_dialog(); + let before = read_file(&r, "f.txt"); + + let err = discard_lines(&r.path, "f.txt", 1, &[0], &shown, 3).unwrap_err(); + assert!(err.contains("nothing was discarded"), "unexpected error: {}", err); + assert_eq!( + read_file(&r, "f.txt"), + before, + "a refused discard must not touch the worktree" + ); + } + + /// The refusal has to be a retry, not a dead end: re-reading the diff and discarding against + /// THAT succeeds, and hits the hunk the fresh diff actually names. + #[test] + fn discard_succeeds_once_the_snapshot_is_refreshed() { + let (r, shown, seen_state) = repo_with_diff_changed_under_the_dialog(); + assert!(discard_hunk(&r.path, "f.txt", 1, &shown, 3).is_err()); + + discard_hunk_now(&r, "f.txt", 1, 3).unwrap(); + assert_eq!( + read_file(&r, "f.txt"), + seen_state, + "hunk 1 of the FRESH diff is the H2 insertion" + ); + } + + /// The `-` path: discard must put a line BACK, not just remove one. The discard tests + /// that came before this one all used a pure-addition hunk, so this branch of + /// `build_partial_hunk` was only ever exercised against the INDEX by `unstage_lines`, + /// never against the working tree. + #[test] + fn discard_lines_restores_a_deleted_line() { + let r = TempRepo::new(); + r.commit_file("f.txt", "a\nb\nc\n", "init"); + r.write("f.txt", "a\nc\n"); + discard_lines_now(&r, "f.txt", 0, &[0], 3).unwrap(); + assert_eq!(fs::read_to_string(r.path.join("f.txt")).unwrap(), "a\nb\nc\n"); + } + + /// A mixed hunk where only the deletion is discarded: `b` comes back and the + /// addition `B2` stays. Asserts exact file content so an off-by-one in the + /// context anchoring is caught rather than passing a substring check. + #[test] + fn discard_lines_mixed_hunk_restores_only_the_deletion() { + let r = TempRepo::new(); + r.commit_file("f.txt", "a\nb\nc\n", "init"); + r.write("f.txt", "a\nB2\nc\n"); + // Ordinals: -b=0, +B2=1. Discard only the deletion. + discard_lines_now(&r, "f.txt", 0, &[0], 3).unwrap(); + assert_eq!(fs::read_to_string(r.path.join("f.txt")).unwrap(), "a\nb\nB2\nc\n"); + } + + // --------------------------------------------------------------------- + // Partial selections at a no-newline end of file + // + // When the last line of a file with no trailing newline changes, git emits a + // `-`/`+` pair where BOTH sides carry `\ No newline at end of file`. Selecting + // one half of that pair asks for a file where the restored line is no longer + // final — so it must GAIN a trailing newline that the marker denies it. + // + // The two directions are not symmetric. Reversing (discard/unstage) keeps the + // other half as context on the side that still ends there, so dropping the + // stale marker expresses it exactly. Going forward (stage) would need the + // demoted line to be newline-terminated on the new side and not on the old — + // which a single context line cannot say — so that one is refused. + // --------------------------------------------------------------------- + + fn index_content(r: &TempRepo, f: &str) -> String { + let o = Command::new("git") + .current_dir(&r.path) + .args(["cat-file", "-p", &format!(":{}", f)]) + .output() + .unwrap(); + assert!(o.status.success(), "git cat-file failed"); + String::from_utf8_lossy(&o.stdout).to_string() + } + + /// Restoring the deletion must leave `t` and `T2` on SEPARATE lines. Before the + /// fix this returned Ok and silently produced "a\ntT2" — the two lines merged — + /// on the discard path, which has no reflog to recover from. + #[test] + fn discard_lines_at_no_newline_eof_keeps_lines_separate() { + let r = TempRepo::new(); + r.commit_file("f.txt", "a\nt", "init"); + r.write("f.txt", "a\nT2"); + // Ordinals: -t=0, +T2=1. Restore the deletion, keep the addition. + discard_lines_now(&r, "f.txt", 0, &[0], 3).unwrap(); + assert_eq!(read_file(&r, "f.txt"), "a\nt\nT2"); + } + + /// The index twin of the case above: same shape, same merge, same fix. + #[test] + fn unstage_lines_at_no_newline_eof_keeps_lines_separate() { + let r = TempRepo::new(); + r.commit_file("f.txt", "a\nt", "init"); + r.write("f.txt", "a\nT2"); + r.git(&["add", "f.txt"]); + // Ordinals in the STAGED diff: -t=0, +T2=1. Unstage the deletion. + unstage_lines(&r.path, "f.txt", 0, &[0], 3).unwrap(); + assert_eq!(index_content(&r, "f.txt"), "a\nt\nT2"); + } + + /// Staging only the addition cannot be expressed: `t` would have to be + /// newline-terminated in the index and not in HEAD, and one context line cannot + /// carry both. Refuse rather than emit a patch that applies cleanly and corrupts. + #[test] + fn stage_lines_at_no_newline_eof_refuses_rather_than_corrupting() { + let r = TempRepo::new(); + r.commit_file("f.txt", "a\nt", "init"); + r.write("f.txt", "a\nT2"); + // Ordinals: -t=0, +T2=1. Stage only the addition. + let err = stage_lines(&r.path, "f.txt", 0, &[1], 3).unwrap_err(); + assert!( + err.contains("no-newline"), + "error should name the cause, got: {err}" + ); + // Nothing staged, and the working tree is untouched. + assert_eq!(index_content(&r, "f.txt"), "a\nt"); + assert_eq!(read_file(&r, "f.txt"), "a\nT2"); + } + + /// The escape hatch the refusal leaves open: whole-hunk ops replay git's own + /// hunk verbatim, so they are unaffected by any of this. + #[test] + fn whole_hunk_ops_at_no_newline_eof_are_unaffected() { + let r = TempRepo::new(); + r.commit_file("f.txt", "a\nt", "init"); + r.write("f.txt", "a\nT2"); + stage_hunk(&r.path, "f.txt", 0, 3).unwrap(); + assert_eq!(index_content(&r, "f.txt"), "a\nT2"); + + let r2 = TempRepo::new(); + r2.commit_file("f.txt", "a\nt", "init"); + r2.write("f.txt", "a\nT2"); + discard_hunk_now(&r2, "f.txt", 0, 3).unwrap(); + assert_eq!(read_file(&r2, "f.txt"), "a\nt"); + } + + /// `chmod +x` plus an edit makes git put `old mode`/`new mode` in the FILE header, and the + /// whole header is what gets reverse-applied. Discarding text would then also revert the + /// executable bit — a change the confirmation never mentions and the line count never counts. + #[test] + fn discard_lines_leaves_a_mode_change_alone() { + use std::os::unix::fs::PermissionsExt; + let r = TempRepo::new(); + r.commit_file("f.sh", "a\nb\nc\n", "init"); + r.write("f.sh", "a\nB2\nc\n"); + fs::set_permissions(r.path.join("f.sh"), fs::Permissions::from_mode(0o755)).unwrap(); + + // Ordinals: -b=0, +B2=1. Discard only the deletion. + discard_lines_now(&r, "f.sh", 0, &[0], 3).unwrap(); + + assert_eq!(read_file(&r, "f.sh"), "a\nb\nB2\nc\n"); + let mode = fs::metadata(r.path.join("f.sh")).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o755, "the chmod +x must survive a line discard"); + } + + /// `git add -N` makes a path tracked-but-unstaged, so the UI offers line-level discard on it. + /// Its diff header says `new file mode` / `--- /dev/null`, and a partial selection keeps the + /// unselected additions as CONTEXT — giving the patch a non-empty old side the header denies. + /// Git refused the whole thing with "new file n.txt depends on old contents". + #[test] + fn discard_lines_on_an_intent_to_add_file_keeps_the_unselected_lines() { + let r = TempRepo::new(); + r.commit_file("base.txt", "base\n", "init"); + r.write("n.txt", "a\nb\nc\n"); + r.git(&["add", "-N", "--", "n.txt"]); + + // Ordinals: +a=0, +b=1, +c=2. Discard only the first added line. + discard_lines_now(&r, "n.txt", 0, &[0], 3).unwrap(); + + assert_eq!(read_file(&r, "n.txt"), "b\nc\n"); + } + + /// `diff.mnemonicPrefix` renames the path prefixes per-command (`i/`, `w/`), and `diff.noprefix` + /// drops them. This output is not merely displayed — it is fed back to `git apply` — so an + /// unpinned prefix aims the patch at the wrong path. Caught by review: rewriting the `---` side + /// by replacing the first `b/` anywhere turned `w/lib/util.js` into `w/lia/util.js`, which git + /// reads as a rename; it emptied the file the user was editing, rewrote an untouched committed + /// one, and returned Ok. `diff()` now pins `--src-prefix`/`--dst-prefix`. + #[test] + fn discard_lines_is_unaffected_by_diff_prefix_config() { + for (key, value) in [("diff.mnemonicPrefix", "true"), ("diff.noprefix", "true")] { + let r = TempRepo::new(); + r.git(&["config", key, value]); + // A committed neighbour whose name is one byte from the `b/`-mangled form of `lib/…`. + r.git(&["config", "user.email", "t@example.com"]); + r.write("lia.txt", "a\nb\nc\n"); + r.git(&["add", "--", "lia.txt"]); + r.git(&["commit", "-qm", "seed"]); + fs::create_dir_all(r.path.join("lib")).unwrap(); + r.write("lib/util.js", "a\nb\nc\n"); + r.git(&["add", "-N", "--", "lib/util.js"]); + + discard_lines_now(&r, "lib/util.js", 0, &[0], 3) + .unwrap_or_else(|e| panic!("{key}={value}: discard failed: {e}")); + + assert_eq!(read_file(&r, "lib/util.js"), "b\nc\n", "{key}={value}: wrong file content"); + assert_eq!(read_file(&r, "lia.txt"), "a\nb\nc\n", "{key}={value}: neighbour was touched"); + } + } + + /// The other side of that repair: when the selection covers every addition the patch has no + /// old side at all, so the `new file` header is still true and must be left alone — the file + /// goes away, exactly as a whole-hunk discard would do. + #[test] + fn discard_lines_on_an_intent_to_add_file_selecting_all_removes_the_file() { + let r = TempRepo::new(); + r.commit_file("base.txt", "base\n", "init"); + r.write("n.txt", "a\nb\nc\n"); + r.git(&["add", "-N", "--", "n.txt"]); + + discard_lines_now(&r, "n.txt", 0, &[0, 1, 2], 3).unwrap(); + + assert!(!r.path.join("n.txt").exists(), "every added line discarded — file should be gone"); + } + + /// Same leak one function over: `discard_hunk` reverse-applies the same header. + #[test] + fn discard_hunk_leaves_a_mode_change_alone() { + use std::os::unix::fs::PermissionsExt; + let r = TempRepo::new(); + r.commit_file("f.sh", "a\nb\nc\n", "init"); + r.write("f.sh", "a\nB2\nc\n"); + fs::set_permissions(r.path.join("f.sh"), fs::Permissions::from_mode(0o755)).unwrap(); + + discard_hunk_now(&r, "f.sh", 0, 3).unwrap(); + + assert_eq!(read_file(&r, "f.sh"), "a\nb\nc\n"); + let mode = fs::metadata(r.path.join("f.sh")).unwrap().permissions().mode() & 0o777; + assert_eq!(mode, 0o755, "the chmod +x must survive a hunk discard"); + } + #[test] fn working_changes_handles_space_in_path() { let r = TempRepo::new(); @@ -953,7 +2044,7 @@ mod tests { fn partial_hunk_reverse_mixed_select_plus_drops_minus() { // ordinal 0 = `-removed`, ordinal 1 = `+added`; select only 1 (the add) let hunk = "@@ -12,4 +12,4 @@\n ctx1\n-removed\n+added\n ctx2\n"; - let result = build_partial_hunk(hunk, &set(&[1]), true).expect("should produce patch"); + let result = build_partial_hunk(hunk, &set(&[1]), true).unwrap().expect("should produce patch"); // `-removed` must be dropped entirely in reverse mode (absent from staged image) assert!(!result.contains("-removed"), "unselected minus dropped in reverse"); assert!(!result.contains(" removed"), "demoted context must NOT appear in reverse"); diff --git a/docs/superpowers/plans/2026-08-06-diff-hunk-affordance.md b/docs/superpowers/plans/2026-08-06-diff-hunk-affordance.md new file mode 100644 index 0000000..e2e5faf --- /dev/null +++ b/docs/superpowers/plans/2026-08-06-diff-hunk-affordance.md @@ -0,0 +1,1534 @@ +# Diff Hunk/Block Affordance + Line Discard — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add a nested hover affordance (hunk ring + change-block ring with a floating Stage/Discard/Unstage toolbar) to the working-copy diff, add hunk- and line-level Discard, and replace the line-selection visual that is invisible in the NERV theme. + +**Architecture:** Pure block/ordinal math moves to a vitest-covered `src/lib/diff/blocks.ts`. `DiffView.svelte` emits one `` per hunk (so the hunk ring is a plain CSS `outline`) and draws the block/selection ring with four *separate CSS custom properties* feeding one `box-shadow` on every `` — these merge instead of overriding, which is what makes a ring around a run of ``s possible without JS measurement. Only the floating toolbar needs JS, and only for its vertical offset. In Rust, `git_apply` gains a `cached` flag; dropping `--cached` turns the existing reverse-apply into a worktree discard. + +**Tech Stack:** SvelteKit 5 (runes), TypeScript, vitest, Rust (cargo workspace), Tauri 2. + +## Global Constraints + +- **Never `:global()` in `src/lib/theme/nerv.css`** — it is not a Svelte `