From 20b73adb2ac10b3aefbae2c9c7beda01a66a3e9f Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 21:00:55 +0800 Subject: [PATCH 01/10] chore(release): v26.9.1 --- CHANGELOG.md | 39 +++++++++++++++++++++++++++++++++++++++ Cargo.lock | 8 ++++---- Cargo.toml | 2 +- 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f108c4b2..2c83283d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,43 @@ All notable changes to tty7 are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [26.9.1] - 2026-09-07 + +### Fixed + +- **A remote server that will not start now says why** (#774). A remote + workspace could sit in a loop nobody could get out of: every reconnect failed + with "started but nothing was answering on the control socket after 15s", the + strip showed a copy bar frozen at 100%, and no button was offered. A daemon + whose control listener would not open logged one line and kept running — and a + running daemon holds the single-server lock, so every later start stood down + at once and every probe failed, forever. Whether something else is serving is + now settled by connecting rather than by reading the errno, and a daemon that + cannot listen exits. The reason is kept too: the remote daemon's output and + exit status land beside the binary, stamped with the launch's own nonce so a + restart never reads the outgoing daemon's status as the incoming one's, and a + start that has already failed no longer waits out the full timeout. On the + window side, an automatic reconnect retires its install progress instead of + drawing an install still in flight, and a long error no longer stretches the + status card past the window and takes the retry button off screen with it. + +- **A stale pane socket no longer stops every later daemon** (#779). A daemon + that died without unlinking its Unix socket left a file `bind` refuses, so the + client launched a daemon, it exited on the bind, and the client launched + another — forever. The removal is now decided by the single-server seat rather + than the pidfile: holding the seat means nobody else can be serving that config + dir, so anything still at the endpoint belongs to a process that is gone. + Where there is no seat, a socket that answers is refused rather than removed. + +### Changed + +- **The control dialect is v9.** v8 added the project verbs; this build takes + them back out and speaks v7's messages again, message for message — but the + number does not go back with them. v8 is deployed, and a number that moves + backwards stops being an identity: a 7 on the wire would mean either "before + projects" or "after them" depending on which build put it there, and the + handshake has nothing but the number to tell the two apart. + ## [26.9.0] - 2026-09-04 ### Added @@ -4401,6 +4438,8 @@ Initial release. - zsh shell integration (OSC 7 cwd + OSC 133 prompt marks) via a throwaway `ZDOTDIR`. - Native macOS light/dark themes that follow the system appearance. +[26.9.1]: https://github.com/l0ng-ai/tty7/compare/v26.9.0...v26.9.1 +[26.9.0]: https://github.com/l0ng-ai/tty7/compare/v26.8.3...v26.9.0 [26.8.3]: https://github.com/l0ng-ai/tty7/compare/v26.8.2...v26.8.3 [26.8.2]: https://github.com/l0ng-ai/tty7/compare/v26.8.1...v26.8.2 [26.8.1]: https://github.com/l0ng-ai/tty7/compare/v26.8.0...v26.8.1 diff --git a/Cargo.lock b/Cargo.lock index 7974f7e1..4e345fcb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9775,7 +9775,7 @@ dependencies = [ [[package]] name = "tty7" -version = "26.9.0" +version = "26.9.1" dependencies = [ "alacritty_terminal", "anyhow", @@ -9820,7 +9820,7 @@ dependencies = [ [[package]] name = "tty7-cli" -version = "26.9.0" +version = "26.9.1" dependencies = [ "alacritty_terminal", "anyhow", @@ -9835,7 +9835,7 @@ dependencies = [ [[package]] name = "tty7-core" -version = "26.9.0" +version = "26.9.1" dependencies = [ "anyhow", "base64 0.22.1", @@ -9869,7 +9869,7 @@ dependencies = [ [[package]] name = "tty7-server" -version = "26.9.0" +version = "26.9.1" dependencies = [ "libc", "tempfile", diff --git a/Cargo.toml b/Cargo.toml index 31d358e5..55848281 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -271,7 +271,7 @@ edition = "2024" # they must never drift apart. This is the line a release bump edits (and the # one nightly's `awk '/^version = /'` stamps: it is the first such line in the # file, since the package entries above are all `version.workspace = true`). -version = "26.9.0" +version = "26.9.1" [workspace.dependencies] # Our fork's `tty7` branch carries the local customizations tty7 relies on: From f6358494b608bc6e2790760a364d1a4fb8c21af7 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:09:07 +0800 Subject: [PATCH 02/10] test(remote): give each machine in the route-origin tests its own key (#798) --- src/ui/remote_connect.rs | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/ui/remote_connect.rs b/src/ui/remote_connect.rs index b04ea116..42d101b4 100644 --- a/src/ui/remote_connect.rs +++ b/src/ui/remote_connect.rs @@ -960,6 +960,7 @@ mod tests { fn native_spec(user: &str, host: &str, port: u16) -> NativeSshSpec { let mut profile = crate::core::ssh_profile::SshProfile::new(host.to_string()); + profile.host = host.to_string(); profile.user = user.to_string(); profile.port = port; crate::ui::ssh_connect::build_native_ssh_spec( @@ -970,6 +971,36 @@ mod tests { ) } + /// `SshProfile::new` takes a *name*, and the address lives in a separate + /// `host` field; every production caller assigns both. `native_spec` used + /// to assign only the name, so the spec it handed back addressed nobody + /// and `ConnectionKey::from_spec` spelled it `me@:22` — one key for every + /// machine in this module that talks to port 22. + /// + /// `ORIGINS` is keyed by exactly that string, so the two tests below that + /// note an origin and read it back were writing to and reading from the + /// same slot. Whichever noted last won, and the other was handed the wrong + /// machine: 17 failures in 20 runs of this module alone, 6 in 20 of the + /// whole binary, and none when either test ran by itself. + #[test] + fn two_machines_do_not_share_one_route_origin_key() { + use crate::daemon::router::RouteTarget; + + let build = RouteTarget::Ssh(Box::new(native_spec("me", "build-box", 22))); + let twin = RouteTarget::Ssh(Box::new(native_spec("me", "twin-box", 22))); + + assert_eq!( + build.origin_key(), + "me@build-box:22", + "a route origin key names the machine it dials" + ); + assert_ne!( + build.origin_key(), + twin.origin_key(), + "two machines sharing one key make `note_origin` overwrite the other's" + ); + } + #[test] fn a_routed_auth_prompt_carries_the_machine_that_raised_it() { let _turn = claim_mailbox(); @@ -978,6 +1009,7 @@ mod tests { let route = crate::daemon::router::RouteTarget::Ssh(Box::new(native_spec("me", "build-box", 22))); note_origin(&route, &target); + let origin_key = route.origin_key(); let handle = std::thread::spawn(move || { use crate::daemon::router::RouteAuthResponder as _; @@ -1005,7 +1037,12 @@ mod tests { ); std::thread::sleep(Duration::from_millis(5)); }; - assert_eq!(pending.host, target.host_id()); + assert_eq!( + pending.host, + target.host_id(), + "the prompt names the machine noted under {:?}", + origin_key + ); pending.answer(AuthResponse::Secret("hunter2".into())); assert_eq!( handle.join().unwrap(), From 6474e25a24a725aafa4956959ad0dc04931c9ab3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:09:19 +0800 Subject: [PATCH 03/10] deps: bump the cargo-minor-patch group across 1 directory with 3 updates (#776) --- Cargo.lock | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4e345fcb..c48f27fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4581,9 +4581,9 @@ dependencies = [ [[package]] name = "keyring" -version = "4.1.6" +version = "4.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72585bb6cc9bc370d1d545b7e23fcce71dfd4461c5e15275e3cf51bdfd9a980a" +checksum = "2270074a3d26bcac93c1dc5d2845eb4c089e8d761ccf6e0ea266a16004640627" dependencies = [ "apple-native-keyring-store", "keyring-core", @@ -4901,9 +4901,9 @@ dependencies = [ [[package]] name = "log" -version = "0.4.33" +version = "0.4.34" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ceec5bc11778974d1bcb055b18002eba7f4b3518b6a0081b3af5f21666da9ad" +checksum = "f9f8bd3e56ce4dfc153cf470fffbfa98c7620958b312ca5c3a4b8d5181fd13c6" dependencies = [ "serde_core", "value-bag", @@ -10156,9 +10156,9 @@ dependencies = [ [[package]] name = "uuid" -version = "1.24.1" +version = "1.26.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2cefc03fd367c0c6d4305de1b312cf00248c4114f4a0418ce6a6af769e3b0bd9" +checksum = "b5772d71c9be8a8a6ac2117d949c5b224c1b72241bb611d9a3012edcf8af7812" dependencies = [ "getrandom 0.4.3", "js-sys", From dac70ec4c838cffec3bd8e2a38b1bbc39e8201e5 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:11:06 +0800 Subject: [PATCH 04/10] perf(protocol): put a frame's header and payload on the wire in one write (#797) Refs #713. --- crates/tty7-core/src/daemon/protocol.rs | 85 +++++++++++++++++++++++-- 1 file changed, 81 insertions(+), 4 deletions(-) diff --git a/crates/tty7-core/src/daemon/protocol.rs b/crates/tty7-core/src/daemon/protocol.rs index ecdc62bd..0eaa807e 100644 --- a/crates/tty7-core/src/daemon/protocol.rs +++ b/crates/tty7-core/src/daemon/protocol.rs @@ -5,6 +5,9 @@ use serde::{Deserialize, Serialize}; pub const MAX_FRAME: usize = 64 * 1024 * 1024; +/// A frame is a little-endian `u32` length, a one-byte kind, then the payload. +const HEADER: usize = 5; + pub const PROTOCOL_VERSION: u32 = 6; pub const FEATURE_PANE_OWNER: &str = "pane-owner"; @@ -952,9 +955,33 @@ pub fn write_frame(w: &mut W, kind: u8, payload: &[u8]) -> io::Result< "frame payload exceeds MAX_FRAME", )); } - w.write_all(&(len as u32).to_le_bytes())?; - w.write_all(&[kind])?; - w.write_all(payload)?; + // One write, not three. The pane socket is a loopback `TcpStream` with + // `TCP_NODELAY` set, so three `write_all`s put the length, the kind and the + // payload on the wire as three separate segments, and the reader on the far + // side wakes from `read()` three times for one frame. Under a PTY flood the + // daemon frames every ConPTY read, so that is two extra syscalls on each + // side per frame, tens of thousands a second (issue #713). + let mut header = [0u8; HEADER]; + header[..4].copy_from_slice(&(len as u32).to_le_bytes()); + header[4] = kind; + let mut bufs = [io::IoSlice::new(&header), io::IoSlice::new(payload)]; + let mut rest: &mut [io::IoSlice<'_>] = &mut bufs; + while !rest.is_empty() { + match w.write_vectored(rest) { + Ok(0) => { + return Err(io::Error::new( + io::ErrorKind::WriteZero, + "the frame could not be written in full", + )); + } + // A writer that does not implement `write_vectored` natively falls + // back to writing the first non-empty slice, so this loop still + // terminates — it just costs the two writes it used to cost. + Ok(n) => io::IoSlice::advance_slices(&mut rest, n), + Err(e) if e.kind() == io::ErrorKind::Interrupted => {} + Err(e) => return Err(e), + } + } Ok(()) } @@ -984,7 +1011,6 @@ pub fn is_error_kind(kind: u8) -> bool { } pub fn take_frame(buf: &mut Vec) -> io::Result)>> { - const HEADER: usize = 5; if buf.len() < HEADER { return Ok(None); } @@ -2046,6 +2072,57 @@ mod tests { assert!(buf.is_empty()); } + /// The pane socket has `TCP_NODELAY` set, so a write is a segment and a + /// segment is a wakeup on the far side. A frame must therefore cost one + /// write, not one for the length, one for the kind and one for the payload + /// (issue #713) — at flood rates that difference is tens of thousands of + /// syscalls a second on each end. + #[test] + fn a_frame_is_one_write_on_a_vectored_writer() { + #[derive(Default)] + struct Counting { + writes: usize, + bytes: Vec, + } + impl Write for Counting { + fn write(&mut self, buf: &[u8]) -> io::Result { + self.writes += 1; + self.bytes.extend_from_slice(buf); + Ok(buf.len()) + } + fn write_vectored(&mut self, bufs: &[io::IoSlice<'_>]) -> io::Result { + self.writes += 1; + let mut n = 0; + for b in bufs { + self.bytes.extend_from_slice(b); + n += b.len(); + } + Ok(n) + } + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + let mut w = Counting::default(); + write_frame(&mut w, kind::OUTPUT, b"a chunk of pty output").expect("write the frame"); + assert_eq!(w.writes, 1, "one frame must cost one write"); + + // An empty payload is a frame too — the header still has to land, and + // the empty second slice must not spin the loop. + let mut empty = Counting::default(); + write_frame(&mut empty, kind::DETACH, &[]).expect("write the empty frame"); + assert_eq!(empty.writes, 1); + + // Whatever the write count, the bytes on the wire are unchanged: a + // `read_frame` over them gives back exactly what went in. + let mut cursor = io::Cursor::new(w.bytes); + assert_eq!( + read_frame(&mut cursor).expect("read it back"), + (kind::OUTPUT, b"a chunk of pty output".to_vec()) + ); + } + #[test] fn from_frame_rejects_unknown_kind() { assert!(ClientMsg::from_frame(99, vec![]).is_err()); From 081e191bb0918695dc56d5461b2d74f4a8c15ae2 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:13:57 +0800 Subject: [PATCH 05/10] perf(diff-overlay): draw the patch as a virtualised row list (#799) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The overlay built its whole patch as a nested element tree on every frame: a card per file, a header per hunk, six elements per line. gpui notifies the view on each scroll wheel event, so a few hundred lines of diff rebuilt tens of thousands of elements tens of times a second, and the window stalled. Flatten the tree into one row per line in a new `diff_list` module and draw it with `gpui::list`, which builds only the rows on screen. The rows are rebuilt only when what they are built from changes, so scrolling no longer re-splits hunks or re-clones every line, and a change to one file splices just the rows it touched rather than resetting the list and losing the scroll position. The key that decides a rebuild takes the snapshot each frame was asked about even when it matched only by contents. A probe that finds nothing new still lands a fresh `Arc` over an equal snapshot; a key left pointing at the old one would go on walking the whole patch to prove the two equal, once per wheel event, which is the cost the key exists to avoid. A list counts a row it has not laid out yet as zero tall, which left the scrollbar reading an 800-line patch as one viewport: its thumb filled the track, and a drag from top to bottom travelled 248px and stopped. The rows below the fold are counted at the 19px both views already give a line of a patch, through `ListState::with_size_hint` — added to the gpui fork for this, `Cargo.lock` following its `tty7` branch to `ece710e3`. A card cannot survive that flattening — its rows are separate items now — so the frame it drew is gone, and with it the grey header bars and hunk bands that made the overlay the one view in the app still speaking gpui-component's default container language. The rows take the source control panel's own measurements instead: 26px, 10px inset, 5px radius, colour only under the pointer. The title bar's view switch loses its border for the same reason. --- Cargo.lock | 90 +-- Cargo.toml | 10 +- src/ui/diff_list.rs | 484 +++++++++++++ src/ui/diff_overlay.rs | 1480 ++++++++++++++++++++++++---------------- src/ui/diff_rows.rs | 3 + src/ui/mod.rs | 1 + src/ui/rounding.rs | 34 - src/ui/scrollbar.rs | 10 +- 8 files changed, 1447 insertions(+), 665 deletions(-) create mode 100644 src/ui/diff_list.rs diff --git a/Cargo.lock b/Cargo.lock index c48f27fc..a0f31ea1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -298,7 +298,7 @@ version = "1.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40c48f72fd53cd289104fc64099abca73db4166ad86ea0b4341abe65af83dadc" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -309,7 +309,7 @@ checksum = "291e6a250ff86cd4a820112fb8898808a366d8f9f58ce16d1f538353ad55747d" dependencies = [ "anstyle", "once_cell_polyfill", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -891,7 +891,7 @@ dependencies = [ "bitflags 2.13.1", "cexpr", "clang-sys", - "itertools 0.13.0", + "itertools 0.11.0", "log", "prettyplease", "proc-macro2", @@ -1483,7 +1483,7 @@ dependencies = [ [[package]] name = "collections" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "gpui_util", "indexmap", @@ -2047,7 +2047,7 @@ dependencies = [ [[package]] name = "derive_refineable" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "proc-macro2", "quote", @@ -2104,7 +2104,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -2142,7 +2142,7 @@ version = "0.5.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab8ecd87370524b461f8557c119c405552c396ed91fc0a8eec68679eab26f94a" dependencies = [ - "libloading 0.8.9", + "libloading 0.7.4", ] [[package]] @@ -2394,7 +2394,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "39cab71617ae0d63f51a36d69f866391735b51691dbda63cf6f96d042b63efeb" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -3199,7 +3199,7 @@ dependencies = [ "log", "presser", "thiserror 2.0.20", - "windows 0.62.2", + "windows 0.58.0", ] [[package]] @@ -3225,7 +3225,7 @@ dependencies = [ [[package]] name = "gpui" version = "0.2.2" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "accesskit", "anyhow", @@ -3416,7 +3416,7 @@ dependencies = [ [[package]] name = "gpui_linux" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "accesskit", "accesskit_unix", @@ -3467,7 +3467,7 @@ dependencies = [ [[package]] name = "gpui_macos" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "accesskit", "accesskit_macos", @@ -3514,7 +3514,7 @@ dependencies = [ [[package]] name = "gpui_macros" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "heck 0.5.0", "proc-macro2", @@ -3525,7 +3525,7 @@ dependencies = [ [[package]] name = "gpui_platform" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "console_error_panic_hook", "gpui", @@ -3538,7 +3538,7 @@ dependencies = [ [[package]] name = "gpui_shared_string" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "schemars", "serde", @@ -3548,7 +3548,7 @@ dependencies = [ [[package]] name = "gpui_util" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "log", @@ -3557,7 +3557,7 @@ dependencies = [ [[package]] name = "gpui_web" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "console_error_panic_hook", @@ -3581,7 +3581,7 @@ dependencies = [ [[package]] name = "gpui_wgpu" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "bytemuck", @@ -3610,7 +3610,7 @@ dependencies = [ [[package]] name = "gpui_windows" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "accesskit", "accesskit_windows", @@ -3939,7 +3939,7 @@ dependencies = [ [[package]] name = "http_client" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "async-compression", @@ -3964,7 +3964,7 @@ dependencies = [ [[package]] name = "http_client_tls" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "rustls", "rustls-platform-verifier", @@ -4057,7 +4057,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.62.2", + "windows-core 0.58.0", ] [[package]] @@ -5092,7 +5092,7 @@ checksum = "7ebb8d8732c6a6df3d8f032a82911cfc747e00efb95cc46e8d0acd5b5b88570c" [[package]] name = "media" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "bindgen", @@ -5202,7 +5202,7 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "536bfad37a309d62069485248eeaba1e8d9853aaf951caaeaed0585a95346f08" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -5258,7 +5258,7 @@ dependencies = [ "once_cell", "png", "thiserror 2.0.20", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -5481,7 +5481,7 @@ version = "0.50.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7957b9740744892f114936ab4a57b3f487491bbeafaf8083688b16841a4240e5" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -6204,7 +6204,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perf" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "collections", "serde", @@ -6900,7 +6900,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -7207,7 +7207,7 @@ dependencies = [ [[package]] name = "refineable" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "derive_refineable", ] @@ -7250,7 +7250,7 @@ checksum = "19b30a45b0cd0bcca8037f3d0dc3421eaf95327a17cad11964fb8179b4fc4832" [[package]] name = "reqwest_client" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "bytes", @@ -7611,7 +7611,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -7784,7 +7784,7 @@ dependencies = [ [[package]] name = "scheduler" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "async-task", "backtrace", @@ -8389,7 +8389,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c3d1e2c7f27f8d4cb10542a02c49005dbd6e93095799d6f3be745fae9f8fedd4" dependencies = [ "libc", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -8515,7 +8515,7 @@ dependencies = [ "cfg-if", "libc", "psm", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -8627,7 +8627,7 @@ checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" [[package]] name = "sum_tree" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "heapless", "log", @@ -8917,7 +8917,7 @@ dependencies = [ "getrandom 0.4.3", "once_cell", "rustix 1.1.4", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -9384,7 +9384,7 @@ dependencies = [ "once_cell", "png", "thiserror 2.0.20", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -9896,7 +9896,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e" dependencies = [ "memoffset 0.9.1", "tempfile", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -10108,7 +10108,7 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "util" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "async-fs", @@ -10147,7 +10147,7 @@ dependencies = [ [[package]] name = "util_macros" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "perf", "quote", @@ -10740,7 +10740,7 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c2a7b1c03c876122aa43f3020e6c3c3ee5c05081c9a00739faf7503aeba10d22" dependencies = [ - "windows-sys 0.61.2", + "windows-sys 0.48.0", ] [[package]] @@ -11491,7 +11491,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7d6f32a0ff4a9f6f01231eb2059cc85479330739333e0e58cadf03b6af2cca10" dependencies = [ "cfg-if", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -12048,7 +12048,7 @@ dependencies = [ [[package]] name = "zlog" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "anyhow", "chrono", @@ -12093,7 +12093,7 @@ dependencies = [ [[package]] name = "ztracing" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" dependencies = [ "tracing", "tracing-subscriber", @@ -12104,7 +12104,7 @@ dependencies = [ [[package]] name = "ztracing_macro" version = "0.1.0" -source = "git+https://github.com/l0ng-ai/zed?branch=tty7#d99a40a5f79a173465feec00363d480f75881a81" +source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a8d20488769b2de9a6" [[package]] name = "zune-core" diff --git a/Cargo.toml b/Cargo.toml index 55848281..48f90ceb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -367,7 +367,7 @@ lto = "thin" codegen-units = 1 # ---- gpui fork ------------------------------------------------------------ -# Our `tty7` branch (cut from the pinned upstream rev, three commits on top) carries: +# Our `tty7` branch (cut from the pinned upstream rev) carries: # # 1. `prefers_ime_for_printable_keys` takes the keystroke, so an input handler can # answer per key instead of per view. tty7 needs it for Option-as-Meta — macOS @@ -386,6 +386,14 @@ codegen-units = 1 # 3. resvg/usvg bumped 0.45 → 0.47 so gpui's SVG stack unifies with the resvg # tty7 pins directly above (follow-up to the #227 dependabot bump). # +# 4. `ListState::with_size_hint`. A list counts an item it has not laid out yet +# as zero tall, so a long document reports itself as about one viewport and +# a scrollbar reading that height drags one viewport and stops — the diff +# overlay scrolls a whole working tree through one of these. The hint counts +# the unmeasured items at an estimate instead, which `measure_all` would +# settle exactly at the price of laying out the whole document on the first +# frame (see `ui::diff_overlay::DIFF_LINE_H`). +# # Patching by source rather than editing the `gpui`/`gpui_platform` pins above is # deliberate: `gpui-component` declares its own `gpui` from the upstream URL, and # a plain pin swap would put two incompatible copies of gpui in the tree. `[patch]` diff --git a/src/ui/diff_list.rs b/src/ui/diff_list.rs new file mode 100644 index 00000000..08a77818 --- /dev/null +++ b/src/ui/diff_list.rs @@ -0,0 +1,484 @@ +//! Flattening a diff snapshot into the one-dimensional list of rows the +//! overlay scrolls. +//! +//! The overlay used to build its whole patch as a nested element tree — +//! a card per file, a hunk header and one element per line inside it, every +//! one of them constructed on every frame. A `20_000` line budget is around +//! `120_000` elements to lay out, and gpui notifies the view on each scroll +//! wheel event, so a diff of any size rebuilt the entire tree tens of times a +//! second. +//! +//! [`gpui::list`] only builds the rows it can see, but it is one-dimensional: +//! it takes an index, not a tree. So the tree is flattened here, once per +//! change to what is on screen. + +use std::collections::HashMap; + +use crate::core::config::DiffViewMode; +use crate::terminal::git_diff::{ + AUTO_COLLAPSE_LINES, DiffSnapshot, FileDiff, FileStatus, MAX_RENDERED_FILES, Truncation, +}; +use crate::ui::diff_rows::{SplitRow, UnifiedRow, split_hunk, unified_rows}; + +/// Everything the file header row draws, lifted out of its [`FileDiff`]. +/// +/// A copy rather than a borrow: the row list outlives the frame that built it, +/// and these are a handful of small fields against a file's whole patch. +#[derive(PartialEq, Eq)] +pub(crate) struct FileHead { + /// Position in `snap.files`, for the element id and nothing else. + pub(crate) index: usize, + pub(crate) path: String, + /// `old → new` for a rename, the path itself otherwise. + pub(crate) shown_path: String, + pub(crate) status: FileStatus, + pub(crate) added: u32, + pub(crate) removed: u32, + pub(crate) binary: bool, + pub(crate) expandable: bool, + pub(crate) expanded: bool, +} + +#[derive(PartialEq, Eq)] +pub(crate) enum DiffRow { + /// The space that used to be the `gap_3` of a flex column. + Gap, + /// The banner above an auto-collapsed tree. Its text is derived from the + /// snapshot at render time, so nothing is carried here. + Oversized, + FileHeader(FileHead), + HunkHeader { + text: String, + /// The first hunk of a file follows its header and needs no rule + /// above it; every later one is separating itself from the lines of + /// the hunk before. + leads: bool, + }, + Split(SplitRow), + Unified(UnifiedRow), + Truncated(Truncation), + MoreFiles { + rest: usize, + }, + UntrackedHeader { + total: usize, + }, + Untracked { + index: usize, + path: String, + }, + MoreUntracked { + rest: usize, + }, +} + +/// The rows of the whole overlay, in the order they scroll past. +pub(crate) fn build_rows( + snap: &DiffSnapshot, + expanded: &HashMap, + focused: Option, + mode: DiffViewMode, + oversized: bool, +) -> Vec { + let mut out: Vec = Vec::new(); + // Stands in for the gap between the groups this list used to be a flex + // column of: gpui's list stacks its items with nothing between them. + let gap = |out: &mut Vec| { + if !out.is_empty() { + out.push(DiffRow::Gap); + } + }; + + if oversized { + out.push(DiffRow::Oversized); + } + + 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; + } + let is_expanded = if focused == Some(idx) { + expanded.get(&file.path).copied().unwrap_or(true) + } else { + file_expanded(file, expanded, oversized) + }; + gap(&mut out); + out.extend(file_rows(idx, file, is_expanded, mode)); + } + + if focused.is_none() && snap.files.len() > shown { + gap(&mut out); + out.push(DiffRow::MoreFiles { + rest: snap.files.len() - shown, + }); + } + + if focused.is_none() && !snap.untracked.is_empty() { + gap(&mut out); + out.extend(untracked_rows(snap)); + } + + out +} + +/// The single synthesized file an untracked file's preview shows. +/// +/// `usize::MAX` keeps its element ids clear of the real list's. +pub(crate) fn preview_rows(file: &FileDiff, mode: DiffViewMode) -> Vec { + file_rows(usize::MAX, file, true, mode) +} + +fn file_rows(index: usize, file: &FileDiff, expanded: bool, mode: DiffViewMode) -> Vec { + let expandable = + !file.binary && (!file.hunks.is_empty() || file.truncated == Some(Truncation::Budget)); + let shown_path = match &file.old_path { + Some(old) => format!("{old} → {}", file.path), + None => file.path.clone(), + }; + let mut rows = vec![DiffRow::FileHeader(FileHead { + index, + path: file.path.clone(), + shown_path, + status: file.status, + added: file.added, + removed: file.removed, + binary: file.binary, + expandable, + expanded, + })]; + + if expanded && (!file.hunks.is_empty() || file.truncated.is_some()) { + for (h, hunk) in file.hunks.iter().enumerate() { + rows.push(DiffRow::HunkHeader { + text: hunk.header.clone(), + leads: h == 0, + }); + match mode { + DiffViewMode::Split => { + rows.extend(split_hunk(&hunk.lines).into_iter().map(DiffRow::Split)) + } + DiffViewMode::Unified => { + rows.extend(unified_rows(&hunk.lines).into_iter().map(DiffRow::Unified)) + } + } + } + if let Some(reason) = file.truncated { + rows.push(DiffRow::Truncated(reason)); + } + } + + rows +} + +fn untracked_rows(snap: &DiffSnapshot) -> Vec { + let total = snap.untracked_count(); + let shown = &snap.untracked[..snap.untracked.len().min(MAX_RENDERED_FILES)]; + let mut rows = vec![DiffRow::UntrackedHeader { total }]; + for (index, path) in shown.iter().enumerate() { + rows.push(DiffRow::Untracked { + index, + path: path.clone(), + }); + } + if total > shown.len() { + rows.push(DiffRow::MoreUntracked { + rest: total - shown.len(), + }); + } + rows +} + +/// Whether a file's lines show on their own, absent an explicit choice. +pub(crate) fn file_expanded( + file: &FileDiff, + expanded: &HashMap, + collapse_all: bool, +) -> bool { + if let Some(&want) = expanded.get(&file.path) { + return want; + } + !collapse_all && file.added + file.removed <= AUTO_COLLAPSE_LINES +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::terminal::git_diff::{DiffLine, Hunk, LineKind}; + + fn line(kind: LineKind, old: Option, new: Option) -> DiffLine { + DiffLine { + kind, + old_no: old, + new_no: new, + text: "x".to_string(), + } + } + + /// One removal answered by one addition, wrapped in a context line either + /// side — four lines unified, three rows split. + fn hunk() -> Hunk { + Hunk { + header: "@@ -1,3 +1,3 @@".to_string(), + lines: vec![ + line(LineKind::Context, Some(1), Some(1)), + line(LineKind::Removed, Some(2), None), + line(LineKind::Added, None, Some(2)), + line(LineKind::Context, Some(3), Some(3)), + ], + } + } + + fn file(path: &str, hunks: usize) -> FileDiff { + FileDiff { + path: path.to_string(), + old_path: None, + status: FileStatus::Modified, + added: 1, + removed: 1, + binary: false, + truncated: None, + hunks: std::iter::repeat_n(hunk(), hunks).collect(), + } + } + + fn snapshot(files: Vec) -> DiffSnapshot { + DiffSnapshot { + files, + ..Default::default() + } + } + + fn shape(rows: &[DiffRow]) -> Vec<&'static str> { + rows.iter() + .map(|row| match row { + DiffRow::Gap => "gap", + DiffRow::Oversized => "oversized", + DiffRow::FileHeader(_) => "file", + DiffRow::HunkHeader { .. } => "hunk", + DiffRow::Split(_) => "split", + DiffRow::Unified(_) => "unified", + DiffRow::Truncated(_) => "truncated", + DiffRow::MoreFiles { .. } => "more-files", + DiffRow::UntrackedHeader { .. } => "untracked-header", + DiffRow::Untracked { .. } => "untracked", + DiffRow::MoreUntracked { .. } => "more-untracked", + }) + .collect() + } + + fn open(paths: [&str; 1]) -> HashMap { + paths.into_iter().map(|p| (p.to_string(), true)).collect() + } + + #[test] + fn an_open_file_flattens_to_a_header_a_hunk_header_and_a_row_per_line() { + let snap = snapshot(vec![file("a.rs", 1)]); + let rows = build_rows(&snap, &HashMap::new(), None, DiffViewMode::Unified, false); + assert_eq!( + shape(&rows), + ["file", "hunk", "unified", "unified", "unified", "unified"] + ); + + let rows = build_rows(&snap, &HashMap::new(), None, DiffViewMode::Split, false); + assert_eq!( + shape(&rows), + ["file", "hunk", "split", "split", "split"], + "the split view pairs the removal with the addition that replaced it" + ); + } + + #[test] + fn only_the_first_hunk_of_a_file_follows_its_header() { + let rows = build_rows( + &snapshot(vec![file("a.rs", 2)]), + &HashMap::new(), + None, + DiffViewMode::Unified, + false, + ); + let leads: Vec = rows + .iter() + .filter_map(|r| match r { + DiffRow::HunkHeader { leads, .. } => Some(*leads), + _ => None, + }) + .collect(); + assert_eq!( + leads, + [true, false], + "the second hunk draws the rule that separates it from the first" + ); + } + + #[test] + fn a_collapsed_file_is_a_single_row() { + let snap = snapshot(vec![file("a.rs", 1)]); + let shut: HashMap = [("a.rs".to_string(), false)].into_iter().collect(); + let rows = build_rows(&snap, &shut, None, DiffViewMode::Unified, false); + assert_eq!(shape(&rows), ["file"]); + } + + #[test] + fn a_truncation_note_lands_under_the_lines_it_is_about() { + let mut f = file("a.rs", 1); + f.truncated = Some(Truncation::Budget); + let rows = build_rows( + &snapshot(vec![f]), + &HashMap::new(), + None, + DiffViewMode::Unified, + false, + ); + assert_eq!(*shape(&rows).last().unwrap(), "truncated"); + } + + #[test] + fn files_are_separated_by_a_gap_and_the_list_never_opens_with_one() { + let snap = snapshot(vec![file("a.rs", 1), file("b.rs", 1)]); + let shut: HashMap = + snap.files.iter().map(|f| (f.path.clone(), false)).collect(); + let rows = build_rows(&snap, &shut, None, DiffViewMode::Unified, false); + assert_eq!(shape(&rows), ["file", "gap", "file"]); + } + + #[test] + fn the_oversized_banner_leads_and_collapses_what_was_not_asked_for() { + let snap = snapshot(vec![file("a.rs", 1), file("b.rs", 1)]); + let rows = build_rows(&snap, &open(["b.rs"]), None, DiffViewMode::Unified, true); + assert_eq!( + shape(&rows), + [ + "oversized", + "gap", + "file", + "gap", + "file", + "hunk", + "unified", + "unified", + "unified", + "unified" + ], + "the file the reader opened stays open; the other one does not" + ); + } + + #[test] + fn a_focused_file_is_the_only_one_flattened() { + let snap = snapshot(vec![file("a.rs", 1), file("b.rs", 1)]); + let rows = build_rows( + &snap, + &HashMap::new(), + Some(1), + DiffViewMode::Unified, + false, + ); + assert_eq!( + shape(&rows), + ["file", "hunk", "unified", "unified", "unified", "unified"] + ); + let DiffRow::FileHeader(head) = &rows[0] else { + panic!("the focused file leads"); + }; + assert_eq!(head.path, "b.rs"); + assert_eq!(head.index, 1, "the element id still points into `files`"); + } + + #[test] + fn the_file_list_is_capped_and_the_tail_gets_one_line() { + let snap = snapshot( + (0..MAX_RENDERED_FILES + 25) + .map(|i| { + let mut f = file(&format!("f{i}.rs"), 1); + f.hunks.clear(); + f + }) + .collect(), + ); + let rows = build_rows(&snap, &HashMap::new(), None, DiffViewMode::Unified, false); + let files = rows + .iter() + .filter(|r| matches!(r, DiffRow::FileHeader(_))) + .count(); + assert_eq!(files, MAX_RENDERED_FILES); + assert!(matches!(rows.last(), Some(DiffRow::MoreFiles { rest: 25 }))); + } + + #[test] + fn the_untracked_list_is_capped_but_its_count_stays_true() { + let mut snap = snapshot(Vec::new()); + snap.untracked = (0..MAX_RENDERED_FILES + 5) + .map(|i| format!("p{i}")) + .collect(); + snap.untracked_total = 9_000; + let rows = build_rows(&snap, &HashMap::new(), None, DiffViewMode::Unified, false); + + assert!(matches!( + rows.first(), + Some(DiffRow::UntrackedHeader { total: 9_000 }) + )); + let shown = rows + .iter() + .filter(|r| matches!(r, DiffRow::Untracked { .. })) + .count(); + assert_eq!(shown, MAX_RENDERED_FILES); + assert!(matches!( + rows.last(), + Some(DiffRow::MoreUntracked { rest }) if *rest == 9_000 - MAX_RENDERED_FILES + )); + } + + #[test] + fn a_preview_is_one_open_file() { + let rows = preview_rows(&file("new.md", 1), DiffViewMode::Unified); + assert_eq!( + shape(&rows), + ["file", "hunk", "unified", "unified", "unified", "unified"] + ); + let DiffRow::FileHeader(head) = &rows[0] else { + panic!("a file opens with its header"); + }; + assert_eq!( + head.index, + usize::MAX, + "its element ids stay clear of the real list's" + ); + assert!(head.expanded); + } + + #[test] + fn a_rename_shows_both_names_and_a_binary_file_cannot_be_opened() { + let mut renamed = file("new.rs", 1); + renamed.old_path = Some("old.rs".to_string()); + let rows = preview_rows(&renamed, DiffViewMode::Unified); + let DiffRow::FileHeader(head) = &rows[0] else { + unreachable!() + }; + assert_eq!(head.shown_path, "old.rs → new.rs"); + assert_eq!(head.path, "new.rs", "the key is still the file itself"); + + let mut binary = file("logo.png", 0); + binary.binary = true; + let rows = preview_rows(&binary, DiffViewMode::Unified); + let DiffRow::FileHeader(head) = &rows[0] else { + unreachable!() + }; + assert!(!head.expandable); + } + + /// The threshold that decides whether a file opens on its own, kept where + /// the flattening that reads it lives. + #[test] + fn a_file_over_the_line_count_opens_only_when_asked() { + let mut big = file("big.rs", 1); + big.added = AUTO_COLLAPSE_LINES + 1; + let none = HashMap::new(); + assert!(!file_expanded(&big, &none, false)); + assert!(file_expanded(&big, &open(["big.rs"]), true)); + assert!(file_expanded(&file("small.rs", 1), &none, false)); + } +} diff --git a/src/ui/diff_overlay.rs b/src/ui/diff_overlay.rs index 2e869e31..d3d60b72 100644 --- a/src/ui/diff_overlay.rs +++ b/src/ui/diff_overlay.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; +use std::rc::Rc; use std::sync::Arc; use gpui::{ @@ -13,8 +14,8 @@ use gpui_component::{ActiveTheme as _, Icon, IconName, Sizable as _, h_flex, v_f use crate::core::config::{Config, DiffViewMode}; use crate::core::git::status::DecoStatus; use crate::terminal::git_diff::{ - self, AUTO_COLLAPSE_LINES, CommitLabel, DiffSnapshot, DiffSource, DiffStats, FileDiff, - FileStatus, LineKind, MAX_RENDERED_FILES, Truncation, + self, CommitLabel, DiffSnapshot, DiffSource, DiffStats, FileDiff, FileStatus, LineKind, + Truncation, }; /// How much of an untracked file the preview will read. Past this the card @@ -22,12 +23,12 @@ use crate::terminal::git_diff::{ /// line budget below cuts rendering long before this does anyway. const MAX_PREVIEW_BYTES: u64 = 4 * 1024 * 1024; use crate::ui::app::Tty7App; -use crate::ui::diff_rows::{Side, SplitCell, SplitRow, UnifiedRow, split_hunk, unified_rows}; +use crate::ui::diff_list::{DiffRow, FileHead}; +use crate::ui::diff_rows::{Side, SplitCell, SplitRow, UnifiedRow}; use crate::ui::document_column::DocumentChrome; use crate::ui::i18n::{L10nKey, t, t_fmt, t_plural}; use crate::ui::right_panel::info_chip; use crate::ui::rounding; -use crate::ui::rounding::RoundedCorners as _; use crate::ui::scm::path::relative_time; use crate::ui::scm::status::{status_color, status_glyph}; @@ -56,7 +57,13 @@ pub(crate) struct DiffOverlayState { /// cadence a tracked file's does. pub(crate) preview: Option<(String, Option>)>, pub(crate) preview_loading: Option, - pub(crate) scroll: gpui::ScrollHandle, + /// The virtualised list the rows scroll in. Held across frames: it owns + /// the scroll position, and the row heights gpui has measured. + pub(crate) list: gpui::ListState, + /// The patch, flattened into one row per line — see + /// [`crate::ui::diff_list`]. Rebuilt only when [`RowsKey`] changes. + pub(crate) rows: Rc>, + rows_key: Option, /// The [`ScmData`](crate::terminal::git_data::ScmData) epoch this patch was /// read at, for the two sources that can go stale. /// @@ -67,25 +74,6 @@ pub(crate) struct DiffOverlayState { pub(crate) epoch: Option, } -/// One hunk, already turned into whichever kind of row the current view draws. -enum HunkRows { - Split(Vec), - Unified(Vec), -} - -impl HunkRows { - fn len(&self) -> usize { - match self { - HunkRows::Split(rows) => rows.len(), - HunkRows::Unified(rows) => rows.len(), - } - } - - fn is_empty(&self) -> bool { - self.len() == 0 - } -} - /// Paints the full-window diff surface without inheriting workspace opacity. /// The preset's solid or gradient design remains intact, but neither it nor /// the plain theme fallback may reveal the OS backdrop through diff text. @@ -180,7 +168,10 @@ impl Tty7App { focus, preview: None, preview_loading: None, - scroll: gpui::ScrollHandle::new(), + list: gpui::ListState::new(0, gpui::ListAlignment::Top, px(256.)) + .with_size_hint(DIFF_LINE_H), + rows: Rc::new(Vec::new()), + rows_key: None, epoch: None, }); window.focus(&focus_handle, cx); @@ -395,38 +386,12 @@ impl Tty7App { cx: &mut Context, ) -> Option { self.spawn_untracked_preview_if_needed(cx); + let body = self.sync_diff_rows(cx)?; let overlay = self.tabs.get(self.active)?.diff_overlay.as_ref()?; - let content = match &overlay.load { - DiffLoad::Loading => self.diff_message(t(L10nKey::DiffReading), cx), - DiffLoad::NotARepo => self.diff_message(t(L10nKey::DiffNotARepo), cx), - DiffLoad::Ready(snap) if empty_snapshot(snap) && snap.read_failed => { - self.diff_message(t(L10nKey::DiffReadFailed), cx) - } - DiffLoad::Ready(snap) if empty_snapshot(snap) => { - self.diff_message(t(L10nKey::DiffWorkingTreeClean), cx) - } - // A focused *untracked* file has no patch in the snapshot; its - // card is synthesized from the file's own bytes — see `preview`. - DiffLoad::Ready(snap) if untracked_focus(snap, overlay.focus.as_deref()).is_some() => { - let path = untracked_focus(snap, overlay.focus.as_deref()).unwrap(); - match &overlay.preview { - Some((held, Some(file))) if held == path => { - self.diff_preview_card(file.as_ref(), &overlay.scroll, cx) - } - Some((held, None)) if held == path => { - self.diff_message(t(L10nKey::DiffReadFailed), cx) - } - _ => self.diff_message(t(L10nKey::DiffReading), cx), - } - } - DiffLoad::Ready(snap) => self.diff_file_list( - snap, - &overlay.expanded, - focused_file(snap, overlay), - &overlay.scroll, - cx, - ), + let content = match body { + DiffBody::Message(text) => self.diff_message(text, cx), + DiffBody::Rows(snap) => self.diff_rows_list(overlay, snap, cx), }; let header = chrome @@ -675,25 +640,12 @@ impl Tty7App { .when(subject.label.is_none() && !subject_takes_the_slack, |bar| { bar.child(div().flex_1()) }) - .child(div().occlude().flex_shrink_0().child({ - let sf = cx.global::().window; - let selected = usize::from(view_mode(cx) == DiffViewMode::Unified); - self.segmented_on( - sf, - "diff-overlay-view", - &[t(L10nKey::DiffViewSplit), t(L10nKey::DiffViewUnified)], - selected, - cx, - |this, index, _window, cx| { - let mode = if index == 0 { - DiffViewMode::Split - } else { - DiffViewMode::Unified - }; - this.update_config(cx, |cfg| cfg.diff_view = mode); - }, - ) - })) + .child( + div() + .occlude() + .flex_shrink_0() + .child(self.diff_view_switch(cx)), + ) .child( div().occlude().flex_shrink_0().child( crate::ui::tab_strip::chrome_tile_sized( @@ -715,6 +667,54 @@ impl Tty7App { }) } + /// The two views, as a switch rather than a control. + /// + /// Not [`Tty7App::segmented_on`]: that one is a bordered track, which is + /// right in a settings row, where it ends a line of prose and has to + /// announce itself as something you operate. On a title bar it was the + /// only bordered thing on the strip — the close tile beside it is a bare + /// glyph, and so is every tile at the other end of the window — so it read + /// as pasted on. Same two choices, no frame: the live one carries a soft + /// fill, the other is quiet text that lights up under the pointer. + fn diff_view_switch(&self, cx: &mut Context) -> AnyElement { + let sf = cx.global::().window; + let current = view_mode(cx); + let cells = [ + (DiffViewMode::Split, t(L10nKey::DiffViewSplit)), + (DiffViewMode::Unified, t(L10nKey::DiffViewUnified)), + ]; + h_flex() + .id("diff-overlay-view") + .flex_shrink_0() + .gap(px(2.)) + .children(cells.into_iter().enumerate().map(|(i, (mode, label))| { + let live = mode == current; + h_flex() + .id(("diff-overlay-view-cell", i)) + .items_center() + .h(px(22.)) + .px(px(8.)) + .rounded(ROW_RADIUS) + .text_sm() + .cursor_pointer() + .when(live, |cell| { + cell.bg(gpui::rgb(sf.selected)) + .text_color(gpui::rgb(sf.text_selected)) + .font_weight(FontWeight::MEDIUM) + }) + .when(!live, |cell| { + cell.text_color(cx.theme().muted_foreground) + .hover(|h| h.bg(gpui::rgb(sf.hover))) + }) + .active(|cell| cell.bg(gpui::rgb(sf.pressed))) + .on_click(cx.listener(move |this, _, _window, cx| { + this.update_config(cx, |cfg| cfg.diff_view = mode); + })) + .child(label) + })) + .into_any_element() + } + /// Dispatch the byte read behind an untracked file's preview, at most /// once per (path, snapshot). Runs from `render`, so the guards are the /// point: `preview` says the answer is in hand, `preview_loading` says it @@ -788,33 +788,6 @@ impl Tty7App { ); } - /// The one synthesized card, in the same scroll shell the file list uses. - fn diff_preview_card( - &self, - file: &FileDiff, - scroll: &gpui::ScrollHandle, - cx: &mut Context, - ) -> AnyElement { - let mode = view_mode(cx); - let list = v_flex() - .gap_3() - .p_4() - .w_full() - // `usize::MAX` keeps the element ids clear of the real list's. - .child(self.diff_file_card(usize::MAX, file, true, mode, cx)); - crate::ui::scrollbar::with_vertical_scrollbar( - "diff-overlay-scrollbar", - div() - .id("diff-overlay-scroll") - .flex_1() - .min_h_0() - .overflow_y_scroll() - .track_scroll(scroll) - .child(list), - scroll, - ) - } - fn diff_message(&self, text: &'static str, cx: &Context) -> AnyElement { div() .flex_1() @@ -827,500 +800,627 @@ impl Tty7App { .into_any_element() } - fn diff_file_list( - &self, - snap: &DiffSnapshot, - expanded: &HashMap, - focused: Option, - scroll: &gpui::ScrollHandle, - cx: &mut Context, - ) -> AnyElement { - let stats = snap.stats(); + /// Brings the active overlay's flattened rows up to date with what it is + /// meant to be showing, and says what to draw. + /// + /// Called from `render`, so the [`RowsKey`] comparison is what keeps it + /// cheap: flattening a twenty-thousand-line patch allocates a row per + /// line, and nothing about that changes between two frames of scrolling. + fn sync_diff_rows(&mut self, cx: &mut Context) -> Option { let mode = view_mode(cx); - let oversized = focused.is_none() && stats.oversized; - let mut list = v_flex().gap_3().p_4().w_full(); - if oversized { - list = list.child(self.diff_oversized_notice(snap, &stats, cx)); - } - 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; - } - let is_expanded = if focused == Some(idx) { - expanded.get(&file.path).copied().unwrap_or(true) - } else { - file_expanded(file, expanded, oversized) - }; - list = list.child(self.diff_file_card(idx, file, is_expanded, mode, 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(t_plural(L10nKey::DiffMoreFiles, rest, &[])), - ); - } - if focused.is_none() && !snap.untracked.is_empty() { - list = list.child(self.diff_untracked_section(snap, cx)); - } - // A whole working tree can scroll past here with nothing to say how - // far it runs or where in it you are — the one long document in the - // app without the bar every other scroll area has. - crate::ui::scrollbar::with_vertical_scrollbar( - "diff-overlay-scrollbar", - div() - .id("diff-overlay-scroll") - .flex_1() - .min_h_0() - .overflow_y_scroll() - .track_scroll(scroll) - .child(list), - scroll, - ) - } + let active = self.active; + let overlay = self.tabs.get_mut(active)?.diff_overlay.as_mut()?; - fn diff_oversized_notice( - &self, - snap: &DiffSnapshot, - stats: &DiffStats, - cx: &Context, - ) -> AnyElement { - let text = t_fmt( - L10nKey::DiffOversizedNotice, - &[("summary", &oversized_summary(snap, stats))], - ); - 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() - } - - fn diff_file_card( - &self, - idx: usize, - file: &FileDiff, - expanded: bool, - mode: DiffViewMode, - cx: &mut Context, - ) -> AnyElement { - let expandable = - !file.binary && (!file.hunks.is_empty() || file.truncated == Some(Truncation::Budget)); - let deco = deco_status(file.status); - let (glyph, glyph_color) = (status_glyph(deco), status_color(deco, cx)); - let shown_path = match &file.old_path { - Some(old) => format!("{old} → {}", file.path), - None => file.path.clone(), + let snap = match &overlay.load { + DiffLoad::Loading => return Some(DiffBody::Message(t(L10nKey::DiffReading))), + DiffLoad::NotARepo => return Some(DiffBody::Message(t(L10nKey::DiffNotARepo))), + DiffLoad::Ready(snap) if empty_snapshot(snap) && snap.read_failed => { + return Some(DiffBody::Message(t(L10nKey::DiffReadFailed))); + } + DiffLoad::Ready(snap) if empty_snapshot(snap) => { + return Some(DiffBody::Message(t(L10nKey::DiffWorkingTreeClean))); + } + DiffLoad::Ready(snap) => Arc::clone(snap), }; - let has_body = expanded && (!file.hunks.is_empty() || file.truncated.is_some()); + // A focused *untracked* file has no patch in the snapshot; its card is + // synthesized from the file's own bytes — see `preview`. + let preview = match untracked_focus(&snap, overlay.focus.as_deref()) { + Some(path) => match &overlay.preview { + Some((held, file)) if held == path => match file { + Some(file) => Some(Arc::clone(file)), + None => return Some(DiffBody::Message(t(L10nKey::DiffReadFailed))), + }, + _ => return Some(DiffBody::Message(t(L10nKey::DiffReading))), + }, + None => None, + }; - let header_corners = rounding::stack_corners( - 0, - if has_body { 2 } else { 1 }, - rounding::CARD_RADIUS, - rounding::HAIRLINE, - ); - let mut header = h_flex() - .id(("diff-file-header", idx)) - .w_full() - .items_center() - .gap_2() - .px_2p5() - .py_1p5() - .rounded_corners(header_corners) - .bg(cx.theme().secondary) - .when(expandable, |h| { - let path = file.path.clone(); - h.cursor_pointer() - .hover(|s| s.bg(cx.theme().list_hover)) - .on_click(cx.listener(move |this, _, _window, cx| { + let focused = focused_file(&snap, overlay); + let from = RowsFrom { + snap: &snap, + preview: preview.as_ref(), + mode, + focused, + oversized: focused.is_none() && snap.stats().oversized, + expanded: &overlay.expanded, + }; + let stale = overlay + .rows_key + .as_ref() + .is_none_or(|held| !held.describes(&from)); + if stale { + let rows = match from.preview { + Some(file) => crate::ui::diff_list::preview_rows(file, from.mode), + None => crate::ui::diff_list::build_rows( + from.snap, + from.expanded, + from.focused, + from.mode, + from.oversized, + ), + }; + let key = from.to_key(); + resync_list(&overlay.list, &overlay.rows, &rows); + overlay.rows = Rc::new(rows); + overlay.rows_key = Some(key); + } else if let Some(held) = overlay.rows_key.as_mut() { + held.retarget(&from); + } + Some(DiffBody::Rows(snap)) + } + + /// The rows, in the virtualised list that draws only the visible ones. + fn diff_rows_list( + &self, + overlay: &DiffOverlayState, + snap: Arc, + cx: &mut Context, + ) -> AnyElement { + let rows = Rc::clone(&overlay.rows); + let font = SharedString::from(self.font_family.clone()); + let app = cx.entity().downgrade(); + let list = overlay.list.clone(); + let body = gpui::list(list.clone(), move |ix, _window, cx| { + #[cfg(test)] + row_probe::record(); + match rows.get(ix) { + Some(row) => diff_row_element(row, &font, &snap, &app, cx), + // The list is spliced in step with `rows`, so this is + // unreachable — and an empty row is a better answer to a bug + // than an index panic in a paint. + None => div().into_any_element(), + } + }) + .size_full() + // Only the vertical padding: `List` lays every item out at its own + // full width and puts it at its own left edge, so a horizontal + // padding here would be silently ignored. The rows carry their own — + // see `diff_row_element`. + .py_4(); + // The bar reads the list's own height, and a list only counts the + // rows it has measured. Left at that, a patch of any length would + // report itself as one screen long and the thumb would fill the + // track: a drag from top to bottom would travel one screen and stop, + // on the one document in the app long enough to need the bar. The + // rows below the fold are counted at `DIFF_LINE_H` until they are + // laid out — `ListState::measure_all` would settle it exactly, by + // laying out every row on the first frame, which is the cost this + // whole list exists to avoid. + crate::ui::scrollbar::with_vertical_scrollbar("diff-overlay-scrollbar", body, &list) + } +} + +/// Counts the rows the list actually built, so a test can tell that a patch of +/// any size costs the handful of rows on screen rather than all of them. +#[cfg(test)] +pub(crate) mod row_probe { + use std::cell::Cell; + + thread_local! { + static BUILT: Cell = const { Cell::new(0) }; + } + + pub(crate) fn record() { + BUILT.set(BUILT.get() + 1); + } + + /// The count since the last call, and zero from here. + pub(crate) fn take() -> u64 { + BUILT.replace(0) + } +} + +/// What the overlay's scrolling area holds this frame. +enum DiffBody { + Message(&'static str), + /// The rows are in [`DiffOverlayState::rows`]; the snapshot rides along + /// for the few rows whose text is derived from it. + Rows(Arc), +} + +/// What this frame would flatten its rows from, borrowed from the overlay. +struct RowsFrom<'a> { + snap: &'a Arc, + preview: Option<&'a Arc>, + mode: DiffViewMode, + focused: Option, + oversized: bool, + expanded: &'a HashMap, +} + +impl RowsFrom<'_> { + fn to_key(&self) -> RowsKey { + RowsKey { + snap: Arc::clone(self.snap), + preview: self.preview.cloned(), + mode: self.mode, + focused: self.focused, + oversized: self.oversized, + expanded: self.expanded.clone(), + } + } +} + +/// What [`DiffOverlayState::rows`] was flattened from, kept so the next frame +/// can tell whether it would flatten the same rows again. +struct RowsKey { + snap: Arc, + preview: Option>, + mode: DiffViewMode, + focused: Option, + oversized: bool, + expanded: HashMap, +} + +impl RowsKey { + /// Whether the rows built from `from` would be the rows already held. + /// + /// The snapshot is compared by pointer first and by contents second: a + /// probe that found nothing new still lands a fresh `Arc` over an equal + /// snapshot, and rebuilding every row of the patch for that would undo the + /// point of keeping them. + /// + /// The scalars go first so that the walk of the patch behind that second + /// comparison is only ever paid to answer a question the cheap fields + /// have not already answered. + fn describes(&self, from: &RowsFrom<'_>) -> bool { + self.mode == from.mode + && self.focused == from.focused + && self.oversized == from.oversized + && self.expanded == *from.expanded + && self.same_preview(from) + && (Arc::ptr_eq(&self.snap, from.snap) || self.snap == *from.snap) + } + + /// The preview, by pointer and then by contents — a re-read of an + /// untracked file lands a fresh `Arc` over bytes that did not change. + fn same_preview(&self, from: &RowsFrom<'_>) -> bool { + match (&self.preview, from.preview) { + (None, None) => true, + (Some(a), Some(b)) => Arc::ptr_eq(a, b) || a == b, + _ => false, + } + } + + /// Points the key at the `Arc`s this frame was asked about, having just + /// found them equal to the ones held. + /// + /// Without this the key goes on holding the snapshot from the last + /// *rebuild*, so every frame after a probe that found nothing new proves + /// the two equal the long way — a walk of every line of the patch, once + /// per wheel event, which is the cost this key exists to avoid. + fn retarget(&mut self, from: &RowsFrom<'_>) { + self.snap = Arc::clone(from.snap); + self.preview = from.preview.cloned(); + } +} + +/// Tells the list which rows changed, rather than that all of them did. +/// +/// `ListState::reset` would drop the scroll position, so collapsing one file +/// would throw the reader back to the top of the tree. The rows either side of +/// an edit are untouched, so the shared prefix and suffix are kept and only +/// what is between them is spliced. +fn resync_list(list: &gpui::ListState, old: &[DiffRow], new: &[DiffRow]) { + let (replaced, with) = spliced_range(old, new); + list.splice(replaced, with); +} + +/// Which of the old rows were replaced, and by how many new ones. +fn spliced_range(old: &[DiffRow], new: &[DiffRow]) -> (std::ops::Range, usize) { + let prefix = old.iter().zip(new).take_while(|(a, b)| a == b).count(); + // Whatever is left of the shorter list once the shared head is off it — + // the most the shared tail can be, and what keeps the two slices below in + // step with each other. + let rest = old.len().min(new.len()) - prefix; + let suffix = old[old.len() - rest..] + .iter() + .rev() + .zip(new[new.len() - rest..].iter().rev()) + .take_while(|(a, b)| a == b) + .count(); + (prefix..old.len() - suffix, new.len() - prefix - suffix) +} + +/// The row inset every row of the list shares, matching the source control +/// panel's — the overlay is a second view of that panel's list, and the two +/// stopped looking like one app when this one drew cards. +const ROW_INSET: Pixels = px(10.); + +/// The height of a row that is a *file* rather than a line of one: the same +/// 26px the panel gives its file rows. +const FILE_ROW_H: Pixels = px(26.); + +/// The radius on a row that lights up under the pointer. Matches the panel's. +const ROW_RADIUS: Pixels = px(5.); + +/// The height of one line of a patch, in either view. +/// +/// Also what the list counts a row it has not laid out yet at. A list knows +/// only the rows it has measured, so without an estimate for the rest a patch +/// of any length reports itself as one screen long — and the scrollbar, which +/// reads that height, drags the reader one screen and stops. The file and hunk +/// rows are a few pixels taller, so the estimate runs a little short until +/// they have been measured; a diff is overwhelmingly its lines. +const DIFF_LINE_H: Pixels = px(19.); + +/// The rule between one hunk and the last line of the one before it. +/// +/// Barely there on purpose: with the cards gone it is the only line left in +/// the list, and it is separating two parts of one file rather than two +/// files. +fn hunk_rule(cx: &gpui::App) -> Hsla { + cx.theme().border.opacity(0.6) +} + +/// One row, inset the way every row in the list is. +fn diff_row_element( + row: &DiffRow, + font: &SharedString, + snap: &Arc, + app: &gpui::WeakEntity, + cx: &mut gpui::App, +) -> AnyElement { + match row { + // Stands in for the gap between the groups this list used to be a + // flex column of. + DiffRow::Gap => div().w_full().h(gpui::rems(0.75)).into_any_element(), + DiffRow::Oversized => padded(diff_oversized_notice(snap, cx)), + DiffRow::FileHeader(head) => padded(diff_file_header(head, font, app, cx)), + DiffRow::HunkHeader { text, leads } => padded( + div() + .w_full() + .px(ROW_INSET) + .py_1() + .when(!leads, |h| { + h.mt_1().border_t_1().border_color(hunk_rule(cx)) + }) + .text_xs() + .font_family(font.clone()) + .text_color(cx.theme().muted_foreground) + .truncate() + .child(text.clone()) + .into_any_element(), + ), + // The lines run the full width of the list. A diff is read as a + // column of code, and code that is inset from both sides reads as a + // quotation of itself. + DiffRow::Split(row) => diff_split_row(row, font, cx).into_any_element(), + DiffRow::Unified(row) => diff_unified_row(row, font, cx).into_any_element(), + DiffRow::Truncated(reason) => { + let note = match reason { + Truncation::PerFile => t_fmt( + L10nKey::DiffTruncatedPerFile, + &[("limit", &git_diff::MAX_LINES_PER_FILE.to_string())], + ), + Truncation::Budget => t(L10nKey::DiffTruncatedBudget).to_string(), + }; + padded(note_row(note, cx)) + } + DiffRow::MoreFiles { rest } => { + padded(note_row(t_plural(L10nKey::DiffMoreFiles, *rest, &[]), cx)) + } + // A section label, in the shape the sidebar gives its group headings: + // small, quiet, and carried by the space around it rather than a bar + // of its own. + DiffRow::UntrackedHeader { total } => padded( + div() + .w_full() + .px(ROW_INSET) + .py_1() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(t_plural(L10nKey::DiffUntrackedHeader, *total, &[])) + .into_any_element(), + ), + DiffRow::Untracked { index, path } => { + padded(diff_untracked_row(*index, path, font, app, cx)) + } + DiffRow::MoreUntracked { rest } => padded(note_row( + t_plural(L10nKey::DiffMoreUntracked, *rest, &[]), + cx, + )), + } +} + +/// The margin the file rows keep from the edge of the list. +fn padded(row: AnyElement) -> AnyElement { + div().w_full().px_2().child(row).into_any_element() +} + +/// An aside in the list's own voice — a cap that was hit, a tail that was not +/// drawn. Never a row you can act on, so never one that lights up. +fn note_row(text: String, cx: &gpui::App) -> AnyElement { + div() + .w_full() + .px(ROW_INSET) + .py_1() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(text) + .into_any_element() +} + +fn diff_oversized_notice(snap: &DiffSnapshot, cx: &gpui::App) -> AnyElement { + let stats = snap.stats(); + let text = t_fmt( + L10nKey::DiffOversizedNotice, + &[("summary", &oversized_summary(snap, &stats))], + ); + div() + .w_full() + .px(ROW_INSET) + .py_2() + .rounded(rounding::CARD_RADIUS) + .bg(cx.theme().secondary) + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(text) + .into_any_element() +} + +fn diff_file_header( + head: &FileHead, + font: &SharedString, + app: &gpui::WeakEntity, + cx: &gpui::App, +) -> AnyElement { + let hover = gpui::rgb(cx.global::().window.hover); + let deco = deco_status(head.status); + let (glyph, glyph_color) = (status_glyph(deco), status_color(deco, cx)); + let mut header = h_flex() + .id(("diff-file-header", head.index)) + .w_full() + .items_center() + .gap_2() + .h(FILE_ROW_H) + .px(ROW_INSET) + .rounded(ROW_RADIUS) + .when(head.expandable, |h| { + let path = head.path.clone(); + let want = !head.expanded; + let app = app.clone(); + h.cursor_pointer() + .hover(|s| s.bg(hover)) + .on_click(move |_, _window, cx| { + let path = path.clone(); + app.update(cx, |this, cx| { let active = this.active; if let Some(overlay) = this .tabs .get_mut(active) .and_then(|t| t.diff_overlay.as_mut()) { - overlay.expanded.insert(path.clone(), !expanded); + overlay.expanded.insert(path, want); cx.notify(); } - })) - .child( - Icon::new(if expanded { - IconName::ChevronDown - } else { - IconName::ChevronRight - }) - .small() - .text_color(cx.theme().muted_foreground), - ) - }) - .child( - div() - .flex_shrink_0() - .font_family(self.font_family.clone()) - .text_xs() - .font_weight(FontWeight::BOLD) - .text_color(glyph_color) - .child(glyph), - ) - .child( - div() - .flex_1() - .min_w_0() - .truncate() - .text_xs() - .font_family(self.font_family.clone()) - .child(shown_path), - ); - if file.binary { - header = header.child( - div() - .flex_shrink_0() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child(t(L10nKey::Binary)), - ); - } - if file.added > 0 { - header = header.child( - div() - .flex_shrink_0() - .text_xs() - .text_color(cx.theme().success) - .child(format!("+{}", file.added)), - ); - } - if file.removed > 0 { - header = header.child( - div() - .flex_shrink_0() - .text_xs() - .text_color(cx.theme().danger) - .child(format!("−{}", file.removed)), - ); - } - - let mut card = v_flex() - .w_full() - .border_1() - .border_color(cx.theme().border) - .rounded(rounding::CARD_RADIUS) - .overflow_hidden() - .child(header); - - if has_body { - let mut body = v_flex().w_full(); - let hunks: Vec<_> = file - .hunks - .iter() - .map(|hunk| { - let rows = match mode { - DiffViewMode::Split => HunkRows::Split(split_hunk(&hunk.lines)), - DiffViewMode::Unified => HunkRows::Unified(unified_rows(&hunk.lines)), - }; - (hunk, rows) + }) + .ok(); }) - .collect(); - let closing_row = if file.truncated.is_some() { - None - } else { - hunks - .iter() - .rposition(|(_, rows)| !rows.is_empty()) - .map(|h| (h, hunks[h].1.len() - 1)) - }; - for (h, (hunk, rows)) in hunks.iter().enumerate() { - body = body.child( - div() - .w_full() - .px_2() - .py_0p5() - .bg(cx.theme().muted) - .text_xs() - .font_family(self.font_family.clone()) - .text_color(cx.theme().muted_foreground) - .truncate() - .child(hunk.header.clone()), - ); - match rows { - HunkRows::Split(rows) => { - for (r, row) in rows.iter().enumerate() { - body = body.child(self.diff_split_row( - row, - closing_row == Some((h, r)), - cx, - )); - } - } - HunkRows::Unified(rows) => { - for (r, row) in rows.iter().enumerate() { - body = body.child(self.diff_unified_row( - row, - closing_row == Some((h, r)), - cx, - )); - } - } - } - } - if let Some(reason) = file.truncated { - let note = match reason { - Truncation::PerFile => t_fmt( - L10nKey::DiffTruncatedPerFile, - &[("limit", &git_diff::MAX_LINES_PER_FILE.to_string())], - ), - Truncation::Budget => t(L10nKey::DiffTruncatedBudget).to_string(), + .child( + Icon::new(if head.expanded { + IconName::ChevronDown + } else { + IconName::ChevronRight + }) + .small() + .text_color(cx.theme().muted_foreground), + ) + }) + .child( + div() + .flex_shrink_0() + .font_family(font.clone()) + .text_xs() + .font_weight(FontWeight::BOLD) + .text_color(glyph_color) + .child(glyph), + ) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .text_xs() + .font_family(font.clone()) + .child(head.shown_path.clone()), + ); + if head.binary { + header = header.child( + div() + .flex_shrink_0() + .text_xs() + .text_color(cx.theme().muted_foreground) + .child(t(L10nKey::Binary)), + ); + } + if head.added > 0 { + header = header.child( + div() + .flex_shrink_0() + .text_xs() + .text_color(cx.theme().success) + .child(format!("+{}", head.added)), + ); + } + if head.removed > 0 { + header = header.child( + div() + .flex_shrink_0() + .text_xs() + .text_color(cx.theme().danger) + .child(format!("−{}", head.removed)), + ); + } + header.into_any_element() +} + +// An untracked file has no patch in the snapshot, so it cannot be expanded in +// place the way the files above it are — its contents are read one file at a +// time and shown on their own. The row asks for that read, which until now +// only the Source Control panel could: in the overlay these rows were the only +// files in a list of files that did nothing when clicked. +fn diff_untracked_row( + index: usize, + path: &str, + font: &SharedString, + app: &gpui::WeakEntity, + cx: &gpui::App, +) -> AnyElement { + let hover = gpui::rgb(cx.global::().window.hover); + let for_focus = path.to_string(); + let app = app.clone(); + h_flex() + .id(("diff-untracked", index)) + .w_full() + .items_center() + .gap_2() + .h(FILE_ROW_H) + .px(ROW_INSET) + .rounded(ROW_RADIUS) + .text_xs() + .font_family(font.clone()) + .cursor_pointer() + .hover(|s| s.bg(hover)) + .on_click(move |_, window, cx| { + let for_focus = for_focus.clone(); + app.update(cx, |this, cx| { + let Some((host, cwd, source)) = this + .tabs + .get(this.active) + .and_then(|t| t.diff_overlay.as_ref()) + .map(|o| (o.host_id, o.cwd.clone(), o.source.clone())) + else { + return; }; - body = body.child( - div() - .w_full() - .px_2() - .py_1() - .text_xs() - .text_color(cx.theme().muted_foreground) - .child(note), - ); - } - card = card.child(body); - } - card.into_any_element() - } + this.open_diff_overlay(host, cwd, source, Some(for_focus), window, cx); + }) + .ok(); + }) + .child( + div() + .flex_shrink_0() + .font_weight(FontWeight::BOLD) + .text_color(status_color(DecoStatus::Untracked, cx)) + .child(status_glyph(DecoStatus::Untracked)), + ) + .child(div().flex_1().min_w_0().truncate().child(path.to_string())) + .into_any_element() +} - fn diff_split_row(&self, row: &SplitRow, closes_card: bool, cx: &Context) -> AnyElement { - let radius = if closes_card { - rounding::inner_radius(rounding::CARD_RADIUS, rounding::HAIRLINE) - } else { - px(0.) - }; - h_flex() - .w_full() - .h(px(19.)) - .items_stretch() - .text_xs() - .font_family(self.font_family.clone()) - .child(self.diff_split_cell(row.left.as_ref(), Side::Old, radius, cx)) - .child(div().flex_shrink_0().w(px(1.)).bg(cx.theme().border)) - .child(self.diff_split_cell(row.right.as_ref(), Side::New, radius, cx)) - .into_any_element() - } +fn diff_split_row(row: &SplitRow, font: &SharedString, cx: &gpui::App) -> impl IntoElement { + h_flex() + .w_full() + .h(DIFF_LINE_H) + .items_stretch() + .text_xs() + .font_family(font.clone()) + .child(diff_split_cell(row.left.as_ref(), Side::Old, cx)) + .child(div().flex_shrink_0().w(px(1.)).bg(hunk_rule(cx))) + .child(diff_split_cell(row.right.as_ref(), Side::New, cx)) +} - fn diff_split_cell( - &self, - cell: Option<&SplitCell>, - side: Side, - outer_radius: Pixels, - cx: &Context, - ) -> AnyElement { - let base = h_flex().flex_1().min_w_0().h_full().items_center(); - let base = match side { - Side::Old => base.rounded_bl(outer_radius), - Side::New => base.rounded_br(outer_radius), - }; - let Some(cell) = cell else { - return base.bg(cx.theme().muted.opacity(0.3)).into_any_element(); - }; - let (marker, tint) = match (cell.changed, side) { - (true, Side::Old) => ("−", Some(cx.theme().danger.opacity(0.12))), - (true, Side::New) => ("+", Some(cx.theme().success.opacity(0.12))), - (false, _) => (" ", None), - }; - base.when_some(tint, |row, bg| row.bg(bg)) - .child( - h_flex() - .flex_shrink_0() - .w(px(42.)) - .justify_end() - .pr_1p5() - .text_color(cx.theme().muted_foreground.opacity(0.7)) - .child(cell.no.map(|n| n.to_string()).unwrap_or_default()), - ) - .child( - div() - .flex_1() - .min_w_0() - .truncate() - .child(format!("{marker} {}", cell.text)), - ) - .into_any_element() - } - - /// One line of the unified view. - /// - /// Every measurement it shares with [`Self::diff_split_cell`] is shared on - /// purpose — the same 19px row, the same `text_xs` in the same family, and - /// above all the same `0.12` wash behind an addition and a removal. The two - /// views are one diff seen twice; a different green would read as a - /// different thing. - /// - /// What differs is forced by the shape. The line numbers get 34px a side - /// rather than 42 (there are two gutters here in front of one column of - /// text, not one in front of each), and the `+`/`−` gets a column of its - /// own rather than riding in the text: with three kinds of line stacked in - /// one column, an inlined marker would leave the context lines' code - /// starting two characters left of everything else. - fn diff_unified_row( - &self, - row: &UnifiedRow, - closes_card: bool, - cx: &Context, - ) -> AnyElement { - let radius = if closes_card { - rounding::inner_radius(rounding::CARD_RADIUS, rounding::HAIRLINE) - } else { - px(0.) - }; - let (marker_color, tint) = match row.kind { - LineKind::Added => (cx.theme().success, Some(cx.theme().success.opacity(0.12))), - LineKind::Removed => (cx.theme().danger, Some(cx.theme().danger.opacity(0.12))), - LineKind::Context => (cx.theme().muted_foreground, None), - }; - let gutter = |no: Option| { +fn diff_split_cell(cell: Option<&SplitCell>, side: Side, cx: &gpui::App) -> AnyElement { + let base = h_flex().flex_1().min_w_0().h_full().items_center(); + let Some(cell) = cell else { + return base.bg(cx.theme().muted.opacity(0.3)).into_any_element(); + }; + let (marker, tint) = match (cell.changed, side) { + (true, Side::Old) => ("−", Some(cx.theme().danger.opacity(0.12))), + (true, Side::New) => ("+", Some(cx.theme().success.opacity(0.12))), + (false, _) => (" ", None), + }; + base.when_some(tint, |row, bg| row.bg(bg)) + .child( h_flex() .flex_shrink_0() - .w(px(34.)) + .w(px(42.)) .justify_end() .pr_1p5() .text_color(cx.theme().muted_foreground.opacity(0.7)) - .child(no.map(|n| n.to_string()).unwrap_or_default()) - }; - h_flex() - .w_full() - .h(px(19.)) - .items_center() - .text_xs() - .font_family(self.font_family.clone()) - .rounded_bl(radius) - .rounded_br(radius) - .when_some(tint, |line, bg| line.bg(bg)) - .child(gutter(row.old)) - .child(gutter(row.new)) - // The split view's centre rule, in the one place it still means the - // same thing: everything left of it is a number, everything right - // of it is the file. - .child( - div() - .flex_shrink_0() - .w(px(1.)) - .h_full() - .bg(cx.theme().border), - ) - .child( - div() - .flex_shrink_0() - .w(px(12.)) - .text_center() - .text_color(marker_color) - .child(unified_marker(row.kind)), - ) - .child(div().flex_1().min_w_0().truncate().child(row.text.clone())) - .into_any_element() - } + .child(cell.no.map(|n| n.to_string()).unwrap_or_default()), + ) + .child( + div() + .flex_1() + .min_w_0() + .truncate() + .child(format!("{marker} {}", cell.text)), + ) + .into_any_element() +} - fn diff_untracked_section(&self, snap: &DiffSnapshot, cx: &Context) -> AnyElement { - let total = snap.untracked_count(); - let untracked = &snap.untracked[..snap.untracked.len().min(MAX_RENDERED_FILES)]; - let header_corners = rounding::stack_corners( - 0, - if total == 0 { 1 } else { 2 }, - rounding::CARD_RADIUS, - rounding::HAIRLINE, - ); - let mut section = v_flex() - .w_full() - .border_1() - .border_color(cx.theme().border) - .rounded(rounding::CARD_RADIUS) - .overflow_hidden() - .child( - div() - .w_full() - .px_2p5() - .py_1p5() - .rounded_corners(header_corners) - .bg(cx.theme().secondary) - .text_xs() - .text_color(cx.theme().muted_foreground) - .child(t_plural(L10nKey::DiffUntrackedHeader, total, &[])), - ); - for (i, path) in untracked.iter().enumerate() { - // An untracked file has no patch in the snapshot, so it cannot be - // expanded in place the way the cards above it are — its contents - // are read one file at a time and shown on their own. The row - // asks for that read, which until now only the Source Control - // panel could: in the overlay these rows were the only files in a - // list of files that did nothing when clicked. - let for_focus = path.clone(); - section = section.child( - h_flex() - .id(("diff-untracked", i)) - .w_full() - .items_center() - .gap_2() - .px_2p5() - .py_1() - .text_xs() - .font_family(self.font_family.clone()) - .cursor_pointer() - .hover(|s| s.bg(cx.theme().secondary)) - .on_click(cx.listener(move |this, _, window, cx| { - let Some((host, cwd, source)) = this - .tabs - .get(this.active) - .and_then(|t| t.diff_overlay.as_ref()) - .map(|o| (o.host_id, o.cwd.clone(), o.source.clone())) - else { - return; - }; - this.open_diff_overlay( - host, - cwd, - source, - Some(for_focus.clone()), - window, - cx, - ); - })) - .child( - div() - .flex_shrink_0() - .font_weight(FontWeight::BOLD) - .text_color(status_color(DecoStatus::Untracked, cx)) - .child(status_glyph(DecoStatus::Untracked)), - ) - .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(t_plural(L10nKey::DiffMoreUntracked, rest, &[])), - ); - } - section.into_any_element() - } +/// One line of the unified view. +/// +/// Every measurement it shares with [`diff_split_cell`] is shared on purpose — +/// the same 19px row, the same `text_xs` in the same family, and above all the +/// same `0.12` wash behind an addition and a removal. The two views are one +/// diff seen twice; a different green would read as a different thing. +/// +/// What differs is forced by the shape. The line numbers get 34px a side +/// rather than 42 (there are two gutters here in front of one column of text, +/// not one in front of each), and the `+`/`−` gets a column of its own rather +/// than riding in the text: with three kinds of line stacked in one column, an +/// inlined marker would leave the context lines' code starting two characters +/// left of everything else. +fn diff_unified_row(row: &UnifiedRow, font: &SharedString, cx: &gpui::App) -> impl IntoElement { + let (marker_color, tint) = match row.kind { + LineKind::Added => (cx.theme().success, Some(cx.theme().success.opacity(0.12))), + LineKind::Removed => (cx.theme().danger, Some(cx.theme().danger.opacity(0.12))), + LineKind::Context => (cx.theme().muted_foreground, None), + }; + let gutter = |no: Option| { + h_flex() + .flex_shrink_0() + .w(px(34.)) + .justify_end() + .pr_1p5() + .text_color(cx.theme().muted_foreground.opacity(0.7)) + .child(no.map(|n| n.to_string()).unwrap_or_default()) + }; + h_flex() + .w_full() + .h(DIFF_LINE_H) + .items_center() + .text_xs() + .font_family(font.clone()) + .when_some(tint, |line, bg| line.bg(bg)) + .child(gutter(row.old)) + .child(gutter(row.new)) + // The split view's centre rule, in the one place it still means the + // same thing: everything left of it is a number, everything right of + // it is the file. + .child(div().flex_shrink_0().w(px(1.)).h_full().bg(hunk_rule(cx))) + .child( + div() + .flex_shrink_0() + .w(px(12.)) + .text_center() + .text_color(marker_color) + .child(unified_marker(row.kind)), + ) + .child(div().flex_1().min_w_0().truncate().child(row.text.clone())) } /// Which layout the overlay draws. One setting for the window, not one per @@ -1480,13 +1580,6 @@ fn empty_snapshot(snap: &DiffSnapshot) -> bool { snap.files.is_empty() && snap.untracked.is_empty() } -fn file_expanded(file: &FileDiff, expanded: &HashMap, collapse_all: bool) -> bool { - if let Some(&want) = expanded.get(&file.path) { - return want; - } - !collapse_all && file.added + file.removed <= AUTO_COLLAPSE_LINES -} - fn oversized_summary(snap: &DiffSnapshot, stats: &DiffStats) -> String { let mut parts = vec![t_plural(L10nKey::DiffChangedFiles, snap.files.len(), &[])]; let (added, removed) = stats.totals; @@ -1544,7 +1637,9 @@ fn probe_key( #[cfg(test)] mod tests { use super::*; - use crate::terminal::git_diff::{DiffLine, LineKind}; + use crate::terminal::git_diff::{AUTO_COLLAPSE_LINES, DiffLine, LineKind, MAX_RENDERED_FILES}; + use crate::ui::diff_list::{build_rows, file_expanded}; + use crate::ui::diff_rows::split_hunk; use crate::ui::i18n::set_locale; #[test] @@ -2000,6 +2095,109 @@ mod tests { ); } + /// `ListState::reset` would drop the scroll position, so opening or + /// closing one file in a long tree would throw the reader back to the top. + /// Only the rows that actually changed are spliced. + #[test] + fn only_the_rows_that_changed_are_spliced() { + let snap = DiffSnapshot { + files: vec![ + small_file("a.rs", 2), + small_file("b.rs", 2), + small_file("c.rs", 2), + ], + ..Default::default() + }; + let shut = |path: &str| -> HashMap { + [(path.to_string(), false)].into_iter().collect() + }; + let open = build_rows(&snap, &HashMap::new(), None, DiffViewMode::Unified, false); + let middle_shut = build_rows(&snap, &shut("b.rs"), None, DiffViewMode::Unified, false); + + let (replaced, with) = spliced_range(&open, &middle_shut); + assert_eq!( + (replaced.start, with), + (5, 1), + "the first card and the gap after it are untouched" + ); + assert_eq!( + open.len() - replaced.end, + middle_shut.len() - (replaced.start + with), + "and so is everything below the file that closed" + ); + + let (replaced, with) = spliced_range(&open, &open); + assert_eq!( + (replaced.start, replaced.end, with), + (open.len(), open.len(), 0) + ); + } + + #[test] + fn a_list_that_was_empty_or_becomes_empty_splices_in_one_piece() { + let snap = DiffSnapshot { + files: vec![small_file("a.rs", 2)], + ..Default::default() + }; + let rows = build_rows(&snap, &HashMap::new(), None, DiffViewMode::Unified, false); + + let (replaced, with) = spliced_range(&[], &rows); + assert_eq!((replaced.start, replaced.end, with), (0, 0, rows.len())); + + let (replaced, with) = spliced_range(&rows, &[]); + assert_eq!((replaced.start, replaced.end, with), (0, rows.len(), 0)); + } + + /// A probe that found nothing new still lands a fresh `Arc` over an equal + /// snapshot. The rows are rightly kept — and the key has to come away + /// holding the `Arc` that landed, or every frame from then on proves the + /// two equal the long way: a walk of every line of the patch, per wheel + /// event. + #[test] + fn an_equal_snapshot_leaves_the_key_pointing_at_the_one_that_landed() { + let held = Arc::new(DiffSnapshot { + files: vec![small_file("a.rs", 2)], + ..Default::default() + }); + let landed = Arc::new(DiffSnapshot { + files: vec![small_file("a.rs", 2)], + ..Default::default() + }); + assert!( + !Arc::ptr_eq(&held, &landed), + "two separate Arcs over equal contents" + ); + + let expanded = HashMap::new(); + let mut key = RowsFrom { + snap: &held, + preview: None, + mode: DiffViewMode::Unified, + focused: None, + oversized: false, + expanded: &expanded, + } + .to_key(); + let landed_from = RowsFrom { + snap: &landed, + preview: None, + mode: DiffViewMode::Unified, + focused: None, + oversized: false, + expanded: &expanded, + }; + + assert!( + key.describes(&landed_from), + "nothing about the rows changed" + ); + key.retarget(&landed_from); + assert!( + Arc::ptr_eq(&key.snap, &landed), + "so the next frame settles it by pointer rather than by contents" + ); + } + #[test] fn a_focused_untracked_file_asks_for_a_preview_not_the_list() { let snap = DiffSnapshot { @@ -2490,6 +2688,128 @@ mod render_idle_gpui_tests { assert!(out.status.success(), "git {args:?} failed"); } + /// The whole point of the flattened row list: a patch of any size costs + /// the rows on screen, not the rows in the patch. + /// + /// Before this, the overlay built a card per file and an element per line + /// on every frame, and gpui notifies the view on every scroll wheel event + /// — so a few hundred lines of diff rebuilt tens of thousands of elements + /// tens of times a second, and the window visibly stalled. + #[gpui::test] + fn a_long_patch_builds_only_the_rows_on_screen(cx: &mut TestAppContext) { + const LINES: usize = 800; + + let root = std::env::temp_dir().join(format!("tty7-diff-rows-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).unwrap(); + let root = std::fs::canonicalize(&root).unwrap(); + git(&root, &["init", "--quiet"]); + let before: String = (0..LINES).map(|i| format!("line {i}\n")).collect(); + std::fs::write(root.join("long.txt"), &before).unwrap(); + git(&root, &["add", "long.txt"]); + git( + &root, + &[ + "-c", + "user.email=t@x", + "-c", + "user.name=t", + "commit", + "-qm", + "one", + ], + ); + // Every line rewritten, so the patch is a removal and an addition per + // line rather than a handful of hunks in a sea of context. + let after: String = (0..LINES).map(|i| format!("edited {i}\n")).collect(); + std::fs::write(root.join("long.txt"), &after).unwrap(); + + let (app, mut vcx, _pane) = test_window::harness_with_tabs(cx, 1); + let open = root.clone(); + app.update_in(&mut vcx, |app, window, cx| { + app.open_diff_overlay(HostId::LOCAL, open, DiffSource::Head, None, window, cx); + }); + + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(30); + loop { + vcx.background_executor.run_until_parked(); + let ready = app.update_in(&mut vcx, |app, _, _| { + app.tabs[app.active] + .diff_overlay + .as_ref() + .is_some_and(|o| matches!(o.load, DiffLoad::Ready(_)) && !o.loading) + }); + if ready { + break; + } + assert!( + std::time::Instant::now() < deadline, + "the overlay never landed a snapshot" + ); + std::thread::sleep(std::time::Duration::from_millis(20)); + } + // 1600 changed lines is well past the auto-collapse threshold, so the + // card starts shut. Open it: a reader opening a large file is exactly + // the frame this is about. + app.update_in(&mut vcx, |app, _, cx| { + let active = app.active; + app.tabs[active] + .diff_overlay + .as_mut() + .expect("the overlay is open") + .expanded + .insert("long.txt".to_string(), true); + cx.notify(); + }); + vcx.background_executor.run_until_parked(); + + let rows = app.update_in(&mut vcx, |app, _, _| { + app.tabs[app.active] + .diff_overlay + .as_ref() + .map(|o| o.rows.len()) + .unwrap_or(0) + }); + assert!( + rows > LINES, + "the patch flattens to a row per changed line ({rows} rows)" + ); + + row_probe::take(); + app.update_in(&mut vcx, |_, _, cx| cx.notify()); + vcx.background_executor.run_until_parked(); + let built = row_probe::take(); + assert!( + built > 0, + "the list drew something — a probe that counts nothing proves nothing" + ); + assert!( + built < rows as u64 / 4, + "a frame built {built} of {rows} rows: the list is not virtualised" + ); + + // The other half of virtualising: a list counts the rows it has not + // laid out at zero unless it is given an estimate, and the scrollbar + // reads that count as the length of the document. Without the size + // hint the bar reaches 248px into this patch — its thumb fills the + // track, and dragging it to the bottom lands a screen down. + let reach = app.update_in(&mut vcx, |app, _, _| { + app.tabs[app.active] + .diff_overlay + .as_ref() + .expect("the overlay is open") + .list + .max_offset_for_scrollbar() + .y + }); + assert!( + reach > DIFF_LINE_H * (rows as f32 * 0.75), + "the scrollbar reaches {reach} into a patch of {rows} rows" + ); + + let _ = std::fs::remove_dir_all(&root); + } + #[gpui::test] fn an_overlay_over_a_stale_branch_reaches_render_idle(cx: &mut TestAppContext) { let root = std::env::temp_dir().join(format!("tty7-diff-idle-{}", std::process::id())); diff --git a/src/ui/diff_rows.rs b/src/ui/diff_rows.rs index 4aa16876..b5316d50 100644 --- a/src/ui/diff_rows.rs +++ b/src/ui/diff_rows.rs @@ -23,12 +23,14 @@ pub(crate) enum Side { New, } +#[derive(PartialEq, Eq)] pub(crate) struct SplitCell { pub(crate) no: Option, pub(crate) text: String, pub(crate) changed: bool, } +#[derive(PartialEq, Eq)] pub(crate) struct SplitRow { pub(crate) left: Option, pub(crate) right: Option, @@ -85,6 +87,7 @@ pub(crate) fn split_hunk(lines: &[DiffLine]) -> Vec { rows } +#[derive(PartialEq, Eq)] pub(crate) struct UnifiedRow { pub(crate) old: Option, pub(crate) new: Option, diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 31f360c8..149da06b 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -1,6 +1,7 @@ pub mod app; pub mod assets; pub mod code_editor; +pub mod diff_list; pub mod diff_overlay; pub mod diff_rows; pub mod document_column; diff --git a/src/ui/rounding.rs b/src/ui/rounding.rs index 86d0ad44..87b3dd7f 100644 --- a/src/ui/rounding.rs +++ b/src/ui/rounding.rs @@ -40,24 +40,6 @@ pub(crate) fn segment_corners( } } -pub(crate) fn stack_corners( - i: usize, - count: usize, - outer: Pixels, - border: Pixels, -) -> Corners { - let r = inner_radius(outer, border); - let zero = px(0.); - let first = i < count && i == 0; - let last = i < count && i + 1 == count; - Corners { - top_left: if first { r } else { zero }, - top_right: if first { r } else { zero }, - bottom_left: if last { r } else { zero }, - bottom_right: if last { r } else { zero }, - } -} - #[cfg(test)] mod tests { use super::*; @@ -109,20 +91,4 @@ mod tests { Corners::all(px(0.)) ); } - - #[test] - fn a_stack_caps_its_first_and_last_band() { - let r = inner_radius(CARD_RADIUS, HAIRLINE); - let zero = px(0.); - - let top = stack_corners(0, 2, CARD_RADIUS, HAIRLINE); - assert_eq!((top.top_left, top.top_right), (r, r)); - assert_eq!((top.bottom_left, top.bottom_right), (zero, zero)); - - let bottom = stack_corners(1, 2, CARD_RADIUS, HAIRLINE); - assert_eq!((bottom.bottom_left, bottom.bottom_right), (r, r)); - assert_eq!((bottom.top_left, bottom.top_right), (zero, zero)); - - assert_eq!(stack_corners(0, 1, CARD_RADIUS, HAIRLINE), Corners::all(r)); - } } diff --git a/src/ui/scrollbar.rs b/src/ui/scrollbar.rs index 3e6efea4..5a8102f0 100644 --- a/src/ui/scrollbar.rs +++ b/src/ui/scrollbar.rs @@ -1,5 +1,5 @@ use gpui::{AnyElement, ElementId, Pixels, ScrollHandle, div, prelude::*, px}; -use gpui_component::scroll::Scrollbar; +use gpui_component::scroll::{Scrollbar, ScrollbarHandle}; use gpui_component::v_flex; /// Overlays the shared vertical scrollbar on a scroll area. @@ -9,10 +9,10 @@ use gpui_component::v_flex; /// dropping it straight into whatever is around it. Without that the wrapper /// sizes to its content, the `size_full` scroll area inside grows with it, and /// the pane stops scrolling because nothing overflows any more. -pub(crate) fn with_vertical_scrollbar( +pub(crate) fn with_vertical_scrollbar( id: impl Into, scroll_area: impl IntoElement, - handle: &ScrollHandle, + handle: &H, ) -> AnyElement { with_inset_vertical_scrollbar(id, scroll_area, handle, px(0.)) } @@ -23,10 +23,10 @@ pub(crate) fn with_vertical_scrollbar( /// the border is already the edge. A scroll area that *is* the window has no /// such edge, and a bar that runs to the last pixel lands on the rounded /// corner and reads as if it had been clipped. -pub(crate) fn with_inset_vertical_scrollbar( +pub(crate) fn with_inset_vertical_scrollbar( id: impl Into, scroll_area: impl IntoElement, - handle: &ScrollHandle, + handle: &H, inset_y: Pixels, ) -> AnyElement { v_flex() From cb710c4d7984aa9d93b9a4a57c565361d972707e Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:23:28 +0800 Subject: [PATCH 06/10] deps: bump async_zip from 0.0.18 to 0.0.19 (#764) Also bump the version requirement in Cargo.toml, which dependabot left at 0.0.18 and which made every --locked job fail. --- Cargo.lock | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++-- Cargo.toml | 2 +- 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0f31ea1..5f3168d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -376,6 +376,12 @@ dependencies = [ "password-hash", ] +[[package]] +name = "array-init" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3d62b7694a562cdf5a74227903507c56ab2cc8bdd1f781ed5cb4cf9c9f810bfc" + [[package]] name = "arraydeque" version = "0.5.1" @@ -681,6 +687,20 @@ dependencies = [ "thiserror 2.0.20", ] +[[package]] +name = "async_zip" +version = "0.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb7f5f40e1eb30949a266fc900d37fd3267c7baf50c3705ac09c5d8ced5def63" +dependencies = [ + "async-compression", + "binrw", + "crc32fast", + "futures-lite 2.6.1", + "pin-project", + "thiserror 2.0.20", +] + [[package]] name = "atk" version = "0.18.2" @@ -902,6 +922,30 @@ dependencies = [ "syn 2.0.119", ] +[[package]] +name = "binrw" +version = "0.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6ad120d555272286c1017d25165ab8bd74806f13fc85b258484ec7e4ce75458f" +dependencies = [ + "array-init", + "binrw_derive", + "bytemuck", +] + +[[package]] +name = "binrw_derive" +version = "0.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6df92e0e9baae4dc82c7bad7715ca40c0a5c71539057bf2ea04a5c29c980410b" +dependencies = [ + "either", + "owo-colors", + "proc-macro2", + "quote", + "syn 2.0.119", +] + [[package]] name = "bit-set" version = "0.8.0" @@ -6006,6 +6050,12 @@ dependencies = [ "pin-project-lite", ] +[[package]] +name = "owo-colors" +version = "4.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "13c45bb4a6ae1280ec0803b1ef9d3455eb50f01efbbe1447ab020f1d54fba9d8" + [[package]] name = "p256" version = "0.14.0-rc.15" @@ -9779,7 +9829,7 @@ version = "26.9.1" dependencies = [ "alacritty_terminal", "anyhow", - "async_zip", + "async_zip 0.0.19", "core-foundation 0.10.0", "gpui", "gpui-component", @@ -10112,7 +10162,7 @@ source = "git+https://github.com/l0ng-ai/zed?branch=tty7#ece710e365575bc08656c4a dependencies = [ "anyhow", "async-fs", - "async_zip", + "async_zip 0.0.18", "collections", "command-fds", "dirs", diff --git a/Cargo.toml b/Cargo.toml index 48f90ceb..84a83f5b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -172,7 +172,7 @@ windows-sys = { version = "0.61", features = [ # The standalone updater extracts only Windows release ZIPs. Reuse the async # reader already present in Cargo.lock and enable only the Deflate codec emitted # by PowerShell's Compress-Archive. -async_zip = { version = "0.0.18", default-features = false, features = ["deflate"], optional = true } +async_zip = { version = "0.0.19", default-features = false, features = ["deflate"], optional = true } # COM for toast branding (`core::aumid`): IShellLinkW + IPropertyStore stamp # System.AppUserModel.ID onto the Start Menu shortcut, which Windows requires From d8a0245e7560ba7a484e568c76575af1668c9f7a Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:26:39 +0800 Subject: [PATCH 07/10] fix(terminal): name every ligature feature off, not just calt (#788) --- docs/customization/fonts.mdx | 13 ++++- src/terminal/element.rs | 94 +++++++++++++++++++++++++++++++----- 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/docs/customization/fonts.mdx b/docs/customization/fonts.mdx index 6be3ea61..3e1b59f5 100644 --- a/docs/customization/fonts.mdx +++ b/docs/customization/fonts.mdx @@ -14,7 +14,7 @@ description: "The bundled default, fallback chains, ligatures, and why CJK needs | **Interface font family** | system UI font | The face for everything outside the grid | | **Line height** | 1.4 | A multiple of the font size | | **Bold font** / **Italic font** | — | Distinct faces, when you want them | -| **Font ligatures** | off | Contextual alternates stay off unless you ask | +| **Font ligatures** | off | `calt`, `liga` and `clig` all stay off unless you ask | ## Hack is bundled @@ -60,6 +60,17 @@ A tag must be four alphanumeric characters; `true`/`false` map to `1`/`0`. Anything malformed is skipped with a log line rather than failing the whole config. +Writing *any* tag hands the whole feature set to you: the terminal only names +`calt: 0`, `liga: 0` and `clig: 0` itself while `font_features` is unset. So +`{"font_features": {"zero": 1}}` turns slashed zeroes on **and** ligatures back +on. Name them explicitly if you want both: + +```json +{ + "font_features": { "zero": 1, "calt": 0, "liga": 0, "clig": 0 } +} +``` + ## CJK and the two-column grid diff --git a/src/terminal/element.rs b/src/terminal/element.rs index 6eee0416..5dc1da49 100644 --- a/src/terminal/element.rs +++ b/src/terminal/element.rs @@ -149,11 +149,39 @@ fn build_font(base: &Font, bold: bool, italic: bool) -> Font { FontStyle::Normal }; if f.features.tag_value_list().is_empty() { - f.features = gpui::FontFeatures::disable_ligatures(); + f.features = ligatures_off(); } f } +/// The feature set that actually turns ligatures off in a terminal grid. +/// +/// gpui's own `FontFeatures::disable_ligatures()` emits `calt: 0` and nothing +/// else. `calt` is only the contextual-alternate feature a programming face +/// drives `=>` and `!=` from; the `fi`/`ffl`/`ffi` ligatures live in `liga` and +/// `clig`, which that call never mentions — so they were left at the shaper's +/// default, and CoreText, DirectWrite and rustybuzz all default them on. A +/// terminal cannot have them: a ligature is one glyph where the grid budgeted a +/// cell per character, so the row drifts. +/// +/// Naming all three is therefore the whole fix, and it is not platform +/// specific. Windows is the sharpest case only because gpui's DirectWrite +/// backend always attaches an `IDWriteTypography` to the run: an empty feature +/// list leaves that object empty, which suppresses DirectWrite's own defaults, +/// while any non-empty list has `liga: 1`/`clig: 1` appended to it +/// (`gpui_windows/src/direct_write.rs`, `apply_font_features`). Asking for +/// `calt: 0` there bought ligatures that asking for nothing would not have. +/// +/// `rlig` is deliberately left alone: it carries the required ligatures a +/// script cannot be written without. +fn ligatures_off() -> gpui::FontFeatures { + gpui::FontFeatures(std::sync::Arc::new(vec![ + ("calt".to_string(), 0), + ("liga".to_string(), 0), + ("clig".to_string(), 0), + ])) +} + fn snapshot_cell( cell: &Cell, point: AlacPoint, @@ -2717,20 +2745,64 @@ mod tests { assert_eq!(seg_budget(false, 1, false, cell), px(12.5)); } + /// The emitted feature set is the whole contract with the shaper, so pin it + /// tag for tag rather than asking after one tag at a time. + /// + /// `calt: 0` alone is not "ligatures off" on any platform: it never asks + /// for `liga`/`clig`, which every shaper defaults on. Shaping "office + /// waffle fluffier" in Calibri through the real DirectWrite shaper gives 15 + /// glyphs for 22 characters with `calt: 0`, and 22 once all three are named + /// zero. #[test] - fn build_font_disables_ligatures_unless_features_are_configured() { - let font = build_font(&gpui::font("Test"), false, false); - assert_eq!(font.features.is_calt_enabled(), Some(false)); - - let mut configured = gpui::font("Test"); - configured.features = serde_json::from_str(r#"{"calt":true,"liga":1}"#).unwrap(); - let font = build_font(&configured, false, false); - assert_eq!(font.features.is_calt_enabled(), Some(true)); - assert!( + fn build_font_emits_the_pinned_feature_set_for_each_ligature_setting() { + fn tags(font: &Font) -> Vec<(&str, u32)> { font.features .tag_value_list() .iter() - .any(|(tag, value)| tag == "liga" && *value == 1) + .map(|(tag, value)| (tag.as_str(), *value)) + .collect() + } + + // Default config, and the settings toggle switched off: both leave + // `font_features` unset, so the terminal names its own. + let font = build_font(&gpui::font("Test"), false, false); + assert_eq!( + tags(&font), + vec![("calt", 0), ("liga", 0), ("clig", 0)], + "an unconfigured face must name every ligature feature off", + ); + assert_eq!(font.features.is_calt_enabled(), Some(false)); + + // Every face the grid paints with gets the same set, not just the plain one. + for (bold, italic) in [(true, false), (false, true), (true, true)] { + assert_eq!( + tags(&build_font(&gpui::font("Test"), bold, italic)), + vec![("calt", 0), ("liga", 0), ("clig", 0)], + "bold={bold} italic={italic}", + ); + } + + // Explicitly on: what Settings → Appearance → Font ligatures writes. + let mut configured = gpui::font("Test"); + configured.features = crate::core::config::gpui_font_features( + &serde_json::from_str(r#"{"calt":true,"liga":1}"#).unwrap(), + ); + let font = build_font(&configured, false, false); + assert_eq!( + tags(&font), + vec![("calt", 1), ("liga", 1)], + "an explicit request for ligatures must survive untouched", + ); + assert_eq!(font.features.is_calt_enabled(), Some(true)); + + // Explicitly off in `config.json`: also passed through unchanged. + let mut configured = gpui::font("Test"); + configured.features = crate::core::config::gpui_font_features( + &serde_json::from_str(r#"{"calt":0,"liga":0,"clig":0}"#).unwrap(), + ); + assert_eq!( + tags(&build_font(&configured, false, false)), + vec![("calt", 0), ("liga", 0), ("clig", 0)], ); } From 42424c7737608d624fc3d9aa6b82c3f5431b916a Mon Sep 17 00:00:00 2001 From: bytehello Date: Mon, 7 Sep 2026 22:29:08 +0800 Subject: [PATCH 08/10] feat(ui): add a bindable Close Window action (#773) (#778) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 26.9.0's tray-retire model made closing the last window the "keep the daemon, drop the UI process weight" gesture, but that path was reachable only from the OS red close button: no action, no palette entry, nothing to bind. `⌘W` closes a pane or tab, `⌘H` hides the window but keeps it in memory, and `⌘Q` stops the server. There was nothing that closed the window and left the shells running. `CloseWindow` is that action, with no default key — the slot is left free. The logic that decided what a window close means (detach the workspace, and on the last window retire to the tray if an icon is actually up, otherwise quit) moves out of `on_window_should_close` into `prepare_window_close`, so the button and the action share one decision instead of two that can drift. Notably not routed through `close_window_for`: that one recycles the last window onto a fresh workspace, which is what deleting a workspace wants and not what closing a window wants. It reaches the command palette, the Keybindings UI, and the reference table of actions with no default key. In the palette it sits beside Quit, because that pair is the whole point of the action: both end the window in front of you and only one takes your shells with it. Their subtitles now say which — including Quit's, which had been promising "shells keep running" while calling `daemon::spawn::stop()`. Co-authored-by: bytehello https://claude.ai/code/session_01LKMZVh6mUBxXAn6v7P6JC6 --- docs/reference/keyboard-shortcuts.mdx | 2 +- src/core/actions.rs | 1 + src/ui/app.rs | 137 +++++++++++++++++++++----- src/ui/i18n/en.rs | 4 +- src/ui/i18n/ja.rs | 4 +- src/ui/i18n/mod.rs | 2 + src/ui/i18n/zh.rs | 4 +- src/ui/keymap.rs | 7 ++ src/ui/palette.rs | 8 ++ 9 files changed, 143 insertions(+), 26 deletions(-) diff --git a/docs/reference/keyboard-shortcuts.mdx b/docs/reference/keyboard-shortcuts.mdx index e82a1077..81314b1a 100644 --- a/docs/reference/keyboard-shortcuts.mdx +++ b/docs/reference/keyboard-shortcuts.mdx @@ -112,7 +112,7 @@ Keybindings**: | Git | `ScmStageAll` · `ScmUnstageAll` · `ScmDiscardAll` · `ScmCommitAmend` · `ScmRefresh` · `ScmSync` · `ScmPush` · `ScmPull` · `ScmFetch` · `ScmCheckoutBranch` · `ScmCreateBranch` · `ScmToggleGraph` · `ToggleDiffViewMode` | | Panels | `ShowRightPanelInfo` · `ShowRightPanelChanges` · `ShowRightPanelFiles` | | SSH | `ToggleSftp` · `ShowSshForwards` · `OpenSshProfiles` | -| Application | `About` · `CheckForUpdates` · `OpenDocumentation` · `OpenDiscord` · `ReportIssue` · `ShowAll` · `ZoomWindow` | +| Application | `CloseWindow` · `About` · `CheckForUpdates` · `OpenDocumentation` · `OpenDiscord` · `ReportIssue` · `ShowAll` · `ZoomWindow` | ## Rebinding syntax diff --git a/src/core/actions.rs b/src/core/actions.rs index 09604411..30788b42 100644 --- a/src/core/actions.rs +++ b/src/core/actions.rs @@ -18,6 +18,7 @@ actions!( SelectWorkspace7, SelectWorkspace8, SelectWorkspace9, + CloseWindow, CloseActiveTab, RenameTab, NewWorktreeTab, diff --git a/src/ui/app.rs b/src/ui/app.rs index dea55890..e3b74ca3 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -1395,29 +1395,8 @@ impl Tty7App { let weak_app = cx.weak_entity(); window.on_window_should_close(cx, move |_window, cx| { - let last_window = crate::ui::windows::WindowRegistry::count(cx) <= 1; if let Some(app) = weak_app.upgrade() { - app.update(cx, |app, cx| app.detach_workspace(cx)); - } - if last_window { - // With a tray icon, closing the last window retires to the - // tray: the daemon stays reachable (show / quit-and-stop) - // instead of being orphaned behind a dead icon. Without one - // the app quits — the only way it stays visible at all. - // - // The icon has to actually be up, not merely asked for: the - // backend can fail for the whole run (a Linux session with no - // StatusNotifier host), and retiring into an icon that never - // appeared leaves a process with no window and no tray — no - // way back in, and the daemon still held. - let retire_to_tray = - cx.global::().show_tray_icon && crate::ui::tray::icon_is_up(); - if !retire_to_tray { - cx.spawn(async move |cx| { - let _ = cx.update(|cx| cx.quit()); - }) - .detach(); - } + app.update(cx, |app, cx| app.prepare_window_close(cx)); } true }); @@ -1468,6 +1447,36 @@ impl Tty7App { crate::ui::windows::refresh_menu(cx); } + fn prepare_window_close(&self, cx: &mut App) { + let last_window = crate::ui::windows::WindowRegistry::count(cx) <= 1; + self.detach_workspace(cx); + if last_window { + // With a tray icon, closing the last window retires to the + // tray: the daemon stays reachable (show / quit-and-stop) + // instead of being orphaned behind a dead icon. Without one + // the app quits — the only way it stays visible at all. + // + // The icon has to actually be up, not merely asked for: the + // backend can fail for the whole run (a Linux session with no + // StatusNotifier host), and retiring into an icon that never + // appeared leaves a process with no window and no tray — no + // way back in, and the daemon still held. + let retire_to_tray = + cx.global::().show_tray_icon && crate::ui::tray::icon_is_up(); + if !retire_to_tray { + cx.spawn(async move |cx| { + let _ = cx.update(|cx| cx.quit()); + }) + .detach(); + } + } + } + + fn close_window(&self, window: &mut Window, cx: &mut App) { + self.prepare_window_close(cx); + window.remove_window(); + } + pub(crate) fn teardown_workspace_forwards(&self, cx: &gpui::App) { let Some(route) = self .tabs @@ -4895,6 +4904,7 @@ impl Tty7App { OpenWorkspacePicker => self.open_switcher(window, cx), StopWorkspace => self.stop_workspace(self.workspace, window, cx), DeleteWorkspace => self.delete_workspace(self.workspace, window, cx), + CloseWindow => self.close_window(window, cx), SplitRight => self.split(Axis::Horizontal, window, cx), SplitDown => self.split(Axis::Vertical, window, cx), ClosePane => self.close_pane(window, cx), @@ -7391,6 +7401,9 @@ impl Render for Tty7App { .on_action(cx.listener(|this, _: &NewWorkspace, window, cx| { this.open_workspace_form(window, cx); })) + .on_action( + cx.listener(|this, _: &CloseWindow, window, cx| this.close_window(window, cx)), + ) .on_action(cx.listener(|this, _: &CloseActiveTab, window, cx| { if !this.editor_close_active_if_focused(window, cx) { this.close_pane(window, cx) @@ -10244,3 +10257,83 @@ mod managed_forward_gpui_tests { }); } } + +#[cfg(test)] +mod close_window_action_tests { + use crate::core::actions::CloseWindow; + use crate::core::config::Config; + use crate::core::session::Session; + use crate::ui::app::Tty7App; + use crate::ui::windows::WindowRegistry; + use gpui::{AppContext as _, TestAppContext, VisualTestContext}; + + /// `CloseWindow` has to close the window, and close it the way the red + /// button does. + /// + /// The action is otherwise all table entries — the `actions!` row, the + /// keymap slot, the palette command, the Keybindings label — and every one + /// of those can be in place while the action reaches nothing at all. So + /// this drives the real dispatch path and then asks two separate + /// questions: the window is gone from gpui, *and* it left the + /// `WindowRegistry` on the way out. The second is what makes it the same + /// close as the native one — `detach_workspace` is where the session is + /// saved and the workspace is retired, and a `remove_window` that skipped + /// it would still pass the first assertion while quietly dropping a + /// window's tabs on the floor. + #[gpui::test] + fn dispatching_close_window_takes_the_window_down_with_its_registration( + cx: &mut TestAppContext, + ) { + crate::core::config::pin_test_config_dir(); + cx.executor().allow_parking(); + cx.update(|cx| { + gpui_component::init(cx); + cx.set_global(Config::default()); + crate::ui::keymap::init(cx); + WindowRegistry::init(cx); + }); + let window = cx.add_window(|window, cx| { + let app = + cx.new(|cx| Tty7App::with_session(None, Some(Session::default()), window, cx)); + gpui_component::Root::new(app, window, cx) + }); + let app = window + .update(cx, |root, _, _| { + root.view() + .clone() + .downcast::() + .ok() + .expect("window root wraps a Tty7App") + }) + .unwrap(); + // Registered the way an opened window registers itself; the registry + // is where the close has to show up, so an unregistered window would + // make the assertion below pass for the wrong reason. + let handle = window.into(); + let weak = app.downgrade(); + app.update(cx, |app, cx| { + WindowRegistry::register(cx, app.workspace, handle, weak); + }); + + let mut vcx = VisualTestContext::from_window(handle, cx); + vcx.background_executor.run_until_parked(); + assert_eq!( + vcx.update(|_, cx| WindowRegistry::count(cx)), + 1, + "the harness starts with exactly the one window" + ); + + vcx.dispatch_action(CloseWindow); + drop(vcx); + + assert!( + cx.update(|cx| cx.windows().is_empty()), + "CloseWindow has to reach `remove_window`; the window is still open" + ); + assert!( + cx.update(|cx| WindowRegistry::open_windows(cx).is_empty()), + "the close has to run the same cleanup the red button runs, \ + which is what takes the window out of the registry" + ); + } +} diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index eb62d36f..81949a5f 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -1430,6 +1430,8 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::CmdForkSessionSubtitle => "branch this agent session into a new tab", L10nKey::CmdMarkTabAsUnread => "Mark Tab as Unread", L10nKey::CmdClosePaneTab => "Close Pane / Tab", + L10nKey::CmdCloseWindow => "Close Window", + L10nKey::CmdCloseWindowSubtitle => "shells keep running", L10nKey::CmdCloseOtherTabs => "Close Other Tabs", L10nKey::CmdCloseTabsToTheRight => "Close Tabs to the Right", L10nKey::CmdReopenClosedTab => "Reopen Closed Tab", @@ -1506,7 +1508,7 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::CmdRestartServer => "Restart Server…", L10nKey::CmdRestartServerSubtitle => "ends every running shell; layout is kept", L10nKey::CmdQuitTty7 => "Quit tty7", - L10nKey::CmdQuitTty7Subtitle => "shells keep running", + L10nKey::CmdQuitTty7Subtitle => "stops the server; every running shell ends", L10nKey::CmdQuickConnect => "Connect to \"{target}\"", L10nKey::CmdQuickConnectSaveProfile => "Save \"{target}\" as profile…", L10nKey::CmdRecent => "Recent", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 367c4d95..16305f28 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -1487,6 +1487,8 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::CmdForkSessionSubtitle => "このエージェントのセッションを新しいタブにフォーク", L10nKey::CmdMarkTabAsUnread => "タブを未読としてマーク", L10nKey::CmdClosePaneTab => "ペイン / タブを閉じる", + L10nKey::CmdCloseWindow => "ウィンドウを閉じる", + L10nKey::CmdCloseWindowSubtitle => "シェルは実行を継続", L10nKey::CmdCloseOtherTabs => "他のタブを閉じる", L10nKey::CmdCloseTabsToTheRight => "右側のタブを閉じる", L10nKey::CmdReopenClosedTab => "閉じたタブをもう一度開く", @@ -1561,7 +1563,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::CmdRestartServer => "サーバーを再起動…", L10nKey::CmdRestartServerSubtitle => "実行中のすべてのシェルを終了し、レイアウトは保持", L10nKey::CmdQuitTty7 => "tty7 を終了", - L10nKey::CmdQuitTty7Subtitle => "シェルは実行を継続", + L10nKey::CmdQuitTty7Subtitle => "サーバーを停止し、実行中のすべてのシェルを終了", L10nKey::CmdQuickConnect => "「{target}」に接続", L10nKey::CmdQuickConnectSaveProfile => "「{target}」をプロファイルとして保存…", L10nKey::CmdRecent => "最近", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 1b54fbca..1219d497 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -1150,6 +1150,8 @@ l10n_keys! { CmdForkSessionSubtitle, CmdMarkTabAsUnread, CmdClosePaneTab, + CmdCloseWindow, + CmdCloseWindowSubtitle, CmdCloseOtherTabs, CmdCloseTabsToTheRight, CmdReopenClosedTab, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index f802207a..6cf65437 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -1348,6 +1348,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::CmdForkSessionSubtitle => "将此 agent 会话 fork 到新标签页", L10nKey::CmdMarkTabAsUnread => "将标签页标记为未读", L10nKey::CmdClosePaneTab => "关闭窗格/标签页", + L10nKey::CmdCloseWindow => "关闭窗口", + L10nKey::CmdCloseWindowSubtitle => "shell 保持运行", L10nKey::CmdCloseOtherTabs => "关闭其他标签页", L10nKey::CmdCloseTabsToTheRight => "关闭右侧标签页", L10nKey::CmdReopenClosedTab => "重新打开已关闭标签页", @@ -1422,7 +1424,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::CmdRestartServer => "重启 server…", L10nKey::CmdRestartServerSubtitle => "结束所有运行中的 shell；保留布局", L10nKey::CmdQuitTty7 => "退出 tty7", - L10nKey::CmdQuitTty7Subtitle => "shell 保持运行", + L10nKey::CmdQuitTty7Subtitle => "停止服务；结束所有运行中的 shell", L10nKey::CmdQuickConnect => "连接到“{target}”", L10nKey::CmdQuickConnectSaveProfile => "将“{target}”保存为主机配置…", L10nKey::CmdRecent => "最近使用", diff --git a/src/ui/keymap.rs b/src/ui/keymap.rs index 67fd2c97..7c175e57 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -265,6 +265,7 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { vec![ ("NewTab", per_platform("secondary-t", "secondary-shift-t")), ("NewWorkspace", "secondary-shift-n"), + ("CloseWindow", ""), ( "CloseActiveTab", per_platform("secondary-w", "secondary-shift-w"), @@ -733,6 +734,10 @@ fn authored_entry(action: &str) -> Option<(CommandGroup, String)> { CommandGroup::Application, t(L10nKey::AppMenuCommandPalette).to_string(), ), + "CloseWindow" => ( + CommandGroup::Application, + t(L10nKey::CmdCloseWindow).to_string(), + ), "OpenSettings" => ( CommandGroup::Application, t(L10nKey::CmdSettings).to_string(), @@ -1036,6 +1041,7 @@ fn make_binding(action: &str, keystroke: &str) -> Option { "DeleteWorkspace" => KeyBinding::new(keystroke, DeleteWorkspace, None), "RenameWorkspace" => KeyBinding::new(keystroke, RenameWorkspace, None), "ToggleSwitcher" => KeyBinding::new(keystroke, ToggleSwitcher, None), + "CloseWindow" => KeyBinding::new(keystroke, CloseWindow, None), "CloseActiveTab" => KeyBinding::new(keystroke, CloseActiveTab, None), "RenameTab" => KeyBinding::new(keystroke, RenameTab, None), "NewWorktreeTab" => KeyBinding::new(keystroke, NewWorktreeTab, None), @@ -1217,6 +1223,7 @@ mod tests { // palette and the docs all say Zoom Pane. assert_eq!(action_entry("ToggleMaximizePane").1, "Zoom Pane"); assert_eq!(action_entry("CloseActiveTab").1, "Close Pane / Tab"); + assert_eq!(action_entry("CloseWindow").1, "Close Window"); assert_eq!(action_entry("ClearScrollback").1, "Clear Scrollback"); assert_eq!(action_entry("TogglePalette").1, "Command Palette…"); assert_eq!(action_entry("ToggleSwitcher").1, "Switch Workspace…"); diff --git a/src/ui/palette.rs b/src/ui/palette.rs index 28272380..fd749760 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -22,6 +22,7 @@ pub enum CommandKind { RenameWorkspace, StopWorkspace, DeleteWorkspace, + CloseWindow, SplitRight, SplitDown, ClosePane, @@ -124,6 +125,7 @@ impl CommandKind { RenameWorkspace => "rename-workspace", StopWorkspace => "stop-workspace", DeleteWorkspace => "delete-workspace", + CloseWindow => "close-window", SplitRight => "split-right", SplitDown => "split-down", ClosePane => "close-pane", @@ -230,6 +232,7 @@ impl CommandKind { RenameWorkspace => "RenameWorkspace", StopWorkspace => "StopWorkspace", DeleteWorkspace => "DeleteWorkspace", + CloseWindow => "CloseWindow", SplitRight => "SplitRight", SplitDown => "SplitDown", ClosePane => "CloseActiveTab", @@ -574,6 +577,11 @@ impl Command { Command::localized(L10nKey::CmdReportIssue, ReportIssue), Command::localized(L10nKey::CmdRestartServer, RestartDaemon) .with_subtitle(t(L10nKey::CmdRestartServerSubtitle)), + // Beside Quit, because the pair is the whole point of the action: + // both end the window you are looking at, and only one of them + // takes your shells with it. Read together the subtitles say which. + Command::localized(L10nKey::CmdCloseWindow, CloseWindow) + .with_subtitle(t(L10nKey::CmdCloseWindowSubtitle)), Command::localized(L10nKey::CmdQuitTty7, Quit) .with_subtitle(t(L10nKey::CmdQuitTty7Subtitle)), ]; From c125803077aafbdb3b4c23c22939e146a175f380 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:31:23 +0800 Subject: [PATCH 09/10] feat(daemon): list a Windows pane's listening ports (#787) --- crates/tty7-core/Cargo.toml | 3 + crates/tty7-core/src/daemon/procinfo.rs | 404 +++++++++++++++++++++++- 2 files changed, 406 insertions(+), 1 deletion(-) diff --git a/crates/tty7-core/Cargo.toml b/crates/tty7-core/Cargo.toml index 9c12b7d5..a016ab0c 100644 --- a/crates/tty7-core/Cargo.toml +++ b/crates/tty7-core/Cargo.toml @@ -163,8 +163,11 @@ getrandom = "0.3" # tree on hangup (ConPTY's `kill` only reaches the shell itself). `Registry` # additionally lets `daemon::windows_env` re-read the two environment hives at # pane-spawn time so a long-lived daemon stops handing out its startup `PATH`. +# `IpHelper` is `GetExtendedTcpTable`, which is how `daemon::procinfo` answers +# the Info tab's Ports section here — the platform has no `lsof` to shell out to. windows-sys = { version = "0.59", features = [ "Win32_Foundation", + "Win32_NetworkManagement_IpHelper", "Win32_Security", "Win32_System_Console", "Win32_System_Diagnostics_ToolHelp", diff --git a/crates/tty7-core/src/daemon/procinfo.rs b/crates/tty7-core/src/daemon/procinfo.rs index 03732ccb..30a8fe85 100644 --- a/crates/tty7-core/src/daemon/procinfo.rs +++ b/crates/tty7-core/src/daemon/procinfo.rs @@ -296,7 +296,225 @@ fn listening_ports(procs: &[ProcEntry]) -> Vec { ports } -#[cfg(not(unix))] +/// Windows has no `lsof`, and the Ports section was simply never drawn there — +/// the daemon answered `QueryProcs` with an empty list no matter what the pane +/// was running, so a `npm run dev` in a Windows pane showed processes and no +/// port to click. +/// +/// `GetExtendedTcpTable` is the same answer without a subprocess: the kernel's +/// own table of listening sockets, each already tagged with the pid that owns +/// it. The table is machine-wide, so the filter against the pane's tree below +/// is the whole difference between this panel and `netstat -ano`. +/// +/// **Cost.** Two calls per poll — one per address family — into a buffer sized +/// for far more listeners than a real machine has; a family only pays for a +/// second call when its table outgrew that. The Info tab re-polls every two +/// seconds while it is open, so this is a fixed handful of microseconds, with +/// no process spawn and nothing allocated per pid. +#[cfg(windows)] +fn listening_ports(procs: &[ProcEntry]) -> Vec { + use std::net::{Ipv4Addr, Ipv6Addr}; + + use windows_sys::Win32::NetworkManagement::IpHelper::{ + MIB_TCP6ROW_OWNER_PID, MIB_TCP6TABLE_OWNER_PID, MIB_TCPROW_OWNER_PID, + MIB_TCPTABLE_OWNER_PID, + }; + + if procs.is_empty() { + return Vec::new(); + } + let by_pid: HashMap = procs.iter().map(|p| (p.pid, p.name.as_str())).collect(); + let mut ports: Vec = Vec::new(); + + let v4 = tcp_table(AF_INET); + // SAFETY: `tcp_table` hands back either an empty buffer or one the kernel + // filled with a `MIB_TCPTABLE_OWNER_PID`; the `Vec` gives it the 4-byte + // alignment every field of that struct wants, and `rows` is clamped to what + // the buffer can actually hold before anything is read out of it. + unsafe { + if let Some((rows, count)) = table_rows::(&v4) + { + for i in 0..count { + let row = &*rows.add(i); + // Filter before spelling the address: the table is the whole + // machine's, and formatting a string for every stranger's + // socket is the one avoidable allocation on this path. + let Some(name) = by_pid.get(&row.dwOwningPid) else { + continue; + }; + let addr = Ipv4Addr::from(row.dwLocalAddr.to_ne_bytes()); + record_listener( + &mut ports, + name, + local_port(row.dwLocalPort), + row.dwOwningPid, + spell_v4(addr), + ); + } + } + } + + let v6 = tcp_table(AF_INET6); + // SAFETY: as above, for the IPv6 shape of the same table. + unsafe { + if let Some((rows, count)) = + table_rows::(&v6) + { + for i in 0..count { + let row = &*rows.add(i); + let Some(name) = by_pid.get(&row.dwOwningPid) else { + continue; + }; + let addr = Ipv6Addr::from(row.ucLocalAddr); + record_listener( + &mut ports, + name, + local_port(row.dwLocalPort), + row.dwOwningPid, + spell_v6(addr), + ); + } + } + } + + ports.sort_by_key(|e| (e.port, e.pid)); + ports +} + +/// The two Winsock address families, named here rather than by switching on +/// `windows-sys`'s `Win32_Networking_WinSock`: the whole socket module is a +/// long compile for two integers the ABI froze decades ago. +#[cfg(windows)] +const AF_INET: u32 = 2; +#[cfg(windows)] +const AF_INET6: u32 = 23; + +/// One `GetExtendedTcpTable` snapshot of the listening sockets in `family`, as +/// the raw buffer the kernel filled, or an empty buffer if it would not answer. +/// +/// Failure is soft, the way an absent `lsof` is soft on unix: the panel shows no +/// ports rather than an error. A machine with IPv6 disabled takes that path for +/// `AF_INET6` alone and still gets its IPv4 ports. +#[cfg(windows)] +fn tcp_table(family: u32) -> Vec { + use windows_sys::Win32::Foundation::{ERROR_INSUFFICIENT_BUFFER, NO_ERROR}; + use windows_sys::Win32::NetworkManagement::IpHelper::{ + GetExtendedTcpTable, TCP_TABLE_OWNER_PID_LISTENER, + }; + + // 8 KiB: room for ~340 IPv4 or ~146 IPv6 listeners, where a busy desktop has + // a few dozen. Sizing it up front is what keeps the common poll to one call + // per family instead of the usual size-then-fetch pair. + let mut buf = vec![0u32; 2048]; + // Two rounds, not a loop until it fits: the table can keep growing between + // calls, and this runs on a 2 s timer where giving up costs one poll. + for _ in 0..2 { + let mut size = (buf.len() * std::mem::size_of::()) as u32; + // SAFETY: `buf` is at least `size` bytes, 4-aligned, and writable; the + // kernel writes no more than `size` and reports what it needed instead. + let rc = unsafe { + GetExtendedTcpTable( + buf.as_mut_ptr().cast(), + &mut size, + // No kernel-side sort: the rows are ordered by (port, pid) below + // anyway, and this one is over the address, not the port. + 0, + family, + TCP_TABLE_OWNER_PID_LISTENER, + 0, + ) + }; + match rc { + NO_ERROR => return buf, + ERROR_INSUFFICIENT_BUFFER => { + buf = vec![0u32; (size as usize).div_ceil(std::mem::size_of::()) + 64] + } + _ => break, + } + } + Vec::new() +} + +/// Where the rows of a `MIB_*TABLE_OWNER_PID` start in `buf`, and how many of +/// them the buffer can be trusted for. +/// +/// The count is `min`'d against the buffer's own capacity rather than taken from +/// `dwNumEntries` alone: that field is the kernel's, but the read that follows +/// it is ours, and a table shorter than its header claims must not walk off the +/// end of the allocation. +/// +/// # Safety +/// +/// `buf` must be empty or hold a `Table` the kernel filled. +#[cfg(windows)] +unsafe fn table_rows(buf: &[u32]) -> Option<(*const Row, usize)> { + let bytes = std::mem::size_of_val(buf); + if bytes < std::mem::size_of::() { + return None; + } + let table = buf.as_ptr().cast::
(); + // Every `MIB_*TABLE_OWNER_PID` is `{ dwNumEntries: u32, table: [Row; 1] }`, + // so the count is the first word and the rows begin where the padding ends. + let count = buf[0] as usize; + let offset = std::mem::size_of::
() - std::mem::size_of::(); + let capacity = (bytes - offset) / std::mem::size_of::(); + let rows = unsafe { table.cast::().add(offset) }.cast::(); + Some((rows, count.min(capacity))) +} + +/// `dwLocalPort` carries the port in *network* byte order in its low 16 bits. +/// Reading it as a plain number is the classic way to end up showing 41247 for +/// a server on 8099. +#[cfg(windows)] +fn local_port(raw: u32) -> u16 { + u16::from_be(raw as u16) +} + +/// The wildcard binds are spelled `*`, exactly as `lsof -n` spells them on the +/// other platforms, so the same server reads the same in the panel wherever it +/// runs — and so `PortEntry::authority` resolves it to `localhost`. +#[cfg(windows)] +fn spell_v4(addr: std::net::Ipv4Addr) -> String { + match addr.is_unspecified() { + true => "*".to_string(), + false => addr.to_string(), + } +} + +/// See `spell_v4`. A specific IPv6 address keeps `lsof`'s brackets, which is +/// what makes `[::1]:5173` a pastable authority. +#[cfg(windows)] +fn spell_v6(addr: std::net::Ipv6Addr) -> String { + match addr.is_unspecified() { + true => "*".to_string(), + false => format!("[{addr}]"), + } +} + +/// Adds one listening socket to the list, merging it with a row already there +/// for the same port and pid. +/// +/// The merge rule is the unix path's, for the same reason: a process bound to +/// both `192.168.1.5` and `*` is on localhost, and the row the panel turns into +/// a clickable URL should say so rather than whichever address the kernel +/// happened to list first. +#[cfg(windows)] +fn record_listener(ports: &mut Vec, name: &str, port: u16, pid: u32, addr: String) { + if let Some(seen) = ports.iter_mut().find(|e| e.port == port && e.pid == pid) { + if !PortEntry::reaches_loopback(&seen.addr) && PortEntry::reaches_loopback(&addr) { + seen.addr = addr; + } + return; + } + ports.push(PortEntry { + port, + pid, + addr, + name: name.to_string(), + }); +} + +#[cfg(not(any(unix, windows)))] fn listening_ports(_procs: &[ProcEntry]) -> Vec { Vec::new() } @@ -408,3 +626,187 @@ mod tests { assert_eq!(entry("192.168.1.20").authority(), "192.168.1.20:8080"); } } + +#[cfg(all(test, windows))] +mod windows_tests { + use std::io::Write as _; + use std::net::TcpListener; + use std::time::{Duration, Instant}; + + use super::*; + + /// The port lives in the low half of a DWORD in *network* order. Getting + /// this wrong does not fail loudly — it yields a plausible port number for + /// a socket nobody is listening on. + #[test] + fn a_local_port_is_read_out_of_network_order() { + // 8099 == 0x1FA3, so the wire spells it 0xA31F. + assert_eq!(local_port(0x0000_A31F), 8099); + assert_eq!(local_port(0xBB01), 443); + assert_eq!(local_port(0x5000), 80); + } + + /// The panel renders one server the same way on every platform, so a + /// Windows wildcard bind has to arrive spelled the way `lsof -n` spells it. + #[test] + fn addresses_are_spelled_the_way_lsof_spells_them() { + assert_eq!(spell_v4("0.0.0.0".parse().unwrap()), "*"); + assert_eq!(spell_v4("127.0.0.1".parse().unwrap()), "127.0.0.1"); + assert_eq!(spell_v4("192.168.1.20".parse().unwrap()), "192.168.1.20"); + assert_eq!(spell_v6("::".parse().unwrap()), "*"); + assert_eq!(spell_v6("::1".parse().unwrap()), "[::1]"); + } + + /// A dual-stack server shows up twice in the kernel's tables, once per + /// family, and is one row in the panel — the reachable one. + #[test] + fn a_dual_stack_listener_collapses_to_its_reachable_address() { + let mut ports = Vec::new(); + record_listener(&mut ports, "node.exe", 3000, 42, "192.168.1.5".into()); + record_listener(&mut ports, "node.exe", 3000, 42, "*".into()); + assert_eq!(ports.len(), 1, "one port, not one per address family"); + assert_eq!(ports[0].addr, "*", "the loopback-reachable bind wins"); + + // ...and never the other way round: a wildcard already recorded is not + // downgraded to an interface nobody can reach on localhost. + let mut ports = Vec::new(); + record_listener(&mut ports, "node.exe", 3000, 42, "[::]".into()); + record_listener(&mut ports, "node.exe", 3000, 42, "192.168.1.5".into()); + assert_eq!(ports[0].addr, "[::]"); + + // Two processes on the same port number are two rows. + record_listener(&mut ports, "python.exe", 3000, 43, "127.0.0.1".into()); + assert_eq!(ports.len(), 2); + } + + /// The whole feature, against the live kernel table: a real + /// `cmd.exe -> powershell.exe` chain holding a real socket. + /// + /// Both halves matter. The port has to show up with the right number and + /// the right owner — that is the half that was missing entirely, since + /// `listening_ports` was `#[cfg(unix)]` and Windows got an empty list. And + /// a socket held *outside* the tree must not show up, because + /// `GetExtendedTcpTable` answers for the whole machine: without the pid + /// filter this panel would list every port on the box and still pass the + /// first assertion. + #[test] + fn listening_ports_finds_the_pane_tree_s_socket_and_only_its_tree_s() { + // The out-of-tree listener. It belongs to the test process, which is + // the parent of the chain and so is never inside the tree rooted at it. + let outsider = TcpListener::bind("127.0.0.1:0").expect("bind an out-of-tree listener"); + let outside_port = outsider.local_addr().expect("read its port").port(); + + let stamp = format!("tty7-ports-{}", std::process::id()); + let dir = std::env::temp_dir(); + let script = dir.join(format!("{stamp}.ps1")); + let port_file = dir.join(format!("{stamp}.port")); + let _ = std::fs::remove_file(&port_file); + // The port comes back through a file rather than a pipe: PowerShell + // buffers redirected stdout, and a test that waits on a flush that + // never comes is a test that hangs. + let mut f = std::fs::File::create(&script).expect("write the listener script"); + write!( + f, + "$l = [System.Net.Sockets.TcpListener]::new([System.Net.IPAddress]::Loopback, 0)\r\n\ + $l.Start()\r\n\ + [System.IO.File]::WriteAllText('{}', [string]$l.LocalEndpoint.Port)\r\n\ + while ($true) {{ Start-Sleep -Seconds 1 }}\r\n", + port_file.display().to_string().replace('\'', "''") + ) + .expect("write the listener script"); + drop(f); + + // `cmd.exe` in front of PowerShell is what makes this a *tree* and not + // one child: the listener sits at depth 1, reached only by walking. + let mut child = std::process::Command::new("cmd.exe") + .args([ + "/c", + "powershell", + "-NoProfile", + "-ExecutionPolicy", + "Bypass", + "-File", + ]) + .arg(&script) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("spawn the cmd -> powershell chain"); + let root = child.id(); + + let cleanup = |child: &mut std::process::Child| { + let doomed = crate::daemon::winproc::descendants( + &crate::daemon::winproc::snapshot(), + child.id(), + ); + let _ = child.kill(); + let _ = child.wait(); + crate::daemon::winproc::terminate_and_wait_all( + &doomed, + Instant::now() + Duration::from_secs(5), + ); + }; + + // PowerShell's startup is measured in seconds on a cold machine, and + // `WriteAllText` can be observed mid-write, so parse until it parses. + let deadline = Instant::now() + Duration::from_secs(60); + let inside_port = loop { + if let Some(port) = std::fs::read_to_string(&port_file).ok().and_then(|text| { + text.trim() + .trim_start_matches('\u{feff}') + .parse::() + .ok() + }) { + break port; + } + if Instant::now() >= deadline { + cleanup(&mut child); + let _ = std::fs::remove_file(&script); + panic!("the in-tree listener never reported its port"); + } + std::thread::sleep(Duration::from_millis(100)); + }; + + let got = snapshot(root, None); + cleanup(&mut child); + let _ = std::fs::remove_file(&script); + let _ = std::fs::remove_file(&port_file); + drop(outsider); + + assert!( + got.procs.iter().any(|p| p.depth > 0), + "the chain must be walked past its root: {:?}", + got.procs + ); + let found = got + .ports + .iter() + .find(|e| e.port == inside_port) + .unwrap_or_else(|| { + panic!("port {inside_port} is missing from {:?}", got.ports); + }); + assert_eq!( + found.addr, "127.0.0.1", + "a loopback bind keeps its address, as it does under lsof" + ); + assert!( + got.procs.iter().any(|p| p.pid == found.pid), + "the port's owner must be one of the pane's own processes" + ); + assert!( + found.name.eq_ignore_ascii_case("powershell.exe"), + "the row names the process holding the socket, got {:?}", + found.name + ); + assert!( + !got.ports.iter().any(|e| e.port == outside_port), + "port {outside_port} is held outside the tree and must not be listed: {:?}", + got.ports + ); + assert!( + got.ports.windows(2).all(|w| w[0].port <= w[1].port), + "rows arrive ordered by port, as they do on unix" + ); + } +} From 5ff0567c7df76a38a40294b316248ae32634fddb Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 7 Sep 2026 22:36:58 +0800 Subject: [PATCH 10/10] fix(terminal): keep a solo glyph inside its cell when the next one is taken (#783) --- src/terminal/element.rs | 135 +++++++++++++++++++++++++++++++++++----- 1 file changed, 119 insertions(+), 16 deletions(-) diff --git a/src/terminal/element.rs b/src/terminal/element.rs index 5dc1da49..55618483 100644 --- a/src/terminal/element.rs +++ b/src/terminal/element.rs @@ -1003,12 +1003,26 @@ fn native_cell_residue(style: &GlyphStyle) -> Option { /// and letting it spill. This follows WezTerm, whose answer shows the glyph /// whole most often: a quarter cell of slack always, and a whole extra cell /// when the neighbouring cell is blank and has nothing to lose. -fn seg_budget(solo: bool, cells: usize, room: bool, cell_width: Pixels) -> Pixels { +fn seg_budget(solo: bool, measured: bool, cells: usize, room: bool, cell_width: Pixels) -> Pixels { if solo { - // Single-cell glyphs have always been allowed to lean into the next - // cell. Narrowing that here would shrink a pile of symbols that look - // fine today, so it stays a separate decision. - cell_width * 2. + // Single-cell glyphs are allowed to lean into the next cell — that is + // what keeps the pile of symbols that look fine today from shrinking — + // but only while that cell is empty. A Nerd Font icon pulled from a + // fallback face inks well past a narrow primary's cell (1.6 cells is + // typical), and where the next cell has a glyph of its own the lean is + // not a lean, it is an overlap: the neighbour is painted afterwards + // and lands on top of the overshoot. Hand those their own cell and let + // `fit_scale` bring them down into it, the way kitty and ghostty do. + // + // Only where the ink was measured, though. This number is the clip as + // well as the threshold to shrink at, so narrowing it for a segment + // nothing could measure would cut the glyph instead of scaling it — + // see `ink_covers_segment`. + if room || !measured { + cell_width * 2. + } else { + cell_width + } } else if room { cell_width * (cells as f32 + 1.) } else { @@ -1065,6 +1079,21 @@ fn ink_extent( }) } +/// Whether [`ink_extent`]'s answer speaks for the whole segment. +/// +/// It measures the segment's first character in the run's first face. That is +/// all of a one-character segment and only part of anything longer: a +/// cluster's combining marks can ink well to the right of the base they hang +/// off — Devanagari `\u{915}` + `\u{93E}` — and are not in the number. `None` +/// is a face that answered no bounds at all, which is no measurement either. +/// +/// Neither can be scaled to fit, so neither may be held to one cell: the +/// budget doubles as the clip, and a segment that cannot shrink into a +/// narrowed one is simply cut off at it. +fn ink_covers_segment(ink: Option, text: &str) -> bool { + ink.is_some() && text.chars().nth(1).is_none() +} + /// Whether the cell after a segment is free for its glyph to lean into. /// /// A blank still owns its cell if it paints anything there: a background, a @@ -1201,9 +1230,20 @@ fn paint_glyphs( }; let x = geom.origin.x + geom.cell_width * (start as f32); + + let mut shaped = + window + .text_system() + .shape_line(text.clone(), font_size, run_buf, force_width); + // Measured before the budget is set, because how far a solo glyph + // may reach turns on whether its ink is known at all. + let ink = fit + .then(|| ink_extent(cx, &shaped, &text, font_size)) + .flatten(); let budget = if fit { seg_budget( solo, + ink_covers_segment(ink, &text), cells, has_room_after(row_cells, start, cells), geom.cell_width, @@ -1211,12 +1251,7 @@ fn paint_glyphs( } else { geom.cell_width * cells as f32 }; - - let mut shaped = - window - .text_system() - .shape_line(text.clone(), font_size, run_buf, force_width); - if fit && let Some(ink) = ink_extent(cx, &shaped, &text, font_size) { + if let Some(ink) = ink { let scale = fit_scale(ink, budget); if scale < 1. { shaped = window.text_system().shape_line( @@ -2386,7 +2421,7 @@ mod tests { let ink = px(18.75); let scale = |advance_em: f32, room: bool| { let cell = px(15. * advance_em); - fit_scale(ink, seg_budget(false, 2, room, cell)) + fit_scale(ink, seg_budget(false, true, 2, room, cell)) }; // Menlo and friends: a quarter cell of slack is enough on its own. @@ -2735,14 +2770,82 @@ mod tests { #[test] fn seg_budget_frees_solo_symbols_and_lends_a_cell_only_when_one_is_free() { let cell = px(10.); - assert_eq!(seg_budget(true, 1, false, cell), px(20.), "solo keeps two"); assert_eq!( - seg_budget(false, 2, false, cell), + seg_budget(true, true, 1, true, cell), + px(20.), + "a solo glyph leans into a free cell" + ); + assert_eq!( + seg_budget(true, true, 1, false, cell), + px(10.), + "but keeps to its own once the next cell is taken" + ); + assert_eq!( + seg_budget(false, true, 2, false, cell), px(22.5), "a quarter cell" ); - assert_eq!(seg_budget(false, 2, true, cell), px(30.), "a whole cell"); - assert_eq!(seg_budget(false, 1, false, cell), px(12.5)); + assert_eq!( + seg_budget(false, true, 2, true, cell), + px(30.), + "a whole cell" + ); + assert_eq!(seg_budget(false, true, 1, false, cell), px(12.5)); + } + + #[test] + fn a_fallback_icon_is_fitted_to_its_cell_only_when_the_next_one_is_taken() { + // Measured on Windows: Maple Mono NF CN supplying U+F059 to a + // 15px Cascadia Mono grid inks 13.85px across an 8.79px cell. + let cell = px(8.789); + let ink = px(13.845); + + let leaning = fit_scale(ink, seg_budget(true, true, 1, true, cell)); + assert_eq!(leaning, 1., "a blank neighbour still lends its cell"); + + let crowded = fit_scale(ink, seg_budget(true, true, 1, false, cell)); + assert!(crowded < 1., "an occupied neighbour does not"); + assert!( + ink * crowded <= cell, + "and the icon has to end inside its own cell" + ); + } + + #[test] + fn a_solo_segment_is_held_to_its_cell_only_where_its_ink_was_measured() { + let cell = px(10.); + let budget = |ink, text| seg_budget(true, ink_covers_segment(ink, text), 1, false, cell); + + // One character the face answered bounds for: the whole of it was + // measured, so `fit_scale` can bring it into one cell and the clip + // may be drawn there. + assert!(ink_covers_segment(Some(px(15.)), "\u{f059}")); + assert_eq!(budget(Some(px(15.)), "\u{f059}"), px(10.)); + + // A base with a combining mark hanging off it reaches paint_glyphs as + // a one-cell `Cluster`, which counts as solo. `ink_extent` read the + // base alone: Devanagari ka plus the aa matra inks to the right of the + // base, and none of that overhang is in the 9px. Nothing would shrink + // it, so a one-cell clip would cut the matra clean off. + assert!(!ink_covers_segment(Some(px(9.)), "\u{915}\u{93E}")); + assert_eq!(budget(Some(px(9.)), "\u{915}\u{93E}"), px(20.)); + + // A face that answers no bounds at all is the same story from the + // other side: `fit_scale` is never reached, so halving the clip only + // clips. + assert!(!ink_covers_segment(None, "\u{f059}")); + assert_eq!(budget(None, "\u{f059}"), px(20.)); + + // A free neighbour lends its cell either way \u2014 the measurement only + // decides whether the lean can be withdrawn. + for ink in [Some(px(15.)), None] { + for text in ["\u{f059}", "\u{915}\u{93E}"] { + assert_eq!( + seg_budget(true, ink_covers_segment(ink, text), 1, true, cell), + px(20.) + ); + } + } } /// The emitted feature set is the whole contract with the shaper, so pin it