From 5d1f6dc4ca4c3833dfc82feab7b7c2540c3415a2 Mon Sep 17 00:00:00 2001 From: ymote <151983+ymote@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:21:50 -0700 Subject: [PATCH] terminal: settings lists scroll with the mouse; a change applies even when it cannot be saved Found on the OctoSense desktop (process-hosted Terminal) and reproduced with the remote instrument: - The settings list and every chooser (Theme, Font, ...) ignored the mouse wheel: the wheel moved `scroll`, but each draw scrolled back to keep the selection visible, so only the arrow keys moved the list. The draw now brings a selection into view only when it CHANGED (tracked as `shown_selected`); a wheel scroll stays where it was put. Verified: three wheel steps move the Theme list to its end (the old build stays at the top). - settings::update returned before updating the live copy when writing the file failed (read-only or sandboxed home), so theme, cursor and every other change silently did nothing. It now applies the change and bumps the generation first, then returns the save error for the caller to report. Verified with a read-only MAKEPAD_HOME: the theme and the cursor shape change live, the save error is logged. Co-Authored-By: Claude Opus 5.5 --- apps/terminal/src/settings.rs | 20 ++++++++++++------- apps/terminal/src/tabs.rs | 36 +++++++++++++++++++++++++---------- 2 files changed, 39 insertions(+), 17 deletions(-) diff --git a/apps/terminal/src/settings.rs b/apps/terminal/src/settings.rs index 0f711c6fe4..8a7fce975d 100644 --- a/apps/terminal/src/settings.rs +++ b/apps/terminal/src/settings.rs @@ -437,18 +437,24 @@ pub fn generation() -> u64 { /// Save and apply: the file is replaced whole (write, then rename), so a /// reader never sees half of it. +/// +/// The change applies to this process even when the file cannot be written +/// (a read-only or sandboxed home): the error is returned for the caller to +/// report, but the person's choice still takes effect until they quit. pub fn update(settings: Settings) -> std::io::Result<()> { let path = path(); - if let Some(dir) = path.parent() { - std::fs::create_dir_all(dir)?; - } - let tmp = path.with_extension("conf.tmp"); - std::fs::write(&tmp, settings.to_text())?; - std::fs::rename(&tmp, &path)?; + let saved = (|| { + if let Some(dir) = path.parent() { + std::fs::create_dir_all(dir)?; + } + let tmp = path.with_extension("conf.tmp"); + std::fs::write(&tmp, settings.to_text())?; + std::fs::rename(&tmp, &path) + })(); let mut guard = LIVE.write().unwrap_or_else(|e| e.into_inner()); *guard = Some(Live { settings, mtime: file_mtime() }); GENERATION.fetch_add(1, Ordering::AcqRel); - Ok(()) + saved } static LAST_POLL: Mutex> = Mutex::new(None); diff --git a/apps/terminal/src/tabs.rs b/apps/terminal/src/tabs.rs index e63a2a73ea..63b0b13588 100644 --- a/apps/terminal/src/tabs.rs +++ b/apps/terminal/src/tabs.rs @@ -394,6 +394,9 @@ struct Panel { selected: usize, /// Rows scrolled off the top. scroll: usize, + /// The selection last scrolled into view: the draw keeps a NEW + /// selection visible, but leaves a mouse-wheel scroll alone. + shown_selected: Option, rect: Rect, hits: Vec<(Rect, PanelHit)>, /// Rows (or list entries) that fit, from the last draw. @@ -422,6 +425,8 @@ struct Chooser { /// Index into the filtered list. selected: usize, scroll: usize, + /// The selection last scrolled into view (see `Panel::shown_selected`). + shown_selected: Option, /// The settings when the list opened: Esc goes back to them. original: Settings, /// Opened before the font scan finished: refill when it has. @@ -1588,6 +1593,7 @@ impl TermTabs { filter: String::new(), selected, scroll: selected.saturating_sub(4), + shown_selected: Some(selected), original: self.settings.clone(), awaiting_fonts: matches!(row, Row::Font | Row::CjkFont) && !crate::fonts::ready(), }); @@ -1913,15 +1919,19 @@ impl TermTabs { hits: &mut Vec<(Rect, PanelHit)>, ) -> usize { let shown_settings = self.current_settings(); - let (selected, mut scroll, confirm_delete) = - self.panel.as_ref().map_or((0, 0, false), |p| (p.selected, p.scroll, p.confirm_delete)); - // Keep the selected row in view, counting section headers. - scroll = scroll.min(selected); - while selected >= scroll + rows_fitting(scroll, bottom - top, row_h) { - scroll += 1; + let (selected, mut scroll, confirm_delete, shown) = + self.panel.as_ref().map_or((0, 0, false, None), |p| (p.selected, p.scroll, p.confirm_delete, p.shown_selected)); + // Keep a newly selected row in view, counting section headers; a + // mouse-wheel scroll (same selection) stays where it was put. + if shown != Some(selected) { + scroll = scroll.min(selected); + while selected >= scroll + rows_fitting(scroll, bottom - top, row_h) { + scroll += 1; + } } if let Some(panel) = self.panel.as_mut() { panel.scroll = scroll; + panel.shown_selected = Some(selected); } let mut y = top; let mut index = 0; @@ -2034,11 +2044,17 @@ impl TermTabs { let shown = settings_panel::filter_choices(&chooser.choices, &chooser.filter); let visible = (((bottom - top - row_h * 2.0) / row_h).floor().max(1.0)) as usize; chooser.selected = chooser.selected.min(shown.len().saturating_sub(1)); - if chooser.selected < chooser.scroll { - chooser.scroll = chooser.selected; - } else if chooser.selected >= chooser.scroll + visible { - chooser.scroll = chooser.selected + 1 - visible; + // Keep a newly selected entry in view; a mouse-wheel scroll (same + // selection) stays where it was put. + if chooser.shown_selected != Some(chooser.selected) { + if chooser.selected < chooser.scroll { + chooser.scroll = chooser.selected; + } else if chooser.selected >= chooser.scroll + visible { + chooser.scroll = chooser.selected + 1 - visible; + } + chooser.shown_selected = Some(chooser.selected); } + chooser.scroll = chooser.scroll.min(shown.len().saturating_sub(visible)); let (row, filter, selected, scroll) = (chooser.row, chooser.filter.clone(), chooser.selected, chooser.scroll); let current = row.current(&shown_settings);