diff --git a/crates/tty7-core/src/core/git/diff.rs b/crates/tty7-core/src/core/git/diff.rs index 5bf692a3..209c2991 100644 --- a/crates/tty7-core/src/core/git/diff.rs +++ b/crates/tty7-core/src/core/git/diff.rs @@ -19,15 +19,172 @@ pub const MAX_RENDERED_FILES: usize = 300; pub const MAX_UNTRACKED: usize = 500; +/// `-U`. Git's own default, spelled out because the request carries it. +pub const DEFAULT_CONTEXT: u32 = 3; + #[derive(Clone, Copy, PartialEq, Eq, Debug)] pub enum Truncation { PerFile, Budget, } +/// Which patch to ask git for. +/// +/// The three working-tree variants are the same three questions the SCM panel +/// asks — `Worktree` is what is not staged, `Staged` is what is, and `Head` is +/// both at once, which is what the overlay has always shown. +#[derive(Clone, PartialEq, Eq, Hash, Debug, Default)] +pub enum DiffSource { + /// `git diff` — unstaged changes. + Worktree, + /// `git diff --cached` — what a commit right now would contain. + Staged, + /// `git diff HEAD` — staged and unstaged together. + #[default] + Head, + /// One commit against its first parent. + Commit { rev: String }, + /// `base...head`: what `head` added since the two diverged. + Range { base: String, head: String }, +} + +impl DiffSource { + /// The whole argv, minus pathspecs. Every diff tty7 runs is built here so + /// there is one place to read, and one place to test, what git is asked. + pub fn args(&self, context: u32, ignore_whitespace: bool) -> Vec { + // `core.quotePath=false` is not tidiness: with it on, git renders a + // non-ASCII path in the `diff --git` header as C octal escapes, and + // nothing downstream decodes those — the path would simply be wrong. + let mut argv = strings(&["-c", "core.quotePath=false"]); + match self { + DiffSource::Worktree => argv.push("diff".to_string()), + DiffSource::Staged => argv.extend(strings(&["diff", "--cached"])), + DiffSource::Head => argv.extend(strings(&["diff", "HEAD"])), + // `log -p -1`, not `diff-tree`: `diff-tree` does not honour + // `--first-parent` as a *narrowing* of a merge. Measured on git + // 2.50.1 against a merge of two branches that each added a file: + // `diff-tree -p -m --first-parent` emits both files (one patch per + // parent, concatenated), and dropping `-m` emits nothing at all. + // `log -p -1 --first-parent` gives the one first-parent patch for a + // merge, an ordinary commit and the initial commit alike, so there + // is no special case and no need for `--root`. `--format=` empties + // the commit header, at the cost of one blank line the parser + // ignores. + DiffSource::Commit { rev } => { + argv.extend(strings(&["log", "-p", "-1", "--format=", "--first-parent"])); + argv.push(rev.clone()); + } + // Three dots: measured from the merge base, so unrelated work on + // `base` does not show up as if `head` had reverted it. + DiffSource::Range { base, head } => { + argv.push("diff".to_string()); + argv.push(format!("{base}...{head}")); + } + } + argv.extend(strings(&[ + "--no-color", + "--no-ext-diff", + "--no-textconv", + "-M", + ])); + argv.push(format!("-U{context}")); + if ignore_whitespace { + argv.push("-w".to_string()); + } + argv + } + + /// Whether untracked files belong in the snapshot. They are a property of + /// the working tree, so a commit or a range has none, and a staged diff + /// does not either — an untracked file is by definition not in the index. + pub fn lists_untracked(&self) -> bool { + matches!(self, DiffSource::Worktree | DiffSource::Head) + } +} + +fn strings(args: &[&str]) -> Vec { + args.iter().map(|a| a.to_string()).collect() +} + +/// How much of a patch is worth keeping in memory. +/// +/// The line counts on [`FileDiff`] stay exact past every one of these; only the +/// retained text is bounded. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct DiffBudget { + pub max_lines_per_file: usize, + pub max_total_lines: usize, + pub max_files_with_hunks: usize, +} + +impl DiffBudget { + /// A whole tree at once: no single file may crowd out the rest. + pub const PANEL: DiffBudget = DiffBudget { + max_lines_per_file: MAX_LINES_PER_FILE, + max_total_lines: MAX_TOTAL_LINES, + max_files_with_hunks: MAX_FILES_WITH_HUNKS, + }; + + /// One file the user asked for by name. There is nothing to crowd out, so + /// the per-file cap rises to where it is a defence against a generated + /// file rather than a rationing rule. + pub const SINGLE_FILE: DiffBudget = DiffBudget { + max_lines_per_file: 50_000, + max_total_lines: MAX_TOTAL_LINES, + max_files_with_hunks: MAX_FILES_WITH_HUNKS, + }; +} + +/// The whole point of the second budget. Pinned here so that trimming +/// [`MAX_LINES_PER_FILE`] one day cannot silently make the two identical. +const _: () = + assert!(DiffBudget::SINGLE_FILE.max_lines_per_file > DiffBudget::PANEL.max_lines_per_file); + +impl Default for DiffBudget { + fn default() -> DiffBudget { + DiffBudget::PANEL + } +} + +/// One patch to fetch and parse. +pub struct DiffRequest<'a> { + pub source: DiffSource, + /// Empty means the whole tree. Every entry must already be a pathspec — + /// `:(literal)…` — because git globs a bare path and a file named `a[b].c` + /// would then match nothing. + pub paths: &'a [String], + pub context: u32, + pub budget: DiffBudget, + pub ignore_whitespace: bool, +} + +impl Default for DiffRequest<'_> { + fn default() -> DiffRequest<'static> { + DiffRequest { + source: DiffSource::default(), + paths: &[], + context: DEFAULT_CONTEXT, + budget: DiffBudget::PANEL, + ignore_whitespace: false, + } + } +} + +impl DiffRequest<'_> { + pub fn args(&self) -> Vec { + let mut argv = self.source.args(self.context, self.ignore_whitespace); + if !self.paths.is_empty() { + argv.push("--".to_string()); + argv.extend(self.paths.iter().cloned()); + } + argv + } +} + #[derive(Clone, PartialEq, Eq, Debug, Default)] pub struct DiffSnapshot { pub root: PathBuf, + pub source: DiffSource, pub branch: String, pub files: Vec, pub untracked: Vec, @@ -91,6 +248,12 @@ pub enum FileStatus { Modified, Deleted, Renamed, + Copied, + /// Regular file ↔ symlink ↔ submodule. Not a mode change: `100644` to + /// `100755` is still [`FileStatus::Modified`]. + TypeChanged, + /// A path with conflict markers, or one git refused to diff at all. + Unmerged, } #[derive(Clone, PartialEq, Eq, Debug)] @@ -126,43 +289,66 @@ pub struct DiffLine { pub text: String, } +/// The whole working tree against `HEAD`, on the panel's budget. pub fn probe(host: &dyn Host, cwd: &Path) -> Option { - let root = git::git(host, cwd, &["rev-parse", "--show-toplevel"])?; - let root = PathBuf::from(root.trim_end_matches(['\n', '\r'])); - let branch = git::branch_name(host, cwd)?; - let mut parser = DiffParser::default(); - let diffed = host.git_lines( - cwd, - &["diff", "--no-color", "--no-ext-diff", "-M", "HEAD"], - &mut |line| parser.push_line(line), - ); + probe_diff(host, cwd, &DiffRequest::default()) +} + +pub fn probe_diff(host: &dyn Host, root: &Path, req: &DiffRequest<'_>) -> Option { + let toplevel = git::git(host, root, &["rev-parse", "--show-toplevel"])?; + let toplevel = PathBuf::from(toplevel.trim_end_matches(['\n', '\r'])); + let branch = git::branch_name(host, root)?; + + let argv = req.args(); + let argv: Vec<&str> = argv.iter().map(String::as_str).collect(); + let mut parser = DiffParser::with_budget(req.budget); + let diffed = host.git_lines(root, &argv, &mut |line| parser.push_line(line)); let files = match diffed { Ok(Some(0)) => parser.finish(), _ => Vec::new(), }; + let mut untracked: Vec = Vec::new(); let mut untracked_total = 0usize; - let listed = host.git_lines( - cwd, - &["ls-files", "--others", "--exclude-standard", "--full-name"], - &mut |line| { - untracked_total += 1; - if untracked.len() < MAX_UNTRACKED { - untracked.push(line.to_string()); - } - }, - ); - if !matches!(listed, Ok(Some(0))) { + // A pathspec-limited request is answering "what changed in these files", + // so a list of everything else in the tree would be noise. + let list_untracked = req.source.lists_untracked() && req.paths.is_empty(); + let listed = list_untracked.then(|| { + host.git_lines( + root, + &[ + "-c", + "core.quotePath=false", + "ls-files", + "--others", + "--exclude-standard", + "--full-name", + ], + &mut |line| { + untracked_total += 1; + if untracked.len() < MAX_UNTRACKED { + untracked.push(line.to_string()); + } + }, + ) + }); + let untracked_ok = match &listed { + Some(listed) => matches!(listed, Ok(Some(0))), + None => true, + }; + if !untracked_ok { untracked.clear(); untracked_total = 0; } + Some(DiffSnapshot { - root, + root: toplevel, + source: req.source.clone(), branch, files, untracked, untracked_total, - read_failed: !matches!(diffed, Ok(Some(0))) || !matches!(listed, Ok(Some(0))), + read_failed: !matches!(diffed, Ok(Some(0))) || !untracked_ok, }) } @@ -177,6 +363,7 @@ pub fn parse_unified(out: &str) -> Vec { #[derive(Default)] pub struct DiffParser { + budget: DiffBudget, files: Vec, old_no: u32, new_no: u32, @@ -184,26 +371,44 @@ pub struct DiffParser { total_lines: usize, files_with_hunks: usize, in_hunk: bool, + /// How many marker columns the current hunk's body lines carry: one for an + /// ordinary patch, one per parent for a combined (`diff --cc`) one. + markers: usize, + old_mode: String, } impl DiffParser { + pub fn with_budget(budget: DiffBudget) -> DiffParser { + DiffParser { + budget, + ..DiffParser::default() + } + } + pub fn push_line(&mut self, line: &str) { if let Some(rest) = line.strip_prefix("diff --git ") { let (old_p, new_p) = parse_git_header_paths(rest); - self.files.push(FileDiff { - path: new_p.clone(), - old_path: (old_p != new_p).then_some(old_p), - status: FileStatus::Modified, - added: 0, - removed: 0, - binary: false, - truncated: None, - hunks: Vec::new(), - }); - self.file_lines = 0; - self.in_hunk = false; + self.start_file(new_p.clone(), (old_p != new_p).then_some(old_p), None); return; } + // A conflicted path comes out as a combined diff against every merge + // parent at once, which is git's only way of saying "unmerged" in a + // patch — the `diff --git` header has no status field. + if let Some(rest) = line + .strip_prefix("diff --cc ") + .or_else(|| line.strip_prefix("diff --combined ")) + { + let path = parse_combined_header_path(rest); + self.start_file(path, None, Some(FileStatus::Unmerged)); + return; + } + // `git diff --cached` cannot show a conflicted path at all and says so + // on its own line, before any patch. + if let Some(rest) = line.strip_prefix("* Unmerged path ") { + self.start_file(rest.to_string(), None, Some(FileStatus::Unmerged)); + return; + } + let old_mode = std::mem::take(&mut self.old_mode); let Some(file) = self.files.last_mut() else { return; }; @@ -219,6 +424,20 @@ impl DiffParser { file.status = FileStatus::Renamed; return; } + if line.starts_with("copy from ") { + file.status = FileStatus::Copied; + return; + } + if let Some(mode) = line.strip_prefix("old mode ") { + self.old_mode = mode.trim().to_string(); + return; + } + if let Some(mode) = line.strip_prefix("new mode ") { + if !old_mode.is_empty() && object_type(&old_mode) != object_type(mode.trim()) { + file.status = FileStatus::TypeChanged; + } + return; + } if line.starts_with("Binary files ") || line.starts_with("GIT binary patch") { file.binary = true; return; @@ -231,12 +450,16 @@ impl DiffParser { } if line.starts_with("@@") { self.in_hunk = true; + // `@@` is one marker column, `@@@` two — a combined diff carries + // one per merge parent. Set before any early return: the line + // counts below stay exact even for a file whose body was dropped. + self.markers = line.bytes().take_while(|b| *b == b'@').count().max(2) - 1; if file.truncated.is_some() { return; } let first_hunk = file.hunks.is_empty(); - if (first_hunk && self.files_with_hunks >= MAX_FILES_WITH_HUNKS) - || self.total_lines >= MAX_TOTAL_LINES + if (first_hunk && self.files_with_hunks >= self.budget.max_files_with_hunks) + || self.total_lines >= self.budget.max_total_lines { file.truncated = Some(Truncation::Budget); return; @@ -256,11 +479,8 @@ impl DiffParser { if !self.in_hunk { return; } - let (kind, text) = match line.as_bytes().first() { - Some(b'+') => (LineKind::Added, &line[1..]), - Some(b'-') => (LineKind::Removed, &line[1..]), - Some(b' ') => (LineKind::Context, &line[1..]), - _ => return, + let Some((kind, text)) = split_body_line(line, self.markers) else { + return; }; match kind { LineKind::Added => file.added += 1, @@ -271,11 +491,11 @@ impl DiffParser { return; } self.file_lines += 1; - if self.file_lines > MAX_LINES_PER_FILE { + if self.file_lines > self.budget.max_lines_per_file { file.truncated = Some(Truncation::PerFile); return; } - if self.total_lines >= MAX_TOTAL_LINES { + if self.total_lines >= self.budget.max_total_lines { file.truncated = Some(Truncation::Budget); return; } @@ -310,8 +530,76 @@ impl DiffParser { } pub fn finish(self) -> Vec { - self.files + fold_type_changes(self.files) } + + fn start_file(&mut self, path: String, old_path: Option, status: Option) { + self.files.push(FileDiff { + path, + old_path, + status: status.unwrap_or(FileStatus::Modified), + added: 0, + removed: 0, + binary: false, + truncated: None, + hunks: Vec::new(), + }); + self.file_lines = 0; + self.in_hunk = false; + self.markers = 1; + self.old_mode.clear(); + } +} + +/// A path whose *type* changed is not one patch: git emits the deletion and the +/// creation back to back, and the header of neither says why. The adjacent pair +/// over one path is the whole signal — git has no other reason to produce it — +/// so it is folded back into the single entry the user thinks of. +fn fold_type_changes(files: Vec) -> Vec { + let mut folded: Vec = Vec::with_capacity(files.len()); + for file in files { + let pair = folded.last().is_some_and(|prev| { + prev.status == FileStatus::Deleted + && file.status == FileStatus::Added + && prev.path == file.path + }); + match folded.last_mut().filter(|_| pair) { + Some(prev) => { + prev.status = FileStatus::TypeChanged; + prev.added += file.added; + prev.removed += file.removed; + prev.binary |= file.binary; + prev.truncated = prev.truncated.or(file.truncated); + prev.hunks.extend(file.hunks); + } + None => folded.push(file), + } + } + folded +} + +/// The `100`/`120`/`160` of a git file mode: regular file, symlink, submodule. +/// Permission bits are deliberately dropped — `100644` to `100755` is a +/// modification, not a type change. +fn object_type(mode: &str) -> &str { + &mode[..mode.len().min(3)] +} + +/// Splits a hunk body line into its kind and its text, given how many marker +/// columns the hunk carries. A combined diff marks a line per parent; one `+` +/// or `-` anywhere in those columns settles what happened to the line. +fn split_body_line(line: &str, markers: usize) -> Option<(LineKind, &str)> { + let head = line.get(..markers)?; + let kind = if head.contains('+') { + LineKind::Added + } else if head.contains('-') { + LineKind::Removed + } else if head.bytes().all(|b| b == b' ') { + LineKind::Context + } else { + return None; + }; + Some((kind, &line[markers..])) } fn is_hunk_line(line: &str) -> bool { @@ -366,13 +654,34 @@ fn strip_prefix_ab(p: &str) -> String { .to_string() } +/// A combined hunk header carries one `-` range per merge parent before the +/// single `+` range, so the ranges are read positionally rather than by a fixed +/// shape. The first `-` is the first parent's, which is the side tty7 numbers. fn parse_hunk_starts(line: &str) -> Option<(u32, u32)> { - let rest = line.strip_prefix("@@ -")?; - let (old_part, rest) = rest.split_once(" +")?; - let (new_part, _) = rest.split_once(" @@")?; - let old = old_part.split(',').next()?.parse().ok()?; - let new = new_part.split(',').next()?.parse().ok()?; - Some((old, new)) + let ranges = line.trim_start_matches('@'); + let ranges = &ranges[..ranges.find(" @@")?]; + let mut old = None; + let mut new = None; + for range in ranges.split_whitespace() { + let (sign, rest) = range.split_at_checked(1)?; + let start: u32 = rest.split(',').next()?.parse().ok()?; + match sign { + "-" if old.is_none() => old = Some(start), + "+" => new = Some(start), + _ => {} + } + } + Some((old?, new?)) +} + +/// `diff --cc `: one path, repo-relative, with no `a/` or `b/` prefix. +fn parse_combined_header_path(rest: &str) -> String { + match rest.starts_with('"') { + true => parse_quoted_pair(rest) + .pop() + .unwrap_or_else(|| rest.to_string()), + false => rest.to_string(), + } } #[cfg(test)] @@ -446,6 +755,176 @@ Binary files a/img.png and b/img.png differ assert!(b.hunks.is_empty()); } + fn argv(source: DiffSource) -> Vec { + source.args(DEFAULT_CONTEXT, false) + } + + #[test] + fn every_source_spells_out_its_own_argv() { + let common = ["--no-color", "--no-ext-diff", "--no-textconv", "-M", "-U3"]; + let cases: Vec<(DiffSource, Vec<&str>)> = vec![ + (DiffSource::Worktree, vec!["diff"]), + (DiffSource::Staged, vec!["diff", "--cached"]), + (DiffSource::Head, vec!["diff", "HEAD"]), + ( + DiffSource::Commit { + rev: "deadbeef".into(), + }, + vec!["log", "-p", "-1", "--format=", "--first-parent", "deadbeef"], + ), + ( + DiffSource::Range { + base: "main".into(), + head: "topic".into(), + }, + vec!["diff", "main...topic"], + ), + ]; + for (source, middle) in cases { + let want: Vec = ["-c", "core.quotePath=false"] + .into_iter() + .chain(middle) + .chain(common) + .map(str::to_string) + .collect(); + assert_eq!(argv(source.clone()), want, "{source:?}"); + } + } + + #[test] + fn quote_path_is_off_on_every_source() { + // Left on, a non-ASCII path arrives as C octal escapes that nothing + // downstream decodes — `parse_quoted_pair` would hand back the literal + // digits. Verified against git 2.50.1: `diff --git + // "a/\344\270\255\346\226\207\345\220\215.txt" …` becomes + // `diff --git a/中文名.txt b/中文名.txt` once this is off. + for source in [ + DiffSource::Worktree, + DiffSource::Staged, + DiffSource::Head, + DiffSource::Commit { rev: "HEAD".into() }, + DiffSource::Range { + base: "a".into(), + head: "b".into(), + }, + ] { + assert_eq!(&argv(source.clone())[..2], ["-c", "core.quotePath=false"]); + } + } + + #[test] + fn octal_escaped_paths_are_what_the_flag_prevents() { + let escaped = parse_unified( + "diff --git \"a/\\344\\270\\255\\346\\226\\207\\345\\220\\215.txt\" \ + \"b/\\344\\270\\255\\346\\226\\207\\345\\220\\215.txt\"\n", + ); + assert_ne!( + escaped[0].path, "中文名.txt", + "the escapes are not decoded here, which is why they must not be produced" + ); + + let raw = parse_unified("diff --git a/中文名.txt b/中文名.txt\n"); + assert_eq!(raw[0].path, "中文名.txt"); + } + + #[test] + fn a_request_appends_its_pathspecs_after_a_separator() { + let paths = [":(literal)src/a[b].rs".to_string()]; + let req = DiffRequest { + source: DiffSource::Staged, + paths: &paths, + context: 0, + budget: DiffBudget::SINGLE_FILE, + ignore_whitespace: true, + }; + assert_eq!( + req.args(), + [ + "-c", + "core.quotePath=false", + "diff", + "--cached", + "--no-color", + "--no-ext-diff", + "--no-textconv", + "-M", + "-U0", + "-w", + "--", + ":(literal)src/a[b].rs", + ] + ); + assert_eq!( + DiffRequest::default() + .args() + .iter() + .filter(|a| *a == "--") + .count(), + 0, + "a whole-tree request has nothing to separate" + ); + } + + #[test] + fn the_default_request_is_the_panel_probe() { + let req = DiffRequest::default(); + assert_eq!(req.source, DiffSource::Head); + assert_eq!(req.budget, DiffBudget::PANEL); + assert_eq!(req.args(), argv(DiffSource::Head)); + } + + #[test] + fn a_default_snapshot_still_reads_as_head() { + let snap = DiffSnapshot::default(); + assert_eq!( + snap.source, + DiffSource::Head, + "`..Default::default()` callers keep the source they always had" + ); + } + + #[test] + fn the_single_file_budget_only_lifts_the_per_file_cap() { + assert_eq!(DiffBudget::PANEL, DiffBudget::default()); + assert_eq!( + DiffBudget::SINGLE_FILE.max_total_lines, + DiffBudget::PANEL.max_total_lines + ); + assert_eq!( + DiffBudget::SINGLE_FILE.max_files_with_hunks, + DiffBudget::PANEL.max_files_with_hunks + ); + + let mut out = String::from( + "diff --git a/big.txt b/big.txt\nindex 1..2 100644\n--- a/big.txt\n+++ b/big.txt\n@@ -0,0 +1,3000 @@\n", + ); + for i in 0..3000 { + out.push_str(&format!("+line {i}\n")); + } + let mut parser = DiffParser::with_budget(DiffBudget::SINGLE_FILE); + for line in out.lines() { + parser.push_line(line); + } + let files = parser.finish(); + assert_eq!(files[0].truncated, None, "3000 lines fit under 50_000"); + assert_eq!(files[0].hunks[0].lines.len(), 3000); + } + + #[test] + fn untracked_files_belong_to_the_working_tree_only() { + assert!(DiffSource::Worktree.lists_untracked()); + assert!(DiffSource::Head.lists_untracked()); + assert!(!DiffSource::Staged.lists_untracked()); + assert!(!DiffSource::Commit { rev: "x".into() }.lists_untracked()); + assert!( + !DiffSource::Range { + base: "a".into(), + head: "b".into() + } + .lists_untracked() + ); + } + #[test] fn parses_renames() { let out = "\ @@ -462,6 +941,116 @@ rename to new/name.rs assert_eq!((files[0].added, files[0].removed), (0, 0)); } + #[test] + fn parses_copies() { + let out = "\ +diff --git a/tpl.rs b/copy.rs +similarity index 100% +copy from tpl.rs +copy to copy.rs +"; + let files = parse_unified(out); + assert_eq!(files[0].status, FileStatus::Copied); + assert_eq!(files[0].path, "copy.rs"); + assert_eq!(files[0].old_path.as_deref(), Some("tpl.rs")); + } + + #[test] + fn a_type_change_is_one_file_not_a_delete_and_an_add() { + // git 2.50.1, `git diff` over a regular file replaced by a symlink. + let out = "\ +diff --git a/t.txt b/t.txt +deleted file mode 100644 +index 587be6b..0000000 +--- a/t.txt ++++ /dev/null +@@ -1 +0,0 @@ +-x +diff --git a/t.txt b/t.txt +new file mode 120000 +index 0000000..1de5659 +--- /dev/null ++++ b/t.txt +@@ -0,0 +1 @@ ++target +\\ No newline at end of file +"; + let files = parse_unified(out); + assert_eq!(files.len(), 1, "one path, one row"); + assert_eq!(files[0].status, FileStatus::TypeChanged); + assert_eq!((files[0].added, files[0].removed), (1, 1)); + assert_eq!(files[0].hunks.len(), 2, "both halves stay readable"); + } + + #[test] + fn a_permission_change_is_still_a_modification() { + let files = parse_unified("diff --git a/t.sh b/t.sh\nold mode 100644\nnew mode 100755\n"); + assert_eq!(files[0].status, FileStatus::Modified); + } + + #[test] + fn a_mode_change_across_object_types_is_a_type_change() { + let files = parse_unified("diff --git a/t b/t\nold mode 100644\nnew mode 120000\n"); + assert_eq!(files[0].status, FileStatus::TypeChanged); + } + + #[test] + fn a_delete_and_an_add_of_different_paths_stay_two_files() { + let files = parse_unified( + "diff --git a/gone.txt b/gone.txt\ndeleted file mode 100644\n\ + diff --git a/new.txt b/new.txt\nnew file mode 100644\n", + ); + assert_eq!(files.len(), 2); + assert_eq!(files[0].status, FileStatus::Deleted); + assert_eq!(files[1].status, FileStatus::Added); + } + + #[test] + fn a_conflicted_file_parses_as_a_combined_diff() { + // git 2.50.1, `git diff` during a conflicted merge. Two marker columns, + // and a hunk header with one range per parent. + let out = "\ +diff --cc f.txt +index af70335,f794161..0000000 +--- a/f.txt ++++ b/f.txt +@@@ -1,3 -1,3 +1,7 @@@ + a +++<<<<<<< HEAD + +MAIN +++======= ++ SIDE +++>>>>>>> side + c +"; + let files = parse_unified(out); + assert_eq!(files.len(), 1); + assert_eq!(files[0].path, "f.txt"); + assert_eq!(files[0].status, FileStatus::Unmerged); + + let lines = &files[0].hunks[0].lines; + assert_eq!(lines.len(), 7); + assert_eq!(lines[0].kind, LineKind::Context); + assert_eq!(lines[0].text, "a", "both marker columns come off the text"); + assert_eq!((lines[0].old_no, lines[0].new_no), (Some(1), Some(1))); + assert_eq!(lines[1].kind, LineKind::Added); + assert_eq!(lines[1].text, "<<<<<<< HEAD"); + assert_eq!(lines[4].kind, LineKind::Added); + assert_eq!(lines[4].text, "SIDE", "added on one side is still added"); + assert_eq!(lines[6].text, "c"); + assert_eq!((files[0].added, files[0].removed), (5, 0)); + } + + #[test] + fn a_staged_diff_reports_the_paths_it_refused_to_show() { + let files = parse_unified("* Unmerged path f.txt\ndiff --git a/ok.rs b/ok.rs\n"); + assert_eq!(files.len(), 2); + assert_eq!(files[0].path, "f.txt"); + assert_eq!(files[0].status, FileStatus::Unmerged); + assert!(files[0].hunks.is_empty()); + assert_eq!(files[1].path, "ok.rs"); + } + #[test] fn parses_quoted_paths() { let out = "diff --git \"a/has space.txt\" \"b/has space.txt\"\n"; @@ -655,6 +1244,200 @@ index 1..2 100644 ); } + /// A repo with a merge whose two parents each added a file, plus an + /// ordinary commit and an initial one. Returns `None` if git is missing. + fn merge_repo(name: &str) -> Option { + let dir = std::env::temp_dir().join(format!("tty7-diff-{name}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).ok()?; + let run = |args: &[&str]| { + git::git_output(&dir, args) + .ok() + .filter(|out| out.success()) + .is_some() + }; + if !run(&["init", "-q", "-b", "mainwork", "."]) { + return None; + } + run(&["config", "user.email", "t@tty7.test"]); + run(&["config", "user.name", "tty7 test"]); + std::fs::write(dir.join("base.txt"), "base\n").ok()?; + run(&["add", "-A"]); + run(&["commit", "-qm", "initial"]); + run(&["checkout", "-q", "-b", "side"]); + std::fs::write(dir.join("s.txt"), "side\n").ok()?; + run(&["add", "-A"]); + run(&["commit", "-qm", "side"]); + run(&["checkout", "-q", "mainwork"]); + std::fs::write(dir.join("m.txt"), "main\n").ok()?; + run(&["add", "-A"]); + run(&["commit", "-qm", "main"]); + run(&["merge", "-q", "--no-ff", "-m", "merge side", "side"]); + Some(dir) + } + + fn rev(dir: &Path, spec: &str) -> String { + git::git_output(dir, &["rev-parse", spec]) + .map(|out| String::from_utf8_lossy(&out.stdout).trim().to_string()) + .unwrap_or_default() + } + + fn commit_files(host: &dyn Host, dir: &Path, spec: &str) -> Vec { + let req = DiffRequest { + source: DiffSource::Commit { + rev: rev(dir, spec), + }, + ..Default::default() + }; + probe_diff(host, dir, &req) + .expect("the scratch repo answers") + .files + .into_iter() + .map(|f| f.path) + .collect() + } + + /// The reason `Commit` runs `log -p -1 --first-parent` and not `diff-tree`: + /// on git 2.50.1 `diff-tree -p -m --first-parent` emits *both* parents' + /// patches concatenated, and without `-m` it emits nothing for a merge at + /// all. Either would show a file the merge did not touch on this side. + #[test] + fn a_merge_commit_yields_only_its_first_parent_patch() { + let Some(dir) = merge_repo("merge") else { + return; + }; + let host = crate::host::local::LocalHost::new(); + + assert_eq!(commit_files(&*host, &dir, "HEAD"), ["s.txt"]); + assert_eq!(commit_files(&*host, &dir, "HEAD^"), ["m.txt"]); + assert_eq!( + commit_files(&*host, &dir, "HEAD^^"), + ["base.txt"], + "the initial commit diffs against the empty tree, not an error" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn the_three_working_tree_sources_split_staged_from_unstaged() { + let Some(dir) = merge_repo("sources") else { + return; + }; + let host = crate::host::local::LocalHost::new(); + std::fs::write(dir.join("base.txt"), "base\nstaged\n").unwrap(); + let _ = git::git_output(&dir, &["add", "base.txt"]); + std::fs::write(dir.join("m.txt"), "main\nunstaged\n").unwrap(); + std::fs::write(dir.join("fresh.txt"), "new\n").unwrap(); + + let files = |source: DiffSource| -> Vec { + let req = DiffRequest { + source, + ..Default::default() + }; + probe_diff(&*host, &dir, &req) + .expect("the scratch repo answers") + .files + .into_iter() + .map(|f| f.path) + .collect() + }; + assert_eq!(files(DiffSource::Staged), ["base.txt"]); + assert_eq!(files(DiffSource::Worktree), ["m.txt"]); + assert_eq!(files(DiffSource::Head), ["base.txt", "m.txt"]); + + let staged = probe_diff( + &*host, + &dir, + &DiffRequest { + source: DiffSource::Staged, + ..Default::default() + }, + ) + .unwrap(); + assert!( + staged.untracked.is_empty(), + "an untracked file is by definition not staged" + ); + assert_eq!(probe(&*host, &dir).unwrap().untracked, ["fresh.txt"]); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_non_ascii_path_survives_the_round_trip() { + let Some(dir) = merge_repo("utf8") else { + return; + }; + let host = crate::host::local::LocalHost::new(); + std::fs::write(dir.join("中文名.txt"), "one\n").unwrap(); + let _ = git::git_output(&dir, &["add", "-A"]); + + let snap = probe_diff( + &*host, + &dir, + &DiffRequest { + source: DiffSource::Staged, + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + snap.files + .iter() + .map(|f| f.path.as_str()) + .collect::>(), + ["中文名.txt"], + "octal escapes would have made this `344\\270\\255…`" + ); + + std::fs::write(dir.join("未跟踪.txt"), "two\n").unwrap(); + assert!( + probe(&*host, &dir) + .unwrap() + .untracked + .contains(&"未跟踪.txt".to_string()), + "ls-files quotes the same way, and is unquoted the same way" + ); + + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn a_pathspec_narrows_the_patch_and_drops_the_untracked_list() { + let Some(dir) = merge_repo("pathspec") else { + return; + }; + let host = crate::host::local::LocalHost::new(); + std::fs::write(dir.join("base.txt"), "base\nedit\n").unwrap(); + std::fs::write(dir.join("m.txt"), "main\nedit\n").unwrap(); + std::fs::write(dir.join("fresh.txt"), "new\n").unwrap(); + + let paths = [":(literal)m.txt".to_string()]; + let snap = probe_diff( + &*host, + &dir, + &DiffRequest { + source: DiffSource::Worktree, + paths: &paths, + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + snap.files + .iter() + .map(|f| f.path.as_str()) + .collect::>(), + ["m.txt"] + ); + assert!(snap.untracked.is_empty()); + assert!(!snap.read_failed); + assert_eq!(snap.source, DiffSource::Worktree); + + let _ = std::fs::remove_dir_all(&dir); + } + #[test] #[ignore = "measurement, not an assertion"] fn bench_stream_vs_buffer() { diff --git a/src/ui/diff_overlay.rs b/src/ui/diff_overlay.rs index da52eaa7..17bd7aa8 100644 --- a/src/ui/diff_overlay.rs +++ b/src/ui/diff_overlay.rs @@ -9,14 +9,21 @@ use gpui_component::button::Button; use gpui_component::{ActiveTheme as _, Icon, IconName, Sizable as _, h_flex, v_flex}; use crate::terminal::git_diff::{ - self, AUTO_COLLAPSE_LINES, DiffSnapshot, DiffStats, FileDiff, FileStatus, LineKind, + self, AUTO_COLLAPSE_LINES, DiffSnapshot, DiffSource, DiffStats, FileDiff, FileStatus, MAX_RENDERED_FILES, Truncation, }; use crate::ui::app::Tty7App; +use crate::ui::diff_rows::{Side, SplitCell, SplitRow, split_hunk}; use crate::ui::i18n::{L10nKey, t, t_fmt, t_plural}; use crate::ui::rounding; use crate::ui::rounding::RoundedCorners as _; +/// What the right panel's shared probe still asks git for, and therefore what +/// an overlay opened from the panel has to ask for too: the seed below is only +/// sound while the two agree. Both move together when the panel splits into +/// staged and unstaged groups. +const PANEL_DIFF_SOURCE: DiffSource = DiffSource::Head; + pub(crate) enum DiffLoad { Loading, Ready(Arc), @@ -26,11 +33,15 @@ pub(crate) enum DiffLoad { pub(crate) struct DiffOverlayState { pub(crate) host_id: crate::ui::host_ops::HostId, pub(crate) cwd: PathBuf, + /// Which patch this overlay is showing. Part of its identity, not a + /// setting: two sources over one directory are two different overlays. + pub(crate) source: DiffSource, pub(crate) focus_handle: FocusHandle, pub(crate) load: DiffLoad, pub(crate) loading: bool, pub(crate) expanded: HashMap, pub(crate) focus: Option, + pub(crate) scroll: gpui::ScrollHandle, } impl Tty7App { @@ -51,6 +62,18 @@ impl Tty7App { focus: Option, window: &mut Window, cx: &mut Context, + ) { + self.open_diff_overlay(host, cwd, PANEL_DIFF_SOURCE, focus, window, cx) + } + + pub(crate) fn open_diff_overlay( + &mut self, + host: crate::ui::host_ops::HostId, + cwd: PathBuf, + source: DiffSource, + focus: Option, + window: &mut Window, + cx: &mut Context, ) { let active = self.active; let was_front = self.tabs.get(active).is_some_and(|t| { @@ -59,11 +82,14 @@ impl Tty7App { if let Some(tab) = self.tabs.get_mut(active) { tab.overlay_top = crate::ui::app::OverlayTop::Diff; } + // The source belongs in this filter: an open worktree overlay reused + // for a staged file would only move the focus, and go on showing the + // unstaged patch under the staged file's name. match self .tabs .get_mut(active) .and_then(|t| t.diff_overlay.as_mut()) - .filter(|o| o.cwd == cwd && o.host_id == host) + .filter(|o| o.cwd == cwd && o.host_id == host && o.source == source) { Some(o) if o.focus == focus && was_front => { self.close_diff_overlay(window, cx); @@ -78,8 +104,12 @@ impl Tty7App { } None => {} } + // Skipping the "Reading…" flash is only allowed when the panel's + // snapshot answers the same question this overlay is asking. let seed = match (&self.right_panel.diff_cwd, &self.right_panel.diff) { - (Some(panel_key), Some(Some(snap))) if *panel_key == (host, cwd.clone()) => { + (Some(panel_key), Some(Some(snap))) + if source == PANEL_DIFF_SOURCE && *panel_key == (host, cwd.clone()) => + { DiffLoad::Ready(Arc::clone(snap)) } _ => DiffLoad::Loading, @@ -92,11 +122,13 @@ impl Tty7App { tab.diff_overlay = Some(DiffOverlayState { host_id: host, cwd, + source, focus_handle: focus_handle.clone(), load: seed, loading: false, expanded: HashMap::new(), focus, + scroll: gpui::ScrollHandle::new(), }); window.focus(&focus_handle, cx); self.spawn_diff_probe(cx); @@ -137,12 +169,13 @@ impl Tty7App { return; } let cwd = overlay.cwd.clone(); + let source = overlay.source.clone(); let id = overlay.host_id; let Some(host) = crate::ui::host_registry::HostRegistry::lookup(cx, id) else { return; }; overlay.loading = true; - self.spawn_shared_diff_probe(host, cwd, cx); + self.spawn_diff_probe_for(host, cwd, source, cx); } pub(crate) fn spawn_shared_diff_probe( @@ -151,22 +184,40 @@ impl Tty7App { cwd: PathBuf, cx: &mut Context, ) { - let key = (host.id(), cwd.clone()); + self.spawn_diff_probe_for(host, cwd, PANEL_DIFF_SOURCE, cx) + } + + pub(crate) fn spawn_diff_probe_for( + &mut self, + host: crate::ui::host_ops::SharedHost, + cwd: PathBuf, + source: DiffSource, + cx: &mut Context, + ) { + let key = probe_key(host.id(), &cwd, &source); if !self.diff_probes_inflight.insert(key.clone()) { self.diff_probes_restale.insert(key); return; } let host_for_retry = host.clone(); let probe_cwd = cwd.clone(); + let probe_source = source.clone(); crate::ui::host_ops::HostOps::run( host, cx, - move |h| git_diff::probe(h, &probe_cwd), + move |h| { + let req = git_diff::DiffRequest { + source: probe_source, + ..Default::default() + }; + git_diff::probe_diff(h, &probe_cwd, &req) + }, move |app, result, cx| { + let id = key.0; app.diff_probes_inflight.remove(&key); - app.install_diff_snapshot(key.0, &cwd, result.map(Arc::new), cx); - if app.diff_probes_restale.remove(&(key.0, cwd.clone())) { - app.spawn_shared_diff_probe(host_for_retry, cwd, cx); + app.install_diff_snapshot(id, &cwd, &source, result.map(Arc::new), cx); + if app.diff_probes_restale.remove(&key) { + app.spawn_diff_probe_for(host_for_retry, cwd, source, cx); } }, ); @@ -176,6 +227,7 @@ impl Tty7App { &mut self, host: crate::ui::host_ops::HostId, cwd: &Path, + source: &DiffSource, snap: Option>, cx: &mut Context, ) { @@ -184,7 +236,7 @@ impl Tty7App { let Some(overlay) = tab .diff_overlay .as_mut() - .filter(|o| o.cwd == cwd && o.host_id == host) + .filter(|o| o.cwd == cwd && o.host_id == host && o.source == *source) else { continue; }; @@ -195,6 +247,12 @@ impl Tty7App { }; landed = true; } + if *source != PANEL_DIFF_SOURCE { + if landed { + cx.notify(); + } + return; + } let key = (host, cwd.to_path_buf()); if self.right_panel.diff_pending.as_ref() == Some(&key) { self.right_panel.diff_pending = None; @@ -220,6 +278,13 @@ impl Tty7App { if overlay.loading { return; } + // The cached counts come from `git diff --numstat HEAD`, so only a + // HEAD snapshot is comparable to them. A staged or unstaged snapshot + // would differ the moment anything is staged, and re-probe forever; a + // commit or a range cannot go stale at all. + if overlay.source != DiffSource::Head { + return; + } let DiffLoad::Ready(snap) = &overlay.load else { return; }; @@ -250,9 +315,13 @@ impl Tty7App { DiffLoad::Ready(snap) if empty_snapshot(snap) => { self.diff_message(t(L10nKey::DiffWorkingTreeClean), cx) } - DiffLoad::Ready(snap) => { - self.diff_file_list(snap, &overlay.expanded, focused_file(snap, overlay), cx) - } + DiffLoad::Ready(snap) => self.diff_file_list( + snap, + &overlay.expanded, + focused_file(snap, overlay), + &overlay.scroll, + cx, + ), }; let header = self.diff_header(overlay, window, cx); @@ -442,6 +511,7 @@ impl Tty7App { snap: &DiffSnapshot, expanded: &HashMap, focused: Option, + scroll: &gpui::ScrollHandle, cx: &mut Context, ) -> AnyElement { let stats = snap.stats(); @@ -480,13 +550,17 @@ impl Tty7App { if focused.is_none() && !snap.untracked.is_empty() { list = list.child(self.diff_untracked_section(snap, cx)); } - div() - .id("diff-overlay-scroll") - .flex_1() - .min_h_0() - .overflow_y_scroll() - .child(list) - .into_any_element() + crate::ui::scrollbar::with_vertical_scrollbar( + "diff-overlay-scrollbar", + div() + .id("diff-overlay-scroll") + .flex_1() + .min_h_0() + .overflow_y_scroll() + .track_scroll(scroll) + .child(list), + scroll, + ) } fn diff_oversized_notice( @@ -527,6 +601,9 @@ impl Tty7App { FileStatus::Modified => ("M", cx.theme().warning), FileStatus::Deleted => ("D", cx.theme().danger), FileStatus::Renamed => ("R", cx.theme().muted_foreground), + FileStatus::Copied => ("C", cx.theme().muted_foreground), + FileStatus::TypeChanged => ("T", cx.theme().warning), + FileStatus::Unmerged => ("U", cx.theme().danger), }; let shown_path = match &file.old_path { Some(old) => format!("{old} → {}", file.path), @@ -863,76 +940,19 @@ fn oversized_summary(snap: &DiffSnapshot, stats: &DiffStats) -> String { parts.join(", ") } -#[derive(Clone, Copy)] -enum Side { - Old, - New, -} - -struct SplitCell { - no: Option, - text: String, - changed: bool, -} - -struct SplitRow { - left: Option, - right: Option, -} - -fn split_hunk(lines: &[git_diff::DiffLine]) -> Vec { - fn clean(text: &str) -> String { - text.replace('\t', " ") - } - fn flush( - rows: &mut Vec, - rem: &mut Vec<&git_diff::DiffLine>, - add: &mut Vec<&git_diff::DiffLine>, - ) { - for i in 0..rem.len().max(add.len()) { - rows.push(SplitRow { - left: rem.get(i).map(|l| SplitCell { - no: l.old_no, - text: clean(&l.text), - changed: true, - }), - right: add.get(i).map(|l| SplitCell { - no: l.new_no, - text: clean(&l.text), - changed: true, - }), - }); - } - rem.clear(); - add.clear(); - } - - let mut rows = Vec::new(); - let mut rem: Vec<&git_diff::DiffLine> = Vec::new(); - let mut add: Vec<&git_diff::DiffLine> = Vec::new(); - for line in lines { - match line.kind { - LineKind::Removed => rem.push(line), - LineKind::Added => add.push(line), - LineKind::Context => { - flush(&mut rows, &mut rem, &mut add); - rows.push(SplitRow { - left: Some(SplitCell { - no: line.old_no, - text: clean(&line.text), - changed: false, - }), - right: Some(SplitCell { - no: line.new_no, - text: clean(&line.text), - changed: false, - }), - }); - } - } - } - flush(&mut rows, &mut rem, &mut add); - rows +/// The de-duplication sets on `Tty7App` are keyed by `(HostId, PathBuf)`, so +/// the source rides along inside the path: two sources over one directory are +/// two independent probes and must not cancel one another. `Debug` is what +/// makes the tag unique — it carries the rev of a `Commit` and both ends of a +/// `Range` — and the separator is a byte no path contains. +fn probe_key( + host: crate::ui::host_ops::HostId, + cwd: &Path, + source: &DiffSource, +) -> (crate::ui::host_ops::HostId, PathBuf) { + let mut tagged = std::ffi::OsString::from(format!("{source:?}\u{1}")); + tagged.push(cwd.as_os_str()); + (host, PathBuf::from(tagged)) } #[cfg(test)] @@ -951,40 +971,22 @@ mod tests { } #[test] - fn pairs_removed_and_added_side_by_side() { - let lines = vec![ - line(LineKind::Context, Some(1), Some(1), "a"), - line(LineKind::Removed, Some(2), None, "b"), - line(LineKind::Removed, Some(3), None, "c"), - line(LineKind::Added, None, Some(2), "B"), - line(LineKind::Context, Some(4), Some(3), "d"), - ]; - let rows = split_hunk(&lines); - assert_eq!(rows.len(), 4); - - let l = rows[0].left.as_ref().unwrap(); - let r = rows[0].right.as_ref().unwrap(); - assert_eq!((l.no, l.text.as_str(), l.changed), (Some(1), "a", false)); - assert_eq!((r.no, r.text.as_str(), r.changed), (Some(1), "a", false)); - - let l = rows[1].left.as_ref().unwrap(); - let r = rows[1].right.as_ref().unwrap(); - assert_eq!((l.no, l.text.as_str(), l.changed), (Some(2), "b", true)); - assert_eq!((r.no, r.text.as_str(), r.changed), (Some(2), "B", true)); - - assert_eq!(rows[2].left.as_ref().unwrap().text, "c"); - assert!(rows[2].right.is_none()); - - assert_eq!(rows[3].left.as_ref().unwrap().no, Some(4)); - assert_eq!(rows[3].right.as_ref().unwrap().no, Some(3)); - } - - #[test] - fn expands_tabs_in_cell_text() { - let lines = vec![line(LineKind::Added, None, Some(1), "\tindented")]; - let rows = split_hunk(&lines); - assert_eq!(rows[0].right.as_ref().unwrap().text, " indented"); - assert!(rows[0].left.is_none()); + fn the_probe_key_separates_the_sources_over_one_directory() { + let host = crate::ui::host_ops::HostId::LOCAL; + let cwd = Path::new("/repo"); + let worktree = probe_key(host, cwd, &DiffSource::Worktree); + assert_ne!(worktree, probe_key(host, cwd, &DiffSource::Staged)); + assert_ne!(worktree, probe_key(host, cwd, &DiffSource::Head)); + assert_ne!( + probe_key(host, cwd, &DiffSource::Commit { rev: "a".into() }), + probe_key(host, cwd, &DiffSource::Commit { rev: "b".into() }), + "two commits are two probes" + ); + assert_eq!(worktree, probe_key(host, cwd, &DiffSource::Worktree)); + assert_ne!( + worktree, + probe_key(host, Path::new("/other"), &DiffSource::Worktree) + ); } fn small_file(path: &str, added: u32) -> FileDiff { @@ -1376,3 +1378,103 @@ mod tests { } } } + +#[cfg(all(test, unix))] +mod overlay_gpui_tests { + use super::*; + use crate::ui::app::test_window; + use crate::ui::host_ops::HostId; + use gpui::{Entity, TestAppContext, VisualTestContext}; + + fn overlay_source_and_load( + app: &Entity, + vcx: &mut VisualTestContext, + ) -> (DiffSource, bool) { + app.update_in(vcx, |app, _, _| { + let overlay = app.tabs[app.active] + .diff_overlay + .as_ref() + .expect("an overlay is open"); + ( + overlay.source.clone(), + matches!(overlay.load, DiffLoad::Loading), + ) + }) + } + + /// Opening a staged file while a worktree overlay is up must re-probe. The + /// filter used to match on `(cwd, host)` alone, so it took the "just move + /// the focus" branch and left the unstaged patch on screen under the + /// staged file's name. + #[gpui::test] + fn a_second_source_over_one_directory_is_a_second_overlay(cx: &mut TestAppContext) { + let (app, mut vcx, _pane) = test_window::harness_with_tabs(cx, 1); + let cwd = std::path::PathBuf::from("/no/such/tty7/repo"); + + app.update_in(&mut vcx, |app, window, cx| { + app.open_diff_overlay( + HostId::LOCAL, + cwd.clone(), + DiffSource::Worktree, + Some("a.rs".to_string()), + window, + cx, + ); + }); + assert_eq!( + overlay_source_and_load(&app, &mut vcx), + (DiffSource::Worktree, true) + ); + + // Pretend the worktree probe landed, so a reused overlay would show it. + app.update_in(&mut vcx, |app, _, _| { + let active = app.active; + let overlay = app.tabs[active].diff_overlay.as_mut().unwrap(); + overlay.loading = false; + overlay.load = DiffLoad::Ready(Arc::new(DiffSnapshot { + source: DiffSource::Worktree, + branch: "main".into(), + ..Default::default() + })); + }); + + app.update_in(&mut vcx, |app, window, cx| { + app.open_diff_overlay( + HostId::LOCAL, + cwd.clone(), + DiffSource::Staged, + Some("a.rs".to_string()), + window, + cx, + ); + }); + assert_eq!( + overlay_source_and_load(&app, &mut vcx), + (DiffSource::Staged, true), + "the same file from a different source is a different question" + ); + } + + /// The same source and the same focus still toggles the overlay shut. + #[gpui::test] + fn the_same_source_twice_still_closes(cx: &mut TestAppContext) { + let (app, mut vcx, _pane) = test_window::harness_with_tabs(cx, 1); + let cwd = std::path::PathBuf::from("/no/such/tty7/repo"); + + for _ in 0..2 { + app.update_in(&mut vcx, |app, window, cx| { + app.open_diff_overlay( + HostId::LOCAL, + cwd.clone(), + DiffSource::Worktree, + None, + window, + cx, + ); + }); + } + app.update_in(&mut vcx, |app, _, _| { + assert!(app.tabs[app.active].diff_overlay.is_none()); + }); + } +} diff --git a/src/ui/diff_rows.rs b/src/ui/diff_rows.rs index f93f9f10..8a10215b 100644 --- a/src/ui/diff_rows.rs +++ b/src/ui/diff_rows.rs @@ -3,3 +3,245 @@ //! Side-by-side and unified are two renderings of the same `Vec`, so //! the pairing logic lives here — outside either renderer — and is unit tested //! without a window. + +use crate::terminal::git_diff::{DiffLine, LineKind}; + +/// A tab is worth this many columns. Not configurable: a diff is read next to +/// the file's other lines, not on its own, and the grid has to line up. +const TAB_WIDTH: usize = 4; + +/// Diff text is laid out as a single run, so a literal tab would advance to the +/// renderer's idea of a tab stop rather than the file's. Both views expand +/// them the same way, or the two halves of a split row would drift apart. +pub(crate) fn expand_tabs(text: &str) -> String { + text.replace('\t', &" ".repeat(TAB_WIDTH)) +} + +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub(crate) enum Side { + Old, + New, +} + +pub(crate) struct SplitCell { + pub(crate) no: Option, + pub(crate) text: String, + pub(crate) changed: bool, +} + +pub(crate) struct SplitRow { + pub(crate) left: Option, + pub(crate) right: Option, +} + +/// Pairs each run of removals with the run of additions that follows it, so a +/// rewritten line sits opposite the line it replaced. Whichever run is shorter +/// leaves empty cells at the bottom of the pair. +pub(crate) fn split_hunk(lines: &[DiffLine]) -> Vec { + fn flush(rows: &mut Vec, rem: &mut Vec<&DiffLine>, add: &mut Vec<&DiffLine>) { + for i in 0..rem.len().max(add.len()) { + rows.push(SplitRow { + left: rem.get(i).map(|l| SplitCell { + no: l.old_no, + text: expand_tabs(&l.text), + changed: true, + }), + right: add.get(i).map(|l| SplitCell { + no: l.new_no, + text: expand_tabs(&l.text), + changed: true, + }), + }); + } + rem.clear(); + add.clear(); + } + + let mut rows = Vec::new(); + let mut rem: Vec<&DiffLine> = Vec::new(); + let mut add: Vec<&DiffLine> = Vec::new(); + for line in lines { + match line.kind { + LineKind::Removed => rem.push(line), + LineKind::Added => add.push(line), + LineKind::Context => { + flush(&mut rows, &mut rem, &mut add); + rows.push(SplitRow { + left: Some(SplitCell { + no: line.old_no, + text: expand_tabs(&line.text), + changed: false, + }), + right: Some(SplitCell { + no: line.new_no, + text: expand_tabs(&line.text), + changed: false, + }), + }); + } + } + } + flush(&mut rows, &mut rem, &mut add); + rows +} + +// Read by the unified renderer, which lands with the view toggle. The rows +// themselves belong here now so that both shapes of the same hunk are built — +// and tested — in one place rather than growing a second copy later. +#[allow(dead_code)] +pub(crate) struct UnifiedRow { + pub(crate) old: Option, + pub(crate) new: Option, + pub(crate) kind: LineKind, + pub(crate) text: String, +} + +/// One row per line, in git's own order — every removal in a run first, then +/// every addition. That is the opposite of [`split_hunk`], and it is the whole +/// difference between the two views: unified shows the patch as it was written, +/// side-by-side re-pairs it into before and after. +#[allow(dead_code)] +pub(crate) fn unified_rows(lines: &[DiffLine]) -> Vec { + lines + .iter() + .map(|line| UnifiedRow { + old: line.old_no, + new: line.new_no, + kind: line.kind, + text: expand_tabs(&line.text), + }) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + + fn line(kind: LineKind, old: Option, new: Option, text: &str) -> DiffLine { + DiffLine { + kind, + old_no: old, + new_no: new, + text: text.to_string(), + } + } + + /// A context line, two removals, one addition, a context line — the shape + /// where the two views visibly disagree. + fn hunk() -> Vec { + vec![ + line(LineKind::Context, Some(1), Some(1), "a"), + line(LineKind::Removed, Some(2), None, "b"), + line(LineKind::Removed, Some(3), None, "c"), + line(LineKind::Added, None, Some(2), "B"), + line(LineKind::Context, Some(4), Some(3), "d"), + ] + } + + #[test] + fn pairs_removed_and_added_side_by_side() { + let rows = split_hunk(&hunk()); + assert_eq!(rows.len(), 4); + + let l = rows[0].left.as_ref().unwrap(); + let r = rows[0].right.as_ref().unwrap(); + assert_eq!((l.no, l.text.as_str(), l.changed), (Some(1), "a", false)); + assert_eq!((r.no, r.text.as_str(), r.changed), (Some(1), "a", false)); + + let l = rows[1].left.as_ref().unwrap(); + let r = rows[1].right.as_ref().unwrap(); + assert_eq!((l.no, l.text.as_str(), l.changed), (Some(2), "b", true)); + assert_eq!((r.no, r.text.as_str(), r.changed), (Some(2), "B", true)); + + assert_eq!(rows[2].left.as_ref().unwrap().text, "c"); + assert!(rows[2].right.is_none()); + + assert_eq!(rows[3].left.as_ref().unwrap().no, Some(4)); + assert_eq!(rows[3].right.as_ref().unwrap().no, Some(3)); + } + + #[test] + fn expands_tabs_in_cell_text() { + let lines = vec![line(LineKind::Added, None, Some(1), "\tindented")]; + let rows = split_hunk(&lines); + assert_eq!(rows[0].right.as_ref().unwrap().text, " indented"); + assert!(rows[0].left.is_none()); + } + + #[test] + fn expand_tabs_is_a_fixed_width_substitution() { + assert_eq!(expand_tabs("plain"), "plain"); + assert_eq!(expand_tabs("\tone"), " one"); + assert_eq!(expand_tabs("\t\ttwo"), " two"); + assert_eq!( + expand_tabs("a\tb"), + "a b", + "a fixed width, not the next tab stop — the diff has no column grid" + ); + assert_eq!(expand_tabs(""), ""); + } + + #[test] + fn unified_keeps_gits_own_order() { + let rows = unified_rows(&hunk()); + let shape: Vec<(Option, Option, LineKind, &str)> = rows + .iter() + .map(|r| (r.old, r.new, r.kind, r.text.as_str())) + .collect(); + assert_eq!( + shape, + [ + (Some(1), Some(1), LineKind::Context, "a"), + (Some(2), None, LineKind::Removed, "b"), + (Some(3), None, LineKind::Removed, "c"), + (None, Some(2), LineKind::Added, "B"), + (Some(4), Some(3), LineKind::Context, "d"), + ], + "both removals come before the addition, unlike the split view" + ); + } + + #[test] + fn unified_numbers_each_column_from_the_side_it_belongs_to() { + let rows = unified_rows(&hunk()); + assert!( + rows.iter() + .all(|r| (r.old.is_some() && r.new.is_some()) == (r.kind == LineKind::Context)), + "a context line is the only kind that exists on both sides" + ); + assert!( + rows.iter() + .filter(|r| r.kind == LineKind::Added) + .all(|r| r.old.is_none()) + ); + assert!( + rows.iter() + .filter(|r| r.kind == LineKind::Removed) + .all(|r| r.new.is_none()) + ); + } + + #[test] + fn unified_expands_tabs_the_same_way_split_does() { + let lines = vec![line(LineKind::Added, None, Some(1), "\tindented")]; + assert_eq!(unified_rows(&lines)[0].text, " indented"); + } + + #[test] + fn both_views_render_every_line_exactly_once() { + let lines = hunk(); + let unified = unified_rows(&lines); + assert_eq!(unified.len(), lines.len()); + + let cells: usize = split_hunk(&lines) + .iter() + .map(|r| r.left.is_some() as usize + r.right.is_some() as usize) + .sum(); + let context = lines.iter().filter(|l| l.kind == LineKind::Context).count(); + assert_eq!( + cells, + lines.len() + context, + "a context line fills two cells, a change fills one" + ); + } +}