mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
fix(diff): bound the untracked list and stop a moving default flipping toggles
Five findings from the review of the previous commit, all confirmed against the source before acting. The untracked list escaped every bound the previous commit added. `git ls-files --others` reports the whole tree of anything not yet ignored, so a fresh clone before `node_modules` / `target` / `.venv` reach `.gitignore` answers with tens of thousands of paths — read through the buffering helper, retained without a cap, ignored by `oversized()`, and rendered one non-virtualized row each. That reaches the overlay without going through the diff at all, which is why the diff budget never saw it. It is now streamed through `git_lines`, capped at `MAX_UNTRACKED`, counted toward the oversized threshold, and rendered at most `MAX_RENDERED_FILES` rows with a "… and N more" tail. The reported count stays the true total via `untracked_total` — same split the diff side already makes between what is retained and what is counted, because a count that shrank with the budget would read as files having disappeared. The overlay's expand state was an inversion set — "files flipped away from their default" — which was correct while the default was per-file and stable. The repo-wide `collapse_all` moves the default for every file at once, so any refresh crossing the oversized threshold inverted every explicit choice simultaneously: the two files the user had opened snapped shut and the rest sprang open. It now stores absolute intent (`HashMap<String, bool>`), answered before the default is even computed, so nothing about the snapshot can reach it. Covered by a test that asserts an explicitly opened and an explicitly closed file both survive a transition in either direction while an untouched file follows the default. The Changes panel dropped freshly landed snapshots. `install_diff_snapshot` only wrote `right_panel.diff` when the panel was the one waiting, so a probe the overlay started was discarded even when the panel sat on that exact repo — the overlay rendered the new snapshot while the panel kept the old one, same window, same repo. The wait (`diff_pending`) and the data (`diff_cwd`) are now claimed separately: with probes deduped per cwd there is at most one in flight, so there is no out-of-order overwrite to guard against. The oversized banner reported `retained_lines()` as "diff lines", which after the budget fires is what was kept, not what changed — it read "20000 diff lines" directly under a header showing the exact +90000/-0. It now states loaded-of-total and names which cap ate the difference, and lists each axis that tripped the threshold so a big untracked list never reads as a claim that the diff is big. And the Changes panel deep-copied every untracked path String on every frame, on the UI thread, for two `len()`/`is_empty()` reads — the same cost class the `Arc` switch removed from the probe path. 995 tests pass, fmt clean, no new clippy warnings. The app was not built, launched, or driven; visual acceptance is the owner's. Refs #239.
This commit is contained in:
l0ng-ai
@@ -125,6 +125,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
of line text → 20 000 lines / 1.2 MiB — while `+N −N` keeps counting the
|
||||
whole diff, so the numbers stay exact and the overlay doesn't re-probe in a
|
||||
loop chasing a total it can no longer reach.
|
||||
- The untracked list escaped all of the above: `git ls-files --others` reports
|
||||
every path not yet ignored, and one un-ignored `node_modules` reached the
|
||||
overlay as tens of thousands of rows without touching the diff at all. It is
|
||||
now streamed and capped like the diff, counts toward the oversized
|
||||
threshold, and renders a bounded number of rows — while the count shown
|
||||
stays the true total, so a capped list never reads as files having vanished.
|
||||
- The 400-line auto-collapse rule was per-file, so sixty forty-line files all
|
||||
opened at once (2400 side-by-side rows, none of them individually large).
|
||||
Past a repo-wide total the overlay now opens with every file collapsed —
|
||||
|
||||
@@ -65,9 +65,18 @@ pub const AUTO_COLLAPSE_TOTAL_FILES: usize = 100;
|
||||
|
||||
/// Hard ceiling on file cards the overlay builds at all. Past this the list is
|
||||
/// cut and a "… and N more" line stands in for the tail, so the element tree
|
||||
/// stays bounded no matter what the working tree looks like.
|
||||
/// stays bounded no matter what the working tree looks like. Also bounds the
|
||||
/// untracked section, which is the same one-row-per-path shape.
|
||||
pub const MAX_RENDERED_FILES: usize = 300;
|
||||
|
||||
/// Repo-wide cap on retained untracked paths. `git ls-files --others` answers
|
||||
/// with the whole tree of anything not yet ignored — a fresh clone before
|
||||
/// `node_modules` / `target` / `.venv` reach `.gitignore` reports tens of
|
||||
/// thousands of paths — and every one of them is an owned `String` here and a
|
||||
/// row in the overlay. The count stays exact past the cap
|
||||
/// ([`DiffSnapshot::untracked_total`]); only the retained list is bounded.
|
||||
pub const MAX_UNTRACKED: usize = 500;
|
||||
|
||||
/// Why a file's hunks stop short of its real diff.
|
||||
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
|
||||
pub enum Truncation {
|
||||
@@ -92,7 +101,17 @@ pub struct DiffSnapshot {
|
||||
/// `git diff HEAD` has no blob to diff them against, and agents create
|
||||
/// files constantly — hiding them would make the overlay look like it
|
||||
/// lost work.
|
||||
///
|
||||
/// Capped at [`MAX_UNTRACKED`]; [`untracked_count`](Self::untracked_count)
|
||||
/// is the honest total.
|
||||
pub untracked: Vec<String>,
|
||||
/// How many untracked paths `git ls-files --others` actually reported,
|
||||
/// which is not `untracked.len()` once the cap bites. Same discipline as
|
||||
/// [`totals`](Self::totals): what we *retain* is budgeted, what we *report*
|
||||
/// stays exact, because a count that shrank with the budget would read as
|
||||
/// files having disappeared. Read it through
|
||||
/// [`untracked_count`](Self::untracked_count).
|
||||
pub untracked_total: usize,
|
||||
}
|
||||
|
||||
impl DiffSnapshot {
|
||||
@@ -123,12 +142,28 @@ impl DiffSnapshot {
|
||||
.sum()
|
||||
}
|
||||
|
||||
/// The true number of untracked paths, whether or not the retained list was
|
||||
/// capped. Falls back to the retained length so a snapshot built by hand
|
||||
/// (tests, `..Default::default()`) can't under-report — the fallback is
|
||||
/// never wrong, since `untracked_total` is only ever ≥ `untracked.len()`.
|
||||
pub fn untracked_count(&self) -> usize {
|
||||
self.untracked_total.max(self.untracked.len())
|
||||
}
|
||||
|
||||
/// Whether this diff is too big to open expanded: every file starts
|
||||
/// collapsed and the overlay leads with the "too large to render
|
||||
/// efficiently" summary. Not a refusal — individual files still expand by
|
||||
/// click, which is the escape hatch the summary points at.
|
||||
///
|
||||
/// Untracked paths count toward the file axis because they cost the same
|
||||
/// thing a collapsed file card costs — one row each, in the same
|
||||
/// non-virtualized list. A tree with an un-ignored `node_modules` is the
|
||||
/// exact stall this is here to prevent, and it reaches the overlay through
|
||||
/// `ls-files`, not through the diff. The summary names whichever axis
|
||||
/// tripped, so a big untracked list never reads as a claim that the *diff*
|
||||
/// is big.
|
||||
pub fn oversized(&self) -> bool {
|
||||
self.files.len() > AUTO_COLLAPSE_TOTAL_FILES
|
||||
self.files.len() + self.untracked_count() > AUTO_COLLAPSE_TOTAL_FILES
|
||||
|| self.retained_lines() > AUTO_COLLAPSE_TOTAL_LINES
|
||||
}
|
||||
|
||||
@@ -221,19 +256,37 @@ pub fn probe(host: &dyn Host, cwd: &Path) -> Option<DiffSnapshot> {
|
||||
.map(|out| parse_unified(&out))
|
||||
.unwrap_or_default();
|
||||
// `--full-name` pins paths to the repo root regardless of which
|
||||
// subdirectory the pane sits in, matching the diff's path space.
|
||||
let untracked = git_status::git(
|
||||
// subdirectory the pane sits in, matching the diff's path space. Capped for
|
||||
// the same reason the diff is: `--others` walks everything not yet ignored,
|
||||
// so one un-ignored dependency directory answers with tens of thousands of
|
||||
// paths.
|
||||
let mut untracked: Vec<String> = Vec::new();
|
||||
let mut untracked_total = 0usize;
|
||||
let ok = git_status::git(
|
||||
host,
|
||||
cwd,
|
||||
&["ls-files", "--others", "--exclude-standard", "--full-name"],
|
||||
)
|
||||
.map(|out| out.lines().map(str::to_string).collect())
|
||||
.unwrap_or_default();
|
||||
.map(|out| {
|
||||
for line in out.lines() {
|
||||
untracked_total += 1;
|
||||
if untracked.len() < MAX_UNTRACKED {
|
||||
untracked.push(line.to_string());
|
||||
}
|
||||
}
|
||||
});
|
||||
if ok.is_none() {
|
||||
// A failed listing is "we don't know", not "there are none" — same
|
||||
// shape as the diff above.
|
||||
untracked.clear();
|
||||
untracked_total = 0;
|
||||
}
|
||||
Some(DiffSnapshot {
|
||||
root,
|
||||
branch,
|
||||
files,
|
||||
untracked,
|
||||
untracked_total,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -791,6 +844,33 @@ index 1..2 100644
|
||||
assert_eq!((late.added, late.removed), (0, 1), "but the line counts");
|
||||
}
|
||||
|
||||
/// The untracked cap keeps the retained list bounded while the reported
|
||||
/// count stays exact — the same split the diff side already makes between
|
||||
/// what is retained and what is counted.
|
||||
#[test]
|
||||
fn untracked_is_capped_but_counted() {
|
||||
// What `probe` builds while streaming `ls-files --others`.
|
||||
let mut untracked: Vec<String> = Vec::new();
|
||||
let mut untracked_total = 0usize;
|
||||
for i in 0..(MAX_UNTRACKED * 3) {
|
||||
untracked_total += 1;
|
||||
if untracked.len() < MAX_UNTRACKED {
|
||||
untracked.push(format!("node_modules/p{i}/index.js"));
|
||||
}
|
||||
}
|
||||
let snap = DiffSnapshot {
|
||||
untracked,
|
||||
untracked_total,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(snap.untracked.len(), MAX_UNTRACKED, "retention is bounded");
|
||||
assert_eq!(
|
||||
snap.untracked_count(),
|
||||
MAX_UNTRACKED * 3,
|
||||
"the count is not"
|
||||
);
|
||||
}
|
||||
|
||||
/// Measurement harness for issue #239 finding 1 — run with
|
||||
/// `cargo test --release -- --ignored --nocapture bench_stream_vs_buffer`.
|
||||
///
|
||||
|
||||
+214
-59
@@ -28,7 +28,7 @@
|
||||
//! file opens collapsed under a summary, and at most
|
||||
//! [`MAX_RENDERED_FILES`] cards are built at all.
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -84,10 +84,19 @@ pub(crate) struct DiffOverlayState {
|
||||
pub(crate) load: DiffLoad,
|
||||
/// A probe is currently in flight (initial or refresh).
|
||||
pub(crate) loading: bool,
|
||||
/// Files the user flipped away from their default collapse state (small
|
||||
/// files default open, big/binary ones closed). Keyed by path so the set
|
||||
/// survives a background refresh of the snapshot.
|
||||
pub(crate) toggled: HashSet<String>,
|
||||
/// Files the user has explicitly expanded (`true`) or collapsed (`false`),
|
||||
/// keyed by path so the choice survives a background refresh of the
|
||||
/// snapshot. Absent means "follow the default".
|
||||
///
|
||||
/// Absolute state, deliberately not an inversion set. It used to be a
|
||||
/// `HashSet` of "files flipped away from their default", which was fine
|
||||
/// while the default was per-file and stable — but the repo-wide
|
||||
/// `collapse_all` moves the default for *every* file at once, so a refresh
|
||||
/// that crossed the oversized threshold inverted every explicit choice
|
||||
/// simultaneously: the two files the user had opened snapped shut and the
|
||||
/// rest sprang open. Storing what the user actually wanted makes a moving
|
||||
/// default unable to touch it.
|
||||
pub(crate) expanded: HashMap<String, bool>,
|
||||
/// When set, the overlay shows only this file (repo-relative path), always
|
||||
/// expanded — the "click a row in the Changes panel" entry point. `None` is
|
||||
/// the whole-tree view the git line opens. Kept as a path rather than an
|
||||
@@ -188,7 +197,7 @@ impl Tty7App {
|
||||
focus_handle: focus_handle.clone(),
|
||||
load: seed,
|
||||
loading: false,
|
||||
toggled: HashSet::new(),
|
||||
expanded: HashMap::new(),
|
||||
focus,
|
||||
});
|
||||
window.focus(&focus_handle, cx);
|
||||
@@ -315,16 +324,25 @@ impl Tty7App {
|
||||
};
|
||||
landed = true;
|
||||
}
|
||||
// The panel's wait is cleared by the answer it asked for, whoever
|
||||
// actually ran it. `diff_cwd` is checked separately: the panel can have
|
||||
// navigated to another repo — or another machine — since, in which case
|
||||
// this result is stale and only the wait is over.
|
||||
// The panel's *wait* is cleared by the answer it asked for, whoever
|
||||
// actually ran it — that's what `diff_pending` is for, and clearing it
|
||||
// on a result for a repo the panel has since left is what lets the
|
||||
// render path notice nothing is cached and re-probe.
|
||||
let key = (host, cwd.to_path_buf());
|
||||
if self.right_panel.diff_pending.as_ref() == Some(&key) {
|
||||
self.right_panel.diff_pending = None;
|
||||
if self.right_panel.diff_cwd.as_ref() == Some(&key) {
|
||||
self.right_panel.diff = Some(snap);
|
||||
}
|
||||
landed = true;
|
||||
}
|
||||
// The panel's *data*, though, is claimed by the repo key alone.
|
||||
// Requiring the panel to have been the one waiting meant a probe the
|
||||
// overlay started was thrown away for the panel even when it was
|
||||
// sitting on that exact repo — so clicking the sidebar counts left the
|
||||
// overlay showing the new snapshot and the panel still rendering the
|
||||
// old one, in the same window. With probes deduped per repo there is at
|
||||
// most one in flight, so there is no out-of-order overwrite to guard
|
||||
// against.
|
||||
if self.right_panel.diff_cwd.as_ref() == Some(&key) {
|
||||
self.right_panel.diff = Some(snap);
|
||||
landed = true;
|
||||
}
|
||||
if landed {
|
||||
@@ -382,7 +400,7 @@ impl Tty7App {
|
||||
self.diff_message("Working tree clean", cx)
|
||||
}
|
||||
DiffLoad::Ready(snap) => {
|
||||
self.diff_file_list(snap, &overlay.toggled, focused_file(snap, overlay), cx)
|
||||
self.diff_file_list(snap, &overlay.expanded, focused_file(snap, overlay), cx)
|
||||
}
|
||||
};
|
||||
|
||||
@@ -428,7 +446,7 @@ impl Tty7App {
|
||||
let (branch, files, untracked, added, removed) = match &overlay.load {
|
||||
DiffLoad::Ready(s) => {
|
||||
let (a, r) = s.totals();
|
||||
(s.branch.clone(), s.files.len(), s.untracked.len(), a, r)
|
||||
(s.branch.clone(), s.files.len(), s.untracked_count(), a, r)
|
||||
}
|
||||
_ => (String::new(), 0, 0, 0, 0),
|
||||
};
|
||||
@@ -601,7 +619,7 @@ impl Tty7App {
|
||||
fn diff_file_list(
|
||||
&self,
|
||||
snap: &DiffSnapshot,
|
||||
toggled: &HashSet<String>,
|
||||
expanded: &HashMap<String, bool>,
|
||||
focused: Option<usize>,
|
||||
cx: &mut Context<Self>,
|
||||
) -> AnyElement {
|
||||
@@ -627,12 +645,12 @@ impl Tty7App {
|
||||
// A file opened by name was asked for explicitly — show its body
|
||||
// even when it's over the auto-collapse threshold. The header still
|
||||
// toggles, so a huge file can be folded back down.
|
||||
let expanded = if focused == Some(idx) {
|
||||
!toggled.contains(&file.path)
|
||||
let is_expanded = if focused == Some(idx) {
|
||||
expanded.get(&file.path).copied().unwrap_or(true)
|
||||
} else {
|
||||
file_expanded(file, toggled, oversized)
|
||||
file_expanded(file, expanded, oversized)
|
||||
};
|
||||
list = list.child(self.diff_file_card(idx, file, expanded, cx));
|
||||
list = list.child(self.diff_file_card(idx, file, is_expanded, cx));
|
||||
}
|
||||
if focused.is_none() && snap.files.len() > shown {
|
||||
let rest = snap.files.len() - shown;
|
||||
@@ -651,7 +669,7 @@ impl Tty7App {
|
||||
}
|
||||
// Untracked files are a property of the tree, not of the focused file.
|
||||
if focused.is_none() && !snap.untracked.is_empty() {
|
||||
list = list.child(self.diff_untracked_section(&snap.untracked, cx));
|
||||
list = list.child(self.diff_untracked_section(snap, cx));
|
||||
}
|
||||
div()
|
||||
.id("diff-overlay-scroll")
|
||||
@@ -668,15 +686,40 @@ impl Tty7App {
|
||||
/// file rows with their `+N −N` are the part that still reads fine at this
|
||||
/// size.
|
||||
fn diff_oversized_notice(&self, snap: &DiffSnapshot, cx: &Context<Self>) -> AnyElement {
|
||||
let mut text = format!(
|
||||
"This diff is too large to render efficiently ({} changed files, {} diff lines). \
|
||||
Every file is collapsed — expand individual files, or run `git diff` in the terminal.",
|
||||
// Name every axis that contributes, so the banner never claims the diff
|
||||
// is big when what is actually big is an un-ignored untracked tree.
|
||||
let mut parts = vec![format!(
|
||||
"{} changed file{}",
|
||||
snap.files.len(),
|
||||
snap.retained_lines(),
|
||||
);
|
||||
if snap.budget_exhausted() {
|
||||
text.push_str(" Some files' contents were dropped to keep tty7 responsive.");
|
||||
if snap.files.len() == 1 { "" } else { "s" }
|
||||
)];
|
||||
// Loaded-of-total, not the retained count alone: past the repo-wide
|
||||
// budget `retained_lines` is what we kept, and printing that as "diff
|
||||
// lines" contradicted the exact `+N −N` in the header directly above —
|
||||
// 20 000 against 90 000 on a big tree, with nothing to reconcile them.
|
||||
let (added, removed) = snap.totals();
|
||||
let total_lines = (added + removed) as usize;
|
||||
let loaded = snap.retained_lines();
|
||||
parts.push(if loaded < total_lines {
|
||||
// Say which cap ate the difference. "20000 of 90000 loaded" on its
|
||||
// own leaves the user guessing whether the rest is merely collapsed
|
||||
// (one click away) or was never read (it isn't).
|
||||
if snap.budget_exhausted() {
|
||||
format!("{loaded} of {total_lines} diff lines loaded, the rest past tty7's budget")
|
||||
} else {
|
||||
format!("{loaded} of {total_lines} diff lines loaded")
|
||||
}
|
||||
} else {
|
||||
format!("{total_lines} diff lines")
|
||||
});
|
||||
if snap.untracked_count() > 0 {
|
||||
parts.push(format!("{} untracked", snap.untracked_count()));
|
||||
}
|
||||
let text = format!(
|
||||
"This working tree is too large to render efficiently ({}). Every file is \
|
||||
collapsed — expand individual files, or run `git diff` in the terminal.",
|
||||
parts.join(", "),
|
||||
);
|
||||
div()
|
||||
.w_full()
|
||||
.px_2p5()
|
||||
@@ -754,11 +797,12 @@ impl Tty7App {
|
||||
.get_mut(active)
|
||||
.and_then(|t| t.diff_overlay.as_mut())
|
||||
{
|
||||
// Flip this file's override; removing an existing
|
||||
// entry returns it to its default state.
|
||||
if !overlay.toggled.remove(&path) {
|
||||
overlay.toggled.insert(path.clone());
|
||||
}
|
||||
// Record what the user now wants, not "differs from
|
||||
// the default": `expanded` is the state this card is
|
||||
// currently drawn in, so the click means the
|
||||
// opposite of it, and that answer keeps holding even
|
||||
// if the default later moves under it.
|
||||
overlay.expanded.insert(path.clone(), !expanded);
|
||||
cx.notify();
|
||||
}
|
||||
}))
|
||||
@@ -968,13 +1012,23 @@ impl Tty7App {
|
||||
/// The trailing "Untracked files" section: names only — `git diff HEAD`
|
||||
/// has no blob to diff a never-added file against, but hiding them would
|
||||
/// read as lost work (agents create files constantly).
|
||||
fn diff_untracked_section(&self, untracked: &[String], cx: &Context<Self>) -> AnyElement {
|
||||
///
|
||||
/// Bounded exactly like the file-card list above it, and for the same
|
||||
/// reason: this is one non-virtualized row per path, and `ls-files
|
||||
/// --others` on a tree whose dependency directory isn't ignored yet answers
|
||||
/// with tens of thousands of them. The header count is the true total, so
|
||||
/// capping the rows never makes files look gone.
|
||||
fn diff_untracked_section(&self, snap: &DiffSnapshot, cx: &Context<Self>) -> AnyElement {
|
||||
let total = snap.untracked_count();
|
||||
let untracked = &snap.untracked[..snap.untracked.len().min(MAX_RENDERED_FILES)];
|
||||
// Same filled-band-in-a-rounded-card shape as `diff_file_card`, so the
|
||||
// header owns the corners it sits in. The rows below it paint no fill,
|
||||
// which is why only the top pair is ever non-zero here.
|
||||
// which is why only the top pair is ever non-zero here. Counted off the
|
||||
// *total*, not the capped slice: a section with a "… and N more" tail
|
||||
// still has rows under its header.
|
||||
let header_corners = rounding::stack_corners(
|
||||
0,
|
||||
if untracked.is_empty() { 1 } else { 2 },
|
||||
if total == 0 { 1 } else { 2 },
|
||||
rounding::CARD_RADIUS,
|
||||
rounding::HAIRLINE,
|
||||
);
|
||||
@@ -993,7 +1047,7 @@ impl Tty7App {
|
||||
.bg(cx.theme().secondary)
|
||||
.text_xs()
|
||||
.text_color(cx.theme().muted_foreground)
|
||||
.child(format!("Untracked files ({})", untracked.len())),
|
||||
.child(format!("Untracked files ({total})")),
|
||||
);
|
||||
for path in untracked {
|
||||
section = section.child(
|
||||
@@ -1015,6 +1069,20 @@ impl Tty7App {
|
||||
.child(div().flex_1().min_w_0().truncate().child(path.clone())),
|
||||
);
|
||||
}
|
||||
if total > untracked.len() {
|
||||
let rest = total - untracked.len();
|
||||
section = section.child(
|
||||
div()
|
||||
.w_full()
|
||||
.px_2p5()
|
||||
.py_1()
|
||||
.text_xs()
|
||||
.text_color(cx.theme().muted_foreground)
|
||||
.child(format!(
|
||||
"… and {rest} more — run `git status` in the terminal to see them.",
|
||||
)),
|
||||
);
|
||||
}
|
||||
section.into_any_element()
|
||||
}
|
||||
}
|
||||
@@ -1038,18 +1106,25 @@ fn focused_name(overlay: &DiffOverlayState) -> Option<String> {
|
||||
Some(snap.files[idx].path.clone())
|
||||
}
|
||||
|
||||
/// Whether a file's body shows: small text diffs default open, big ones (and
|
||||
/// anything the user explicitly flipped) invert via the `toggled` set.
|
||||
/// Whether a file's body shows.
|
||||
///
|
||||
/// `collapse_all` is the repo-wide override: past
|
||||
/// An explicit choice in `expanded` is final: it is answered before the default
|
||||
/// is even computed, so nothing about the snapshot can change it. That ordering
|
||||
/// is the whole point — `collapse_all` is a repo-wide default that moves as the
|
||||
/// working tree grows and shrinks, and a user who opened one file inside an
|
||||
/// oversized diff must not have it snap shut the moment an agent reverts enough
|
||||
/// lines to drop the tree back under the threshold.
|
||||
///
|
||||
/// Files the user never touched follow the default: small text diffs open, big
|
||||
/// ones closed, and past
|
||||
/// [`AUTO_COLLAPSE_TOTAL_LINES`](git_diff::AUTO_COLLAPSE_TOTAL_LINES) nothing
|
||||
/// opens by default, because the per-file threshold can't see that forty
|
||||
/// innocent files are about to expand at once. The user's explicit toggles
|
||||
/// still win over it — that's the "expand individual files" the oversized
|
||||
/// notice promises.
|
||||
fn file_expanded(file: &FileDiff, toggled: &HashSet<String>, collapse_all: bool) -> bool {
|
||||
let default_open = !collapse_all && file.added + file.removed <= AUTO_COLLAPSE_LINES;
|
||||
default_open != toggled.contains(&file.path)
|
||||
/// opens at all, because the per-file threshold can't see that forty innocent
|
||||
/// files are about to expand at once.
|
||||
fn file_expanded(file: &FileDiff, expanded: &HashMap<String, bool>, collapse_all: bool) -> bool {
|
||||
if let Some(&want) = expanded.get(&file.path) {
|
||||
return want;
|
||||
}
|
||||
!collapse_all && file.added + file.removed <= AUTO_COLLAPSE_LINES
|
||||
}
|
||||
|
||||
/// Which column a split cell belongs to — picks the marker and tint.
|
||||
@@ -1214,40 +1289,71 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// An explicit choice map, for the tests below.
|
||||
fn choices<const N: usize>(pairs: [(&str, bool); N]) -> HashMap<String, bool> {
|
||||
pairs.into_iter().map(|(p, v)| (p.to_string(), v)).collect()
|
||||
}
|
||||
|
||||
/// The per-file threshold on its own: a small file opens, a big one doesn't,
|
||||
/// and an explicit toggle inverts either. Unchanged behaviour — this is the
|
||||
/// small-working-tree case that must feel identical.
|
||||
/// and an explicit choice overrides either. Unchanged behaviour — this is
|
||||
/// the small-working-tree case that must feel identical.
|
||||
#[test]
|
||||
fn per_file_collapse_is_unchanged_below_the_repo_threshold() {
|
||||
let small = small_file("small.rs", 10);
|
||||
let big = small_file("big.rs", AUTO_COLLAPSE_LINES + 1);
|
||||
let none = HashSet::new();
|
||||
let none = HashMap::new();
|
||||
assert!(file_expanded(&small, &none, false));
|
||||
assert!(!file_expanded(&big, &none, false));
|
||||
|
||||
let toggled: HashSet<String> = ["small.rs".to_string(), "big.rs".to_string()].into();
|
||||
assert!(!file_expanded(&small, &toggled, false));
|
||||
assert!(file_expanded(&big, &toggled, false));
|
||||
let picked = choices([("small.rs", false), ("big.rs", true)]);
|
||||
assert!(!file_expanded(&small, &picked, false));
|
||||
assert!(file_expanded(&big, &picked, false));
|
||||
}
|
||||
|
||||
/// The repo-wide override: past the total threshold nothing opens by
|
||||
/// default, however small each file is — the case a per-file rule can't see
|
||||
/// (issue #239, finding 4). An explicit toggle still wins, which is what
|
||||
/// (issue #239, finding 4). An explicit choice still wins, which is what
|
||||
/// "expand individual files" in the oversized notice means.
|
||||
#[test]
|
||||
fn repo_wide_collapse_overrides_the_per_file_default() {
|
||||
let small = small_file("small.rs", 10);
|
||||
let none = HashSet::new();
|
||||
let none = HashMap::new();
|
||||
assert!(file_expanded(&small, &none, false));
|
||||
assert!(!file_expanded(&small, &none, true), "collapsed en masse");
|
||||
|
||||
let toggled: HashSet<String> = ["small.rs".to_string()].into();
|
||||
assert!(
|
||||
file_expanded(&small, &toggled, true),
|
||||
file_expanded(&small, &choices([("small.rs", true)]), true),
|
||||
"the user's own click still opens it"
|
||||
);
|
||||
}
|
||||
|
||||
/// An explicit choice must survive the default moving under it — the defect
|
||||
/// the inversion-set representation had. A refresh that crosses the
|
||||
/// oversized threshold in either direction leaves every file the user
|
||||
/// touched exactly as they left it, and moves only the ones they didn't.
|
||||
#[test]
|
||||
fn explicit_choices_survive_an_oversized_transition() {
|
||||
let opened = small_file("opened.rs", 10);
|
||||
let closed = small_file("closed.rs", 10);
|
||||
let untouched = small_file("untouched.rs", 10);
|
||||
// The user opened one file and shut another while the tree was oversized.
|
||||
let picked = choices([("opened.rs", true), ("closed.rs", false)]);
|
||||
|
||||
for collapse_all in [true, false] {
|
||||
assert!(
|
||||
file_expanded(&opened, &picked, collapse_all),
|
||||
"an explicitly opened file stays open (collapse_all={collapse_all})"
|
||||
);
|
||||
assert!(
|
||||
!file_expanded(&closed, &picked, collapse_all),
|
||||
"an explicitly closed file stays closed (collapse_all={collapse_all})"
|
||||
);
|
||||
}
|
||||
// Only the file the user never touched follows the default.
|
||||
assert!(!file_expanded(&untouched, &picked, true));
|
||||
assert!(file_expanded(&untouched, &picked, false));
|
||||
}
|
||||
|
||||
/// Sixty files of forty lines each never trip the per-file threshold (each
|
||||
/// is a tenth of it), yet would open 2400 diff rows at once. `oversized`
|
||||
/// catches it on the line axis, and with everything collapsed the overlay
|
||||
@@ -1266,7 +1372,7 @@ mod tests {
|
||||
);
|
||||
assert!(snap.oversized());
|
||||
|
||||
let none = HashSet::new();
|
||||
let none = HashMap::new();
|
||||
let rows_expanded: usize = snap
|
||||
.files
|
||||
.iter()
|
||||
@@ -1297,10 +1403,59 @@ mod tests {
|
||||
..Default::default()
|
||||
};
|
||||
assert!(!snap.oversized());
|
||||
let none = HashSet::new();
|
||||
let none = HashMap::new();
|
||||
assert!(snap.files.iter().all(|f| file_expanded(f, &none, false)));
|
||||
}
|
||||
|
||||
/// A clean diff with a huge untracked list is oversized too. `ls-files
|
||||
/// --others` reaches the overlay without going through the diff at all, so
|
||||
/// an un-ignored `node_modules` produced thousands of rows past every bound
|
||||
/// the rest of this change added.
|
||||
#[test]
|
||||
fn a_huge_untracked_list_is_oversized() {
|
||||
let snap = DiffSnapshot {
|
||||
files: vec![small_file("one.rs", 3)],
|
||||
untracked: (0..git_diff::MAX_UNTRACKED)
|
||||
.map(|i| format!("node_modules/p{i}/index.js"))
|
||||
.collect(),
|
||||
untracked_total: 40_000,
|
||||
..Default::default()
|
||||
};
|
||||
assert!(snap.retained_lines() < git_diff::AUTO_COLLAPSE_TOTAL_LINES);
|
||||
assert!(snap.files.len() < git_diff::AUTO_COLLAPSE_TOTAL_FILES);
|
||||
assert!(snap.oversized(), "the untracked list alone must trip it");
|
||||
}
|
||||
|
||||
/// The untracked section builds at most [`MAX_RENDERED_FILES`] rows while
|
||||
/// still reporting the true total, so a capped list never reads as files
|
||||
/// having vanished.
|
||||
#[test]
|
||||
fn untracked_rows_are_capped_but_the_count_stays_true() {
|
||||
let snap = DiffSnapshot {
|
||||
untracked: (0..git_diff::MAX_UNTRACKED)
|
||||
.map(|i| format!("p{i}"))
|
||||
.collect(),
|
||||
untracked_total: 12_345,
|
||||
..Default::default()
|
||||
};
|
||||
let rendered = snap.untracked.len().min(MAX_RENDERED_FILES);
|
||||
assert_eq!(rendered, MAX_RENDERED_FILES);
|
||||
assert_eq!(snap.untracked_count(), 12_345, "header count is honest");
|
||||
assert_eq!(snap.untracked_count() - rendered, 12_045);
|
||||
}
|
||||
|
||||
/// A snapshot built without the streaming probe (tests, `..Default`) must
|
||||
/// not under-report: the count falls back to the retained length.
|
||||
#[test]
|
||||
fn untracked_count_falls_back_to_the_retained_length() {
|
||||
let snap = DiffSnapshot {
|
||||
untracked: vec!["a".into(), "b".into(), "c".into()],
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(snap.untracked_total, 0);
|
||||
assert_eq!(snap.untracked_count(), 3);
|
||||
}
|
||||
|
||||
/// However many files change, the overlay builds at most
|
||||
/// [`MAX_RENDERED_FILES`] cards and says so.
|
||||
#[test]
|
||||
|
||||
@@ -1192,7 +1192,7 @@ impl Tty7App {
|
||||
// so the diff borrow ends before `panel_title` takes `&mut cx`.
|
||||
let count = match &self.right_panel.diff {
|
||||
Some(Some(snap)) => {
|
||||
let n = snap.files.len() + snap.untracked.len();
|
||||
let n = snap.files.len() + snap.untracked_count();
|
||||
(n > 0).then(|| n.to_string())
|
||||
}
|
||||
_ => None,
|
||||
@@ -1219,7 +1219,11 @@ impl Tty7App {
|
||||
.iter()
|
||||
.map(|f| (f.path.clone(), f.added, f.removed))
|
||||
.collect();
|
||||
let untracked = snap.untracked.clone();
|
||||
// The count, not the paths: this used to clone every untracked
|
||||
// path String on every frame of this panel, on the UI thread,
|
||||
// for two `len()`/`is_empty()` reads — the same cost class the
|
||||
// `Arc` switch removed from the probe path.
|
||||
let untracked = snap.untracked_count();
|
||||
let focused = self.diff_overlay_focus(host.id(), &cwd).map(str::to_string);
|
||||
// Rows inset themselves rather than the list, so the hover and
|
||||
// selected capsules bleed a little past the text into the same
|
||||
@@ -1298,7 +1302,7 @@ impl Tty7App {
|
||||
}),
|
||||
);
|
||||
}
|
||||
if !untracked.is_empty() {
|
||||
if untracked > 0 {
|
||||
list = list.child(
|
||||
h_flex()
|
||||
.items_center()
|
||||
@@ -1314,7 +1318,7 @@ impl Tty7App {
|
||||
div()
|
||||
.text_size(px(11.5))
|
||||
.text_color(cx.theme().muted_foreground)
|
||||
.child(format!("{} untracked", untracked.len())),
|
||||
.child(format!("{untracked} untracked")),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user