From 43961cae207a84b305456f2aefea7eeb50a025e2 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 16 Aug 2026 05:28:01 +0800 Subject: [PATCH] docs: restore seven invariants that the comment strip took out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #268 ("strip every comment from the Rust sources") removed 38,164 comment lines. Much has been rewritten since, but 7,587 blocks still sit in front of code that is unchanged and now undocumented. Last commit I started implementing a pair of deliberate no-op stubs before `git log -L` turned up the stripped comment saying they were deliberate — which is the failure mode this class of loss produces. Seven restored here, chosen for documenting why something must not change rather than what it does: - `LinkShutdown` — "not optional politeness; without it a client cannot be closed". A reader parked in a blocking read is not woken by any flag, so closing has to act on the fd, and no std trait spans the transports. Nothing else says this. - `peek_frame_kind` / `is_error_kind` — why Attach is classified before the payload is paid for. - `FontFeatures` — a frozen config key deliberately replicating gpui's type so tty7-core parses without linking gpui. - `desired_tabs` / `every_leaf_is_native_ssh` — held vs permanently-invisible tabs, and why conflating them either deletes a daemon tab mid-revival or freezes a window's ordering forever. - `control_for` / `TreeLink` / `classify_tree_link` — local and remote links unified, and unserved as a fact about the peer rather than a transient down. Restored against the current code, not verbatim: three claims had gone stale and are corrected. `Duplex` has since adopted `LinkShutdown` (`Halves` carries one) rather than needing to; `peek_frame_kind` has two callers now, not one; and `gpui_font_features` moved out of `ui::app`. --- crates/tty7-core/src/core/config.rs | 19 ++++++++++++ crates/tty7-core/src/daemon/control.rs | 21 +++++++++++++ crates/tty7-core/src/daemon/protocol.rs | 11 +++++++ src/ui/tree_sync.rs | 40 +++++++++++++++++++++++++ 4 files changed, 91 insertions(+) diff --git a/crates/tty7-core/src/core/config.rs b/crates/tty7-core/src/core/config.rs index 9b348c86..e071896e 100644 --- a/crates/tty7-core/src/core/config.rs +++ b/crates/tty7-core/src/core/config.rs @@ -6,6 +6,25 @@ use serde::{Deserialize, Serialize}; pub(crate) const SUPPORTED_GUI_LANGUAGES: &[&str] = &["en", "zh-CN", "ja-JP"]; +/// The OpenType features configured for terminal text, as an ordered tag → value +/// list (`[("calt", 1), ("liga", 1)]`). +/// +/// This is a deliberate, behavior-identical replica of `gpui::FontFeatures`: the +/// field it backs is a real key in the user's `config.json`, so its wire format +/// is frozen, but `Config` itself has to parse on a headless machine that never +/// links gpui. The GUI crate converts this into the gpui type in its own +/// `core::config::gpui_font_features`, and a test beside that function pins the +/// two serializations together. +/// +/// Wire format, matching gpui byte for byte: +/// - a JSON object of four-character alphanumeric tags to `true` / `false` / +/// a non-negative integer; +/// - `true` → 1, `false` → 0, an integer passes through; +/// - a tag that isn't four alphanumeric characters, a negative or fractional +/// value, or a `null` value is logged and skipped rather than failing the +/// whole config parse; +/// - serialization always writes integers, so `{"calt":true}` round-trips as +/// `{"calt":1}`. #[derive(Default, Clone, Eq, PartialEq, Hash)] pub struct FontFeatures(pub Arc>); diff --git a/crates/tty7-core/src/daemon/control.rs b/crates/tty7-core/src/daemon/control.rs index 42dad2ea..388afff8 100644 --- a/crates/tty7-core/src/daemon/control.rs +++ b/crates/tty7-core/src/daemon/control.rs @@ -939,7 +939,28 @@ pub struct ControlResponse { pub type EventSink = Box; +/// Whatever can force a parked reader out of a blocking `read`. +/// +/// **This is not optional politeness; without it a client cannot be closed.** +/// The reader thread spends its whole life inside `read_frame`, which blocks +/// until the peer sends something. Setting a "we're closed now" flag does not +/// wake it, because nothing is looking at the flag — the thread is inside a +/// syscall. And the peer has no reason to send anything: it is waiting for the +/// next request. Both ends wait for the other forever. +/// +/// So closing has to act on the file descriptor itself. Every transport has +/// *some* way to do that, but they share no trait in std — a socket has +/// `shutdown`, a child process has `kill`, an SSH channel has `close` — hence +/// this one-method abstraction rather than a bound on the stream type. +/// +/// The server side has the same problem in mirror image, and resolves it with +/// this same trait rather than a parallel one: every +/// [`Duplex::split`](crate::daemon::duplex::Duplex::split) hands back a +/// [`Halves`](crate::daemon::duplex::Halves) carrying an +/// `Arc` for its own read half. pub trait LinkShutdown: Send + Sync + 'static { + /// Force the read half to return. Called at most once, and may be called + /// while the reader is blocked inside `read`. fn shutdown_link(&self) -> io::Result<()>; } diff --git a/crates/tty7-core/src/daemon/protocol.rs b/crates/tty7-core/src/daemon/protocol.rs index 65d05412..6e0e4737 100644 --- a/crates/tty7-core/src/daemon/protocol.rs +++ b/crates/tty7-core/src/daemon/protocol.rs @@ -942,10 +942,21 @@ pub fn read_frame(r: &mut R) -> io::Result<(u8, Vec)> { Ok((kind[0], payload)) } +/// The kind byte of the frame at the front of `buf`, once its 5-byte header has +/// arrived — the payload need not have. +/// +/// For the callers that have to classify a reply *before* paying for it. The +/// motivating case is `Attach`, answered either by a tiny `Error` or by a +/// `Size` + `Snapshot` replay that can run to megabytes: waiting for the whole +/// first frame to tell them apart would stall every successful attach behind +/// its own scrollback. pub fn peek_frame_kind(buf: &[u8]) -> Option { (buf.len() >= 5).then(|| buf[4]) } +/// Whether `kind` is the [`DaemonMsg::Error`] frame. The kind bytes themselves +/// stay private — this is the one classification a client makes without +/// decoding, and naming it keeps the numbering in one file. pub fn is_error_kind(kind: u8) -> bool { kind == kind::ERROR } diff --git a/src/ui/tree_sync.rs b/src/ui/tree_sync.rs index 2acf29a7..05ffe1d4 100644 --- a/src/ui/tree_sync.rs +++ b/src/ui/tree_sync.rs @@ -16,6 +16,15 @@ use crate::ui::app::Tty7App; use crate::ui::i18n::{L10nKey, t}; use crate::ui::pane::{Pane, PaneSlot}; +/// The control link to `host`'s daemon, if one is up right now. +/// +/// The unification the whole design leans on: the local machine's link lives in +/// [`LocalLink`](crate::ui::local_link::LocalLink), a remote machine's in +/// [`HostLinks`](crate::ui::remote_connect::HostLinks), and +/// everything above this function stops caring which. `None` is always +/// transient (both holders have supervisors reconnecting), so callers treat it +/// as "not now": mark dirty and let the re-pull that follows reconnection +/// resend what still matters. pub(crate) fn control_for(cx: &mut App, host: HostId) -> Option> { if host.is_local() { crate::ui::local_link::LocalLink::client(cx) @@ -26,6 +35,16 @@ pub(crate) fn control_for(cx: &mut App, host: HostId) -> Option), Unserved, @@ -36,6 +55,8 @@ pub(crate) fn tree_control_for(cx: &mut App, host: HostId) -> TreeLink { classify_tree_link(control_for(cx, host)) } +/// The judgement half of [`tree_control_for`]: what the handshake's +/// capability bits say this link is good for. fn classify_tree_link(client: Option>) -> TreeLink { match client { Some(client) @@ -108,6 +129,19 @@ impl DesiredNode { } } +/// Read the window's tabs into the daemon's shape. Tabs with nothing +/// representable yet (every pane still spawning) are omitted from the desired +/// list — but their identities are answered separately as *held*: the tab is +/// occupied, its panes just have no ids yet, and a diff that read its absence +/// as "closed" would delete the daemon tab (and spend the very records) a +/// revival in flight is about to replace. +/// +/// Held is strictly for the *transient* case. A remote window's tab that is +/// native-SSH through and through is unrepresentable **forever** — its panes +/// live in this client's daemon — and is neither desired nor held: as far as +/// this machine's tree is concerned, it does not exist. Holding it instead +/// would freeze the whole window's ordering and active-tab sync permanently, +/// because [`diff`] waits out held tabs before touching either. pub(crate) fn desired_tabs( app: &Tty7App, cx: &App, @@ -143,6 +177,12 @@ pub(crate) fn desired_tabs( (out, active, held) } +/// Whether every leaf of `pane` is a *ready* native-SSH view — the one kind +/// of leaf a remote window can never name in its machine's tree, because the +/// pane lives in this client's own daemon. Only meaningful for a tab whose +/// desired root came out `None`: it decides permanently-invisible versus +/// held (see [`desired_tabs`]). A connecting or empty leaf answers `false` — +/// those are pending, not foreign. fn every_leaf_is_native_ssh(pane: &Pane, cx: &App) -> bool { match pane { Pane::Leaf(PaneSlot::Ready(view)) => view.read(cx).ssh_spec().is_some(),