mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-22 00:02:23 +00:00
feat(diff): make the sidebar diff preview optional and bound its cost
Clicking a sidebar row's `+N −N` opens the working-tree diff overlay. On a big tree that could stall the window, and not everyone wants an in-app diff viewer in the first place. Two halves, matching the report. The setting: `sidebar_diff_preview` (Settings → Window & Tabs, on by default, persisted in `config.json`). Off, the branch and the counts stay exactly where they are and read exactly the same; they lose only the pointer cursor and the `toggle_diff_overlay` handler, so the press falls through to ordinary tab activation. Both come off one value — `diff_click_cwd` — so they cannot get out of step. The performance work. All five of the reporter's hypotheses held up against v26.7.6, and each fix is measured on a 300-file / 90 000-line / 4.5 MB diff (release, macOS arm64): 1. The full diff was buffered before parsing — `git_status::git` uses `Command::output()`. Now streamed line by line through the new `git_status::git_lines` into an incremental `DiffParser`: peak transient buffer 4 552 060 bytes → 50 bytes, at ~1.7× the parse CPU (3.97 ms → 6.76 ms) on the background thread, where it never touches a frame. 2. The snapshot was deep-cloned per holder inside `this.update`, i.e. on the UI thread. Now shared behind `Arc`: 2.41 ms → 11 ns per holder. 3. The element tree is not virtualized — confirmed, not cured. Rendering is not being redesigned here; instead the element count is bounded (see 4) and `MAX_RENDERED_FILES` caps the cards built at all, with a "… and N more" line for the tail. 4. Auto-collapse was per file, and counted only +/− while the rendered body also has context lines. Added `AUTO_COLLAPSE_TOTAL_LINES` over *retained* lines: sixty forty-line files, none individually large, went from 2400 side-by-side rows to zero, under a summary saying the diff is too large to render efficiently and pointing at expanding individual files or `git diff`. 5. The Changes panel probed independently and kept its own snapshot. Both now go through `spawn_shared_diff_probe`, which dedupes by cwd and installs one `Arc` into every watcher; opening the overlay while the panel already shows that repo now paints from the panel's snapshot instead of re-probing. Plus a repo-wide retention budget (`MAX_TOTAL_LINES`, `MAX_FILES_WITH_HUNKS`): 90 000 lines / 6.2 MiB of line text → 20 000 / 1.2 MiB. The `+N −N` totals deliberately escape every cap — they are compared against `--numstat` to detect staleness, so a capped total would disagree forever and re-probe in a loop. Small diffs are untouched: a forty-file, twelve-lines-each tree is not oversized and still opens expanded, asserted directly. Not verified: anything requiring the GUI. No frame timings, no visual check of the oversized banner or the settings row, and `AUTO_COLLAPSE_TOTAL_LINES` is a judgement call anchored on row count rather than a measured frame budget. Refs #239. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -58,6 +58,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
session" means: paste it into `codex resume`, a bug report, or another tool.
|
||||
(#211)
|
||||
|
||||
- **Sidebar diff preview is optional** — clicking a sidebar row's `+N −N`
|
||||
working-tree counts opens the diff overlay, which is the point of them for
|
||||
most people but not for everyone. Settings → Window & Tabs → *Open diff
|
||||
preview from sidebar counts* turns the click off (`sidebar_diff_preview` in
|
||||
`config.json`, on by default). Off, the branch and the counts stay exactly
|
||||
where they are and read exactly the same; they simply stop being their own
|
||||
click target, so the press falls through to ordinary tab activation like any
|
||||
other part of the row. (#239)
|
||||
|
||||
### Changed
|
||||
|
||||
- **The prompt editor's soft newline is now a rebindable action** — `Shift+Enter`
|
||||
@@ -97,6 +106,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
the 80px grab handle, so chips reach their minimum width and truncate a tab or
|
||||
two sooner. (#221)
|
||||
|
||||
- **Large working-tree diffs no longer stall the window** — the diff overlay
|
||||
had four costs that all scaled with the size of the tree rather than with
|
||||
what it could show, and issue #239's source-level analysis found each of
|
||||
them. Measured on a 300-file, 90 000-line, 4.5 MB working-tree diff:
|
||||
|
||||
- The full `git diff HEAD` was read into one `String` before parsing began.
|
||||
It is now streamed off the pipe a line at a time, so peak transient memory
|
||||
is the longest line — 4 552 060 bytes resident → 50 bytes — at a cost of
|
||||
~1.7× the parse CPU (3.97 ms → 6.76 ms) on the background thread, where it
|
||||
never touches a frame.
|
||||
- The parsed snapshot was deep-cloned onto every tab's overlay *on the UI
|
||||
update path*. It is now shared behind an `Arc`: 2.41 ms of main-thread
|
||||
copying per holder → 11 ns.
|
||||
- The per-file 2000-line cap bounded one pathological file but not the sum,
|
||||
so two hundred ordinary files could retain 90 000 `DiffLine`s. A repo-wide
|
||||
budget now caps retained lines and files-with-hunks — 90 000 lines / 6.2 MiB
|
||||
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 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 —
|
||||
zero rows built — leads with a summary saying the diff is too large to
|
||||
render efficiently, and points at expanding individual files or `git diff`
|
||||
in the terminal.
|
||||
|
||||
- **One `git diff` per repository, not two** — the Changes panel and the diff
|
||||
overlay each ran their own full-diff probe and kept their own snapshot of the
|
||||
same repository. They now share one probe and one snapshot, so opening both
|
||||
costs one shell-out and one parse, and opening the overlay while the panel is
|
||||
already showing that repo paints immediately instead of re-probing. (#239)
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Rounded UI controls no longer square off their corners**
|
||||
|
||||
@@ -285,6 +285,16 @@ pub struct Config {
|
||||
/// (`repo`, the default), or one flat list (`none`).
|
||||
#[serde(default, deserialize_with = "de_lenient")]
|
||||
pub sidebar_grouping: SidebarGrouping,
|
||||
/// Whether clicking a sidebar row's `+N −N` working-tree counts opens the
|
||||
/// diff overlay. Off leaves the branch and the counts exactly as they are —
|
||||
/// they are a readout worth having on their own — and only takes away the
|
||||
/// click target and its pointer cursor, so the press falls through to
|
||||
/// ordinary tab activation. On by default: the overlay is the reason the
|
||||
/// counts are there for most people, and the large-diff cost it used to
|
||||
/// carry is now bounded by the diff parser's budgets. This is the escape
|
||||
/// hatch for anyone who wants the numbers without the viewer.
|
||||
#[serde(default = "default_true")]
|
||||
pub sidebar_diff_preview: bool,
|
||||
/// When to post a desktop notification after a long foreground command
|
||||
/// finishes.
|
||||
#[serde(default, deserialize_with = "de_lenient")]
|
||||
@@ -751,6 +761,8 @@ impl Default for Config {
|
||||
right_panel_width: default_right_panel_width(),
|
||||
right_panel_tab: RightPanelTab::Info,
|
||||
sidebar_grouping: SidebarGrouping::Repo,
|
||||
// Today's behaviour, unchanged: the counts open the overlay.
|
||||
sidebar_diff_preview: true,
|
||||
notify_on_command_finish: NotifyMode::Unfocused,
|
||||
// Opt-out, not opt-in: a stale terminal that never tells you it's
|
||||
// outdated is the status quo we're fixing. One cheap GET at startup.
|
||||
@@ -1196,6 +1208,27 @@ mod tests {
|
||||
assert_eq!(back.ssh_profile_frecency.get(&id).unwrap().count, 4);
|
||||
}
|
||||
|
||||
/// The sidebar diff preview is opt-*out*: a config written before the switch
|
||||
/// existed keeps today's clickable counts, and turning it off survives a
|
||||
/// write/read cycle of `config.json` (issue #239).
|
||||
#[test]
|
||||
fn sidebar_diff_preview_defaults_on_and_round_trips() {
|
||||
assert!(Config::default().sidebar_diff_preview);
|
||||
|
||||
let old: Config = serde_json::from_str(r#"{"font_size": 15.0}"#).unwrap();
|
||||
assert!(
|
||||
old.sidebar_diff_preview,
|
||||
"absent key means today's behaviour"
|
||||
);
|
||||
|
||||
let off: Config = serde_json::from_str(r#"{"sidebar_diff_preview": false}"#).unwrap();
|
||||
assert!(!off.sidebar_diff_preview);
|
||||
let json = serde_json::to_string(&off).unwrap();
|
||||
assert!(json.contains("\"sidebar_diff_preview\":false"), "persisted");
|
||||
let back: Config = serde_json::from_str(&json).unwrap();
|
||||
assert!(!back.sidebar_diff_preview);
|
||||
}
|
||||
|
||||
/// Opt-*out*, unlike most flags here: a config written before this setting
|
||||
/// existed must keep the prompt, or an update would silently take away the
|
||||
/// one thing telling people their sessions survive a quit.
|
||||
|
||||
+1
-1
@@ -58,7 +58,7 @@ it never wraps or replaces the agent.
|
||||
- **Brand avatars** — the tab chip / sidebar row shows which agent runs where; custom wrappers map in via `agent_commands` in `config.json`
|
||||
- **Status dot** — working (blue) / needs your input (amber) / done (green), driven by agent-reported events over an OSC channel; Settings → Agents installs the hooks that feed it (Claude Code, Codex, Copilot CLI, OpenCode, Pi, Grok Build)
|
||||
- **Notifications** — "needs your permission…" the moment an agent blocks on you, and "finished after Ns" per turn, honoring your notification policy
|
||||
- **Branch at a glance** — each sidebar row shows its pane's git branch and working-tree diff (`+N −M`), refreshed on `cd` and when a command finishes
|
||||
- **Branch at a glance** — each sidebar row shows its pane's git branch and working-tree diff (`+N −M`), refreshed on `cd` and when a command finishes; clicking the counts opens the diff overlay, and turning that off (Settings → Window & Tabs, or `sidebar_diff_preview: false` in `config.json`) keeps the readout while making it non-clickable
|
||||
- **Session resume** — panes lost to a reboot re-launch their agent conversation on restore, carrying the original launch flags (`claude --dangerously-skip-permissions --resume …`) (`restore_agent_sessions`, on by default)
|
||||
- **Fork session** — branch a live agent conversation into a second, independent one by shelling the agent's own fork command (`codex fork <id>`, `claude --resume <id> --fork-session`, also OpenCode and Grok Build); the original is untouched and both continue separately. Right-click a pane to pick a split placement, or right-click the tab / sidebar row to open the fork in a new tab. Needs the agent's hooks installed, since the fork targets the session id they report; a remote pane can't fork, because the command would run against the local agent — and note a fork copies the whole transcript, so repeated forking costs real disk in the agent's own session store
|
||||
- **Copy Session ID** — put the agent's native session id on the clipboard, beside *Copy Working Directory*, for pasting into `codex resume`, a bug report, or another tool
|
||||
|
||||
@@ -55,7 +55,7 @@ Aider、Amp、OpenCode 等约 17 个)并在其外围加功能 —— 绝不包
|
||||
- **品牌头像** —— 标签 chip / 侧栏行显示每个 pane 跑的是哪个 agent;自定义包装命令可通过 `config.json` 的 `agent_commands` 映射
|
||||
- **状态点** —— 工作中(蓝)/ 等你输入(琥珀)/ 完成(绿),由 agent 自己上报的 OSC 事件驱动;在 设置 → Agents 一键装好对应 hooks(Claude Code、Codex、Copilot CLI、OpenCode、Pi、Grok Build)
|
||||
- **通知** —— agent 卡在等你批准的那一刻弹 "needs your permission…",每轮结束弹 "finished after Ns",遵循你的通知策略
|
||||
- **一眼看分支** —— 侧栏每行显示该 pane 的 git 分支和工作区改动(`+N −M`),`cd` 或命令跑完时自动刷新
|
||||
- **一眼看分支** —— 侧栏每行显示该 pane 的 git 分支和工作区改动(`+N −M`),`cd` 或命令跑完时自动刷新;点改动数字会打开 diff 浮层,关掉它(设置 → 窗口与标签,或 `config.json` 的 `sidebar_diff_preview: false`)分支和数字照常显示,只是不再可点
|
||||
- **会话恢复** —— 重启后无法重连的 pane 会自动续上 agent 对话,并带上原始启动 flags(`claude --dangerously-skip-permissions --resume …`;`restore_agent_sessions`,默认开启)
|
||||
- **Fork 会话** —— 直接调 agent 自己的 fork 命令(`codex fork <id>`、`claude --resume <id> --fork-session`,OpenCode 和 Grok Build 同样支持),把当前对话分叉成一个独立会话;原会话原封不动,两边各自往下走。在 pane 上右键可选择分屏位置,在标签 / 侧栏行上右键则直接开新标签。需要先装好该 agent 的 hooks(fork 认的是 hooks 上报的 session id);远程 pane 不能 fork,因为命令会跑在本机的 agent 上;另外 fork 会整份复制对话历史,反复 fork 会在 agent 自己的会话目录里占掉不少磁盘
|
||||
- **复制 Session ID** —— 把 agent 的原生 session id 复制到剪贴板,就在 *Copy Working Directory* 旁边,方便粘进 `codex resume`、bug 报告或别的工具
|
||||
|
||||
+420
-50
@@ -10,6 +10,14 @@
|
||||
//! previous snapshot (or a loading state) until a probe lands. Asking the pane's
|
||||
//! host rather than this machine is also what makes the overlay work at all for
|
||||
//! a pane whose repository lives somewhere else.
|
||||
//!
|
||||
//! And never trusted to be *small*, either. `git diff HEAD` is the one git read
|
||||
//! in this app whose output scales with the working tree rather than with what
|
||||
//! the UI can show, so the parser retains at most [`MAX_LINES_PER_FILE`] per
|
||||
//! file and [`MAX_TOTAL_LINES`] / [`MAX_FILES_WITH_HUNKS`] across the
|
||||
//! repository. The `+`/`−` counts deliberately escape all of it: they are
|
||||
//! compared against `git diff --numstat` to decide whether the overlay is
|
||||
//! stale, so a capped total would disagree forever and re-probe in a loop.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
@@ -22,10 +30,54 @@ use crate::ui::host_ops::Host;
|
||||
/// tree. Generous enough that real hand-written changes never hit it.
|
||||
pub const MAX_LINES_PER_FILE: usize = 2000;
|
||||
|
||||
/// Repo-wide cap on retained diff lines. [`MAX_LINES_PER_FILE`] bounds one
|
||||
/// pathological file; this bounds the *sum*, which is the shape a working tree
|
||||
/// full of agent edits actually takes — two hundred files of three hundred
|
||||
/// lines each never trip the per-file cap yet retain 60k `DiffLine`s, each an
|
||||
/// owned `String`. Past this the parser keeps counting `+`/`−` (the header
|
||||
/// numbers must stay honest — see the note in [`parse_unified`]) but stops
|
||||
/// retaining line text.
|
||||
pub const MAX_TOTAL_LINES: usize = 20_000;
|
||||
|
||||
/// Repo-wide cap on how many files keep their hunks. A branch that renames a
|
||||
/// vendored tree can list thousands of files whose diffs are each tiny; every
|
||||
/// one of them still costs a `Vec<Hunk>`. Files past this keep their header row
|
||||
/// (path, status, counts) and lose only the body.
|
||||
pub const MAX_FILES_WITH_HUNKS: usize = 500;
|
||||
|
||||
/// A file's added+removed size at which the overlay collapses it by default
|
||||
/// (GitHub's "Load diff" treatment) — the user can still expand it by click.
|
||||
pub const AUTO_COLLAPSE_LINES: u32 = 400;
|
||||
|
||||
/// Repo-wide counterpart to [`AUTO_COLLAPSE_LINES`]: once the snapshot's
|
||||
/// *retained* lines exceed this, every file starts collapsed and the overlay
|
||||
/// leads with the oversized-diff summary. Two differences from the per-file
|
||||
/// threshold matter here — this counts context lines too (they are rendered,
|
||||
/// so they are what costs), and it is a sum, so many medium files add up the
|
||||
/// way one big file does.
|
||||
pub const AUTO_COLLAPSE_TOTAL_LINES: usize = 2_000;
|
||||
|
||||
/// The same idea by file count. Set well clear of a busy-but-ordinary tree —
|
||||
/// forty changed files of a few lines each is a normal afternoon and must still
|
||||
/// open expanded — because rows, not cards, are what actually cost: this axis
|
||||
/// only catches the tree so wide that a card per file is itself the problem.
|
||||
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.
|
||||
pub const MAX_RENDERED_FILES: usize = 300;
|
||||
|
||||
/// Why a file's hunks stop short of its real diff.
|
||||
#[derive(Clone, Copy, PartialEq, Eq, Debug)]
|
||||
pub enum Truncation {
|
||||
/// This one file exceeded [`MAX_LINES_PER_FILE`].
|
||||
PerFile,
|
||||
/// The repo-wide budget ([`MAX_TOTAL_LINES`] / [`MAX_FILES_WITH_HUNKS`])
|
||||
/// ran out — the file itself may be small.
|
||||
Budget,
|
||||
}
|
||||
|
||||
/// One parsed `git diff HEAD` for a repo, plus the untracked files `diff`
|
||||
/// itself can't see. This is the overlay's whole model.
|
||||
#[derive(Clone, PartialEq, Eq, Debug, Default)]
|
||||
@@ -47,11 +99,47 @@ impl DiffSnapshot {
|
||||
/// Total added/removed line counts across all files — the overlay's
|
||||
/// header numbers, matching the sidebar's `+N −N` by construction (both
|
||||
/// sum per-file counts of the same `HEAD` diff).
|
||||
///
|
||||
/// Deliberately *not* affected by any truncation: the parser keeps counting
|
||||
/// past every cap, because this number is compared against the status
|
||||
/// cache's `git diff --numstat` totals to decide whether the overlay is
|
||||
/// stale. A capped total would never match, and the overlay would re-probe
|
||||
/// in a loop.
|
||||
pub fn totals(&self) -> (u32, u32) {
|
||||
self.files
|
||||
.iter()
|
||||
.fold((0, 0), |(a, r), f| (a + f.added, r + f.removed))
|
||||
}
|
||||
|
||||
/// Diff lines actually kept, summed over every file. This — not
|
||||
/// [`totals`](Self::totals) — is what the overlay would have to build rows
|
||||
/// for if every file were expanded, so it's what the render-side thresholds
|
||||
/// compare against. Cheap: one `len()` per hunk, not per line.
|
||||
pub fn retained_lines(&self) -> usize {
|
||||
self.files
|
||||
.iter()
|
||||
.flat_map(|f| &f.hunks)
|
||||
.map(|h| h.lines.len())
|
||||
.sum()
|
||||
}
|
||||
|
||||
/// 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.
|
||||
pub fn oversized(&self) -> bool {
|
||||
self.files.len() > AUTO_COLLAPSE_TOTAL_FILES
|
||||
|| self.retained_lines() > AUTO_COLLAPSE_TOTAL_LINES
|
||||
}
|
||||
|
||||
/// Whether the repo-wide budget dropped hunks that a smaller diff would
|
||||
/// have kept — the overlay says so, so a missing body reads as a cap rather
|
||||
/// than as tty7 losing the change.
|
||||
pub fn budget_exhausted(&self) -> bool {
|
||||
self.files
|
||||
.iter()
|
||||
.any(|f| f.truncated == Some(Truncation::Budget))
|
||||
}
|
||||
}
|
||||
|
||||
/// How a file changed vs `HEAD` — drives the status glyph in its header row.
|
||||
@@ -76,9 +164,10 @@ pub struct FileDiff {
|
||||
pub removed: u32,
|
||||
/// Binary file — no hunks, the header row says "binary" instead.
|
||||
pub binary: bool,
|
||||
/// Hunk parsing stopped at [`MAX_LINES_PER_FILE`]; the overlay appends a
|
||||
/// "truncated" footer under the last hunk.
|
||||
pub truncated: bool,
|
||||
/// Hunk parsing stopped short of the file's real diff, and why; the overlay
|
||||
/// appends a "truncated" footer under the last hunk. `None` means the body
|
||||
/// is complete.
|
||||
pub truncated: Option<Truncation>,
|
||||
pub hunks: Vec<Hunk>,
|
||||
}
|
||||
|
||||
@@ -152,117 +241,174 @@ pub fn probe(host: &dyn Host, cwd: &Path) -> Option<DiffSnapshot> {
|
||||
/// construction: unrecognized metadata lines between the `diff --git` header
|
||||
/// and the first hunk (modes, index, similarity) are simply skipped, so a git
|
||||
/// version printing extra headers degrades to "fewer facts", never a panic.
|
||||
///
|
||||
/// The whole-string form the tests drive the parser through; [`probe`] streams
|
||||
/// the same [`DiffParser`] line by line off git's stdout instead, so no caller
|
||||
/// in the app ever holds the full diff as one `String`.
|
||||
#[cfg(test)]
|
||||
pub fn parse_unified(out: &str) -> Vec<FileDiff> {
|
||||
let mut files: Vec<FileDiff> = Vec::new();
|
||||
// Line-number counters for the hunk currently being filled.
|
||||
let (mut old_no, mut new_no) = (0u32, 0u32);
|
||||
// Lines consumed by the current file's hunks, for the per-file cap.
|
||||
let mut file_lines = 0usize;
|
||||
|
||||
let mut parser = DiffParser::default();
|
||||
for line in out.lines() {
|
||||
parser.push_line(line);
|
||||
}
|
||||
parser.finish()
|
||||
}
|
||||
|
||||
/// The unified-diff parser as an incremental state machine, so `git diff`'s
|
||||
/// output can be consumed a line at a time off a pipe rather than buffered
|
||||
/// whole. Enforces both the per-file and the repo-wide retention budgets; see
|
||||
/// [`MAX_LINES_PER_FILE`] and [`MAX_TOTAL_LINES`].
|
||||
#[derive(Default)]
|
||||
pub struct DiffParser {
|
||||
files: Vec<FileDiff>,
|
||||
/// Line-number counters for the hunk currently being filled.
|
||||
old_no: u32,
|
||||
new_no: u32,
|
||||
/// Lines consumed by the current file's hunks, for the per-file cap.
|
||||
file_lines: usize,
|
||||
/// Lines retained across every file so far, for the repo-wide cap.
|
||||
total_lines: usize,
|
||||
/// Files that actually kept at least one hunk, for the repo-wide file cap.
|
||||
/// Counted at the first `@@`, not at the file header: a pure rename or a
|
||||
/// binary blob has no body and must not spend the budget on nothing.
|
||||
files_with_hunks: usize,
|
||||
/// Whether the last `@@` header opened a body we're still inside. Tracked
|
||||
/// explicitly rather than inferred from "the file has hunks": a file the
|
||||
/// budget truncated never gets a `Hunk` pushed, but its lines still have to
|
||||
/// be *counted*, and a removed line whose own text starts with `--- ` must
|
||||
/// not be mistaken for the file header it looks like.
|
||||
in_hunk: bool,
|
||||
}
|
||||
|
||||
impl DiffParser {
|
||||
/// Feed one line of `git diff` output (no trailing newline).
|
||||
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);
|
||||
files.push(FileDiff {
|
||||
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: false,
|
||||
truncated: None,
|
||||
hunks: Vec::new(),
|
||||
});
|
||||
file_lines = 0;
|
||||
continue;
|
||||
self.file_lines = 0;
|
||||
self.in_hunk = false;
|
||||
return;
|
||||
}
|
||||
let Some(file) = files.last_mut() else {
|
||||
continue; // preamble before any header (shouldn't happen)
|
||||
let Some(file) = self.files.last_mut() else {
|
||||
return; // preamble before any header (shouldn't happen)
|
||||
};
|
||||
// ── File-level metadata between the header and the first hunk ──────
|
||||
if line.starts_with("new file mode") {
|
||||
file.status = FileStatus::Added;
|
||||
continue;
|
||||
return;
|
||||
}
|
||||
if line.starts_with("deleted file mode") {
|
||||
file.status = FileStatus::Deleted;
|
||||
continue;
|
||||
return;
|
||||
}
|
||||
if line.starts_with("rename from ") {
|
||||
file.status = FileStatus::Renamed;
|
||||
continue;
|
||||
return;
|
||||
}
|
||||
if line.starts_with("Binary files ") || line.starts_with("GIT binary patch") {
|
||||
file.binary = true;
|
||||
continue;
|
||||
return;
|
||||
}
|
||||
// `--- a/x` / `+++ b/x` repeat what the header said; `rename to`,
|
||||
// `index`, modes and similarity scores add nothing we render. But only
|
||||
// skip them *outside* hunk bodies — a removed line legitimately starts
|
||||
// with `--- ` inside one.
|
||||
if file.hunks.is_empty()
|
||||
if !self.in_hunk
|
||||
&& (line.starts_with("--- ") || line.starts_with("+++ ") || !is_hunk_line(line))
|
||||
&& !line.starts_with("@@")
|
||||
{
|
||||
continue;
|
||||
return;
|
||||
}
|
||||
// ── Hunks ───────────────────────────────────────────────────────────
|
||||
if line.starts_with("@@") {
|
||||
if file.truncated {
|
||||
continue; // past the cap: swallow the rest of this file
|
||||
// A truncated file still enters the body — its lines have to be
|
||||
// counted — it just doesn't get a `Hunk` to keep them in.
|
||||
self.in_hunk = true;
|
||||
if file.truncated.is_some() {
|
||||
return;
|
||||
}
|
||||
// The repo-wide budget is charged here rather than at the file
|
||||
// header, so a file with no body at all (a pure rename, a binary
|
||||
// blob) neither consumes the file budget nor gets flagged as
|
||||
// truncated for a body it never had.
|
||||
let first_hunk = file.hunks.is_empty();
|
||||
if (first_hunk && self.files_with_hunks >= MAX_FILES_WITH_HUNKS)
|
||||
|| self.total_lines >= MAX_TOTAL_LINES
|
||||
{
|
||||
file.truncated = Some(Truncation::Budget);
|
||||
return;
|
||||
}
|
||||
if first_hunk {
|
||||
self.files_with_hunks += 1;
|
||||
}
|
||||
let (o, n) = parse_hunk_starts(line).unwrap_or((0, 0));
|
||||
old_no = o;
|
||||
new_no = n;
|
||||
self.old_no = o;
|
||||
self.new_no = n;
|
||||
file.hunks.push(Hunk {
|
||||
header: line.to_string(),
|
||||
lines: Vec::new(),
|
||||
});
|
||||
continue;
|
||||
return;
|
||||
}
|
||||
if file.hunks.is_empty() {
|
||||
continue; // stray content outside any hunk
|
||||
if !self.in_hunk {
|
||||
return; // stray content outside any hunk
|
||||
}
|
||||
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..]),
|
||||
// `\ No newline at end of file` and anything else: not a diff line.
|
||||
_ => continue,
|
||||
_ => return,
|
||||
};
|
||||
// Count added/removed *before* the truncation gate: the cap is about
|
||||
// element volume, but the header numbers must stay honest, so lines
|
||||
// past the cap still count even though they're never kept.
|
||||
// Count added/removed *before* the truncation gate: the caps are about
|
||||
// element volume, but the header numbers must stay honest (they are
|
||||
// compared against `--numstat` to detect staleness), so lines past a cap
|
||||
// still count even though they're never kept.
|
||||
match kind {
|
||||
LineKind::Added => file.added += 1,
|
||||
LineKind::Removed => file.removed += 1,
|
||||
LineKind::Context => {}
|
||||
}
|
||||
if file.truncated {
|
||||
continue;
|
||||
if file.truncated.is_some() {
|
||||
return;
|
||||
}
|
||||
file_lines += 1;
|
||||
if file_lines > MAX_LINES_PER_FILE {
|
||||
file.truncated = true;
|
||||
continue;
|
||||
self.file_lines += 1;
|
||||
if self.file_lines > MAX_LINES_PER_FILE {
|
||||
file.truncated = Some(Truncation::PerFile);
|
||||
return;
|
||||
}
|
||||
if self.total_lines >= MAX_TOTAL_LINES {
|
||||
file.truncated = Some(Truncation::Budget);
|
||||
return;
|
||||
}
|
||||
let Some(hunk) = file.hunks.last_mut() else {
|
||||
continue;
|
||||
return;
|
||||
};
|
||||
let (o, n) = match kind {
|
||||
LineKind::Added => {
|
||||
let n = new_no;
|
||||
new_no += 1;
|
||||
let n = self.new_no;
|
||||
self.new_no += 1;
|
||||
(None, Some(n))
|
||||
}
|
||||
LineKind::Removed => {
|
||||
let o = old_no;
|
||||
old_no += 1;
|
||||
let o = self.old_no;
|
||||
self.old_no += 1;
|
||||
(Some(o), None)
|
||||
}
|
||||
LineKind::Context => {
|
||||
let (o, n) = (old_no, new_no);
|
||||
old_no += 1;
|
||||
new_no += 1;
|
||||
let (o, n) = (self.old_no, self.new_no);
|
||||
self.old_no += 1;
|
||||
self.new_no += 1;
|
||||
(Some(o), Some(n))
|
||||
}
|
||||
};
|
||||
@@ -272,10 +418,15 @@ pub fn parse_unified(out: &str) -> Vec<FileDiff> {
|
||||
new_no: n,
|
||||
text: text.to_string(),
|
||||
});
|
||||
self.total_lines += 1;
|
||||
}
|
||||
|
||||
/// The parsed files. A truncated file still counts +/− for its whole diff
|
||||
/// (the parser keeps counting past every cap), so totals stay consistent
|
||||
/// with `--numstat`.
|
||||
pub fn finish(self) -> Vec<FileDiff> {
|
||||
self.files
|
||||
}
|
||||
// A truncated file still counts +/− for its whole diff (the loop above
|
||||
// keeps counting past the cap), so totals stay consistent with numstat.
|
||||
files
|
||||
}
|
||||
|
||||
/// Whether a line can only belong to a hunk body (`+`/`-`/space/`\` lead).
|
||||
@@ -486,7 +637,7 @@ index 1111111..2222222 100644
|
||||
out.push_str(&format!("+line {i}\n"));
|
||||
}
|
||||
let files = parse_unified(&out);
|
||||
assert!(files[0].truncated);
|
||||
assert_eq!(files[0].truncated, Some(Truncation::PerFile));
|
||||
assert_eq!(files[0].added, 3000);
|
||||
let kept: usize = files[0].hunks.iter().map(|h| h.lines.len()).sum();
|
||||
assert_eq!(kept, MAX_LINES_PER_FILE);
|
||||
@@ -520,4 +671,223 @@ index 1..2 100644
|
||||
};
|
||||
assert_eq!(snap.totals(), (4, 2));
|
||||
}
|
||||
|
||||
/// Build `files` synthetic modified files of `lines_each` added lines —
|
||||
/// the "many medium files" shape the per-file cap alone can't bound.
|
||||
fn many_files(files: usize, lines_each: usize) -> String {
|
||||
let mut out = String::new();
|
||||
for f in 0..files {
|
||||
out.push_str(&format!(
|
||||
"diff --git a/f{f}.rs b/f{f}.rs\nindex 1..2 100644\n--- a/f{f}.rs\n+++ b/f{f}.rs\n@@ -0,0 +1,{lines_each} @@\n"
|
||||
));
|
||||
for i in 0..lines_each {
|
||||
out.push_str(&format!("+file {f} line {i}\n"));
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// The repo-wide budget bounds retained lines even when no single file is
|
||||
/// anywhere near [`MAX_LINES_PER_FILE`] — the case the reporter of #239
|
||||
/// called out as the one the per-file cap misses.
|
||||
#[test]
|
||||
fn repo_wide_budget_caps_retained_lines() {
|
||||
// 300 files × 300 lines = 90k lines, none of which trips the 2000-line
|
||||
// per-file cap.
|
||||
let files = parse_unified(&many_files(300, 300));
|
||||
assert_eq!(files.len(), 300, "every file keeps its header row");
|
||||
let retained: usize = files
|
||||
.iter()
|
||||
.flat_map(|f| &f.hunks)
|
||||
.map(|h| h.lines.len())
|
||||
.sum();
|
||||
assert!(
|
||||
retained <= MAX_TOTAL_LINES,
|
||||
"retained {retained} lines, budget is {MAX_TOTAL_LINES}"
|
||||
);
|
||||
assert!(
|
||||
files
|
||||
.iter()
|
||||
.any(|f| f.truncated == Some(Truncation::Budget))
|
||||
);
|
||||
}
|
||||
|
||||
/// Budget or no budget, the +/− totals must stay exact: they are compared
|
||||
/// against `git diff --numstat` to decide whether the overlay is stale, and
|
||||
/// a short count would make every comparison disagree and re-probe forever.
|
||||
#[test]
|
||||
fn repo_wide_budget_keeps_totals_exact() {
|
||||
let snap = DiffSnapshot {
|
||||
files: parse_unified(&many_files(300, 300)),
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(snap.totals(), (90_000, 0));
|
||||
assert!(snap.budget_exhausted());
|
||||
}
|
||||
|
||||
/// The file cap keeps a rename-the-world diff from allocating a `Vec<Hunk>`
|
||||
/// per file, while every file still lists its path and counts.
|
||||
#[test]
|
||||
fn repo_wide_budget_caps_files_with_hunks() {
|
||||
// One line each: far under the line budget, so only the file cap can
|
||||
// stop this.
|
||||
let files = parse_unified(&many_files(MAX_FILES_WITH_HUNKS + 50, 1));
|
||||
assert_eq!(files.len(), MAX_FILES_WITH_HUNKS + 50);
|
||||
let with_hunks = files.iter().filter(|f| !f.hunks.is_empty()).count();
|
||||
assert_eq!(with_hunks, MAX_FILES_WITH_HUNKS);
|
||||
// The tail still counts, so totals stay honest.
|
||||
assert_eq!(
|
||||
files.iter().map(|f| f.added).sum::<u32>(),
|
||||
(MAX_FILES_WITH_HUNKS + 50) as u32
|
||||
);
|
||||
assert_eq!(files.last().unwrap().truncated, Some(Truncation::Budget));
|
||||
}
|
||||
|
||||
/// A small diff is untouched by the budget — the setting-enabled,
|
||||
/// small-working-tree case must behave exactly as before.
|
||||
#[test]
|
||||
fn small_diff_is_not_truncated() {
|
||||
let snap = DiffSnapshot {
|
||||
files: parse_unified(SAMPLE),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(snap.files.iter().all(|f| f.truncated.is_none()));
|
||||
assert!(!snap.oversized());
|
||||
assert!(!snap.budget_exhausted());
|
||||
}
|
||||
|
||||
/// `oversized` trips on either axis: many files, or many retained lines.
|
||||
#[test]
|
||||
fn oversized_trips_on_files_or_lines() {
|
||||
let by_files = DiffSnapshot {
|
||||
files: parse_unified(&many_files(AUTO_COLLAPSE_TOTAL_FILES + 1, 1)),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(by_files.oversized());
|
||||
|
||||
// Few files, but past the line threshold.
|
||||
let by_lines = DiffSnapshot {
|
||||
files: parse_unified(&many_files(2, AUTO_COLLAPSE_TOTAL_LINES)),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(by_lines.files.len() <= AUTO_COLLAPSE_TOTAL_FILES);
|
||||
assert!(by_lines.retained_lines() > AUTO_COLLAPSE_TOTAL_LINES);
|
||||
assert!(by_lines.oversized());
|
||||
}
|
||||
|
||||
/// Even a budget-truncated file keeps a removed line whose *content* starts
|
||||
/// with `--- ` out of the metadata skip — that line still has to count.
|
||||
#[test]
|
||||
fn truncated_file_counts_dash_prefixed_content() {
|
||||
let mut out = many_files(MAX_FILES_WITH_HUNKS, 1);
|
||||
out.push_str(
|
||||
"diff --git a/late.md b/late.md\nindex 1..2 100644\n--- a/late.md\n+++ b/late.md\n@@ -1,2 +1,1 @@\n keep\n--- a heading rule\n",
|
||||
);
|
||||
let files = parse_unified(&out);
|
||||
let late = files.last().unwrap();
|
||||
assert_eq!(late.path, "late.md");
|
||||
assert_eq!(late.truncated, Some(Truncation::Budget));
|
||||
assert!(late.hunks.is_empty(), "no body kept past the file cap");
|
||||
assert_eq!((late.added, late.removed), (0, 1), "but the line counts");
|
||||
}
|
||||
|
||||
/// Measurement harness for issue #239 finding 1 — run with
|
||||
/// `cargo test --release -- --ignored --nocapture bench_stream_vs_buffer`.
|
||||
///
|
||||
/// Stands in for `Command::output()` vs [`git_status::git_lines`] with a
|
||||
/// file on disk, so the two paths differ only in whether the whole diff is
|
||||
/// materialised as one `String` before parsing starts.
|
||||
#[test]
|
||||
#[ignore = "measurement, not an assertion"]
|
||||
fn bench_stream_vs_buffer() {
|
||||
use std::io::{BufRead as _, BufReader, Write as _};
|
||||
use std::time::Instant;
|
||||
|
||||
let path = std::env::temp_dir().join("tty7-diff-bench.patch");
|
||||
{
|
||||
let mut f = std::io::BufWriter::new(std::fs::File::create(&path).unwrap());
|
||||
// ~90k changed lines over 300 files — a big but not absurd agent
|
||||
// session's working tree.
|
||||
for file in 0..300 {
|
||||
write!(
|
||||
f,
|
||||
"diff --git a/f{file}.rs b/f{file}.rs\nindex 1..2 100644\n--- a/f{file}.rs\n+++ b/f{file}.rs\n@@ -0,0 +1,300 @@\n"
|
||||
)
|
||||
.unwrap();
|
||||
for i in 0..300 {
|
||||
writeln!(f, "+file {file} line {i} of a fairly typical source line").unwrap();
|
||||
}
|
||||
}
|
||||
}
|
||||
let size = std::fs::metadata(&path).unwrap().len();
|
||||
|
||||
let t = Instant::now();
|
||||
let buffered = std::fs::read_to_string(&path).unwrap();
|
||||
let read = t.elapsed();
|
||||
let t = Instant::now();
|
||||
let a = parse_unified(&buffered);
|
||||
println!(
|
||||
"buffered: read {size} bytes in {read:?} (all resident), parse {:?}, {} files",
|
||||
t.elapsed(),
|
||||
a.len()
|
||||
);
|
||||
drop(buffered);
|
||||
|
||||
let t = Instant::now();
|
||||
let mut parser = DiffParser::default();
|
||||
// Same capacity `git_lines` uses.
|
||||
let mut reader = BufReader::with_capacity(64 * 1024, std::fs::File::open(&path).unwrap());
|
||||
let mut buf = Vec::new();
|
||||
let mut longest = 0usize;
|
||||
loop {
|
||||
buf.clear();
|
||||
if reader.read_until(b'\n', &mut buf).unwrap() == 0 {
|
||||
break;
|
||||
}
|
||||
while buf.last().is_some_and(|b| *b == b'\n' || *b == b'\r') {
|
||||
buf.pop();
|
||||
}
|
||||
longest = longest.max(buf.len());
|
||||
parser.push_line(&String::from_utf8_lossy(&buf));
|
||||
}
|
||||
let b = parser.finish();
|
||||
println!(
|
||||
"streamed: read+parse {:?}, {} files, peak transient buffer {longest} bytes \
|
||||
(vs {size} buffered)",
|
||||
t.elapsed(),
|
||||
b.len()
|
||||
);
|
||||
assert_eq!(a.len(), b.len());
|
||||
let _ = std::fs::remove_file(&path);
|
||||
}
|
||||
|
||||
/// Measurement harness for issue #239, not a correctness gate — run with
|
||||
/// `cargo test -- --ignored --nocapture bench_parse_budget`.
|
||||
#[test]
|
||||
#[ignore = "measurement, not an assertion"]
|
||||
fn bench_parse_budget() {
|
||||
use std::time::Instant;
|
||||
let out = many_files(300, 300);
|
||||
println!("input: {} bytes, 300 files × 300 lines", out.len());
|
||||
let t = Instant::now();
|
||||
let files = parse_unified(&out);
|
||||
let elapsed = t.elapsed();
|
||||
let retained: usize = files
|
||||
.iter()
|
||||
.flat_map(|f| &f.hunks)
|
||||
.map(|h| h.lines.len())
|
||||
.sum();
|
||||
let bytes: usize = files
|
||||
.iter()
|
||||
.flat_map(|f| &f.hunks)
|
||||
.flat_map(|h| &h.lines)
|
||||
.map(|l| l.text.capacity() + std::mem::size_of::<DiffLine>())
|
||||
.sum();
|
||||
println!(
|
||||
"parse {elapsed:?} → {retained} retained lines, ~{} KiB of DiffLine text \
|
||||
(unbudgeted would be 90000 lines / ~{} KiB)",
|
||||
bytes / 1024,
|
||||
90_000 * (24 + std::mem::size_of::<DiffLine>()) / 1024,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -734,6 +734,14 @@ pub struct Tty7App {
|
||||
pub(crate) sftp_panel: crate::ui::sftp::SftpPanelState,
|
||||
/// Right detail panel (info / changes / files) docked beside the terminal.
|
||||
pub(crate) right_panel: crate::ui::right_panel::RightPanelState,
|
||||
/// Repositories with a `git diff HEAD` probe in flight, keyed by machine
|
||||
/// *and* working directory — the same path on two hosts is two different
|
||||
/// work trees. One probe answers everyone: the diff overlay on any number of
|
||||
/// tabs and the Changes panel all read the same result, instead of each
|
||||
/// running its own copy of the same invocation and parse. See
|
||||
/// [`Tty7App::spawn_shared_diff_probe`](crate::ui::app::Tty7App::spawn_shared_diff_probe).
|
||||
pub(crate) diff_probes_inflight:
|
||||
std::collections::HashSet<(crate::ui::host_ops::HostId, std::path::PathBuf)>,
|
||||
/// Local project file tree (left column of the body).
|
||||
pub(crate) file_tree: crate::ui::file_tree::FileTreeState,
|
||||
/// Code-editor panel (right column of the body).
|
||||
@@ -1207,6 +1215,7 @@ impl Tty7App {
|
||||
},
|
||||
sftp_panel,
|
||||
right_panel: Default::default(),
|
||||
diff_probes_inflight: Default::default(),
|
||||
file_tree,
|
||||
editor,
|
||||
sidebar_width: Rc::new(Cell::new(sidebar_width)),
|
||||
@@ -2850,6 +2859,14 @@ impl Tty7App {
|
||||
self.update_config(cx, |cfg| cfg.sidebar_grouping = grouping);
|
||||
}
|
||||
|
||||
/// Set whether the sidebar's `+N −N` counts open the diff overlay
|
||||
/// (Settings → Window & Tabs). The counts themselves are unaffected either
|
||||
/// way — this only governs the click. Persists the choice; the sidebar
|
||||
/// re-derives from the `Config` global on the next render.
|
||||
pub(crate) fn set_sidebar_diff_preview(&mut self, on: bool, cx: &mut Context<Self>) {
|
||||
self.update_config(cx, |cfg| cfg.sidebar_diff_preview = on);
|
||||
}
|
||||
|
||||
/// `ToggleTabSidebar`: flip the tab bar between the horizontal title-bar strip
|
||||
/// (`Top`) and the vertical left sidebar (`Left`), persisting the choice.
|
||||
pub(crate) fn toggle_tab_sidebar(&mut self, cx: &mut Context<Self>) {
|
||||
|
||||
+362
-37
@@ -17,9 +17,20 @@
|
||||
//! snapshot whose branch or counts disagree with what's shown — so a finishing
|
||||
//! command or agent turn refreshes the overlay through the exact trigger
|
||||
//! machinery the sidebar numbers already use.
|
||||
//!
|
||||
//! One probe, one snapshot, however many watchers: every tab's overlay and the
|
||||
//! Changes panel go through
|
||||
//! [`spawn_shared_diff_probe`](Tty7App::spawn_shared_diff_probe) and hold the
|
||||
//! result behind an `Arc`. The element tree, meanwhile, is *not* virtualized —
|
||||
//! so what keeps a big working tree from stalling the window is refusing to
|
||||
//! build the rows in the first place: past
|
||||
//! [`AUTO_COLLAPSE_TOTAL_LINES`](git_diff::AUTO_COLLAPSE_TOTAL_LINES) every
|
||||
//! file opens collapsed under a summary, and at most
|
||||
//! [`MAX_RENDERED_FILES`] cards are built at all.
|
||||
|
||||
use std::collections::HashSet;
|
||||
use std::path::PathBuf;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
use gpui::{
|
||||
AnyElement, FocusHandle, FontWeight, KeyDownEvent, Pixels, Window, div, prelude::*, px,
|
||||
@@ -28,7 +39,8 @@ 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, FileDiff, FileStatus, LineKind,
|
||||
self, AUTO_COLLAPSE_LINES, DiffSnapshot, FileDiff, FileStatus, LineKind, MAX_RENDERED_FILES,
|
||||
Truncation,
|
||||
};
|
||||
use crate::ui::app::Tty7App;
|
||||
use crate::ui::rounding;
|
||||
@@ -39,7 +51,11 @@ use crate::ui::rounding::RoundedCorners as _;
|
||||
pub(crate) enum DiffLoad {
|
||||
/// First probe still in flight.
|
||||
Loading,
|
||||
Ready(DiffSnapshot),
|
||||
/// A landed snapshot, shared rather than owned: one probe result reaches
|
||||
/// every tab whose overlay watches this cwd *and* the Changes panel, and
|
||||
/// the snapshot is a deep tree of owned strings — cloning it per holder on
|
||||
/// the UI update path is exactly the cost issue #239 measured as a stall.
|
||||
Ready(Arc<DiffSnapshot>),
|
||||
/// The probe came back "not a work tree" (repo deleted, dir gone).
|
||||
NotARepo,
|
||||
}
|
||||
@@ -147,6 +163,18 @@ impl Tty7App {
|
||||
}
|
||||
None => {}
|
||||
}
|
||||
// The Changes panel may already hold this very repo's snapshot — it is
|
||||
// the same `git diff HEAD`. Opening on it makes the overlay paint
|
||||
// immediately instead of flashing "Reading diff…" for a probe whose
|
||||
// answer is already in the process, and costs an `Arc` bump. A refresh
|
||||
// probe still flies below, so the seeded view is never the last word.
|
||||
// Read here, before the `&mut` borrow of the tab.
|
||||
let seed = match (&self.right_panel.diff_cwd, &self.right_panel.diff) {
|
||||
(Some(panel_cwd), Some(Some(snap))) if *panel_cwd == cwd => {
|
||||
DiffLoad::Ready(Arc::clone(snap))
|
||||
}
|
||||
_ => DiffLoad::Loading,
|
||||
};
|
||||
// The overlay steals focus (it needs Esc); snapshot the active pane so
|
||||
// closing lands back on the same terminal — same discipline as Settings.
|
||||
self.remember_active_pane(window, cx);
|
||||
@@ -158,7 +186,7 @@ impl Tty7App {
|
||||
host_id: host,
|
||||
cwd,
|
||||
focus_handle: focus_handle.clone(),
|
||||
load: DiffLoad::Loading,
|
||||
load: seed,
|
||||
loading: false,
|
||||
toggled: HashSet::new(),
|
||||
focus,
|
||||
@@ -219,39 +247,91 @@ impl Tty7App {
|
||||
return;
|
||||
};
|
||||
overlay.loading = true;
|
||||
self.spawn_shared_diff_probe(host, cwd, cx);
|
||||
}
|
||||
|
||||
/// One `git diff HEAD` per repository, however many things are waiting on
|
||||
/// it — where "repository" is the machine *and* the path, since the same
|
||||
/// path on two hosts is two different work trees.
|
||||
///
|
||||
/// The overlay and the Changes panel used to probe the same repository
|
||||
/// independently and each keep its own `DiffSnapshot` — issue #239's fifth
|
||||
/// finding. Deduping here means opening both costs one invocation and one
|
||||
/// parse, and [`install_diff_snapshot`](Self::install_diff_snapshot) hands
|
||||
/// the *same* `Arc` to both rather than a second copy.
|
||||
///
|
||||
/// Callers still mark themselves as waiting first (the overlay's `loading`
|
||||
/// flag, the panel's `diff_pending`): that's the "refreshing…" hint, and it
|
||||
/// is cleared by whichever probe lands for this repo, not necessarily the
|
||||
/// one the caller thought it started.
|
||||
pub(crate) fn spawn_shared_diff_probe(
|
||||
&mut self,
|
||||
host: crate::ui::host_ops::SharedHost,
|
||||
cwd: PathBuf,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let key = (host.id(), cwd.clone());
|
||||
if !self.diff_probes_inflight.insert(key.clone()) {
|
||||
return; // someone is already asking this exact question
|
||||
}
|
||||
let probe_cwd = cwd.clone();
|
||||
crate::ui::host_ops::HostOps::run(
|
||||
host,
|
||||
cx,
|
||||
move |h| git_diff::probe(h, &probe_cwd),
|
||||
move |app, result, cx| {
|
||||
// Land on every tab whose overlay shows this repo on this
|
||||
// machine — the spawning tab may no longer be active, and
|
||||
// sibling tabs on the same repo are equally stale. A slot
|
||||
// closed or swapped to another repo while we flew is skipped.
|
||||
let mut landed = false;
|
||||
for tab in app.tabs.iter_mut() {
|
||||
let Some(overlay) = tab
|
||||
.diff_overlay
|
||||
.as_mut()
|
||||
.filter(|o| o.cwd == cwd && o.host_id == id)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
overlay.loading = false;
|
||||
overlay.load = match &result {
|
||||
Some(snap) => DiffLoad::Ready(snap.clone()),
|
||||
None => DiffLoad::NotARepo,
|
||||
};
|
||||
landed = true;
|
||||
}
|
||||
if landed {
|
||||
cx.notify();
|
||||
}
|
||||
app.diff_probes_inflight.remove(&key);
|
||||
app.install_diff_snapshot(key.0, &cwd, result.map(Arc::new), cx);
|
||||
},
|
||||
);
|
||||
}
|
||||
|
||||
/// Hand a landed probe to everything watching `cwd`: every tab whose
|
||||
/// overlay shows it (the spawning tab may no longer be active, and sibling
|
||||
/// tabs on the same repo are equally stale) and the Changes panel when it
|
||||
/// is on the same cwd. Slots that closed or swapped repos while the probe
|
||||
/// flew are skipped.
|
||||
fn install_diff_snapshot(
|
||||
&mut self,
|
||||
host: crate::ui::host_ops::HostId,
|
||||
cwd: &Path,
|
||||
snap: Option<Arc<DiffSnapshot>>,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
let mut landed = false;
|
||||
for tab in self.tabs.iter_mut() {
|
||||
let Some(overlay) = tab
|
||||
.diff_overlay
|
||||
.as_mut()
|
||||
.filter(|o| o.cwd == cwd && o.host_id == host)
|
||||
else {
|
||||
continue;
|
||||
};
|
||||
overlay.loading = false;
|
||||
// `Arc::clone`, not a deep copy of the file/hunk/line tree.
|
||||
overlay.load = match &snap {
|
||||
Some(snap) => DiffLoad::Ready(Arc::clone(snap)),
|
||||
None => DiffLoad::NotARepo,
|
||||
};
|
||||
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.
|
||||
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;
|
||||
}
|
||||
if landed {
|
||||
cx.notify();
|
||||
}
|
||||
}
|
||||
|
||||
/// Re-probe the open overlay when the shared status cache learned
|
||||
/// something newer than what's shown — called from the app's
|
||||
/// `observe_global::<GitStatusCache>` hook, i.e. on the very triggers
|
||||
@@ -525,21 +605,50 @@ impl Tty7App {
|
||||
focused: Option<usize>,
|
||||
cx: &mut Context<Self>,
|
||||
) -> AnyElement {
|
||||
// An oversized tree opens fully collapsed: the file rows are still the
|
||||
// useful part, and the bodies are what cost. A file the user opened by
|
||||
// name is exempt — that's an explicit request for one body, not for the
|
||||
// whole tree. See `DiffSnapshot::oversized`.
|
||||
let oversized = focused.is_none() && snap.oversized();
|
||||
let mut list = v_flex().gap_3().p_4().w_full();
|
||||
if oversized {
|
||||
list = list.child(self.diff_oversized_notice(snap, cx));
|
||||
}
|
||||
// Hard ceiling on cards built at all: even collapsed, one card per file
|
||||
// is one card per file, and the list is not virtualized.
|
||||
let shown = snap.files.len().min(MAX_RENDERED_FILES);
|
||||
for (idx, file) in snap.files.iter().enumerate() {
|
||||
if focused.is_some_and(|f| f != idx) {
|
||||
continue;
|
||||
}
|
||||
if focused.is_none() && idx >= shown {
|
||||
break;
|
||||
}
|
||||
// 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)
|
||||
} else {
|
||||
file_expanded(file, toggled)
|
||||
file_expanded(file, toggled, oversized)
|
||||
};
|
||||
list = list.child(self.diff_file_card(idx, file, expanded, cx));
|
||||
}
|
||||
if focused.is_none() && snap.files.len() > shown {
|
||||
let rest = snap.files.len() - shown;
|
||||
list = list.child(
|
||||
div()
|
||||
.w_full()
|
||||
.px_2p5()
|
||||
.py_1p5()
|
||||
.text_xs()
|
||||
.text_color(cx.theme().muted_foreground)
|
||||
.child(format!(
|
||||
"… and {rest} more changed file{} — run `git diff` in the terminal to see them.",
|
||||
if rest == 1 { "" } else { "s" }
|
||||
)),
|
||||
);
|
||||
}
|
||||
// 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));
|
||||
@@ -553,6 +662,35 @@ impl Tty7App {
|
||||
.into_any_element()
|
||||
}
|
||||
|
||||
/// The banner an oversized diff leads with: says why every file is folded
|
||||
/// shut and points at the two ways out (expand one file, or use the
|
||||
/// terminal). Deliberately *above* the list rather than instead of it — the
|
||||
/// 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.",
|
||||
snap.files.len(),
|
||||
snap.retained_lines(),
|
||||
);
|
||||
if snap.budget_exhausted() {
|
||||
text.push_str(" Some files' contents were dropped to keep tty7 responsive.");
|
||||
}
|
||||
div()
|
||||
.w_full()
|
||||
.px_2p5()
|
||||
.py_2()
|
||||
.rounded_md()
|
||||
.border_1()
|
||||
.border_color(cx.theme().border)
|
||||
.bg(cx.theme().secondary)
|
||||
.text_xs()
|
||||
.text_color(cx.theme().muted_foreground)
|
||||
.child(text)
|
||||
.into_any_element()
|
||||
}
|
||||
|
||||
/// One file's card: a clickable header row and, when expanded, the hunks.
|
||||
fn diff_file_card(
|
||||
&self,
|
||||
@@ -562,8 +700,11 @@ impl Tty7App {
|
||||
cx: &mut Context<Self>,
|
||||
) -> AnyElement {
|
||||
// Binary files and pure renames have no hunk body to reveal; their
|
||||
// header is inert (no chevron, no click).
|
||||
let expandable = !file.binary && !file.hunks.is_empty();
|
||||
// header is inert (no chevron, no click). A file the repo-wide budget
|
||||
// emptied *is* expandable even with no hunks — what it reveals is the
|
||||
// note explaining why, which is otherwise unreachable.
|
||||
let expandable =
|
||||
!file.binary && (!file.hunks.is_empty() || file.truncated == Some(Truncation::Budget));
|
||||
let (glyph, glyph_color) = match file.status {
|
||||
FileStatus::Added => ("A", cx.theme().success),
|
||||
FileStatus::Modified => ("M", cx.theme().warning),
|
||||
@@ -722,7 +863,21 @@ impl Tty7App {
|
||||
body = body.child(self.diff_split_row(row, closing_row == Some((h, r)), cx));
|
||||
}
|
||||
}
|
||||
if file.truncated {
|
||||
if let Some(reason) = file.truncated {
|
||||
let note = match reason {
|
||||
Truncation::PerFile => format!(
|
||||
"Diff truncated at {} lines — run `git diff` in the terminal for the rest.",
|
||||
git_diff::MAX_LINES_PER_FILE
|
||||
),
|
||||
// Naming the repo-wide budget matters: this file may be
|
||||
// three lines long, and "truncated" without a why reads as
|
||||
// tty7 having lost the change.
|
||||
Truncation::Budget => {
|
||||
"Body not loaded — this working tree is past tty7's diff budget. \
|
||||
Run `git diff` in the terminal for this file."
|
||||
.to_string()
|
||||
}
|
||||
};
|
||||
body = body.child(
|
||||
div()
|
||||
.w_full()
|
||||
@@ -730,10 +885,7 @@ impl Tty7App {
|
||||
.py_1()
|
||||
.text_xs()
|
||||
.text_color(cx.theme().muted_foreground)
|
||||
.child(format!(
|
||||
"Diff truncated at {} lines — run `git diff` in the terminal for the rest.",
|
||||
git_diff::MAX_LINES_PER_FILE
|
||||
)),
|
||||
.child(note),
|
||||
);
|
||||
}
|
||||
card = card.child(body);
|
||||
@@ -888,8 +1040,15 @@ fn focused_name(overlay: &DiffOverlayState) -> Option<String> {
|
||||
|
||||
/// Whether a file's body shows: small text diffs default open, big ones (and
|
||||
/// anything the user explicitly flipped) invert via the `toggled` set.
|
||||
fn file_expanded(file: &FileDiff, toggled: &HashSet<String>) -> bool {
|
||||
let default_open = file.added + file.removed <= AUTO_COLLAPSE_LINES;
|
||||
///
|
||||
/// `collapse_all` is the repo-wide override: 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)
|
||||
}
|
||||
|
||||
@@ -1034,4 +1193,170 @@ mod tests {
|
||||
assert_eq!(rows[0].right.as_ref().unwrap().text, " indented");
|
||||
assert!(rows[0].left.is_none());
|
||||
}
|
||||
|
||||
/// A file of `added` changed lines, small enough to open by default on its
|
||||
/// own.
|
||||
fn small_file(path: &str, added: u32) -> FileDiff {
|
||||
FileDiff {
|
||||
path: path.to_string(),
|
||||
old_path: None,
|
||||
status: FileStatus::Modified,
|
||||
added,
|
||||
removed: 0,
|
||||
binary: false,
|
||||
truncated: None,
|
||||
hunks: vec![git_diff::Hunk {
|
||||
header: "@@ -1,1 +1,1 @@".to_string(),
|
||||
lines: (0..added)
|
||||
.map(|i| line(LineKind::Added, None, Some(i + 1), "x"))
|
||||
.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.
|
||||
#[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();
|
||||
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));
|
||||
}
|
||||
|
||||
/// 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
|
||||
/// "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();
|
||||
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),
|
||||
"the user's own click still opens it"
|
||||
);
|
||||
}
|
||||
|
||||
/// 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
|
||||
/// builds zero rows.
|
||||
#[test]
|
||||
fn many_medium_files_are_oversized_and_build_no_rows() {
|
||||
let snap = DiffSnapshot {
|
||||
files: (0..60)
|
||||
.map(|i| small_file(&format!("f{i}.rs"), 40))
|
||||
.collect(),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(
|
||||
snap.files.iter().all(|f| f.added <= AUTO_COLLAPSE_LINES),
|
||||
"no single file is over the per-file threshold"
|
||||
);
|
||||
assert!(snap.oversized());
|
||||
|
||||
let none = HashSet::new();
|
||||
let rows_expanded: usize = snap
|
||||
.files
|
||||
.iter()
|
||||
.filter(|f| file_expanded(f, &none, false))
|
||||
.flat_map(|f| &f.hunks)
|
||||
.map(|h| split_hunk(&h.lines).len())
|
||||
.sum();
|
||||
let rows_collapsed: usize = snap
|
||||
.files
|
||||
.iter()
|
||||
.filter(|f| file_expanded(f, &none, true))
|
||||
.flat_map(|f| &f.hunks)
|
||||
.map(|h| split_hunk(&h.lines).len())
|
||||
.sum();
|
||||
assert_eq!(rows_expanded, 2400, "what the old rule would have built");
|
||||
assert_eq!(rows_collapsed, 0);
|
||||
}
|
||||
|
||||
/// The other side of the same coin: a busy-but-ordinary afternoon — forty
|
||||
/// files, a few lines each — is *not* oversized and opens expanded exactly
|
||||
/// as it does today. The thresholds must not tax a normal working tree.
|
||||
#[test]
|
||||
fn an_ordinary_busy_tree_is_not_oversized() {
|
||||
let snap = DiffSnapshot {
|
||||
files: (0..40)
|
||||
.map(|i| small_file(&format!("f{i}.rs"), 12))
|
||||
.collect(),
|
||||
..Default::default()
|
||||
};
|
||||
assert!(!snap.oversized());
|
||||
let none = HashSet::new();
|
||||
assert!(snap.files.iter().all(|f| file_expanded(f, &none, false)));
|
||||
}
|
||||
|
||||
/// However many files change, the overlay builds at most
|
||||
/// [`MAX_RENDERED_FILES`] cards and says so.
|
||||
#[test]
|
||||
fn file_cards_are_capped() {
|
||||
let snap = DiffSnapshot {
|
||||
files: (0..MAX_RENDERED_FILES + 25)
|
||||
.map(|i| small_file(&format!("f{i}.rs"), 1))
|
||||
.collect(),
|
||||
..Default::default()
|
||||
};
|
||||
let shown = snap.files.len().min(MAX_RENDERED_FILES);
|
||||
assert_eq!(shown, MAX_RENDERED_FILES);
|
||||
assert_eq!(
|
||||
snap.files.len() - shown,
|
||||
25,
|
||||
"the tail gets one summary line"
|
||||
);
|
||||
}
|
||||
|
||||
/// Measurement harness for issue #239, finding 2 — run with
|
||||
/// `cargo test -- --ignored --nocapture bench_snapshot_share`.
|
||||
#[test]
|
||||
#[ignore = "measurement, not an assertion"]
|
||||
fn bench_snapshot_share() {
|
||||
use std::time::Instant;
|
||||
|
||||
// Two sizes: what v26.7.5 would have held for a big agent session
|
||||
// (300 files × 300 lines, no repo-wide budget), and what this build
|
||||
// retains for the same tree once the budget applies.
|
||||
for (label, files, per_file) in [
|
||||
("unbudgeted (v26.7.5 shape)", 300, 300),
|
||||
("budgeted (this build retains)", 300, 67),
|
||||
] {
|
||||
let snap = Arc::new(DiffSnapshot {
|
||||
files: (0..files)
|
||||
.map(|i| small_file(&format!("f{i}.rs"), per_file))
|
||||
.collect(),
|
||||
..Default::default()
|
||||
});
|
||||
let lines: usize = snap.retained_lines();
|
||||
|
||||
let t = Instant::now();
|
||||
for _ in 0..10 {
|
||||
let _deep = (*snap).clone();
|
||||
}
|
||||
let deep = t.elapsed() / 10;
|
||||
|
||||
let t = Instant::now();
|
||||
for _ in 0..100 {
|
||||
let _shared = Arc::clone(&snap);
|
||||
}
|
||||
let shared = t.elapsed() / 100;
|
||||
println!(
|
||||
"{label}: {files} files / {lines} lines — deep clone {deep:?} vs \
|
||||
Arc::clone {shared:?}, per holder on the UI thread"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+26
-27
@@ -22,10 +22,11 @@ use gpui_component::{
|
||||
ActiveTheme as _, Icon, IconName, InteractiveElementExt as _, Sizable as _, h_flex, v_flex,
|
||||
};
|
||||
use std::path::PathBuf;
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::core::config::{Config, RightPanelTab};
|
||||
use crate::daemon::protocol::PaneProcs;
|
||||
use crate::terminal::git_diff::{self, DiffSnapshot};
|
||||
use crate::terminal::git_diff::DiffSnapshot;
|
||||
use crate::ui::app::{
|
||||
CONTENT_INSET, TILE_GLYPH_SM, TILE_SIZE_SM, Tty7App, tile_trailing_inset,
|
||||
tile_trailing_inset_sm,
|
||||
@@ -61,10 +62,17 @@ pub(crate) struct RightPanelState {
|
||||
/// machines is two repositories.
|
||||
pub(crate) diff_cwd: Option<(crate::ui::host_ops::HostId, PathBuf)>,
|
||||
/// Last completed probe. `Some(None)` and `None` are different answers:
|
||||
/// "probed, not a work tree" versus "never probed".
|
||||
pub(crate) diff: Option<Option<DiffSnapshot>>,
|
||||
/// A probe is in flight; keeps the render path from spawning a second one.
|
||||
pub(crate) diff_loading: bool,
|
||||
/// "probed, not a work tree" versus "never probed". Shared with the diff
|
||||
/// overlay rather than a second copy of the same tree — see
|
||||
/// [`Tty7App::spawn_shared_diff_probe`].
|
||||
pub(crate) diff: Option<Option<Arc<DiffSnapshot>>>,
|
||||
/// The machine-and-cwd this panel is waiting on a probe for; keeps the
|
||||
/// render path from spawning a second one. A key rather than a flag because
|
||||
/// the shared probe (see [`Tty7App::spawn_shared_diff_probe`]) lands per
|
||||
/// repo: the panel has to know *which* answer clears its wait, or a probe
|
||||
/// for the repo it just navigated away from would leave it stuck on
|
||||
/// "Loading…".
|
||||
pub(crate) diff_pending: Option<(crate::ui::host_ops::HostId, PathBuf)>,
|
||||
/// The pane `procs` describes, so a pane switch invalidates it rather than
|
||||
/// showing the previous pane's processes under the new pane's name.
|
||||
pub(crate) procs_pane: Option<u64>,
|
||||
@@ -1171,8 +1179,8 @@ impl Tty7App {
|
||||
if self.right_panel.diff_cwd.as_ref() != Some(&key) {
|
||||
self.right_panel.diff_cwd = Some(key);
|
||||
self.right_panel.diff = None;
|
||||
self.spawn_right_panel_diff(host.clone(), cwd.clone(), cx);
|
||||
} else if self.right_panel.diff.is_none() && !self.right_panel.diff_loading {
|
||||
self.spawn_right_panel_diff(cwd.clone(), cx);
|
||||
} else if self.right_panel.diff.is_none() && self.right_panel.diff_pending.is_none() {
|
||||
// Nothing cached and nothing in flight: a probe for a previous cwd
|
||||
// landed after we had already moved on and dropped its result, so
|
||||
// no one is left to answer for this one. Without this the tab would
|
||||
@@ -1316,33 +1324,24 @@ impl Tty7App {
|
||||
self.panel_scroll(inner, title)
|
||||
}
|
||||
|
||||
/// Off-thread `git diff` for the panel, mirroring the diff overlay's probe.
|
||||
/// Off-thread `git diff` for the panel — the *same* probe the diff overlay
|
||||
/// uses. This used to be its own `git_diff::probe` call keeping its own
|
||||
/// `DiffSnapshot`, so a repo with both open generated, parsed and stored
|
||||
/// its full diff twice (issue #239, finding 5). Now both go through
|
||||
/// [`Tty7App::spawn_shared_diff_probe`], which dedupes by machine-and-cwd
|
||||
/// and installs one `Arc` into whoever is watching — including this panel,
|
||||
/// which is why there is no result handler left here.
|
||||
fn spawn_right_panel_diff(
|
||||
&mut self,
|
||||
host: crate::ui::host_ops::SharedHost,
|
||||
cwd: PathBuf,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
if self.right_panel.diff_loading {
|
||||
if self.right_panel.diff_pending.is_some() {
|
||||
return;
|
||||
}
|
||||
self.right_panel.diff_loading = true;
|
||||
let key = (host.id(), cwd.clone());
|
||||
crate::ui::host_ops::HostOps::run(
|
||||
host,
|
||||
cx,
|
||||
move |h| git_diff::probe(h, &cwd),
|
||||
move |app, result, cx| {
|
||||
app.right_panel.diff_loading = false;
|
||||
// Drop the result if the panel moved on to another repo — or
|
||||
// another machine — while we flew; otherwise a slow probe would
|
||||
// overwrite a newer one.
|
||||
if app.right_panel.diff_cwd.as_ref() == Some(&key) {
|
||||
app.right_panel.diff = Some(result);
|
||||
cx.notify();
|
||||
}
|
||||
},
|
||||
);
|
||||
self.right_panel.diff_pending = Some((host.id(), cwd.clone()));
|
||||
self.spawn_shared_diff_probe(host, cwd, cx);
|
||||
}
|
||||
|
||||
/// Re-probe the Changes list when the shared status cache learned something
|
||||
@@ -1357,7 +1356,7 @@ impl Tty7App {
|
||||
/// Comparing branch + totals first keeps the quiet case free, and re-probing
|
||||
/// in place leaves the rows on screen until the new snapshot lands.
|
||||
pub(crate) fn right_panel_refresh_changes(&mut self, cx: &mut Context<Self>) {
|
||||
if self.right_panel.diff_loading {
|
||||
if self.right_panel.diff_pending.is_some() {
|
||||
return;
|
||||
}
|
||||
let Some((id, cwd)) = self.right_panel.diff_cwd.clone() else {
|
||||
|
||||
@@ -380,6 +380,11 @@ fn settings_search_entries() -> &'static [SearchEntry] {
|
||||
title: "Sidebar grouping",
|
||||
keywords: "tabs group repo repository git scratch header sidebar flat",
|
||||
},
|
||||
SearchEntry {
|
||||
section: WindowTabs,
|
||||
title: "Open diff preview from sidebar counts",
|
||||
keywords: "diff overlay preview sidebar counts git changes click branch lines",
|
||||
},
|
||||
SearchEntry {
|
||||
section: WindowTabs,
|
||||
title: "Notify on command finish",
|
||||
@@ -4458,6 +4463,7 @@ impl Tty7App {
|
||||
TabBarPosition::Top => 0,
|
||||
TabBarPosition::Left => 1,
|
||||
};
|
||||
let sidebar_diff_preview = cfg.sidebar_diff_preview;
|
||||
let sidebar_grouping_idx = match cfg.sidebar_grouping {
|
||||
crate::core::config::SidebarGrouping::Repo => 0,
|
||||
crate::core::config::SidebarGrouping::None => 1,
|
||||
@@ -4568,6 +4574,10 @@ impl Tty7App {
|
||||
this.set_tab_bar_position(pos, cx);
|
||||
},
|
||||
);
|
||||
let sidebar_diff_switch = crate::ui::theme::switch("wt-sidebar-diff-preview", cx)
|
||||
.checked(sidebar_diff_preview)
|
||||
.on_click(cx.listener(|this, on: &bool, _w, cx| this.set_sidebar_diff_preview(*on, cx)))
|
||||
.into_any_element();
|
||||
let sidebar_grouping_radio = self.segmented(
|
||||
"wt-sidebar-grouping",
|
||||
&["By repo", "Flat"],
|
||||
@@ -4645,6 +4655,16 @@ impl Tty7App {
|
||||
sidebar_grouping_radio,
|
||||
cx,
|
||||
))
|
||||
// Phrased around what *stays*: the worry this row answers is "will
|
||||
// turning it off cost me the branch and the numbers", and the
|
||||
// answer is no — only the click goes.
|
||||
.child(self.settings_row(
|
||||
"Open diff preview from sidebar counts",
|
||||
"Click a row's +N −N to open the working-tree diff in an overlay. Off keeps the \
|
||||
branch and the counts on the row and just stops them being clickable.",
|
||||
sidebar_diff_switch,
|
||||
cx,
|
||||
))
|
||||
.child(self.section_rule(cx))
|
||||
.child(self.section_header("Notifications", cx))
|
||||
.child(self.settings_row(
|
||||
|
||||
+63
-13
@@ -245,19 +245,22 @@ impl Tty7App {
|
||||
// a change never disturbs the title; grouping keys on the repo
|
||||
// root only, so a branch switch never relocates the row either
|
||||
// (see `Tab::sidebar_group`). The diff counts are a quiet
|
||||
// green/red readout and double as the diff-overlay toggle: click
|
||||
// them to peek another session's changes in an overlay without
|
||||
// activating this row's tab. The cwd they probe is the same one
|
||||
// the status resolved through, so overlay and counts always
|
||||
// describe the same repo.
|
||||
let git_cwd = tab.pane.focused_or_first(window, cx).and_then(|leaf| {
|
||||
let view = leaf.read(cx);
|
||||
let cwd = view.git_status_cwd()?.to_path_buf();
|
||||
// The id, not the host: opening the overlay needs no live
|
||||
// connection — a disconnected machine's last diff is still
|
||||
// worth showing, and the re-probe resolves the id itself.
|
||||
Some((view.host_id(), cwd))
|
||||
});
|
||||
// green/red readout and, unless `sidebar_diff_preview` is off,
|
||||
// double as the diff-overlay toggle: click them to peek another
|
||||
// session's changes in an overlay without activating this row's
|
||||
// tab. The cwd they probe is the same one the status resolved
|
||||
// through, so overlay and counts always describe the same repo.
|
||||
let git_cwd = diff_click_cwd(
|
||||
cx.global::<Config>(),
|
||||
tab.pane.focused_or_first(window, cx).and_then(|leaf| {
|
||||
let view = leaf.read(cx);
|
||||
let cwd = view.git_status_cwd()?.to_path_buf();
|
||||
// The id, not the host: opening the overlay needs no live
|
||||
// connection — a disconnected machine's last diff is still
|
||||
// worth showing, and the re-probe resolves the id itself.
|
||||
Some((view.host_id(), cwd))
|
||||
}),
|
||||
);
|
||||
let git_line = tab.git_status(Some(window), cx).map(|g| {
|
||||
let mut line = h_flex()
|
||||
.id(("sidebar-git", i))
|
||||
@@ -1285,6 +1288,22 @@ fn group_names(roots: &[&PathBuf]) -> Vec<String> {
|
||||
}
|
||||
}
|
||||
|
||||
/// The machine-and-cwd a sidebar row's `+N −N` counts should open the diff
|
||||
/// overlay for, or `None` when they're a plain readout. Generic over the
|
||||
/// payload so it stays indifferent to what identifies a repo — that pair grew a
|
||||
/// `HostId` when panes learned to live on other machines, and this gate did not
|
||||
/// need to know.
|
||||
///
|
||||
/// One value drives both halves of the interaction: the render path hangs the
|
||||
/// pointer cursor *and* the `toggle_diff_overlay` mouse handler off the same
|
||||
/// `when_some`, so a `None` here provably removes both and the press falls
|
||||
/// through to the row's own activate handler like any other part of the label.
|
||||
/// The branch and the counts themselves don't consult this — turning the
|
||||
/// preview off must not cost you the readout (issue #239).
|
||||
fn diff_click_cwd<T>(cfg: &Config, target: Option<T>) -> Option<T> {
|
||||
cfg.sidebar_diff_preview.then_some(target).flatten()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -1293,6 +1312,37 @@ mod tests {
|
||||
PathBuf::from(s)
|
||||
}
|
||||
|
||||
/// With the preview on (the default) the counts carry the repo's cwd, which
|
||||
/// is what gives them the pointer cursor and the `toggle_diff_overlay`
|
||||
/// handler; with it off they carry nothing and neither is attached.
|
||||
#[test]
|
||||
fn diff_preview_setting_gates_the_click_target() {
|
||||
let mut cfg = Config::default();
|
||||
assert!(cfg.sidebar_diff_preview, "default is today's behaviour");
|
||||
assert_eq!(
|
||||
diff_click_cwd(&cfg, Some(p("/w/repo"))),
|
||||
Some(p("/w/repo")),
|
||||
"enabled: the counts are a click target"
|
||||
);
|
||||
|
||||
cfg.sidebar_diff_preview = false;
|
||||
assert_eq!(
|
||||
diff_click_cwd(&cfg, Some(p("/w/repo"))),
|
||||
None,
|
||||
"disabled: no cwd, so no cursor and no toggle_diff_overlay"
|
||||
);
|
||||
}
|
||||
|
||||
/// A pane outside a git work tree has no cwd to open either way — the
|
||||
/// setting doesn't invent one.
|
||||
#[test]
|
||||
fn diff_click_target_needs_a_repo_either_way() {
|
||||
let mut cfg = Config::default();
|
||||
assert_eq!(diff_click_cwd(&cfg, None), None);
|
||||
cfg.sidebar_diff_preview = false;
|
||||
assert_eq!(diff_click_cwd(&cfg, None), None);
|
||||
}
|
||||
|
||||
/// Groups appear in first-appearance order with Scratch pinned last, and
|
||||
/// an all-`None` key set renders as one headerless flat section.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user