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..5f3168d5 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]] @@ -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" @@ -891,7 +911,7 @@ dependencies = [ "bitflags 2.13.1", "cexpr", "clang-sys", - "itertools 0.13.0", + "itertools 0.11.0", "log", "prettyplease", "proc-macro2", @@ -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" @@ -1483,7 +1527,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 +2091,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 +2148,7 @@ dependencies = [ "libc", "option-ext", "redox_users", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -2142,7 +2186,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 +2438,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 +3243,7 @@ dependencies = [ "log", "presser", "thiserror 2.0.20", - "windows 0.62.2", + "windows 0.58.0", ] [[package]] @@ -3225,7 +3269,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 +3460,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 +3511,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 +3558,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 +3569,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 +3582,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 +3592,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 +3601,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 +3625,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 +3654,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 +3983,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 +4008,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 +4101,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.62.2", + "windows-core 0.58.0", ] [[package]] @@ -4581,9 +4625,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 +4945,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", @@ -5092,7 +5136,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 +5246,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 +5302,7 @@ dependencies = [ "once_cell", "png", "thiserror 2.0.20", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -5481,7 +5525,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]] @@ -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" @@ -6204,7 +6254,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 +6950,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -7207,7 +7257,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 +7300,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 +7661,7 @@ dependencies = [ "errno", "libc", "linux-raw-sys 0.12.1", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -7784,7 +7834,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 +8439,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 +8565,7 @@ dependencies = [ "cfg-if", "libc", "psm", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -8627,7 +8677,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 +8967,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 +9434,7 @@ dependencies = [ "once_cell", "png", "thiserror 2.0.20", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -9775,11 +9825,11 @@ dependencies = [ [[package]] name = "tty7" -version = "26.9.0" +version = "26.9.1" dependencies = [ "alacritty_terminal", "anyhow", - "async_zip", + "async_zip 0.0.19", "core-foundation 0.10.0", "gpui", "gpui-component", @@ -9820,7 +9870,7 @@ dependencies = [ [[package]] name = "tty7-cli" -version = "26.9.0" +version = "26.9.1" dependencies = [ "alacritty_terminal", "anyhow", @@ -9835,7 +9885,7 @@ dependencies = [ [[package]] name = "tty7-core" -version = "26.9.0" +version = "26.9.1" dependencies = [ "anyhow", "base64 0.22.1", @@ -9869,7 +9919,7 @@ dependencies = [ [[package]] name = "tty7-server" -version = "26.9.0" +version = "26.9.1" dependencies = [ "libc", "tempfile", @@ -9896,7 +9946,7 @@ checksum = "f2f6fb2847f6742cd76af783a2a2c49e9375d0a111c7bef6f71cd9e738c72d6e" dependencies = [ "memoffset 0.9.1", "tempfile", - "windows-sys 0.61.2", + "windows-sys 0.60.2", ] [[package]] @@ -10108,11 +10158,11 @@ 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", - "async_zip", + "async_zip 0.0.18", "collections", "command-fds", "dirs", @@ -10147,7 +10197,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", @@ -10156,9 +10206,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", @@ -10740,7 +10790,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 +11541,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 +12098,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 +12143,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 +12154,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 31d358e5..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 @@ -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: @@ -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/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" + ); + } +} 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()); 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/docs/reference/keyboard-shortcuts.mdx b/docs/reference/keyboard-shortcuts.mdx index 2928daae..a526c78c 100644 --- a/docs/reference/keyboard-shortcuts.mdx +++ b/docs/reference/keyboard-shortcuts.mdx @@ -113,7 +113,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 7286eb1f..7f9a551d 100644 --- a/src/core/actions.rs +++ b/src/core/actions.rs @@ -19,6 +19,7 @@ actions!( SelectWorkspace8, SelectWorkspace9, NewWindow, + CloseWindow, CloseActiveTab, RenameTab, NewWorktreeTab, diff --git a/src/terminal/element.rs b/src/terminal/element.rs index 6eee0416..55618483 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, @@ -975,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 { @@ -1037,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 @@ -1173,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, @@ -1183,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( @@ -2358,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. @@ -2707,30 +2770,142 @@ 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 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)); + 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 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)); + 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 + /// 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_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)], ); } diff --git a/src/terminal/search.rs b/src/terminal/search.rs index 09b7d2e9..dff459e1 100644 --- a/src/terminal/search.rs +++ b/src/terminal/search.rs @@ -727,6 +727,103 @@ pub(super) fn local_probe(path: &Path, require_file: bool) -> Probe { } } +/// Which language a pane's paths are written in. +/// +/// The machine tty7 runs on and the machine a pane's paths live on need not +/// agree, and `std::path` only ever speaks the first one's dialect. On a +/// Windows client that is the whole difference between a link and nothing: a +/// leading `/` is not absolute to `Path` there, so `/etc/hosts` printed by a +/// Linux pane used to be measured from that pane's directory instead of +/// standing alone, and a relative `src/lib.rs` was joined onto it with a +/// backslash the far side has never heard of. +/// +/// The two arms are the two dialects, not the two operating systems: a WSL +/// distro and an SSH host both speak [`PathStyle::Posix`] whatever the client +/// is, and a pane on this machine speaks [`PathStyle::NATIVE`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum PathStyle { + /// `/`-rooted and `/`-joined. Everything a Unix host, a WSL distro or a + /// Git-Bash-style shell prints. + Posix, + /// Rooted by a drive letter or a UNC share, joined with `\`. + Windows, +} + +impl Default for PathStyle { + fn default() -> Self { + Self::NATIVE + } +} + +impl PathStyle { + /// The dialect the machine tty7 is running on speaks. + pub const NATIVE: PathStyle = match cfg!(windows) { + true => PathStyle::Windows, + false => PathStyle::Posix, + }; + + /// The dialect a host that called `sample` one of its own directories + /// speaks: a drive letter or a UNC share is a Windows host's, and anything + /// else is read as POSIX. + /// + /// Only a directory that names a *Windows* root counts as one, rather than + /// every directory that does not name a POSIX one. A cwd that is neither — + /// a relative one, or an empty one from a host that has not settled yet — + /// is the same "nothing to go on" as no cwd at all, and has to fall the + /// same way; reading it as Windows would spell a Linux host's paths with + /// backslashes on the strength of a directory it never really reported. + /// + /// This is inference from what a host says about itself in passing. It is + /// sound in the direction that matters: nothing but a POSIX host reports a + /// `/`-rooted cwd. + pub fn of_dir(sample: &Path) -> Self { + match PathStyle::Windows.is_absolute(&sample.to_string_lossy()) { + true => PathStyle::Windows, + false => PathStyle::Posix, + } + } + + /// Whether a token says for itself which filesystem root it hangs off. + /// + /// Deliberately textual rather than [`Path::is_absolute`], which answers + /// for *this* machine: the same string has to be read the pane's way on + /// every client, or a link works on a Mac and not on the Windows box next + /// to it. + pub fn is_absolute(self, path: &str) -> bool { + match self { + PathStyle::Posix => path.starts_with('/'), + // A UNC share, or a drive letter with a separator behind it. + // `C:foo` is drive-*relative* and deliberately not included, which + // is what `Path::is_absolute` says on Windows too. + PathStyle::Windows => { + if path.starts_with("\\\\") { + return true; + } + let mut chars = path.chars(); + chars.next().is_some_and(|c| c.is_ascii_alphabetic()) + && chars.next() == Some(':') + && matches!(chars.next(), Some('\\' | '/')) + } + } + } + + /// `rel` measured from `root`, spelled the way the pane's host spells it. + /// + /// The POSIX arm joins textually because `Path::join` would reach for this + /// machine's separator: on Windows it turns `/home/u` and `src/lib.rs` + /// into `/home/u\src/lib.rs`, which the Linux box on the other end of the + /// probe cannot stat. + pub fn join(self, root: &Path, rel: &str) -> PathBuf { + match self { + PathStyle::Posix => PathBuf::from(format!( + "{}/{rel}", + root.to_string_lossy().trim_end_matches('/') + )), + PathStyle::Windows => root.join(rel), + } + } +} + /// Where a relative path printed by a pane is measured from. /// /// `local_home` is the part that is easy to miss. `~` has to become a real @@ -743,15 +840,18 @@ pub(super) struct LinkRoots { /// Whether this machine's `$HOME` may stand in for a `~` the roots cannot /// explain. pub local_home: bool, + /// How the pane's host spells the paths it prints. + pub style: PathStyle, } impl LinkRoots { /// Roots on the machine tty7 is running on, where `$HOME` means what it - /// says. + /// says and paths are spelled this OS's way. pub fn local(dirs: Vec) -> Self { Self { dirs, local_home: true, + style: PathStyle::NATIVE, } } @@ -841,18 +941,18 @@ impl FileCandidate { /// Every path this token could mean, best guess first: absolute paths /// stand alone, relative ones are joined onto each root in turn. pub fn paths(&self, roots: &LinkRoots) -> Vec { - let Some(expanded) = expand_home(&self.path, roots.cwd(), roots.local_home) else { + let Some(expanded) = expand_home(&self.path, roots) else { return Vec::new(); }; - if expanded.as_os_str().is_empty() { + if expanded.is_empty() { return Vec::new(); } - if expanded.is_absolute() { - return vec![expanded]; + if roots.style.is_absolute(&expanded) { + return vec![PathBuf::from(expanded)]; } let mut out: Vec = Vec::new(); for root in &roots.dirs { - let joined = root.join(&expanded); + let joined = roots.style.join(root, &expanded); if !out.contains(&joined) { out.push(joined); } @@ -864,17 +964,26 @@ impl FileCandidate { /// measured from anywhere. [`Self::paths`] ignores the roots entirely for /// these, so a report about one must not name a directory as the place it /// was looked for. - pub fn is_rooted(&self) -> bool { - self.path.starts_with('~') || Path::new(&self.path).is_absolute() + /// + /// Takes the pane's own dialect for the same reason `paths` does: on a + /// Windows client `/etc/hosts` printed by a Linux pane is rooted and + /// `/etc/hosts` printed by a `cmd.exe` pane is not, and `Path` alone + /// cannot tell those apart. + pub fn is_rooted(&self, style: PathStyle) -> bool { + self.path.starts_with('~') || style.is_absolute(&self.path) } /// Whether the token is written enough like a path to be worth telling the /// user about when nothing answers for it. A bare word is not — every /// modifier-click on ordinary output would raise a notification saying so. - pub fn looks_like_a_path(&self) -> bool { + /// + /// A backslash counts only where it separates directories. In a POSIX + /// pane it is an escape or an ordinary filename character, so `foo\ bar` + /// there is a word, not a path. + pub fn looks_like_a_path(&self, style: PathStyle) -> bool { self.path.starts_with('~') || self.path.contains('/') - || (cfg!(windows) && self.path.contains('\\')) + || (style == PathStyle::Windows && self.path.contains('\\')) } } @@ -1054,14 +1163,29 @@ fn strip_numeric_suffix(token: &str) -> Option<(&str, u32)> { Some((prefix, value)) } -fn expand_home(path: &str, cwd: Option<&Path>, local_home: bool) -> Option { +/// The token with a leading `~` turned into a real directory, still spelled +/// the pane's way — a string rather than a `PathBuf`, because deciding what is +/// absolute and how to join is the [`PathStyle`]'s job from here on and +/// `Path` would answer for the wrong machine. +fn expand_home(path: &str, roots: &LinkRoots) -> Option { + let home = || { + home_dir(roots.cwd(), roots.local_home, roots.style) + .map(|home| home.to_string_lossy().into_owned()) + }; if path == "~" { - return home_dir(cwd, local_home); + return home(); } - if let Some(rest) = path.strip_prefix("~/").or_else(|| path.strip_prefix("~\\")) { - return home_dir(cwd, local_home).map(|home| home.join(rest)); + let rest = match roots.style { + PathStyle::Posix => path.strip_prefix("~/"), + PathStyle::Windows => path.strip_prefix("~/").or_else(|| path.strip_prefix("~\\")), + }; + if let Some(rest) = rest { + return home().map(|home| match roots.style { + PathStyle::Posix => format!("{}/{rest}", home.trim_end_matches('/')), + PathStyle::Windows => Path::new(&home).join(rest).to_string_lossy().into_owned(), + }); } - Some(PathBuf::from(path)) + Some(path.to_string()) } /// The home `~` stands for, read out of the cwd where it can be and out of the @@ -1072,8 +1196,8 @@ fn expand_home(path: &str, cwd: Option<&Path>, local_home: bool) -> Option, local_home: bool) -> Option { - if let Some(home) = cwd.and_then(home_from_cwd) { +fn home_dir(cwd: Option<&Path>, local_home: bool, style: PathStyle) -> Option { + if let Some(home) = cwd.and_then(|cwd| home_from_cwd(cwd, style)) { return Some(home); } if !local_home { @@ -1085,25 +1209,26 @@ fn home_dir(cwd: Option<&Path>, local_home: bool) -> Option { .map(PathBuf::from) } -#[cfg(unix)] -fn home_from_cwd(cwd: &Path) -> Option { - let mut components = cwd.components(); - let root = components.next()?; - let base = components.next()?; - let user = components.next()?; - let base = base.as_os_str().to_str()?; - matches!(base, "Users" | "home").then(|| { - let mut home = PathBuf::new(); - home.push(root.as_os_str()); - home.push(base); - home.push(user.as_os_str()); - home - }) -} - -#[cfg(not(unix))] -fn home_from_cwd(_cwd: &Path) -> Option { - None +/// The `/home/` or `/Users/` a POSIX cwd sits under. +/// +/// Read off the string rather than `Path::components`, and keyed off the +/// pane's dialect rather than `cfg!(unix)`. The old spelling was gated to Unix +/// clients, which meant a Windows tty7 could not say what `~` meant in *any* +/// pane it was looking at — `~/.zshrc` printed by a Linux host resolved on a +/// Mac and silently did not on a Windows box beside it. +/// +/// A Windows-dialect cwd still gets no answer: nothing about `D:\Users\team` +/// says whose home it is, and the environment fallback in [`home_dir`] is both +/// available and right for the panes that spell paths that way. +fn home_from_cwd(cwd: &Path, style: PathStyle) -> Option { + if style != PathStyle::Posix { + return None; + } + let cwd = cwd.to_str()?; + let mut parts = cwd.strip_prefix('/')?.split('/').filter(|p| !p.is_empty()); + let base = parts.next()?; + let user = parts.next()?; + matches!(base, "Users" | "home").then(|| PathBuf::from(format!("/{base}/{user}"))) } fn trim_trailing_punct(token: &mut String) { @@ -1481,13 +1606,19 @@ mod tests { assert_eq!((link.start, link.end), (6, 21)); } + /// Ungated with the rest: what a `~` in a POSIX pane stands for is that + /// pane's business and not its client's, and the `#[cfg(unix)]` this used + /// to carry described the wrong machine. #[test] - #[cfg(unix)] fn tilde_expansion_prefers_home_inferred_from_the_pane_cwd() { - let cwd = Path::new("/Users/alice/clone/tty7"); + let roots = LinkRoots { + dirs: vec![PathBuf::from("/Users/alice/clone/tty7")], + local_home: true, + style: PathStyle::Posix, + }; assert_eq!( - expand_home("~/clone/tty7/src/main.rs", Some(cwd), true), - Some(PathBuf::from("/Users/alice/clone/tty7/src/main.rs")) + expand_home("~/clone/tty7/src/main.rs", &roots), + Some("/Users/alice/clone/tty7/src/main.rs".to_string()) ); } @@ -1495,17 +1626,24 @@ mod tests { /// machine: this machine's `$HOME` describes nobody there, and a path built /// out of it would be asked about — and possibly answered — on the far side. #[test] - #[cfg(unix)] fn tilde_expansion_does_not_borrow_this_machines_home_for_another_one() { - let cwd = Path::new("/srv/app"); - assert_eq!(expand_home("~/.zshrc", Some(cwd), false), None); + let elsewhere = |cwd: &str| LinkRoots { + dirs: vec![PathBuf::from(cwd)], + local_home: false, + style: PathStyle::Posix, + }; + assert_eq!(expand_home("~/.zshrc", &elsewhere("/srv/app")), None); assert_eq!( - expand_home("~/.zshrc", Some(Path::new("/home/deploy/app")), false), - Some(PathBuf::from("/home/deploy/.zshrc")), + expand_home("~/.zshrc", &elsewhere("/home/deploy/app")), + Some("/home/deploy/.zshrc".to_string()), "a cwd that does reveal the home needs nothing from us" ); assert!( - expand_home("~/.zshrc", Some(cwd), true).is_some(), + expand_home( + "~/.zshrc", + &LinkRoots::local(vec![PathBuf::from("/srv/app")]) + ) + .is_some(), "a local pane still falls back to the environment" ); } @@ -1682,14 +1820,25 @@ mod tests { fn a_path_shaped_token_is_kept_apart_from_a_bare_word() { let path_shaped = file_candidate_at("wrote scratchpad/notes.md now", 8).expect("candidate"); assert_eq!(path_shaped.path, "scratchpad/notes.md"); - assert!(path_shaped.looks_like_a_path()); + assert!(path_shaped.looks_like_a_path(PathStyle::NATIVE)); let word = file_candidate_at("wrote notes now", 8).expect("candidate"); assert_eq!(word.path, "notes"); assert!( - !word.looks_like_a_path(), + !word.looks_like_a_path(PathStyle::NATIVE), "a bare word must not raise a notification on every modifier-click" ); + + let escaped = file_candidate_at(r"wrote a\b now", 6).expect("candidate"); + assert_eq!(escaped.path, r"a\b"); + assert!( + escaped.looks_like_a_path(PathStyle::Windows), + "a backslash separates directories in a Windows pane" + ); + assert!( + !escaped.looks_like_a_path(PathStyle::Posix), + "and escapes a space in a POSIX one, whatever this client runs" + ); } #[test] @@ -1721,23 +1870,230 @@ mod tests { /// `is_rooted` decides whether a report about an unresolved token may name /// a directory it was "looked for under", so it has to agree with /// [`FileCandidate::paths`] about when the roots are consulted at all. - /// Both ask `is_absolute`, and on Windows a leading `/` does not make a - /// path that — which is why this only claims to hold where it does. + /// + /// Both used to ask `Path::is_absolute`, which answers for the machine + /// tty7 runs on rather than the one the pane's paths are on — so this only + /// held on Unix and was gated to it. Both now ask the pane's own dialect, + /// and the agreement holds on every client. #[test] - #[cfg(unix)] fn a_rooted_candidate_is_told_apart_from_one_measured_from_a_root() { + let posix_roots = LinkRoots { + dirs: vec![PathBuf::from("/home/u/proj")], + local_home: false, + style: PathStyle::Posix, + }; for line in ["open /etc/hosts now", "open ~/.zshrc now"] { + let candidate = file_candidate_at(line, 6).expect("candidate"); assert!( - file_candidate_at(line, 6).expect("candidate").is_rooted(), + candidate.is_rooted(PathStyle::Posix), + "{line} says for itself where it starts" + ); + let paths = candidate.paths(&posix_roots); + assert!( + !paths.iter().any(|p| p.starts_with("/home/u/proj")), + "{line} was not measured from the pane's directory: {paths:?}" + ); + } + for line in [r"open C:\Windows\win.ini now", "open ~/.gitconfig now"] { + assert!( + file_candidate_at(line, 6) + .expect("candidate") + .is_rooted(PathStyle::Windows), "{line} says for itself where it starts" ); } - assert!( - !file_candidate_at("see src/lib.rs here", 5) - .expect("candidate") - .is_rooted(), - "a relative path is only ever found by measuring from somewhere" + for style in [PathStyle::Posix, PathStyle::Windows] { + assert!( + !file_candidate_at("see src/lib.rs here", 5) + .expect("candidate") + .is_rooted(style), + "a relative path is only ever found by measuring from somewhere" + ); + } + } + + /// The bug this whole [`PathStyle`] exists for: a Windows tty7 looking at + /// a Linux pane — a remote workspace, a native-SSH pane, or a WSL distro + /// reached as a host — used to read `/etc/hosts` as a *relative* path, + /// because `Path::is_absolute` speaks for the client and a leading `/` is + /// not absolute on Windows. The pane's paths were then measured from its + /// own directory and the far side was asked about something it never + /// printed, so nothing ever underlined. + #[test] + fn a_posix_pane_roots_its_own_paths_on_every_client() { + let roots = LinkRoots { + dirs: vec![PathBuf::from("/home/u/proj")], + local_home: false, + style: PathStyle::Posix, + }; + let candidate = file_candidate_at("open /etc/hosts now", 6).expect("candidate"); + assert_eq!( + candidate.paths(&roots), + vec![PathBuf::from("/etc/hosts")], + "the pane said where the path starts; the roots have nothing to add" ); + + // And with no cwd reported at all there is still exactly one thing it + // can mean. This is the case that failed outright: no roots meant no + // paths, so the host was never even asked. + let roots = LinkRoots { + dirs: Vec::new(), + local_home: false, + style: PathStyle::Posix, + }; + assert_eq!(candidate.paths(&roots), vec![PathBuf::from("/etc/hosts")]); + } + + /// A relative path is the other half, and it fails more quietly: the join + /// used to reach for the *client's* separator, so a Windows tty7 asked a + /// Linux host about `/home/u/proj\src/lib.rs`. + #[test] + fn a_posix_pane_joins_a_relative_path_with_its_own_separator() { + let roots = LinkRoots { + dirs: vec![PathBuf::from("/home/u/proj"), PathBuf::from("/home/u")], + local_home: false, + style: PathStyle::Posix, + }; + let paths = file_candidate_at("see src/lib.rs here", 5) + .expect("candidate") + .paths(&roots); + assert_eq!( + paths + .iter() + .map(|p| p.to_string_lossy()) + .collect::>(), + vec!["/home/u/proj/src/lib.rs", "/home/u/src/lib.rs"], + "the string the far side is asked about has to be one it can stat" + ); + } + + /// `~` in a POSIX pane is read out of the pane's own cwd — on every + /// client. The rule was gated to Unix ones, so a Windows tty7 could not + /// resolve `~/.zshrc` in any pane it was looking at. + #[test] + fn a_posix_pane_reads_a_tilde_out_of_its_own_cwd() { + let roots = LinkRoots { + dirs: vec![PathBuf::from("/home/u/proj")], + local_home: false, + style: PathStyle::Posix, + }; + let paths = file_candidate_at("open ~/.zshrc now", 6) + .expect("candidate") + .paths(&roots); + assert_eq!( + paths + .iter() + .map(|p| p.to_string_lossy()) + .collect::>(), + vec!["/home/u/.zshrc"] + ); + + let no_home = LinkRoots { + dirs: vec![PathBuf::from("/srv/app")], + local_home: false, + style: PathStyle::Posix, + }; + assert!( + file_candidate_at("open ~/.zshrc now", 6) + .expect("candidate") + .paths(&no_home) + .is_empty(), + "a cwd that reveals no home may not borrow this machine's (#568)" + ); + } + + /// The deliberate other half: a pane *on this machine* keeps this OS's + /// reading of its own output. `/etc` in a `cmd.exe` pane sitting on `C:` + /// means `C:\etc`, the way `cd /etc` does there — so it is measured from + /// the pane's root and not turned into a link to a file Windows has not + /// got. Pinned because the temptation is to make every `/`-rooted token + /// stand alone, and that would underline `/etc/hosts` in a PowerShell pane + /// with nothing behind it. + #[test] + fn a_local_pane_reads_its_own_output_the_way_its_own_os_does() { + let candidate = file_candidate_at("open /etc/hosts now", 6).expect("candidate"); + assert_eq!( + candidate.is_rooted(PathStyle::NATIVE), + cfg!(unix), + "a leading slash roots a path on Unix and names a drive-relative \ + directory on Windows" + ); + + let roots = LinkRoots::local(vec![PathBuf::from("/w")]); + assert_eq!( + roots.style, + PathStyle::NATIVE, + "a pane on this machine spells paths this machine's way" + ); + assert_eq!( + candidate.paths(&roots), + vec![PathBuf::from("/etc/hosts")], + "which comes to the same thing under a root with no drive letter" + ); + } + + /// The Windows half of the rule above, spelled out where it can be: the + /// drive the pane is on is what a leading `/` there is measured from. + /// Cannot be asserted on a Unix client, where `PathBuf` has no notion of a + /// drive at all — the *rule* is pinned ungated above, this is the reading. + #[test] + #[cfg(windows)] + fn a_local_windows_pane_measures_a_leading_slash_from_its_own_drive() { + let roots = LinkRoots::local(vec![PathBuf::from(r"C:\proj")]); + assert_eq!( + file_candidate_at("open /etc/hosts now", 6) + .expect("candidate") + .paths(&roots), + vec![PathBuf::from(r"C:\etc\hosts")], + "`/etc` in a cmd.exe pane on C: is C:\\etc, the way `cd /etc` is" + ); + } + + #[test] + fn a_windows_pane_roots_a_drive_letter_and_a_share() { + let style = PathStyle::Windows; + for rooted in [r"C:\Windows", "C:/Windows", r"\\server\share\x"] { + assert!(style.is_absolute(rooted), "{rooted} names its own root"); + } + for measured in ["/etc/hosts", "C:notes.txt", r"src\lib.rs", "", "C:"] { + assert!( + !style.is_absolute(measured), + "{measured} has to be measured from somewhere" + ); + } + for measured in [r"C:\Windows", r"src\lib.rs", ""] { + assert!( + !PathStyle::Posix.is_absolute(measured), + "{measured} is not a POSIX root" + ); + } + } + + #[test] + fn a_hosts_dialect_is_read_off_the_directory_it_reports() { + assert_eq!( + PathStyle::of_dir(Path::new("/home/u/proj")), + PathStyle::Posix + ); + assert_eq!( + PathStyle::of_dir(Path::new(r"C:\Users\u\proj")), + PathStyle::Windows + ); + assert_eq!( + PathStyle::of_dir(Path::new(r"\\wsl$\Ubuntu\home\u")), + PathStyle::Windows + ); + // A directory that names no root at all says nothing about the host, + // so it has to fall the way no directory does — POSIX. Reading it as + // Windows would hand a Linux host `\`-joined paths on the strength of + // a cwd it never really reported. + for nothing_to_go_on in ["", "proj", "~/proj", "C:notes"] { + assert_eq!( + PathStyle::of_dir(Path::new(nothing_to_go_on)), + PathStyle::Posix, + "{nothing_to_go_on:?} names no Windows root" + ); + } } #[test] diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 316d10cd..5978fd8f 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -219,6 +219,30 @@ fn cwd_is_on_host(pane_runs_remotely: bool, host_is_local: bool) -> bool { } } +/// Which path dialect a pane's output is written in. +/// +/// A pane running on this machine spells paths the way this OS does, and that +/// is the end of it: `/etc` printed by a `cmd.exe` pane sitting on `C:` means +/// `C:\etc`, exactly as `cd /etc` would there. Reading it as a rooted POSIX +/// path would underline a file this machine has not got, and a link that +/// cannot be opened is worse than no link. +/// +/// A pane whose paths live somewhere else is asked instead — by the only thing +/// that host ever says about its own spelling, the directory it reports. A +/// `/`-rooted cwd is a POSIX host's. A pane that has not said where it is +/// falls to POSIX: there is no local drive to measure it from either way, and +/// every host tty7 installs a server on over SSH or WSL spells paths that way. +fn link_path_style( + paths_are_local: bool, + host_cwd: Option<&std::path::Path>, +) -> super::search::PathStyle { + use super::search::PathStyle; + match paths_are_local { + true => PathStyle::NATIVE, + false => host_cwd.map_or(PathStyle::Posix, PathStyle::of_dir), + } +} + pub struct TerminalView { pub terminal: RemoteTerminal, host_id: crate::ui::host_ops::HostId, @@ -5327,9 +5351,10 @@ impl TerminalView { window: &mut Window, cx: &mut Context, ) -> bool { + let roots = self.link_roots(cx); // A word that is not written like a path was never a link, and saying // so on every modifier-click over ordinary output would be noise. - if !candidate.looks_like_a_path() { + if !candidate.looks_like_a_path(roots.style) { return false; } // The host has not answered yet. The underline is the promise that it @@ -5340,10 +5365,10 @@ impl TerminalView { // An absolute or `~`-rooted path was never measured from anywhere, so // naming a directory it was "looked for under" would send the user to // somewhere nothing was ever asked about. - let rooted = candidate.is_rooted(); + let rooted = candidate.is_rooted(roots.style); let root = match rooted { true => None, - false => self.link_roots(cx).dirs.into_iter().next(), + false => roots.dirs.into_iter().next(), }; let message = match root { Some(root) => t_fmt( @@ -5596,10 +5621,12 @@ impl TerminalView { /// that exists in both is the near one. fn link_roots(&mut self, cx: &mut Context) -> super::search::LinkRoots { let local_home = self.host_id.is_local(); + let style = self.link_path_style(); let Some(cwd) = self.effective_host_cwd() else { return super::search::LinkRoots { dirs: Vec::new(), local_home, + style, }; }; self.request_link_repo_root(&cwd, cx); @@ -5610,7 +5637,17 @@ impl TerminalView { { dirs.push(root.clone()); } - super::search::LinkRoots { dirs, local_home } + super::search::LinkRoots { + dirs, + local_home, + style, + } + } + + /// Which path dialect this pane's output is written in — see + /// [`link_path_style`]. + fn link_path_style(&self) -> super::search::PathStyle { + link_path_style(self.paths_are_local(), self.effective_host_cwd().as_deref()) } fn request_link_repo_root(&mut self, cwd: &std::path::Path, cx: &mut Context) { @@ -7285,7 +7322,7 @@ mod tests { use super::{ COMPLETION_MENU_MAX_W, LoopbackPlan, RawInput, SelectEndCopy, Typeahead, WheelRoute, clipboard_paste_text, compose_notification_title, cwd_is_on_host, display_width, - is_typeahead_interrupt, loopback_plan, observe_typeahead_for_owner, + is_typeahead_interrupt, link_path_style, loopback_plan, observe_typeahead_for_owner, }; use super::{SCROLL_ANIM_FRAME, scroll_anim_step}; use super::{ @@ -8812,6 +8849,39 @@ mod tests { assert!(!cwd_is_on_host(false, false)); } + /// Which machine's spelling a pane's paths are read in. Ungated on + /// purpose: the bug this settles was a Windows-only one that hid behind a + /// `#[cfg(unix)]` on the test that covered it. + #[test] + fn a_panes_paths_are_read_in_its_own_hosts_spelling() { + use super::super::search::PathStyle; + use std::path::Path; + + assert_eq!( + link_path_style(true, Some(Path::new("/home/u/proj"))), + PathStyle::NATIVE, + "a pane on this machine reads its own output this OS's way, \ + whatever its shell spells the cwd like" + ); + assert_eq!( + link_path_style(false, Some(Path::new("/home/u/proj"))), + PathStyle::Posix, + "an SSH host, a remote workspace or a WSL distro reporting a \ + /-rooted cwd is a POSIX one on every client" + ); + assert_eq!( + link_path_style(false, Some(Path::new(r"C:\Users\u\proj"))), + PathStyle::Windows, + "and a remote Windows host is not" + ); + assert_eq!( + link_path_style(false, None), + PathStyle::Posix, + "a remote pane that has not said where it is still has no local \ + drive its paths could hang off" + ); + } + #[test] fn a_panes_host_is_its_workspaces_machine() { use crate::core::session::{RemoteTarget, WorkspaceId}; @@ -9500,7 +9570,7 @@ mod gpui_tests { LinkAt::Unresolved { candidate, pending } => { assert_eq!(candidate.path, "scratchpad/gone.md"); assert!( - candidate.looks_like_a_path(), + candidate.looks_like_a_path(view.link_path_style()), "so the click reports it instead of staying silent" ); assert!(!pending, "a local pane answers on the spot"); diff --git a/src/ui/app.rs b/src/ui/app.rs index 032c787a..f1b0ab92 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 }); @@ -1484,6 +1463,36 @@ impl Tty7App { crate::ui::windows::open(cx, None); } + 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 @@ -4912,6 +4921,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), @@ -7411,6 +7421,9 @@ impl Render for Tty7App { .on_action(cx.listener(|this, _: &NewWindow, _window, cx| { this.new_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) @@ -10381,3 +10394,83 @@ mod new_window_action_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/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/i18n/en.rs b/src/ui/i18n/en.rs index 494a0faf..f2b86352 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -437,7 +437,8 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::SettingsMouseZoomOff => "Off", L10nKey::SettingsReportMouseToApps => "Report mouse to apps", L10nKey::SettingsReportMouseToAppsDesc => { - "Let full-screen apps (vim, tmux) handle clicks and scrolling; hold Shift to keep a gesture local." + "Let full-screen apps (vim, tmux) handle clicks and scrolling; hold Shift to keep a gesture local. \ + Off keeps clicks from reaching them and turns the wheel into arrow keys." } L10nKey::SettingsBell => "Bell", L10nKey::SettingsTerminalBell => "Terminal bell", @@ -1431,6 +1432,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", @@ -1507,7 +1510,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 896cafb4..f9f4e28d 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -448,7 +448,8 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::SettingsMouseZoomOff => "オフ", L10nKey::SettingsReportMouseToApps => "マウスイベントをアプリに報告", L10nKey::SettingsReportMouseToAppsDesc => { - "フルスクリーンアプリ(vim、tmux)にクリックとスクロールを処理させる。Shift を押している間はローカルで処理されます" + "フルスクリーンアプリ(vim、tmux)にクリックとスクロールを処理させる。Shift を押している間はローカルで処理されます。\ + オフにするとクリックは届かず、ホイールは矢印キーとして送られます" } L10nKey::SettingsBell => "ベル通知", L10nKey::SettingsTerminalBell => "ターミナルベル", @@ -1488,6 +1489,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 => "閉じたタブをもう一度開く", @@ -1562,7 +1565,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 f8b27fef..7a18b837 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -1151,6 +1151,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 fabd615f..d3ade9ac 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -385,7 +385,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::SettingsMouseZoomOff => "关闭", L10nKey::SettingsReportMouseToApps => "向应用报告鼠标", L10nKey::SettingsReportMouseToAppsDesc => { - "让全屏应用(如 vim、tmux)处理点击和滚动;按住 Shift 可让操作保持本地。" + "让全屏应用(如 vim、tmux)处理点击和滚动;按住 Shift 可让操作保持本地。\ + 关闭后点击不再传给它们,滚轮也会变成方向键。" } L10nKey::SettingsBell => "铃声", L10nKey::SettingsTerminalBell => "终端铃声", @@ -1349,6 +1350,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 => "重新打开已关闭标签页", @@ -1423,7 +1426,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 f5c5f27f..e7f0015c 100644 --- a/src/ui/keymap.rs +++ b/src/ui/keymap.rs @@ -285,6 +285,7 @@ pub(crate) fn default_bindings() -> Vec<(&'static str, &'static str)> { // shipping unbound: the palette and the Keybindings page both carry // this, so a key is one line of config away. ("NewWindow", per_platform("secondary-n", "")), + ("CloseWindow", ""), ( "CloseActiveTab", per_platform("secondary-w", "secondary-shift-w"), @@ -757,6 +758,10 @@ fn authored_entry(action: &str) -> Option<(CommandGroup, String)> { CommandGroup::Application, t(L10nKey::CmdNewWindow).to_string(), ), + "CloseWindow" => ( + CommandGroup::Application, + t(L10nKey::CmdCloseWindow).to_string(), + ), "OpenSettings" => ( CommandGroup::Application, t(L10nKey::CmdSettings).to_string(), @@ -1061,6 +1066,7 @@ fn make_binding(action: &str, keystroke: &str) -> Option { "RenameWorkspace" => KeyBinding::new(keystroke, RenameWorkspace, None), "ToggleSwitcher" => KeyBinding::new(keystroke, ToggleSwitcher, None), "NewWindow" => KeyBinding::new(keystroke, NewWindow, 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), @@ -1243,6 +1249,7 @@ mod tests { assert_eq!(action_entry("ToggleMaximizePane").1, "Zoom Pane"); assert_eq!(action_entry("CloseActiveTab").1, "Close Pane / Tab"); assert_eq!(action_entry("NewWindow").1, "New Window"); + 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/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/palette.rs b/src/ui/palette.rs index 09c03a7c..3d05371f 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -23,6 +23,7 @@ pub enum CommandKind { StopWorkspace, DeleteWorkspace, NewWindow, + CloseWindow, SplitRight, SplitDown, ClosePane, @@ -126,6 +127,7 @@ impl CommandKind { StopWorkspace => "stop-workspace", DeleteWorkspace => "delete-workspace", NewWindow => "new-window", + CloseWindow => "close-window", SplitRight => "split-right", SplitDown => "split-down", ClosePane => "close-pane", @@ -233,6 +235,7 @@ impl CommandKind { StopWorkspace => "StopWorkspace", DeleteWorkspace => "DeleteWorkspace", NewWindow => "NewWindow", + CloseWindow => "CloseWindow", SplitRight => "SplitRight", SplitDown => "SplitDown", ClosePane => "CloseActiveTab", @@ -578,6 +581,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)), ]; 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(), 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()