From 44d33031e19633b5ed1b6291c091cfc02d318566 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:26:42 +0700 Subject: [PATCH 1/7] fix(terminal): stop repairing a parked cursor on a raw pty (#430) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Typing `:wq!` in vim wrote it onto the row being edited instead of the command line, and left the cursor there. The command line is not what moved: the cursor is. `ParkedCursorRepair` (#362) exists for conhost, which brackets every frame it paints with `?25l` … `?25h` and, on the frames where it did not paint the cursor, commits the show wherever the last erase or write left it. The repair pairs the hide with its show, calls the show parked when the run moved the cursor around to paint but did not end on a move, and puts the cursor back on the cell the hide caught it on. The reader ran it on every platform. Off Windows there is no conhost in between and the application owns the cursor. Captured from vim 9.1 on a raw macOS pty, opening the command line is \x1b[?25l \x1b[11;10H: \x1b[1;1H \x1b[11;1H \x1b[K \x1b[11;1H: \x1b[?25h — a run full of moves that ends on the `:` it wrote, which is exactly the shape the scanner calls parked. So the repair dragged the cursor off the command line and back onto the file, and vim, which echoes the following keystrokes as bare bytes with no positioning of their own, wrote `wq!` over the text. Gate the repair on `cfg!(windows)`, the way `conpty_resize` already is: the artifact is ConPTY's, and on a raw pty the cell a frame leaves the cursor on is the cell the application meant. This also keeps the scanner off the client's output path entirely on Unix, where it used to walk every batch. A macOS or Linux client attached to a *remote* Windows daemon loses the repair with it — the same limit `conpty_resize` has — which costs a stray caret there and buys back an unshredded screen on every local pane. --- src/terminal/parked_cursor.rs | 10 +++- src/terminal/remote.rs | 104 +++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/src/terminal/parked_cursor.rs b/src/terminal/parked_cursor.rs index 8b3fbd25..facad126 100644 --- a/src/terminal/parked_cursor.rs +++ b/src/terminal/parked_cursor.rs @@ -10,13 +10,19 @@ //! on every batch of pty output) draws that stray cursor for a frame, and a //! TUI that repaints on a spinner — Codex while it works — produces one every //! spinner tick, which reads as a second cursor blinking in the wrong place. -//! macOS never shows it: no ConPTY sits in between, and the TUI itself always -//! moves the cursor before it shows it. //! //! [`ParkedCursorScanner`] finds those hide/show pairs in the byte stream and //! [`ParkedCursorRepair`] restores the cell the cursor stood on when it went //! invisible, which is the cell the correcting frame would have moved it back //! to anyway. +//! +//! Only conhost parks a cursor, so the reader runs this on Windows alone +//! (`RemoteTerminal::REPAIR_PARKED_CURSOR`). Off it the pty is raw and the +//! application's cursor is the real one: a TUI is free to end a repaint on the +//! text it just wrote and then echo the next keystroke straight after it, with +//! no positioning of its own — vim opens its `:` command line exactly that way, +//! which a repair on a raw pty turns into `wq!` landing on the row being edited +//! (#430). use std::time::{Duration, Instant}; diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 31e64d31..7917d5cf 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -553,6 +553,17 @@ impl RemoteTerminal { term.set_options(terminal_config_from_user(user_config)); } + /// Whether this build puts back the cursor a repaint parked — see + /// [`crate::terminal::parked_cursor`]. + /// + /// Only conhost parks one, so like `conpty_resize` the repair is Windows' + /// alone. On a raw pty the application owns the cursor and is free to end a + /// repaint on the text it just wrote and then echo the next keystroke + /// straight after it, with no positioning of its own: vim opens its command + /// line that way, and putting the cursor back on the cell the repaint hid it + /// on drops the `wq!` typed next onto the row being edited (#430). + const REPAIR_PARKED_CURSOR: bool = cfg!(windows); + fn spawn_reader( term: Arc>>, proxy: EventProxy, @@ -637,7 +648,9 @@ impl RemoteTerminal { // emulator to the cut, act on the state that // sequence left behind, carry on. let mut cuts: Vec<(usize, CursorCut)> = Vec::new(); - cursor_scan.feed(&out_batch, |off, c| cuts.push((off, c))); + if Self::REPAIR_PARKED_CURSOR { + cursor_scan.feed(&out_batch, |off, c| cuts.push((off, c))); + } { let t0 = trace.then(std::time::Instant::now); let mut term = term.lock(); @@ -3067,6 +3080,7 @@ mod tests { /// Feeds one conhost-shaped repaint and reports the cell the cursor ends on, /// waiting for the `X` the frame paints so the reader is known to be done. fn cursor_after_conpty_frame(frame: &[u8]) -> (i32, usize) { + crate::core::config::pin_test_config_dir(); let (client_side, mut daemon_side) = UnixStream::pair().unwrap(); let term = RemoteTerminal::from_stream(client_side, TermSize::new(80, 24)).unwrap(); @@ -3097,14 +3111,24 @@ mod tests { #[test] fn a_conpty_frame_that_shows_the_cursor_over_an_erase_keeps_the_cell_it_hid_on() { - assert_eq!( - cursor_after_conpty_frame( - b"\x1b[?25l\x1b[20;2HX\x1b[K\x1b[m\x1b[22;42H\x1b[K\x1b[?25h" - ), - (5, 3), - "conhost parked the cursor on the cell it erased last; the cursor \ - belongs where it was when the repaint hid it" + let got = cursor_after_conpty_frame( + b"\x1b[?25l\x1b[20;2HX\x1b[K\x1b[m\x1b[22;42H\x1b[K\x1b[?25h", ); + if RemoteTerminal::REPAIR_PARKED_CURSOR { + assert_eq!( + got, + (5, 3), + "conhost parked the cursor on the cell it erased last; the cursor \ + belongs where it was when the repaint hid it" + ); + } else { + assert_eq!( + got, + (21, 41), + "with no conhost in between the stream is the application's own, \ + and the cell it left the cursor on is the cell it meant" + ); + } } #[test] @@ -3118,6 +3142,70 @@ mod tests { ); } + /// Issue #430. Vim opens its command line with exactly the shape the parked + /// -cursor scanner calls parked — hide, move around to paint, end on the `:` + /// it wrote — and then echoes every following keystroke as a bare byte at + /// wherever that left the cursor. Putting the cursor back on a raw pty + /// therefore does not straighten out a stray caret, it drops `wq!` onto the + /// row vim was editing. Bytes below are a capture of vim 9 on a 20x11 pty. + #[test] + fn a_raw_pty_repaint_keeps_the_cursor_the_frame_left_so_the_echo_lands_on_it() { + crate::core::config::pin_test_config_dir(); + let (client_side, mut daemon_side) = UnixStream::pair().unwrap(); + let term = RemoteTerminal::from_stream(client_side, TermSize::new(20, 11)).unwrap(); + + let mut stream: Vec = Vec::new(); + // `vim test.md`: the alternate screen, the file, cursor home. + stream.extend_from_slice(b"\x1b[?1049h\x1b[H\x1b[2J\x1b[1;1H123456789\x1b[1;1H"); + // Esc, then `:` — two bracketed repaints, the second ending on the `:` + // vim wrote at the head of the command line. + stream.extend_from_slice(b"\x1b[?25l\x1b[m\x1b[11;10H^[\x1b[1;1H\x1b[?25h"); + stream.extend_from_slice(b"\x1b[?25l\x1b[11;10H \x1b[1;1H\x07\x1b[?25h"); + stream.extend_from_slice( + b"\x1b[?25l\x1b[11;10H:\x1b[1;1H\x1b[11;1H\x1b[K\x1b[11;1H:\x1b[?25h", + ); + // `w`, `q`, `!`: vim echoes them with no positioning of their own. + stream.extend_from_slice(b"wq!"); + DaemonMsg::Output(stream).encode(&mut daemon_side).unwrap(); + daemon_side.flush().unwrap(); + + let row = |t: &Term, line: i32| -> String { + (0..20) + .map(|col| { + t.grid()[alacritty_terminal::index::Line(line)] + [alacritty_terminal::index::Column(col)] + .c + }) + .collect::() + .trim_end() + .to_string() + }; + + // The whole batch is applied under one lock, so the `:` landing on the + // command line means every byte after it landed too. + let mut command_line = String::new(); + let mut edited = String::new(); + for _ in 0..600 { + { + let t = term.term.lock(); + command_line = row(&t, 10); + edited = row(&t, 0); + } + if command_line.starts_with(':') { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert_eq!( + command_line, ":wq!", + "the keystrokes belong after the `:` the repaint ended on" + ); + assert_eq!( + edited, "123456789", + "and nothing of them belongs on the row vim was editing" + ); + } + #[test] fn layout_resize_reasserts_geometry_after_a_late_size_frame() { use alacritty_terminal::grid::Dimensions as _; From 68d6b4b0626dff5d88f22d18696e46824bda167d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:12:51 +0700 Subject: [PATCH 2/7] feat(terminal): zoom the font with the platform modifier and the wheel Holding Cmd (Ctrl off macOS) and scrolling over a terminal now resizes the font instead of the scrollback, which is what you reach for when showing a pane to someone else. A wheel detent is one step whatever the platform bills it as, and a trackpad accumulates until the fingers have travelled three lines, so a flick does not run the font end to end. Steps go out as the existing IncreaseFontSize/DecreaseFontSize actions, so the clamp and the saved setting stay in one place. --- docs/features.md | 1 + docs/features.zh-CN.md | 1 + src/terminal/view.rs | 115 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 2 deletions(-) diff --git a/docs/features.md b/docs/features.md index f9879ca9..282aed19 100644 --- a/docs/features.md +++ b/docs/features.md @@ -106,6 +106,7 @@ Keys are shown in macOS notation — on Windows and Linux, read ⌘ a | ⌘ F | search the scrollback | | ⌃ R | fuzzy-search shell history | | ⌘ + · ⌘ − · ⌘ 0 | font size up · down · reset | +| ⌘ + wheel | zoom the font by scrolling over a terminal | **Settings → Keybindings** (⌘ ,) lists every shortcut. Click one, press the new keys (Esc cancels, Backspace resets to diff --git a/docs/features.zh-CN.md b/docs/features.zh-CN.md index 8c205e7a..a66b6293 100644 --- a/docs/features.zh-CN.md +++ b/docs/features.zh-CN.md @@ -103,6 +103,7 @@ Aider、Amp、OpenCode 等共 18 个）并在其外围加功能 —— 绝不包 | ⌘ F | 搜索 scrollback | | ⌃ R | 模糊搜索 shell 历史 | | ⌘ + · ⌘ − · ⌘ 0 | 字号增大 · 减小 · 重置 | +| ⌘ + 滚轮 | 在终端上滚动缩放字号，演示时随手放大 | **设置 → 按键绑定**（⌘ ,）列出全部快捷键。点一行、按下新键即可 （Esc 取消，Backspace 恢复默认），改完立即生效。窗格缩放与 diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 0e488b8b..953618e2 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -23,8 +23,9 @@ use super::reverse_search::{self, ReverseSearch}; use super::search::{LinkTarget, SearchState}; use super::typeahead::{RawInput, Typeahead}; use crate::core::actions::{ - CloseActiveTab, ForkAgentSessionDown, ForkAgentSessionLeft, ForkAgentSessionRight, - ForkAgentSessionUp, NewTab, SendBackTab, SendTab, SplitDown, SplitRight, ToggleMaximizePane, + CloseActiveTab, DecreaseFontSize, ForkAgentSessionDown, ForkAgentSessionLeft, + ForkAgentSessionRight, ForkAgentSessionUp, IncreaseFontSize, NewTab, SendBackTab, SendTab, + SplitDown, SplitRight, ToggleMaximizePane, }; use crate::core::config::{BellMode, Config, NotifyMode}; use crate::daemon::protocol::{RemoteContext, ShellSpec}; @@ -113,6 +114,10 @@ const SCROLL_ANIM_MIN_JUMP: f32 = 1.0; /// to bridge the gaps in a momentum tail, short enough that reaching for the /// wheel right after a swipe is not mistaken for more of the swipe. const SCROLL_GESTURE_IDLE: std::time::Duration = std::time::Duration::from_millis(150); +/// How far a trackpad has to travel, in lines, to earn one font-size step while +/// the platform modifier is held. A wheel detent is one step on its own, so +/// this only ever applies to the continuous stream a gesture produces. +const ZOOM_SCROLL_LINES: f32 = 3.0; fn cwd_is_on_host(pane_runs_remotely: bool, host_is_local: bool) -> bool { match pane_runs_remotely { @@ -160,6 +165,11 @@ pub struct TerminalView { /// modifiers the user actually held. context_menu_allowed: bool, scroll_debt: f32, + /// Lines travelled under the zoom modifier that have not yet added up to a + /// font-size step. Kept apart from [`scroll_debt`](Self::scroll_debt) so + /// letting go of the modifier mid-gesture cannot hand the leftovers of one + /// to the other. + zoom_debt: f32, pub(super) scroll_frac: f32, pub search: Option, pub cursor_visible: bool, @@ -1095,6 +1105,7 @@ impl TerminalView { link_modifier_down: false, context_menu_allowed: true, scroll_debt: 0., + zoom_debt: 0., scroll_frac: 0., search: None, cursor_visible: true, @@ -4259,6 +4270,14 @@ impl TerminalView { } fn on_scroll(&mut self, ev: &ScrollWheelEvent, window: &mut Window, cx: &mut Context) { + // The platform modifier alone turns the wheel into a zoom, the way it + // does in a browser. Any other modifier alongside it is somebody else's + // gesture — shift in particular is the escape hatch that scrolls the + // scrollback out from under a mouse-reporting program. + if ev.modifiers.secondary() && ev.modifiers.number_of_modifiers() == 1 { + self.zoom_scroll(ev, window, cx); + return; + } let mult = cx.global::().mouse_scroll_multiplier; let raw = match ev.delta { ScrollDelta::Lines(p) => p.y, @@ -4293,6 +4312,34 @@ impl TerminalView { } } + /// Resize the terminal font by whole steps under the platform modifier. + /// + /// The event never reaches the buffer, and never reaches the program + /// running in it either: zooming is chrome, and showing a pane to someone + /// standing behind you has to work the same whether or not what is running + /// asked for the wheel. Steps go out as the same actions the keyboard and + /// the View menu send, so the min/max clamp and the saved setting live in + /// one place — [`Tty7App::change_font_size`](crate::ui::app::Tty7App). + fn zoom_scroll(&mut self, ev: &ScrollWheelEvent, window: &mut Window, cx: &mut Context) { + // Whatever the scrollback still had in flight is dropped: it was + // travelling in lines of a font that is about to change size. + self.cancel_scroll_anim(); + let lines = match ev.delta { + ScrollDelta::Lines(p) => p.y, + ScrollDelta::Pixels(p) => p.y.as_f32() / self.line_height.as_f32(), + }; + let gesturing = self.track_scroll_gesture(ev.touch_phase); + let (steps, debt) = zoom_scroll_steps(lines, self.zoom_debt, gesturing); + self.zoom_debt = debt; + for _ in 0..steps.unsigned_abs() { + if steps > 0 { + window.dispatch_action(Box::new(IncreaseFontSize), cx); + } else { + window.dispatch_action(Box::new(DecreaseFontSize), cx); + } + } + } + /// Track whether the pointing device is mid-gesture, which is what tells a /// trackpad apart from a wheel. /// @@ -5852,6 +5899,30 @@ fn wrapped_click_index( } } +/// How many font-size steps a zoom event is worth, and what is left over for +/// the next one. +/// +/// A wheel detent is a discrete click of intent, so it is one step whatever the +/// platform says it covers — macOS calls a single notch five lines, and five +/// points of font per notch would be unusable. A trackpad has no detents and +/// spends a flick over dozens of events, so those accumulate and only pay out +/// once the fingers have travelled [`ZOOM_SCROLL_LINES`]. +fn zoom_scroll_steps(lines: f32, debt: f32, gesturing: bool) -> (i32, f32) { + if !gesturing { + let step = if lines > 0. { + 1 + } else if lines < 0. { + -1 + } else { + 0 + }; + return (step, 0.); + } + let total = debt + lines; + let steps = (total / ZOOM_SCROLL_LINES).trunc(); + (steps as i32, total - steps * ZOOM_SCROLL_LINES) +} + fn smooth_scroll_step(offset: usize, frac: f32, delta: f32, max: usize) -> (i32, f32) { let pos = (offset as f32 + frac + delta).clamp(0., max as f32); let new_offset = pos.floor(); @@ -8952,6 +9023,46 @@ mod gpui_tests { .unwrap(); } + /// Zooming has to take the wheel away from the buffer entirely, or the + /// grid would slide under the pointer while the font changed size. + #[gpui::test] + fn the_zoom_modifier_takes_the_wheel_off_the_scrollback(cx: &mut TestAppContext) { + let (window, _daemon) = harness(cx); + window + .update(cx, |view, w, cx| { + scroll_into_history(view, 10); + let mut ev = notch(view, -4.9); + ev.modifiers = Modifiers::secondary_key(); + view.on_scroll(&ev, w, cx); + assert_eq!(display_offset(view), 10, "the wheel reached the grid"); + assert!(view.scroll_anim.is_none(), "and queued more of it"); + }) + .unwrap(); + } + + /// A detent is one step however many lines the platform bills it as — + /// macOS calls a single notch five. + #[test] + fn a_wheel_detent_zooms_by_exactly_one_step() { + assert_eq!(zoom_scroll_steps(4.9, 0., false), (1, 0.)); + assert_eq!(zoom_scroll_steps(-4.9, 0., false), (-1, 0.)); + assert_eq!(zoom_scroll_steps(0., 0., false), (0, 0.)); + } + + /// A trackpad has no detents, so a flick arrives as a stream of slivers. + /// Paying out a step per sliver would run the font from end to end. + #[test] + fn a_trackpad_flick_adds_up_to_whole_steps() { + let (mut debt, mut steps) = (0., 0); + for _ in 0..20 { + let (s, d) = zoom_scroll_steps(1. / 3., debt, true); + steps += s; + debt = d; + } + assert_eq!(steps, 2, "twenty thirds of a line is two steps, not twenty"); + assert!(debt > 0., "and the remainder was dropped instead of kept"); + } + /// The one thing typing at a prompt must never do is land where the /// person typing cannot see it. #[gpui::test] From 54960be6bf33537db7642ef3d89f8c23e064b044 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:11:37 +0700 Subject: [PATCH 3/7] feat(palette): match commands in every locale, not just the shown one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The palette matched a query against the label it was rendering and nothing else, so a window running in Chinese answered "no matching commands" to `theme` — the English wording of that row did not exist anywhere the filter could see it. Each entry built from a locale key now carries the same key as every other locale words it, plus the stable command id, as hidden search aliases. They are built once with the entry, cost nothing per keystroke, and are never rendered: the row keeps showing its localized label, and an alias hit scores just below the same hit on that label so a visible match still comes first. --- src/ui/i18n/mod.rs | 19 ++++ src/ui/palette.rs | 217 ++++++++++++++++++++++++++++----------------- 2 files changed, 155 insertions(+), 81 deletions(-) diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index 7379833b..5b5fb606 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -1127,6 +1127,25 @@ pub fn t_fmt(key: L10nKey, args: &[(&str, &str)]) -> String { apply_template(t(key), args, None) } +/// The same key as every *other* locale words it. +/// +/// Fingers learn a command's name once. Someone running the UI in Chinese who +/// picked the habit up in English still types `theme`, and until this existed +/// the palette answered "no matching commands" — the label it was matching +/// against was the only one that existed. Callers keep these as hidden search +/// aliases; nothing renders them, so the wording on screen never changes. +pub fn alias_translations(key: L10nKey) -> Vec<&'static str> { + let shown = t(key); + let mut out = Vec::with_capacity(SUPPORTED_LANGUAGES.len() - 1); + for idx in 0..SUPPORTED_LANGUAGES.len() { + let text = translate(idx, key); + if text != shown && !out.contains(&text) { + out.push(text); + } + } + out +} + #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum PluralCategory { Zero, diff --git a/src/ui/palette.rs b/src/ui/palette.rs index 9dbfd769..7d938d28 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -12,7 +12,7 @@ use uuid::Uuid; use crate::core::config::{Config, RightPanelTab, TabBarPosition}; use crate::core::ssh_profile::parse_quick_connect; -use crate::ui::i18n::{L10nKey, t, t_fmt}; +use crate::ui::i18n::{L10nKey, alias_translations, t, t_fmt}; #[derive(Clone, PartialEq, Eq)] pub enum CommandKind { @@ -317,6 +317,10 @@ pub struct ChromeState { pub struct Command { pub title: String, pub subtitle: Option, + /// Text the query may match but the row never shows: the same label as + /// every other locale words it, plus the stable command id. Built once, + /// with the entry — the filter runs on every keystroke. + pub aliases: Vec<&'static str>, pub kind: CommandKind, pub group: CommandGroup, } @@ -326,11 +330,22 @@ impl Command { Self { title: title.into(), subtitle: None, + // The id is English and hyphenated ("change-theme"), which is close + // enough to what someone reaching past a translated label types. + aliases: kind.id().into_iter().collect(), kind, group: CommandGroup::Application, } } + /// A command whose label comes out of the locale table, and which can + /// therefore also be found by the wording any other locale would show. + pub fn localized(key: L10nKey, kind: CommandKind) -> Self { + let mut cmd = Self::new(t(key), kind); + cmd.aliases.extend(alias_translations(key)); + cmd + } + pub fn with_subtitle(mut self, subtitle: impl Into) -> Self { self.subtitle = Some(subtitle.into()); self @@ -349,129 +364,129 @@ impl Command { let right_panel_open = chrome.right_panel_visible; let tabs = [ - Command::new(t(L10nKey::CmdNewTab), NewTab), - Command::new(t(L10nKey::CmdNewWorktreeTab), NewWorktreeTab) + Command::localized(L10nKey::CmdNewTab, NewTab), + Command::localized(L10nKey::CmdNewWorktreeTab, NewWorktreeTab) .with_subtitle(t(L10nKey::CmdNewWorktreeTabSubtitle)), - Command::new(t(L10nKey::CmdRenameTab), RenameTab), - Command::new(t(L10nKey::CmdSplitRight), SplitRight), - Command::new(t(L10nKey::CmdSplitDown), SplitDown), - Command::new(t(L10nKey::CmdZoomPane), ToggleMaximizePane), - Command::new(t(L10nKey::CmdNextPane), NextPane), - Command::new(t(L10nKey::CmdPreviousPane), PrevPane), - Command::new(t(L10nKey::CmdFocusPaneLeft), FocusPaneLeft), - Command::new(t(L10nKey::CmdFocusPaneRight), FocusPaneRight), - Command::new(t(L10nKey::CmdFocusPaneUp), FocusPaneUp), - Command::new(t(L10nKey::CmdFocusPaneDown), FocusPaneDown), - Command::new(t(L10nKey::CmdResizePaneLeft), ResizePaneLeft), - Command::new(t(L10nKey::CmdResizePaneRight), ResizePaneRight), - Command::new(t(L10nKey::CmdResizePaneUp), ResizePaneUp), - Command::new(t(L10nKey::CmdResizePaneDown), ResizePaneDown), - Command::new(t(L10nKey::CmdSwapPaneNext), SwapPaneNext), - Command::new(t(L10nKey::CmdSwapPanePrevious), SwapPanePrev), - Command::new(t(L10nKey::CmdNextTab), NextTab), - Command::new(t(L10nKey::CmdPreviousTab), PrevTab), - Command::new(t(L10nKey::CmdCopyWorkingDirectory), CopyWorkingDirectory), - Command::new(t(L10nKey::CmdCopySessionId), CopyAgentSessionId) + Command::localized(L10nKey::CmdRenameTab, RenameTab), + Command::localized(L10nKey::CmdSplitRight, SplitRight), + Command::localized(L10nKey::CmdSplitDown, SplitDown), + Command::localized(L10nKey::CmdZoomPane, ToggleMaximizePane), + Command::localized(L10nKey::CmdNextPane, NextPane), + Command::localized(L10nKey::CmdPreviousPane, PrevPane), + Command::localized(L10nKey::CmdFocusPaneLeft, FocusPaneLeft), + Command::localized(L10nKey::CmdFocusPaneRight, FocusPaneRight), + Command::localized(L10nKey::CmdFocusPaneUp, FocusPaneUp), + Command::localized(L10nKey::CmdFocusPaneDown, FocusPaneDown), + Command::localized(L10nKey::CmdResizePaneLeft, ResizePaneLeft), + Command::localized(L10nKey::CmdResizePaneRight, ResizePaneRight), + Command::localized(L10nKey::CmdResizePaneUp, ResizePaneUp), + Command::localized(L10nKey::CmdResizePaneDown, ResizePaneDown), + Command::localized(L10nKey::CmdSwapPaneNext, SwapPaneNext), + Command::localized(L10nKey::CmdSwapPanePrevious, SwapPanePrev), + Command::localized(L10nKey::CmdNextTab, NextTab), + Command::localized(L10nKey::CmdPreviousTab, PrevTab), + Command::localized(L10nKey::CmdCopyWorkingDirectory, CopyWorkingDirectory), + Command::localized(L10nKey::CmdCopySessionId, CopyAgentSessionId) .with_subtitle(t(L10nKey::CmdCopySessionIdSubtitle)), - Command::new(t(L10nKey::CmdForkSession), ForkAgentSession) + Command::localized(L10nKey::CmdForkSession, ForkAgentSession) .with_subtitle(t(L10nKey::CmdForkSessionSubtitle)), - Command::new(t(L10nKey::CmdMarkTabAsUnread), MarkTabUnread), - Command::new(t(L10nKey::CmdClosePaneTab), ClosePane), - Command::new(t(L10nKey::CmdCloseOtherTabs), CloseOtherTabs), - Command::new(t(L10nKey::CmdCloseTabsToTheRight), CloseTabsToTheRight), - Command::new(t(L10nKey::CmdReopenClosedTab), ReopenClosedTab), + Command::localized(L10nKey::CmdMarkTabAsUnread, MarkTabUnread), + Command::localized(L10nKey::CmdClosePaneTab, ClosePane), + Command::localized(L10nKey::CmdCloseOtherTabs, CloseOtherTabs), + Command::localized(L10nKey::CmdCloseTabsToTheRight, CloseTabsToTheRight), + Command::localized(L10nKey::CmdReopenClosedTab, ReopenClosedTab), ]; let workspaces = [ - Command::new(t(L10nKey::CmdNewWorkspace), NewWorkspace), - Command::new(t(L10nKey::CmdSwitchWorkspace), OpenWorkspacePicker), - Command::new(t(L10nKey::CmdRenameWorkspace), RenameWorkspace), - Command::new(t(L10nKey::CmdStopWorkspace), StopWorkspace) + Command::localized(L10nKey::CmdNewWorkspace, NewWorkspace), + Command::localized(L10nKey::CmdSwitchWorkspace, OpenWorkspacePicker), + Command::localized(L10nKey::CmdRenameWorkspace, RenameWorkspace), + Command::localized(L10nKey::CmdStopWorkspace, StopWorkspace) .with_subtitle(t(L10nKey::CmdStopWorkspaceSubtitle)), - Command::new(t(L10nKey::CmdDeleteWorkspace), DeleteWorkspace) + Command::localized(L10nKey::CmdDeleteWorkspace, DeleteWorkspace) .with_subtitle(t(L10nKey::CmdDeleteWorkspaceSubtitle)), ]; let view = [ - Command::new( + Command::localized( if sidebar_hidden { - t(L10nKey::CmdShowLeftSidebar) + L10nKey::CmdShowLeftSidebar } else { - t(L10nKey::CmdHideLeftSidebar) + L10nKey::CmdHideLeftSidebar }, ToggleLeftPanel, ), - Command::new( + Command::localized( if right_panel_open { - t(L10nKey::CmdHideRightPanel) + L10nKey::CmdHideRightPanel } else { - t(L10nKey::CmdShowRightPanel) + L10nKey::CmdShowRightPanel }, ToggleRightPanel, ), - Command::new(t(L10nKey::CmdShowCodePanel), ToggleCodePanel), - Command::new( + Command::localized(L10nKey::CmdShowCodePanel, ToggleCodePanel), + Command::localized( if tab_bar_left { - t(L10nKey::CmdTabBarMoveToTop) + L10nKey::CmdTabBarMoveToTop } else { - t(L10nKey::CmdTabBarMoveToLeftSidebar) + L10nKey::CmdTabBarMoveToLeftSidebar }, ToggleTabSidebar, ), - Command::new( - t(L10nKey::CmdRightPanelInfo), + Command::localized( + L10nKey::CmdRightPanelInfo, ShowRightPanel(RightPanelTab::Info), ), - Command::new( - t(L10nKey::CmdRightPanelChanges), + Command::localized( + L10nKey::CmdRightPanelChanges, ShowRightPanel(RightPanelTab::Changes), ), - Command::new( - t(L10nKey::CmdRightPanelFiles), + Command::localized( + L10nKey::CmdRightPanelFiles, ShowRightPanel(RightPanelTab::Files), ), - Command::new(t(L10nKey::CmdChangeTheme), OpenThemePicker), - Command::new(t(L10nKey::CmdResetFontSize), ResetFontSize), - Command::new(t(L10nKey::CmdEnterFullScreen), ToggleFullscreen), + Command::localized(L10nKey::CmdChangeTheme, OpenThemePicker), + Command::localized(L10nKey::CmdResetFontSize, ResetFontSize), + Command::localized(L10nKey::CmdEnterFullScreen, ToggleFullscreen), ]; let terminal = [ - Command::new(t(L10nKey::CmdClearScrollback), ClearTerminal), - Command::new(t(L10nKey::CmdFindInTerminal), FindInTerminal), - Command::new(t(L10nKey::CmdFindNext), FindNext), - Command::new(t(L10nKey::CmdFindPrevious), FindPrevious), - Command::new(t(L10nKey::CmdCopy), CopyText), - Command::new(t(L10nKey::CmdCut), CutText), - Command::new(t(L10nKey::CmdPaste), PasteText), - Command::new(t(L10nKey::CmdSelectAll), SelectAllText), + Command::localized(L10nKey::CmdClearScrollback, ClearTerminal), + Command::localized(L10nKey::CmdFindInTerminal, FindInTerminal), + Command::localized(L10nKey::CmdFindNext, FindNext), + Command::localized(L10nKey::CmdFindPrevious, FindPrevious), + Command::localized(L10nKey::CmdCopy, CopyText), + Command::localized(L10nKey::CmdCut, CutText), + Command::localized(L10nKey::CmdPaste, PasteText), + Command::localized(L10nKey::CmdSelectAll, SelectAllText), ]; let ssh = [ - Command::new(t(L10nKey::CmdSshAddConnection), OpenSshConnectInput), - Command::new(t(L10nKey::CmdSshManageProfiles), OpenSshProfiles), - Command::new(t(L10nKey::CmdSshReconnect), RestartSshSession), - Command::new(t(L10nKey::CmdSshRemoteFiles), ToggleSftp), - Command::new(t(L10nKey::CmdSshPortForwarding), ShowSshForwards), + Command::localized(L10nKey::CmdSshAddConnection, OpenSshConnectInput), + Command::localized(L10nKey::CmdSshManageProfiles, OpenSshProfiles), + Command::localized(L10nKey::CmdSshReconnect, RestartSshSession), + Command::localized(L10nKey::CmdSshRemoteFiles, ToggleSftp), + Command::localized(L10nKey::CmdSshPortForwarding, ShowSshForwards), ]; let agents = [ - Command::new(t(L10nKey::CmdAgentSendSelection), SendSelectionToAgent) + Command::localized(L10nKey::CmdAgentSendSelection, SendSelectionToAgent) .with_subtitle(t(L10nKey::CmdAgentSendSelectionSubtitle)), - Command::new(t(L10nKey::CmdAgentSendGitDiffForReview), SendGitDiffToAgent) + Command::localized(L10nKey::CmdAgentSendGitDiffForReview, SendGitDiffToAgent) .with_subtitle(t(L10nKey::CmdAgentSendGitDiffSubtitle)), ]; let application = [ - Command::new(t(L10nKey::CmdSettings), OpenSettings), - Command::new(t(L10nKey::CmdKeyboardShortcuts), ShowKeyboardShortcuts), - Command::new(t(L10nKey::CmdAboutTty7), About), - Command::new(t(L10nKey::CmdCheckForUpdates), CheckForUpdates), - Command::new(t(L10nKey::CmdDocumentation), OpenDocumentation), - Command::new(t(L10nKey::CmdJoinDiscord), OpenDiscord), - Command::new(t(L10nKey::CmdReportIssue), ReportIssue), - Command::new(t(L10nKey::CmdRestartServer), RestartDaemon) + Command::localized(L10nKey::CmdSettings, OpenSettings), + Command::localized(L10nKey::CmdKeyboardShortcuts, ShowKeyboardShortcuts), + Command::localized(L10nKey::CmdAboutTty7, About), + Command::localized(L10nKey::CmdCheckForUpdates, CheckForUpdates), + Command::localized(L10nKey::CmdDocumentation, OpenDocumentation), + Command::localized(L10nKey::CmdJoinDiscord, OpenDiscord), + Command::localized(L10nKey::CmdReportIssue, ReportIssue), + Command::localized(L10nKey::CmdRestartServer, RestartDaemon) .with_subtitle(t(L10nKey::CmdRestartServerSubtitle)), - Command::new(t(L10nKey::CmdQuitTty7), Quit) + Command::localized(L10nKey::CmdQuitTty7, Quit) .with_subtitle(t(L10nKey::CmdQuitTty7Subtitle)), ]; @@ -573,6 +588,11 @@ pub fn fuzzy_score(query: &str, text: &str) -> Option { Some(score) } +/// How far behind the visible label an alias hit lands. An alias *is* the +/// command's own name, only in another language, so the gap is small — but two +/// commands that both match must still be ordered by the text on screen. +const ALIAS_PENALTY: i32 = 10; + fn command_score(query: &str, cmd: &Command) -> Option { let title = fuzzy_score(query, &cmd.title); let subtitle = cmd @@ -580,10 +600,16 @@ fn command_score(query: &str, cmd: &Command) -> Option { .as_deref() .and_then(|s| fuzzy_score(query, s)) .map(|s| s / 2 - 25); - match (title, subtitle) { - (Some(a), Some(b)) => Some(a.max(b)), - (a, b) => a.or(b), - } + // Aliases only ever decide whether a row is in the list and where it sits. + // The row keeps rendering `title` untouched, so a hit on wording the user + // cannot see can never end up underlined against the wrong characters. + let alias = cmd + .aliases + .iter() + .filter_map(|a| fuzzy_score(query, a)) + .max() + .map(|s| s - ALIAS_PENALTY); + [title, subtitle, alias].into_iter().flatten().max() } /// A bounded nudge, not a re-ranking. Two commands that match the query about @@ -1259,6 +1285,35 @@ mod tests { assert!(title_hit > subtitle_hit); } + #[test] + fn a_translated_command_is_still_found_by_its_english_name() { + crate::ui::i18n::set_locale("zh-CN"); + let cmd = Command::localized(L10nKey::CmdChangeTheme, CommandKind::OpenThemePicker); + assert!( + !cmd.title.to_lowercase().contains("theme"), + "the row shows the Chinese label: {:?}", + cmd.title + ); + assert!( + command_score("theme", &cmd).is_some(), + "an English query must find the Chinese-labelled row" + ); + assert!( + command_score("テーマ", &cmd).is_some(), + "so must the Japanese one" + ); + // The displayed label still works, and still wins: the same query + // against the locale that shows it scores higher. + let zh = command_score("更改主题", &cmd).expect("the shown label matches"); + crate::ui::i18n::set_locale("en"); + let en = Command::localized(L10nKey::CmdChangeTheme, CommandKind::OpenThemePicker); + assert!(zh > 0 && en.title.contains("Theme")); + assert!( + command_score("theme", &en).unwrap() > command_score("theme", &cmd).unwrap(), + "a hit on the visible label must outrank the same hit on an alias" + ); + } + #[test] fn stable_ids_are_unique() { let mut seen = std::collections::HashSet::new(); From 14c092b0d14440ee39ba44ad515989b1c428ae44 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:15:05 +0700 Subject: [PATCH 4/7] feat(palette): preview themes live while the picker is open Picking a theme from the command palette closed it, so finding out what a theme looks like meant reopening the palette and retyping the search for every single one. The theme picker now applies whatever row is highlighted straight to the running window and stays open: Return persists the pick, Escape or any other way of closing the palette puts the previous theme back. A preview only touches the in-memory config, so arrowing through the list never writes config.json. The picker opens on the theme already in use, so opening it changes nothing by itself. --- src/ui/app.rs | 60 +++++++++++++++++++++++++++++- src/ui/palette.rs | 94 ++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 147 insertions(+), 7 deletions(-) diff --git a/src/ui/app.rs b/src/ui/app.rs index 520e98b1..c6699904 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -413,6 +413,10 @@ pub struct Tty7App { _appearance_watch: Subscription, palette: Option>, palette_sub: Option, + /// Preset that was live when the palette's theme picker started previewing. + /// `Some` means the theme on screen is a preview that was never written to + /// disk, and closing the palette without confirming puts this one back. + theme_preview_restore: Option, pub(crate) closed: Vec, pub(crate) renaming: Option, pub(crate) worktree_prompt: Option, @@ -936,6 +940,7 @@ impl Tty7App { _appearance_watch: appearance_watch, palette: None, palette_sub: None, + theme_preview_restore: None, closed: Vec::new(), renaming: None, worktree_prompt: None, @@ -1533,6 +1538,14 @@ impl Tty7App { } pub(crate) fn set_preset(&mut self, id: &str, window: &mut Window, cx: &mut Context) { + // A confirmed pick ends any preview: there is nothing left to roll back. + self.theme_preview_restore = None; + self.write_preset(id, cx); + self.after_theme_change(window, cx); + } + + /// Points whichever preset slot is live at `id`, in memory only. + fn write_preset(&mut self, id: &str, cx: &mut Context) { let dark_now = crate::ui::theme::system_dark(cx); let cfg = cx.global_mut::(); if !cfg.theme_follow_system { @@ -1542,7 +1555,27 @@ impl Tty7App { } else { cfg.theme_preset_light = id.to_string(); } - self.after_theme_change(window, cx); + } + + /// Shows a preset for as long as the palette's theme picker is open, so + /// arrowing through the list is how you find out what a theme looks like. + /// Nothing is written to `config.json` until the pick is confirmed. + pub(crate) fn preview_preset(&mut self, id: &str, window: &mut Window, cx: &mut Context) { + if self.theme_preview_restore.is_none() { + self.theme_preview_restore = Some(crate::ui::theme::effective_preset_id(cx)); + } + self.write_preset(id, cx); + self.apply_theme_change(false, window, cx); + } + + /// Puts back the preset that was live before the preview started. A no-op + /// when nothing is being previewed. + pub(crate) fn cancel_preset_preview(&mut self, window: &mut Window, cx: &mut Context) { + let Some(id) = self.theme_preview_restore.take() else { + return; + }; + self.write_preset(&id, cx); + self.apply_theme_change(false, window, cx); } pub(crate) fn set_slot_preset( @@ -1609,9 +1642,18 @@ impl Tty7App { } fn after_theme_change(&mut self, window: &mut Window, cx: &mut Context) { + self.apply_theme_change(true, window, cx); + } + + /// Repaints everything a theme change touches. `persist` is false for a + /// palette preview, which repaints on every arrow key and must not turn + /// each of those keystrokes into a `config.json` write. + fn apply_theme_change(&mut self, persist: bool, window: &mut Window, cx: &mut Context) { apply_theme(Some(window), cx); set_menus(cx); - cx.global::().save(); + if persist { + cx.global::().save(); + } self.rebuild_theme_editor(window, cx); self.sync_window_opacity_slider(window, cx); cx.notify(); @@ -3749,16 +3791,30 @@ impl Tty7App { match ev { PaletteEvent::Confirm(kind) => { let kind = kind.clone(); + // The picker is already showing this theme; keep it through the + // close instead of reverting and re-applying it. + if matches!(kind, CommandKind::SetTheme(_)) { + self.theme_preview_restore = None; + } self.close_palette(window, cx); self.run_command(kind, window, cx); } PaletteEvent::Dismiss => self.close_palette(window, cx), + PaletteEvent::PreviewTheme(i) => { + if let Some(id) = crate::ui::presets::all(cx).get(*i).map(|t| t.id.clone()) { + self.preview_preset(&id, window, cx); + } + } + PaletteEvent::CancelThemePreview => self.cancel_preset_preview(window, cx), } } pub(crate) fn close_palette(&mut self, window: &mut Window, cx: &mut Context) { self.palette = None; self.palette_sub = None; + // A previewed theme was never persisted: closing the palette any way + // other than confirming the pick puts the old one back. + self.cancel_preset_preview(window, cx); self.focus_active(window, cx); cx.notify(); } diff --git a/src/ui/palette.rs b/src/ui/palette.rs index 7d938d28..d11bb4c2 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -520,6 +520,15 @@ impl Command { .collect() } + /// Row of the preset already in use, so the theme picker can open on it + /// instead of previewing something else the moment it is opened. + pub fn active_theme_index(cx: &App) -> Option { + let active = crate::ui::theme::effective_preset_id(cx); + crate::ui::presets::all(cx) + .iter() + .position(|p| p.id == active) + } + fn ssh_connect_command(input: &str) -> Command { let trimmed = input.trim(); let title = if trimmed.is_empty() { @@ -782,7 +791,7 @@ impl ListDelegate for PaletteDelegate { fn perform_search( &mut self, query: &str, - _window: &mut Window, + window: &mut Window, cx: &mut Context>, ) -> Task<()> { if let Some(PaletteInput::SshConnect) = self.input { @@ -831,7 +840,12 @@ impl ListDelegate for PaletteDelegate { commands, }]; } - self.selected = self.first_row(); + // Through `set_selected_index`, not by hand: the row index may not have + // moved, but the command under it has, and the theme picker previews + // the command — not the index. + self.selected = None; + let first = self.first_row(); + self.set_selected_index(first, window, cx); Task::ready(()) } @@ -949,7 +963,14 @@ impl ListDelegate for PaletteDelegate { _window: &mut Window, cx: &mut Context>, ) { + let moved = self.selected != ix; self.selected = ix; + // The list only emits `Select` for the arrow keys; a query that re-arms + // the first row moves the highlight silently. The theme picker previews + // whatever is highlighted, so it has to hear about both. + if moved && let Some(ix) = ix { + cx.emit(ListEvent::Select(ix)); + } cx.notify(); } } @@ -957,6 +978,11 @@ impl ListDelegate for PaletteDelegate { pub enum PaletteEvent { Confirm(CommandKind), Dismiss, + /// Show the theme at this preset index without persisting it: the theme + /// picker previews the highlighted row while it stays open. + PreviewTheme(usize), + /// Put back the theme that was live before the preview started. + CancelThemePreview, } #[derive(Clone, Copy, PartialEq, Eq)] @@ -970,6 +996,9 @@ pub struct PaletteView { list: Entity>, root: Vec, menu: PaletteMenu, + /// Preset index the theme picker is currently previewing, so the same + /// theme is not re-applied on every redundant selection event. + previewing: Option, _sub: Subscription, } @@ -981,6 +1010,7 @@ impl PaletteView { list, root: commands, menu: PaletteMenu::Root, + previewing: None, _sub, } } @@ -1021,8 +1051,20 @@ impl PaletteView { list } - fn show(&mut self, commands: Vec, window: &mut Window, cx: &mut Context) { + fn show( + &mut self, + commands: Vec, + selected: Option, + window: &mut Window, + cx: &mut Context, + ) { let list = Self::build_list(commands, window, cx); + if let Some(row) = selected { + list.update(cx, |state, cx| { + state.set_selected_index(Some(IndexPath::new(row)), window, cx); + state.scroll_to_selected_item(window, cx); + }); + } self._sub = cx.subscribe_in(&list, window, Self::on_list_event); self.list = list; cx.notify(); @@ -1065,7 +1107,11 @@ impl PaletteView { Some(CommandKind::OpenThemePicker) => { self.menu = PaletteMenu::Theme; let themes = Command::theme_commands(cx); - self.show(themes, window, cx); + // Open on the theme already in use: the picker previews + // the highlighted row, and merely opening it must not + // change what the window looks like. + self.previewing = Command::active_theme_index(cx); + self.show(themes, self.previewing, window, cx); } Some(CommandKind::OpenSshConnectInput) => { self.menu = PaletteMenu::SshConnect; @@ -1081,6 +1127,12 @@ impl PaletteView { } ListEvent::Cancel => { if self.menu != PaletteMenu::Root { + if self.menu == PaletteMenu::Theme { + // Backing out of the picker is not a choice: whatever + // was previewed goes back to what it was. + self.previewing = None; + cx.emit(PaletteEvent::CancelThemePreview); + } self.menu = PaletteMenu::Root; let root = self.root.clone(); let list = Self::build_root_list(root, window, cx); @@ -1091,7 +1143,15 @@ impl PaletteView { cx.emit(PaletteEvent::Dismiss); } } - ListEvent::Select(_) => {} + ListEvent::Select(ix) => { + if self.menu == PaletteMenu::Theme + && let Some(CommandKind::SetTheme(i)) = list.read(cx).delegate().command_at(*ix) + && self.previewing != Some(i) + { + self.previewing = Some(i); + cx.emit(PaletteEvent::PreviewTheme(i)); + } + } } } } @@ -1340,6 +1400,30 @@ mod tests { } } + /// The picker previews whatever row is highlighted, so it has to open on + /// the row of the theme already in use — the one carrying the check mark. + #[gpui::test] + fn the_theme_picker_opens_on_the_theme_in_use(cx: &mut gpui::TestAppContext) { + cx.update(|cx| { + let themes = crate::ui::presets::all(cx); + let last = themes.last().expect("built-in themes").id.clone(); + cx.set_global(Config(crate::core::config::CoreConfig { + theme_follow_system: false, + theme_preset: last, + ..Default::default() + })); + + let ix = Command::active_theme_index(cx).expect("the live preset is listed"); + assert_eq!(ix, themes.len() - 1); + let rows = Command::theme_commands(cx); + assert!( + rows[ix].title.ends_with('✓'), + "row {ix} ({:?}) should be the checked one", + rows[ix].title + ); + }); + } + #[test] fn dynamic_commands_have_no_id() { assert!(CommandKind::ActivateTab(2).id().is_none()); From 150f6ff76fd9197849f396cddb3e82f02e0e8329 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:17:57 +0700 Subject: [PATCH 5/7] fix(settings): stop the page being the only column that gives width back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The settings nav, the SSH host list and the theme panel were fixed widths that never yielded, so the page they frame absorbed every shortfall. In a 641pt window — the one the report came from — that ran all the way down. On SSH, 220 of nav and 280 of host list left the detail panel 141pt and its empty state painted a couple of hundred points past the right edge of the window. On Appearance with the theme panel open the page got about 125pt, and a Chinese description came out one character per line. The columns are now allocated against the window instead of asserted: each list is handed its full width, then gives back a share of whatever is missing until the page reaches 420, and no list goes below the width at which it stops being itself. Below the width where the nav, the panel and a readable page cannot all fit in one row, the theme panel stops being a column and lays itself over the page — it is a temporary layer over one choice, and Escape already closed it first. The floor the page keeps is derived rather than picked: it is what the narrowest window in the wild leaves the SSH page, the one that spends a second list, once both lists stand on their own floors. It is a target for the allocator and not a `min_w` — a floor a flex row cannot honour does not push its siblings back, it overflows, and overflow here means content painted off the window, which is the failure being fixed. What makes the floor liveable instead is that the wide controls can now shrink into it. The thresholds that decide when a row stacks are widths a label needs, so they follow the interface font size — at 24pt every label is half as wide again while the slider beside it is still 240px. The rows that were hand-rolled rather than built by `settings_row` get the same treatment: the keybinding preset and prefix rows stack at the same width, every binding row lets its label wrap and its key caps wrap to a second line, the theme card drops its preview and stops pushing "change theme" off the card, the SSH quick-connect field shrinks instead of running past the pane it sits in, and a port-forwarding rule takes two lines — or three, at the width the report came from — rather than one that does not fit. --- src/ui/app.rs | 9 +- src/ui/settings.rs | 677 +++++++++++++++++++++++++++++++++++++-------- 2 files changed, 569 insertions(+), 117 deletions(-) diff --git a/src/ui/app.rs b/src/ui/app.rs index c6699904..f97dec21 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -440,9 +440,13 @@ pub struct Tty7App { pub(crate) sidebar_dragging: Rc>, /// How much width a settings row will actually get, measured once per /// render. `settings_row` is called from page builders that never see the - /// window, and the answer differs per page — the SSH page spends 500px on - /// its two lists first. + /// window, and the answer differs per page — the SSH page spends a host + /// list on top of the nav before the row gets anything. pub(crate) settings_row_width: Cell, + /// The window width the settings chrome sized itself against, measured in + /// the same pass. The pages that render their own chrome — the SSH host + /// list, the theme panel — are as blind to the window as `settings_row` is. + pub(crate) settings_viewport_w: Cell, /// Cleared at the top of every settings render, then set by the first row /// the live search matched, so exactly one row per page carries the anchor /// the page scrolls to. @@ -971,6 +975,7 @@ impl Tty7App { sidebar_width: Rc::new(Cell::new(sidebar_width)), sidebar_dragging: Rc::new(Cell::new(false)), settings_row_width: Cell::new(f32::MAX), + settings_viewport_w: Cell::new(f32::MAX), settings_hit_anchored: Cell::new(false), right_panel_width: Rc::new(Cell::new(right_panel_width)), right_panel_dragging: Rc::new(Cell::new(false)), diff --git a/src/ui/settings.rs b/src/ui/settings.rs index 7ba7cf42..39a27155 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -22,8 +22,8 @@ use std::sync::Arc; use uuid::Uuid; use crate::core::config::{ - BellMode, Config, CursorStyle, NewTabPosition, NotifyMode, TabBarPosition, UpdateChannel, - WindowBackdrop, + BellMode, Config, CursorStyle, NewTabPosition, NotifyMode, TabBarPosition, + UI_FONT_SIZE_DEFAULT, UpdateChannel, WindowBackdrop, }; use crate::core::keychain::CredentialRef; use crate::core::ssh_profile::{ @@ -39,29 +39,183 @@ use crate::ui::presets; use crate::ui::rounding; use crate::ui::rounding::RoundedCorners as _; -/// The settings nav, the SSH host list, and the padding each page sets — the -/// chrome a row has to share the window with. +/// The settings nav, the SSH host list, the theme panel, and the padding each +/// page sets — the chrome a row has to share the window with. const NAV_W: f32 = 220.; const SSH_LIST_W: f32 = 280.; +const THEME_PANEL_W: f32 = 300.; const SSH_DETAIL_PAD: f32 = 64.; const PAGE_PAD: f32 = 80.; +/// The narrowest window these numbers have to hold for. +/// +/// `ui::windows::MIN_SIZE` declares 720 and there is only one `window_min_size` +/// in the app, but the settings window in the report this file was fixed for +/// measured 641pt — a restored window bound gets under the declared minimum. +/// Derive from what turns up, not from what is declared. +const NARROWEST_WINDOW: f32 = 640.; + +/// The narrowest each list is still itself: a nav item that still shows a label +/// beside its icon (40 of icon, gap and padding, then the longest label), a +/// host row that still shows a name, a theme card that is still a recognisable +/// picture of a theme. +const NAV_W_MIN: f32 = 140.; +const SSH_LIST_W_MIN: f32 = 180.; +const THEME_PANEL_W_MIN: f32 = 240.; + +/// The reading column every scrolled page caps itself at. Wider than this and +/// a description stops being a paragraph and becomes a line to scan across. +const READING_COLUMN: f32 = 640.; + +/// What the page gets before any list does, and the floor it may be pushed to +/// when even that cannot be had — the numbers this file did not have. +/// +/// Every list beside the page was a fixed width that never gave anything back, +/// so the page absorbed the entire shortfall. On a half-width window with the +/// theme panel open that ran all the way down: a Chinese description came out +/// one character per line, twenty-five lines tall, and the theme cards under it +/// were slivers clipped by the window edge. +/// +/// `CONTENT_W` holds a stacked row's widest control — the 260px text fields — +/// with a description beside it that still reads as a paragraph. `CONTENT_MIN_W` +/// is not chosen at all: it is what the narrowest window in the wild leaves the +/// SSH page, the one that spends a second list, once both lists are standing on +/// their own floors. A control wider than it has to be able to shrink, which is +/// what `max_w_full` on the wrappers below is for. +const CONTENT_W: f32 = 420.; +const CONTENT_MIN_W: f32 = NARROWEST_WINDOW - NAV_W_MIN - SSH_LIST_W_MIN - SSH_DETAIL_PAD; + +/// What the settings page is made of at a given window width. +#[derive(Clone, Copy, PartialEq, Debug)] +struct SettingsColumns { + nav: f32, + /// Zero off the SSH page. + ssh_list: f32, + /// The width to *draw* the theme panel at, whether it is taking a column or + /// covering one — zero only when it is closed. + theme_panel: f32, + /// The panel no longer fits beside the page, so it lays itself over the + /// page instead of taking width from it. The panel is a temporary layer + /// over one choice; the page underneath is what the window is for. + panel_overlays: bool, +} + +/// Hand every list its full width, then take the shortfall back from all of +/// them at once — each in proportion to what it has to spare — until the page +/// between them reaches `CONTENT_W`. Once every list is standing on its own +/// floor the page takes whatever is left, which from `NARROWEST_WINDOW` up is +/// never less than `CONTENT_MIN_W`. The theme panel leaves the row altogether +/// rather than let it come to that. +/// +/// Every width here is one this row can actually have, so the columns always +/// add up to the window. That is deliberate: the fix for a page squeezed to +/// nothing is not a `min_w` the row cannot honour — a floor a flex row cannot +/// meet does not push back, it overflows, and overflow here means content +/// painted off the edge of the window, which is the other half of this bug. +fn settings_columns( + section: SettingsSection, + theme_panel_open: bool, + viewport: f32, +) -> SettingsColumns { + let ssh = matches!(section, SettingsSection::Ssh); + // The panel belongs to Appearance; a stale open flag on any other page is + // not a column, the same way `render_settings` does not draw one. + let theme_panel_open = theme_panel_open && matches!(section, SettingsSection::Appearance); + let pad = if ssh { SSH_DETAIL_PAD } else { PAGE_PAD }; + let page = CONTENT_W + pad; + + // Even with the nav and the panel both at their floors there has to be a + // readable page left between them. Below that width the panel stops being a + // column — this is the one place the *floor* is the test, because the panel + // leaving the row is what buys the page its preferred width back. + let panel_overlays = + theme_panel_open && viewport - NAV_W_MIN - THEME_PANEL_W_MIN - PAGE_PAD < CONTENT_MIN_W; + let beside = theme_panel_open && !panel_overlays; + + let mut nav = NAV_W; + let mut ssh_list = only_when(ssh, SSH_LIST_W); + let mut theme_panel = only_when(beside, THEME_PANEL_W); + let (nav_slack, list_slack, panel_slack) = ( + NAV_W - NAV_W_MIN, + only_when(ssh, SSH_LIST_W - SSH_LIST_W_MIN), + only_when(beside, THEME_PANEL_W - THEME_PANEL_W_MIN), + ); + let slack = nav_slack + list_slack + panel_slack; + let short = (nav + ssh_list + theme_panel + page - viewport).max(0.); + if short > 0. && slack > 0. { + let give = (short / slack).min(1.); + nav -= nav_slack * give; + ssh_list -= list_slack * give; + theme_panel -= panel_slack * give; + } + if panel_overlays { + // Covering the page, not replacing it: leave a strip of the page in + // view so the panel reads as something laid on top and dismissible. + theme_panel = THEME_PANEL_W + .min(viewport - nav - CONTENT_MIN_W / 2.) + .max(THEME_PANEL_W_MIN); + } + SettingsColumns { + nav: nav.round(), + ssh_list: ssh_list.round(), + theme_panel: theme_panel.round(), + panel_overlays, + } +} + /// What a row on this page really has to lay out in. /// /// The nav is always in front of it; the SSH page puts its host list there too, -/// and only the scrolled pages cap the reading column at 640. -fn settings_row_width(section: SettingsSection, viewport: f32) -> f32 { +/// the theme panel takes another slice of Appearance for as long as it is open +/// *and* still fits beside it, and only the scrolled pages cap the reading +/// column. +fn settings_row_width( + section: SettingsSection, + theme_panel_open: bool, + viewport: f32, + ui_scale: f32, +) -> f32 { + let cols = settings_columns(section, theme_panel_open, viewport); + let panel = only_when(!cols.panel_overlays, cols.theme_panel); match section { - SettingsSection::Ssh => (viewport - NAV_W - SSH_LIST_W - SSH_DETAIL_PAD).max(0.), - _ => (viewport - NAV_W - PAGE_PAD).clamp(0., 640.), + SettingsSection::Ssh => (viewport - cols.nav - cols.ssh_list - SSH_DETAIL_PAD).max(0.), + _ => (viewport - cols.nav - panel - PAGE_PAD).clamp(0., READING_COLUMN * ui_scale), } } +fn only_when(on: bool, w: f32) -> f32 { + if on { w } else { 0. } +} + +/// How much wider every piece of text on this page is than the px thresholds +/// below assume. +/// +/// Those thresholds are widths a *label* needs, measured at the default +/// interface font. The interface has a font size of its own and it goes up to +/// 24 — half as wide again — while a slider or a text field beside that label +/// stays the px width it was built at. Without this the row that has to stack +/// first is the one that never does. +fn ui_scale(cx: &App) -> f32 { + cx.global::().ui_font_size / UI_FONT_SIZE_DEFAULT +} + /// Width a settings row needs before its label and its control fit side by /// side: a 260px control, the `gap_8` between them, and enough left for a /// description to read as prose rather than as a column of words. const STACK_ROW_BELOW: f32 = 500.; +/// Width a port-forwarding rule needs before its kind switch, its two host:port +/// pairs, its description and its remove button all fit on one line: about 580 +/// with every field at its floor, rounded up. Past this the rule takes two +/// lines instead of running off the page. +const SPLIT_FORWARD_ROW_BELOW: f32 = 620.; + +/// And the width below which even `bind → target` is more than one line holds: +/// two host fields at their narrow floor, two ports and the arrow come to about +/// 310, which is more than `CONTENT_MIN_W`. The SSH page reaches this on the +/// window the report came from, so the two ends take a line each. +const STACK_FORWARD_ENDS_BELOW: f32 = 340.; + fn settings_row_id(label: &str, _desc: &str) -> SharedString { SharedString::from(format!("settings-row-{label}")) } @@ -873,9 +1027,15 @@ impl Tty7App { let query = search.read(cx).value().trim().to_lowercase(); let show_theme_panel = theme_panel_open && section == SettingsSection::Appearance; + let viewport_w = window.viewport_size().width.as_f32(); + let ui_scale = ui_scale(cx); + self.settings_viewport_w.set(viewport_w); + let cols = settings_columns(section, show_theme_panel, viewport_w); self.settings_row_width.set(settings_row_width( section, - window.viewport_size().width.as_f32(), + show_theme_panel, + viewport_w, + ui_scale, )); self.settings_hit_anchored.set(false); @@ -971,7 +1131,7 @@ impl Tty7App { let sidebar = Sidebar::new("settings-sidebar") .collapsible(SidebarCollapsible::None) - .w(px(220.)) + .w(px(cols.nav)) .header( v_flex() .w_full() @@ -1040,6 +1200,12 @@ impl Tty7App { // No fill of its own: the root already paints the opaque surface and // the background image behind it, and repainting here would hide the // image again in the one pane that fills most of the panel. + // + // `min_w_0` and not a `CONTENT_MIN_W` floor: `settings_columns` sized + // the chrome so this pane clears it, and a floor a flex row cannot + // honour does not push the nav back — it overflows, and overflow here + // means content painted off the edge of the window, which is the other + // half of the bug this file is fixing. let content_pane = if section == SettingsSection::Ssh { v_flex() .id("settings-content") @@ -1067,7 +1233,7 @@ impl Tty7App { div().w_full().px_10().py_8().child( div() .w_full() - .max_w(px(640.)) + .max_w(px(READING_COLUMN * ui_scale)) .children(no_match_note) .child(content), ), @@ -1129,7 +1295,25 @@ impl Tty7App { ) .on_double_click(|_, window, _| window.titlebar_double_click()), ) - .when(show_theme_panel, |r| r.child(self.render_theme_panel(cx))) + // Beside the page while there is room for both, over it when there + // is not. As a column it was taking its 300px from the page and + // from nothing else, which is how a half-width window ended up + // rendering a description one character wide. + .when(show_theme_panel && !cols.panel_overlays, |r| { + r.child(self.render_theme_panel(cx)) + }) + .when(show_theme_panel && cols.panel_overlays, |r| { + r.child( + div() + .absolute() + .top_0() + .right_0() + .bottom_0() + .occlude() + .shadow_lg() + .child(self.render_theme_panel(cx)), + ) + }) .when(!show_theme_panel, |r| { r.child( div() @@ -1161,6 +1345,27 @@ impl Tty7App { root } + /// The column widths this render settled on. `settings_columns` is pure and + /// cheap, so the two pages that draw chrome of their own work them out + /// again rather than have the answer threaded through every builder. + fn settings_columns_now(&self) -> SettingsColumns { + let (section, panel_open) = match self.active_settings() { + Some(s) => ( + s.section, + s.theme_panel_open && s.section == SettingsSection::Appearance, + ), + None => (SettingsSection::Appearance, false), + }; + settings_columns(section, panel_open, self.settings_viewport_w.get()) + } + + /// Whether the row measured this render came out narrower than a threshold + /// quoted at the default interface font — the only way those px thresholds + /// mean anything to a reader who scaled the interface up. + fn settings_row_under(&self, at_default_font: f32, cx: &App) -> bool { + self.settings_row_width.get() < at_default_font * ui_scale(cx) + } + fn header_text(&self, title: &str, cx: &Context) -> Div { div() .text_base() @@ -1270,7 +1475,7 @@ impl Tty7App { // where both still fit, put the control on its own line instead. // Measured, not `flex_wrap`: wrapping made the label column size to its // description, which then ran out past the row on every wide page. - let stacked = self.settings_row_width.get() < STACK_ROW_BELOW; + let stacked = self.settings_row_under(STACK_ROW_BELOW, cx); let labels = v_flex() .gap_0p5() .min_w_0() @@ -1309,7 +1514,16 @@ impl Tty7App { .hover(|h| h.bg(gpui::rgb(cx.global::().window.hover))) .on_hover(cx.listener(|_this, _hovered, _window, cx| cx.notify())) .child(labels) - .child(h_flex().flex_shrink_0().child(control)) + // Stacked, the control column takes the row: that is what gives a + // `max_w_full` control a definite width to shrink against, and on + // the SSH page the widest of them is 260 in a column that can be + // `CONTENT_MIN_W`. + .child( + h_flex() + .when(stacked, |c| c.w_full()) + .when(!stacked, |c| c.flex_shrink_0()) + .child(control), + ) } pub(crate) fn segmented( @@ -1643,6 +1857,7 @@ impl Tty7App { .items_center() .gap_3() .w(px(240.)) + .max_w_full() .child(div().flex_1().child(Slider::new(&slider))) .child( div() @@ -1951,7 +2166,7 @@ impl Tty7App { .child( v_flex() .flex_shrink_0() - .w(px(280.)) + .w(px(self.settings_columns_now().ssh_list)) .h_full() .border_r_1() .border_color(border) @@ -2455,7 +2670,12 @@ impl Tty7App { h_flex() .mt_3() .gap_2() - .child(div().w(px(320.)).child(Input::new(&input).small())) + .child( + div() + .flex_1() + .max_w(px(320.)) + .child(Input::new(&input).small()), + ) .child( Button::new("ssh-quick-connect") .label(t(L10nKey::Connect)) @@ -3172,6 +3392,7 @@ impl Tty7App { t(L10nKey::SettingsNameDesc), div() .w(px(260.)) + .max_w_full() .child(Input::new(&form.name).small()) .into_any_element(), cx, @@ -3183,8 +3404,19 @@ impl Tty7App { t(L10nKey::SettingsHostDesc), h_flex() .gap_2() - .child(div().w(px(172.)).child(Input::new(&form.host).small())) - .child(div().w(px(80.)).child(Input::new(&form.port).small())) + .max_w_full() + .child( + div() + .w(px(172.)) + .min_w_0() + .child(Input::new(&form.host).small()), + ) + .child( + div() + .w(px(80.)) + .flex_shrink_0() + .child(Input::new(&form.port).small()), + ) .into_any_element(), cx, ), @@ -3195,6 +3427,7 @@ impl Tty7App { t(L10nKey::SettingsUserDesc), div() .w(px(260.)) + .max_w_full() .child(Input::new(&form.user).small()) .into_any_element(), cx, @@ -3314,6 +3547,7 @@ impl Tty7App { t(L10nKey::SettingsJumpHostDesc), div() .w(px(260.)) + .max_w_full() .child(Input::new(&form.jump).small()) .into_any_element(), cx, @@ -3373,6 +3607,7 @@ impl Tty7App { ) .child( h_flex() + .flex_wrap() .gap_3() .pt_1() .text_xs() @@ -3400,6 +3635,14 @@ impl Tty7App { }; let incomplete = row.collect(cx).is_none() && !row.is_blank(cx); + // Below `SPLIT_FORWARD_ROW_BELOW` the five controls stop fitting on one + // line. The kind switch, the description and the remove button keep the + // first line; the mapping the rule is actually about takes the second, + // where two host fields, two ports and an arrow are what has to fit + // inside `CONTENT_MIN_W` — hence the lower floor on the host field. + let split = self.settings_row_under(SPLIT_FORWARD_ROW_BELOW, cx); + let stack_ends = self.settings_row_under(STACK_FORWARD_ENDS_BELOW, cx); + let host_min = if split { 80. } else { 104. }; let endpoint = |host: &Entity, port: &Entity| { h_flex() .gap_1() @@ -3411,74 +3654,89 @@ impl Tty7App { .child( div() .flex_1() - .min_w(px(104.)) + .min_w(px(host_min)) .child(Input::new(host).xsmall()), ) .child(div().text_xs().text_color(muted).child(":")) .child(div().w(px(58.)).child(Input::new(port).xsmall())) }; + let mapping = |line: Div| { + line.child( + div() + .flex_1() + .when(stack_ends, |end| end.w_full()) + .child(endpoint(&row.bind_host, &row.bind_port)), + ) + .child(div().flex_shrink_0().text_xs().text_color(muted).child("→")) + .child( + div() + .flex_1() + .opacity(if needs_target { + 1.0 + } else { + crate::ui::forwards::NO_TARGET_FADE + }) + .when(stack_ends, |end| end.w_full()) + .child(endpoint(&row.target_host, &row.target_port)), + ) + }; + + let kind_switch = div().flex_shrink_0().child(self.segmented( + format!("ssh-fwd-kind-{idx}"), + &["L", "R", "D"], + kind_idx, + cx, + move |this, ix, _w, cx| { + let kind = match ix { + 1 => ForwardKind::Remote, + 2 => ForwardKind::Dynamic, + _ => ForwardKind::Local, + }; + if let Some(f) = this.ssh_form_mut() + && let Some(r) = f.forwards.get_mut(idx) + { + r.kind = kind; + cx.notify(); + } + }, + )); + let description = div() + .flex_1() + .min_w(px(80.)) + .child(Input::new(&row.description).xsmall()); + let remove = crate::ui::tab_strip::hit_target( + Button::new(("ssh-fwd-remove", idx)) + .icon(Icon::new(IconName::Close)) + .ghost() + .xsmall(), + ) + .tooltip(t(L10nKey::SettingsRemoveRule)) + .on_click(cx.listener(move |this, _, _w, cx| this.remove_forward_rule(idx, cx))); + + let rule = match split { + true => v_flex() + .gap_1() + .child( + h_flex() + .gap_2() + .items_center() + .child(kind_switch) + .child(description) + .child(remove), + ) + .child(match stack_ends { + true => mapping(v_flex().gap_1().items_start()), + false => mapping(h_flex().gap_2().items_center()), + }), + false => mapping(h_flex().gap_2().items_center().child(kind_switch)) + .child(description) + .child(remove), + }; v_flex() .gap_0p5() .py_1() - .child( - h_flex() - .gap_2() - .items_center() - .child(self.segmented( - format!("ssh-fwd-kind-{idx}"), - &["L", "R", "D"], - kind_idx, - cx, - move |this, ix, _w, cx| { - let kind = match ix { - 1 => ForwardKind::Remote, - 2 => ForwardKind::Dynamic, - _ => ForwardKind::Local, - }; - if let Some(f) = this.ssh_form_mut() - && let Some(r) = f.forwards.get_mut(idx) - { - r.kind = kind; - cx.notify(); - } - }, - )) - .child( - div() - .flex_1() - .child(endpoint(&row.bind_host, &row.bind_port)), - ) - .child(div().flex_shrink_0().text_xs().text_color(muted).child("→")) - .child( - div() - .flex_1() - .opacity(if needs_target { - 1.0 - } else { - crate::ui::forwards::NO_TARGET_FADE - }) - .child(endpoint(&row.target_host, &row.target_port)), - ) - .child( - div() - .flex_1() - .min_w(px(80.)) - .child(Input::new(&row.description).xsmall()), - ) - .child( - crate::ui::tab_strip::hit_target( - Button::new(("ssh-fwd-remove", idx)) - .icon(Icon::new(IconName::Close)) - .ghost() - .xsmall(), - ) - .tooltip(t(L10nKey::SettingsRemoveRule)) - .on_click( - cx.listener(move |this, _, _w, cx| this.remove_forward_rule(idx, cx)), - ), - ), - ) + .child(rule) .when(incomplete, |col| { col.child(div().text_xs().text_color(danger).child(if needs_target { t(L10nKey::SettingsFwdNeedsBoth) @@ -3550,6 +3808,7 @@ impl Tty7App { desc.to_string(), div() .w(px(260.)) + .max_w_full() .child(Input::new(input).small()) .into_any_element(), cx, @@ -3874,10 +4133,12 @@ impl Tty7App { }); let program_control = div() .w(px(260.)) + .max_w_full() .child(Input::new(&program_input).small().suffix(program_picker)) .into_any_element(); let args_control = div() .w(px(260.)) + .max_w_full() .child(Input::new(&args_input).small()) .into_any_element(); @@ -3908,6 +4169,7 @@ impl Tty7App { let wd_path_control = if wd_strategy == WdStrategy::Custom { div() .w(px(260.)) + .max_w_full() .child(Input::new(&wd_path_input).small()) .into_any_element() } else { @@ -3997,6 +4259,7 @@ impl Tty7App { .into_any_element(); let link_file_command_control = div() .w(px(300.)) + .max_w_full() .child(Input::new(&link_file_command_input).small()) .into_any_element(); let scrollback_radio = self.segmented( @@ -4059,6 +4322,7 @@ impl Tty7App { .items_center() .gap_3() .w(px(240.)) + .max_w_full() .child(div().flex_1().child(Slider::new(&scroll_slider))) .child( div() @@ -4264,6 +4528,7 @@ impl Tty7App { ), None => (AgentHooksView::Loading, None, HostId::LOCAL), }; + let stacked = self.settings_row_under(STACK_ROW_BELOW, cx); let mut page = v_flex().child(self.section_intro( t(L10nKey::SettingsAgentsIntro), t(L10nKey::SettingsAgentsIntroDesc), @@ -4308,9 +4573,13 @@ impl Tty7App { .filter(|(for_agent, _)| *for_agent == agent) .map(|(_, text)| text.clone()); + // Right-aligned beside its label, left-aligned under it — + // `settings_row` gives the control column the whole row + // once it stacks, and buttons flush to the far edge of a + // row whose label starts at the near one read as unrelated. let control = v_flex() .gap_2() - .items_end() + .when(!stacked, |c| c.items_end()) .child( h_flex() .gap_2() @@ -4344,8 +4613,9 @@ impl Tty7App { col.child( div() .max_w_80() + .max_w_full() .text_xs() - .text_right() + .when(!stacked, |note| note.text_right()) .text_color(muted_fg) .child(text), ) @@ -4797,6 +5067,9 @@ impl Tty7App { let open = self .active_settings() .is_some_and(|s| s.theme_panel_open && s.theme_panel_slot == slot); + // The same width a row stacks at: the card is a row too, just one whose + // control happens to be a whole preview. + let narrow = self.settings_row_under(STACK_ROW_BELOW, cx); div() .id(card_id) @@ -4822,9 +5095,17 @@ impl Tty7App { }) .bg(surface) .hover(|h| h.bg(hover_bg)) - .child(div().w(px(150.)).flex_shrink_0().child(preview)) + // The preview is the first thing to go: it is a picture of a + // choice the two lines beside it already name, and at the + // width where it stops fitting it was pushing the "change + // theme" affordance off the card. + .when(!narrow, |card| { + card.child(div().w(px(150.)).flex_shrink_0().child(preview)) + }) .child( v_flex() + .flex_1() + .min_w_0() .gap_0p5() .child(div().text_xs().text_color(muted_fg).child(caption)) .child( @@ -4836,9 +5117,9 @@ impl Tty7App { ) .child(swatches), ) - .child(div().flex_1()) .child( h_flex() + .flex_shrink_0() .items_center() .gap_1() .text_sm() @@ -4922,7 +5203,7 @@ impl Tty7App { }); let search_box = div().px_4().pb_3().child( - div().w(px(268.)).child( + div().w_full().child( Input::new(&search).small().prefix( Icon::empty() .path("stock/icons/search.svg") @@ -4986,7 +5267,7 @@ impl Tty7App { .cursor_pointer() .child( div() - .w(px(268.)) + .w_full() .rounded(rounding::TRACK_RADIUS) .overflow_hidden() .border_1() @@ -5005,8 +5286,10 @@ impl Tty7App { h_flex() .items_center() .gap_1p5() + .w_full() .child( div() + .truncate() .text_sm() .font_weight(if is_active { FontWeight::SEMIBOLD @@ -5017,7 +5300,12 @@ impl Tty7App { .child(p.name.clone()), ) .when(is_active, |s| { - s.child(Icon::new(IconName::Check).small().text_color(foreground)) + s.child( + Icon::new(IconName::Check) + .small() + .flex_shrink_0() + .text_color(foreground), + ) }), ) .on_click(cx.listener(move |this, _, window, cx| match slot { @@ -5036,7 +5324,7 @@ impl Tty7App { } v_flex() - .w(px(300.)) + .w(px(self.settings_columns_now().theme_panel)) .h_full() .flex_shrink_0() .bg(bg) @@ -5134,13 +5422,18 @@ impl Tty7App { // The label column has to be allowed to shrink, or its description sets // the row's width and the control it belongs to is pushed off the page. // `settings_row` does this for every other row in Settings; these two - // are hand-rolled and were missing it. - let preset_row = h_flex() - .w_full() - .items_center() - .justify_between() - .gap_8() - .py_2() + // are hand-rolled and were missing it. They take its breakpoint too: + // the segmented controls beside them are the widest on the page. + let stacked = self.settings_row_under(STACK_ROW_BELOW, cx); + let hand_rolled_row = |row: Div| { + row.w_full() + .flex() + .when(stacked, |r| r.flex_col().items_start().gap_2()) + .when(!stacked, |r| { + r.flex_row().items_center().justify_between().gap_8() + }) + }; + let preset_row = hand_rolled_row(div().py_2()) .child( v_flex() .min_w_0() @@ -5161,12 +5454,7 @@ impl Tty7App { ) .child(h_flex().flex_shrink_0().child(preset_control)); - let prefix_row = h_flex() - .w_full() - .items_center() - .justify_between() - .gap_8() - .py_2() + let prefix_row = hand_rolled_row(div().py_2()) .child( div() .min_w_0() @@ -5221,8 +5509,11 @@ impl Tty7App { let is_recording = recording.as_ref().is_some_and(|(a, _)| a == &action); let is_overridden = overridden.contains(&action); + // Wrapping, because a four-chord binding is wider than the column a + // narrow window leaves for it, and the alternative to a second line + // is a first one that runs off the page. let keycaps = |spec: &str| { - h_flex().gap_2().children( + h_flex().flex_wrap().gap_2().children( crate::ui::keymap::key_chords(spec) .into_iter() .map(|chord| h_flex().gap_1().children(chord.into_iter().map(&keycap))), @@ -5332,13 +5623,16 @@ impl Tty7App { } let last_in_group = heading_at.contains_key(&(i + 1)) || i + 1 == count; list = list.child( - h_flex() - .items_center() - .justify_between() - .py_1p5() + hand_rolled_row(div().py_1p5()) .when(!last_in_group, |s| s.border_b_1().border_color(border)) - .child(div().text_sm().text_color(foreground).child(label)) - .child(right), + .child( + div() + .min_w_0() + .text_sm() + .text_color(foreground) + .child(label), + ) + .child(right.flex_shrink_0()), ); } @@ -5454,6 +5748,7 @@ impl Tty7App { let http_proxy_control = v_flex() .gap_1() .w(px(260.)) + .max_w_full() .child(Input::new(&http_proxy_input).small()) .when(http_proxy_invalid, |this| { this.child( @@ -5809,21 +6104,173 @@ mod tests { use super::*; /// The row keeps its side-by-side shape while both halves fit, and stacks - /// once they do not. The SSH page reaches that point first — it spends 500px - /// on two lists before the row gets any. + /// once they do not. The SSH page reaches that point first — it spends its + /// host list before the row gets anything. #[test] fn a_row_stacks_once_its_label_and_control_stop_fitting() { use SettingsSection::*; - assert!(settings_row_width(Terminal, 1440.) >= STACK_ROW_BELOW); - assert!(settings_row_width(Terminal, 900.) >= STACK_ROW_BELOW); - assert!(settings_row_width(Terminal, 700.) < STACK_ROW_BELOW); + assert!(settings_row_width(Terminal, false, 1440., 1.) >= STACK_ROW_BELOW); + assert!(settings_row_width(Terminal, false, 900., 1.) >= STACK_ROW_BELOW); + // At the narrowest window that turns up the page is 420 wide, which is + // under the width where a label and a control still share a line. + assert!(settings_row_width(Terminal, false, NARROWEST_WINDOW, 1.) < STACK_ROW_BELOW); // Capped at the reading column, so a wider window never widens the row. - assert_eq!(settings_row_width(Terminal, 4000.), 640.); - // SSH crosses over while the window is still wide. - assert!(settings_row_width(Ssh, 1440.) >= STACK_ROW_BELOW); - assert!(settings_row_width(Ssh, 1000.) < STACK_ROW_BELOW); + assert_eq!( + settings_row_width(Terminal, false, 4000., 1.), + READING_COLUMN + ); + // SSH crosses over while the window is still wide — it is the page that + // spends a host list before its rows get anything. + assert!(settings_row_width(Ssh, false, 1440., 1.) >= STACK_ROW_BELOW); + assert!(settings_row_width(Ssh, false, 900., 1.) < STACK_ROW_BELOW); // And never goes negative on a window narrower than its own chrome. - assert_eq!(settings_row_width(Ssh, 100.), 0.); + assert_eq!(settings_row_width(Ssh, false, 100., 1.), 0.); + } + + /// Every list gives width back before the page does, and no combination of + /// page and panel leaves the page below the width it is derived to keep. + /// 641pt is the window the report came from — under the declared 720 —- so + /// that is the case that has to hold, not the one the manifest promises. + #[test] + fn the_page_keeps_a_readable_width_at_every_window_that_turns_up() { + use SettingsSection::*; + for section in SettingsSection::ALL { + for panel_open in [false, true] { + for viewport in [NARROWEST_WINDOW, 641., 720., 900., 1100., 1440., 2560.] { + let cols = settings_columns(section, panel_open, viewport); + let pad = match section { + Ssh => SSH_DETAIL_PAD, + _ => PAGE_PAD, + }; + let panel = only_when(!cols.panel_overlays, cols.theme_panel); + let page = viewport - cols.nav - cols.ssh_list - panel - pad; + assert!( + page >= CONTENT_MIN_W, + "{viewport}px, {panel_open}: page got {page}, floor is {CONTENT_MIN_W}" + ); + // And the columns add up to the window rather than past it, + // which is what keeps the rightmost one on screen. + assert!(cols.nav + cols.ssh_list + panel + pad + page <= viewport + 1.); + assert!(cols.nav >= NAV_W_MIN && cols.nav <= NAV_W); + } + } + } + } + + /// The three screenshots in the report, by the numbers measured off them. + /// Every one of them is the same 641pt window. + #[test] + fn the_reported_window_lays_out_without_running_off_the_screen() { + use SettingsSection::*; + const REPORTED: f32 = 641.; + + // Shot 1 — SSH. Nav 220 and host list 280 left the detail 141pt and its + // empty state painted ~270 past the window edge. Both lists now stand + // on their floors and the detail keeps the rest. + let ssh = settings_columns(Ssh, false, REPORTED); + assert_eq!((ssh.nav, ssh.ssh_list), (NAV_W_MIN, SSH_LIST_W_MIN)); + let detail = settings_row_width(Ssh, false, REPORTED, 1.); + assert!(detail >= CONTENT_MIN_W, "SSH detail got {detail}"); + + // Shot 2 — Appearance, panel closed. The opacity row measured a 78pt + // label beside a 240pt slider; at this width the row has to stack. + let page = settings_row_width(Appearance, false, REPORTED, 1.); + assert_eq!(page, CONTENT_W); + assert!( + page < STACK_ROW_BELOW, + "the opacity row has to stack at 641" + ); + + // Shot 3 — Appearance with the theme panel. 220 + 300 of chrome left + // the page ~125pt, one Chinese character per line. The panel now lifts + // off the row entirely and the page is back to shot 2's width. + assert!(settings_columns(Appearance, true, REPORTED).panel_overlays); + assert_eq!(settings_row_width(Appearance, true, REPORTED, 1.), page); + } + + /// The list that has to give the most is the one the window has the least + /// room for, and no list is ever asked for more than it has to spare. + #[test] + fn the_lists_shrink_together_and_stop_at_their_floors() { + use SettingsSection::*; + // Wide enough for everyone: nothing moves. + let wide = settings_columns(Ssh, false, 1440.); + assert_eq!((wide.nav, wide.ssh_list), (NAV_W, SSH_LIST_W)); + // The reported window — half of a 1440pt screen, three columns on SSH. + // Both lists give, neither past its floor, and the detail comes out at + // its preferred width instead of the 336 it used to be left with. + let half = settings_columns(Ssh, false, 900.); + assert!(half.nav < NAV_W && half.ssh_list < SSH_LIST_W); + assert!(half.nav >= NAV_W_MIN && half.ssh_list >= SSH_LIST_W_MIN); + assert_eq!( + settings_row_width(Ssh, false, 900., 1.).round(), + CONTENT_W, + "the SSH detail should get its preferred width at 900pt" + ); + // The narrowest window that turns up: both at the floor, page readable. + let tiny = settings_columns(Ssh, false, NARROWEST_WINDOW); + assert_eq!((tiny.nav, tiny.ssh_list), (NAV_W_MIN, SSH_LIST_W_MIN)); + assert_eq!( + settings_row_width(Ssh, false, NARROWEST_WINDOW, 1.), + CONTENT_MIN_W + ); + } + + /// The thresholds are widths a *label* needs, and a reader who scaled the + /// interface up scaled every label with it while the slider beside it kept + /// the px width it was built at. A window that reads fine at the default + /// font is a starved label column at the largest one. + #[test] + fn the_stacking_width_follows_the_interface_font() { + use crate::core::config::UI_FONT_SIZE_MAX; + use SettingsSection::*; + let large = UI_FONT_SIZE_MAX / UI_FONT_SIZE_DEFAULT; + // Side by side at the default font... + assert!(settings_row_width(Appearance, false, 900., 1.) >= STACK_ROW_BELOW); + // ...and stacked at the largest, where the same row holds half as much. + assert!( + settings_row_width(Appearance, false, 900., large) < STACK_ROW_BELOW * large, + "a 900pt window at the largest interface font has to stack" + ); + // The reading column grows with the font, so a wide window does not. + assert!(settings_row_width(Appearance, false, 1600., large) >= STACK_ROW_BELOW * large); + } + + /// The theme panel took its 300px from the page and from nothing else, so + /// a half-width window with it open rendered a description one character + /// wide. It now shrinks with everything else, and stops being a column at + /// all once even that is not enough. + #[test] + fn the_theme_panel_yields_before_the_page_does() { + use SettingsSection::*; + assert!(settings_row_width(Appearance, true, 900., 1.) < STACK_ROW_BELOW); + assert!( + settings_row_width(Appearance, true, 900., 1.) + < settings_row_width(Appearance, false, 900., 1.) + ); + // Beside the page while both fit — which, with the panel and the nav + // both allowed down to their floors, still holds at 720. + assert!(!settings_columns(Appearance, true, 900.).panel_overlays); + assert!(!settings_columns(Appearance, true, 720.).panel_overlays); + // ...and over it once they do not, at which point the page is back to + // the width it has with the panel closed. + assert!(settings_columns(Appearance, true, 641.).panel_overlays); + assert_eq!( + settings_row_width(Appearance, true, 641., 1.), + settings_row_width(Appearance, false, 641., 1.) + ); + // The panel is the only chrome that can leave, so it has to leave in + // time: at 641 there is no arrangement in which it and a readable page + // both fit in a row. + assert!( + NARROWEST_WINDOW - NAV_W_MIN - THEME_PANEL_W_MIN - PAGE_PAD < CONTENT_MIN_W, + "the overlay threshold has to fire at the narrowest window" + ); + // Wide enough and the cap is the reading column either way. + assert_eq!( + settings_row_width(Appearance, true, 1600., 1.), + READING_COLUMN + ); } #[test] From 3dcffe6f4ec89a7efd6312a729707fde101b5aaf Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:20:15 +0700 Subject: [PATCH 6/7] fix(windows): grow a remembered bound back up to the minimum size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit window_min_size governs what a drag may do to a window, not the bounds it opens with, so a remembered bound walked straight under the declared 720pt minimum — the reported settings window measured 641. Clamp the restored size on the way in, keeping the origin. --- src/ui/windows.rs | 50 ++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/src/ui/windows.rs b/src/ui/windows.rs index 8603c795..9d7800e7 100644 --- a/src/ui/windows.rs +++ b/src/ui/windows.rs @@ -549,7 +549,7 @@ fn window_options(cx: &mut App, workspace: Option) -> WindowOptions let existing = WindowRegistry::count(cx); let bounds = match remembered { Some(state) => { - let bounds = state.bounds(); + let bounds = at_least_min_size(state.bounds()); if cx.displays().iter().any(|d| d.bounds().intersects(&bounds)) { bounds } else { @@ -582,6 +582,23 @@ fn window_options(cx: &mut App, workspace: Option) -> WindowOptions } } +/// Grow a remembered bound back up to `MIN_SIZE`. +/// +/// `window_min_size` only governs what a *drag* may do to a window; the bounds +/// we open with are taken as given. A window that got under the minimum — an +/// older build that had no minimum, a hand-edited `views.json`, a display that +/// went away — was reopened at whatever it had been saved at, and the settings +/// page it opened onto had never been laid out for that width. +fn at_least_min_size(bounds: Bounds) -> Bounds { + Bounds { + origin: bounds.origin, + size: size( + bounds.size.width.max(px(MIN_SIZE.0)), + bounds.size.height.max(px(MIN_SIZE.1)), + ), + } +} + fn cascade(bounds: Bounds, existing: usize) -> Bounds { if existing == 0 { return bounds; @@ -626,6 +643,37 @@ mod tests { assert_eq!(cascade(b, 3).size, b.size); } + #[test] + fn a_remembered_bound_under_the_minimum_is_grown_back_to_it() { + // The window in the report this was fixed for: 641x830, saved and + // reopened at a width no settings page is laid out for. + let undersized = Bounds { + origin: point(px(700.), px(60.)), + size: size(px(641.), px(830.)), + }; + let grown = at_least_min_size(undersized); + assert_eq!(grown.size.width, px(MIN_SIZE.0)); + assert_eq!(grown.size.height, px(830.), "a tall enough height is kept"); + assert_eq!(grown.origin, undersized.origin, "the corner does not move"); + + let short = at_least_min_size(Bounds { + origin: point(px(0.), px(0.)), + size: size(px(1200.), px(300.)), + }); + assert_eq!(short.size, size(px(1200.), px(MIN_SIZE.1))); + } + + #[test] + fn a_remembered_bound_at_or_over_the_minimum_is_left_alone() { + let b = bounds_at(100., 100.); + assert_eq!(at_least_min_size(b).size, b.size); + let exact = Bounds { + origin: point(px(0.), px(0.)), + size: size(px(MIN_SIZE.0), px(MIN_SIZE.1)), + }; + assert_eq!(at_least_min_size(exact).size, exact.size); + } + #[test] fn cascade_wraps_so_windows_never_march_off_screen() { let b = bounds_at(100., 100.); From 2f31a11df027ea57a8cfa1d1cf65615fca8a4b67 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 9 Aug 2026 19:20:15 +0700 Subject: [PATCH 7/7] fix(settings): size the nav floor for the longest label in any locale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SidebarMenuItem clips its label rather than eliding it, so a 140pt floor that fits English cut a glyph in half elsewhere: zh-CN lost the right half of the last character of 窗口与标签页. Size the floor for ja-JP ウィンドウとタブ, the widest of the three, which costs the page 35pt at the narrowest window and keeps every nav label whole. --- src/ui/settings.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/ui/settings.rs b/src/ui/settings.rs index 39a27155..f294375c 100644 --- a/src/ui/settings.rs +++ b/src/ui/settings.rs @@ -49,17 +49,26 @@ const PAGE_PAD: f32 = 80.; /// The narrowest window these numbers have to hold for. /// -/// `ui::windows::MIN_SIZE` declares 720 and there is only one `window_min_size` -/// in the app, but the settings window in the report this file was fixed for -/// measured 641pt — a restored window bound gets under the declared minimum. -/// Derive from what turns up, not from what is declared. +/// `ui::windows::MIN_SIZE` declares 720, but the settings window in the report +/// this file was fixed for measured 641pt: `window_min_size` governs dragging, +/// not the bounds a window opens with, so a remembered bound walked straight +/// under it. `ui::windows::at_least_min_size` now closes that path, and this +/// stays below it deliberately — a declared minimum is a claim about the code, +/// and this file would rather be laid out for the window that turns up. const NARROWEST_WINDOW: f32 = 640.; /// The narrowest each list is still itself: a nav item that still shows a label /// beside its icon (40 of icon, gap and padding, then the longest label), a /// host row that still shows a name, a theme card that is still a recognisable /// picture of a theme. -const NAV_W_MIN: f32 = 140.; +/// +/// The nav floor is sized for the longest nav label in *any* locale, not the +/// one the developer happens to be reading. `SidebarMenuItem` clips its label +/// rather than eliding it, so a floor that fits English cuts a glyph in half in +/// Chinese and Japanese: at 140 the zh-CN "窗口与标签页" lost the right half of +/// its last character. The widest is ja-JP "ウィンドウとタブ" — 8 full-width +/// kana beside the icon, which is 36 more than the 6-glyph Chinese label needs. +const NAV_W_MIN: f32 = 176.; const SSH_LIST_W_MIN: f32 = 180.; const THEME_PANEL_W_MIN: f32 = 240.; @@ -6174,8 +6183,12 @@ mod tests { // Shot 2 — Appearance, panel closed. The opacity row measured a 78pt // label beside a 240pt slider; at this width the row has to stack. + // The page does not reach its preferred `CONTENT_W` here: the nav floor + // is sized to show a Japanese nav label whole, and at 641 that costs the + // page the difference. Readable and stacked is the property that matters. let page = settings_row_width(Appearance, false, REPORTED, 1.); - assert_eq!(page, CONTENT_W); + assert_eq!(page, REPORTED - NAV_W_MIN - PAGE_PAD); + assert!(page >= CONTENT_MIN_W, "the page got {page}"); assert!( page < STACK_ROW_BELOW, "the opacity row has to stack at 641"