T17: a font saves, and a settings file starts, only when gpui loads it as the terminal draws it, so the terminal never falls back without a word; numbers save on Enter, on leaving the field and on each step; after a refused change a number field drops its draft and the font picker shows the saved family again

src/app.rs

@@ -59,11 +59,6 @@ pub fn start(cx: &mut App) -> anyhow::Result<()> {
         mac::repeat_held_keys();
         mac::deliver_command_key_ups()?;
     }
-    let family = &SettingsStore::get(cx).font_family;
-    anyhow::ensure!(
-        cx.text_system().all_font_names().contains(family),
-        "settings: font_family is {family:?}, which is not installed"
-    );
     themes::apply(cx);
     let chosen =
         |settings: &settings::Settings| (settings.theme.clone(), settings.dark_theme.clone());

src/settings/store.rs

@@ -24,6 +24,8 @@ impl SettingsStore {
         let folder = dirs::config_dir().context("settings: the platform names no config folder")?;
         let path = folder.join("ELYTerminal").join("settings.json");
         let settings = load(&path)?;
+        loads(&settings.font_family, cx)
+            .with_context(|| format!("settings: {} holds a bad value", path.display()))?;
         cx.set_global(Self { settings, path });
         Ok(())
     }
@@ -41,12 +43,20 @@ impl SettingsStore {
             return Ok(());
         }
         next.validate()?;
+        if next.font_family != store.settings.font_family {
+            loads(&next.font_family, cx)?;
+        }
         save(&store.path, &next)?;
         cx.global_mut::<Self>().settings = next;
         Ok(())
     }
 }
 
+/// Checks the terminal can load `family`.
+fn loads(family: &str, cx: &App) -> anyhow::Result<()> {
+    crate::term::load_font(&gpui::font(family.to_string()), cx.text_system()).map(|_| ())
+}
+
 /// The file's settings; defaults when there is no file. A file that fails to read or check is an error.
 pub fn load(path: &Path) -> anyhow::Result<Settings> {
     let text = match fs::read_to_string(path) {

src/settings/window.rs

@@ -87,6 +87,8 @@ pub struct SettingsWindow {
     refused: Option<SharedString>,
     /// Why the last change did not save.
     failed: Option<SharedString>,
+    /// Refusals so far; a field keyed by it drops its draft and shows the settings again.
+    drafts: usize,
     font: Entity<TextInput>,
     /// Whether the cursor preview shows, as it blinks.
     lit: bool,
@@ -118,6 +120,7 @@ impl SettingsWindow {
             shell,
             refused: None,
             failed: None,
+            drafts: 0,
             font,
             lit: true,
             _typed: typed,
@@ -146,9 +149,10 @@ impl SettingsWindow {
 
     /// Saves `shell` when it names a program; else says why and keeps the saved one.
     fn save_shell(&mut self, shell: String, cx: &mut Context<Self>) {
-        let refused = match SettingsStore::update(cx, |settings| settings.shell = shell.clone()) {
+        self.refused = match SettingsStore::update(cx, |settings| settings.shell = shell.clone()) {
             Ok(()) => {
                 log::info!("settings: new sessions run {shell:?}");
+                self.failed = None;
                 None
             }
             Err(error) => {
@@ -156,10 +160,7 @@ impl SettingsWindow {
                 Some(format!("{error:#}").into())
             }
         };
-        if refused != self.refused {
-            self.refused = refused;
-            cx.notify();
-        }
+        cx.notify();
     }
 
     fn general(&self, cx: &Context<Self>) -> SettingsSection {
@@ -203,17 +204,17 @@ fn start_in(this: &Entity<SettingsWindow>, value: &SharedString, cx: &mut App) {
     );
 }
 
-/// Saves one change and logs it; a refused one changes nothing, and the window says why.
+/// Saves one change and logs it, telling whether the settings hold it; a refused one changes nothing, and the window says why.
 fn set(
     this: &Entity<SettingsWindow>,
     change: String,
     cx: &mut App,
     edit: impl FnOnce(&mut Settings),
-) {
+) -> bool {
     let mut next = SettingsStore::get(cx).clone();
     edit(&mut next);
     if next == *SettingsStore::get(cx) {
-        return;
+        return true;
     }
     let failed = match SettingsStore::update(cx, |settings| *settings = next) {
         Ok(()) => {
@@ -225,10 +226,13 @@ fn set(
             Some(format!("{error:#}").into())
         }
     };
+    let saved = failed.is_none();
     this.update(cx, |this, cx| {
+        this.drafts += usize::from(!saved);
         this.failed = failed;
         cx.notify();
     });
+    saved
 }
 
 impl Render for SettingsWindow {

src/settings/window/appearance.rs

@@ -42,7 +42,7 @@ impl SettingsWindow {
                     format!("separate dark theme is {on}"),
                     cx,
                     |settings| settings.dark_theme = on.then_some(theme),
-                )
+                );
             }
         });
         let light = separate.then_some("Light");
@@ -56,33 +56,40 @@ impl SettingsWindow {
             .selected(settings.font_family.clone())
             .on_change({
                 let this = this.clone();
-                move |family, _, cx| {
+                move |family, window, cx| {
                     let family = family.to_string();
-                    set(
+                    let saved = set(
                         &this,
                         format!("font_family is {family:?}"),
                         cx,
                         |settings| settings.font_family = family,
-                    )
+                    );
+                    // A refused pick shows the saved family again, the list closed.
+                    if !saved {
+                        let saved = SettingsStore::get(cx).font_family.clone();
+                        let input = this.read(cx).font.clone();
+                        input.update(cx, |input, cx| input.set_text(saved, cx));
+                        window.blur(cx);
+                    }
                 }
             });
         let font = SettingsSection::new("Font")
             .row(SettingsRow::new("Family").control(div().w(theme.prose_width()).child(family)))
-            .row(SettingsRow::new("Size").control(number(
-                &this,
+            .row(SettingsRow::new("Size").control(self.number(
                 "font_size",
                 settings.font_size,
                 &FONT_SIZE,
                 (1.0, 1),
                 |settings, size| settings.font_size = size,
+                cx,
             )))
-            .row(SettingsRow::new("Line height").control(number(
-                &this,
+            .row(SettingsRow::new("Line height").control(self.number(
                 "line_height",
                 settings.line_height,
                 &LINE_HEIGHT,
                 (0.05, 2),
                 |settings, height| settings.line_height = height,
+                cx,
             )))
             .row(SettingsRow::new("Ligatures").control(switch(
                 &this,
@@ -114,7 +121,7 @@ impl SettingsWindow {
                         format!("cursor_style is {style:?}"),
                         cx,
                         |settings| settings.cursor_style = style,
-                    )
+                    );
                 }
             });
         let active = themes::active(settings, window.appearance());
@@ -140,7 +147,7 @@ impl SettingsWindow {
                         format!("background_opacity is {value}"),
                         cx,
                         |settings| settings.background_opacity = value,
-                    )
+                    );
                 }
             });
         let opacity = div()
@@ -161,21 +168,21 @@ impl SettingsWindow {
                 settings.background_blur,
                 |settings, on| settings.background_blur = on,
             )))
-            .row(SettingsRow::new("Horizontal padding").control(number(
-                &this,
+            .row(SettingsRow::new("Horizontal padding").control(self.number(
                 "padding_x",
                 settings.padding_x,
                 &PADDING,
                 (1.0, 0),
                 |settings, padding| settings.padding_x = padding,
+                cx,
             )))
-            .row(SettingsRow::new("Vertical padding").control(number(
-                &this,
+            .row(SettingsRow::new("Vertical padding").control(self.number(
                 "padding_y",
                 settings.padding_y,
                 &PADDING,
                 (1.0, 0),
                 |settings, padding| settings.padding_y = padding,
+                cx,
             )));
         div()
             .flex()
@@ -186,6 +193,30 @@ impl SettingsWindow {
             .child(cursor)
             .child(background)
     }
+
+    /// A field over `range`, by `step` to `places`, that saves each committed number through `put`; a refusal starts it again from the settings.
+    fn number(
+        &self,
+        field: &'static str,
+        value: f32,
+        range: &RangeInclusive<f32>,
+        (step, places): (f64, usize),
+        put: fn(&mut Settings, f32),
+        cx: &Context<Self>,
+    ) -> NumberInput {
+        let this = cx.entity();
+        NumberInput::new((field, self.drafts), f64::from(value))
+            .range(f64::from(*range.start()), f64::from(*range.end()))
+            .step(step)
+            .precision(places)
+            .size(ControlSize::Sm)
+            .on_commit(move |value, _, cx| {
+                let value = value as f32;
+                set(&this, format!("{field} is {value}"), cx, |settings| {
+                    put(settings, value)
+                });
+            })
+    }
 }
 
 /// `fraction` in whole percents; the slider moves by them, so a saved fraction carries no float noise.
@@ -197,31 +228,6 @@ fn fraction(percent: f64) -> f32 {
     (percent / 100.) as f32
 }
 
-/// A field over `range`, by `step` to `places`, that saves each settled number through `put`.
-fn number(
-    this: &Entity<SettingsWindow>,
-    field: &'static str,
-    value: f32,
-    range: &RangeInclusive<f32>,
-    (step, places): (f64, usize),
-    put: fn(&mut Settings, f32),
-) -> NumberInput {
-    NumberInput::new(field, f64::from(value))
-        .range(f64::from(*range.start()), f64::from(*range.end()))
-        .step(step)
-        .precision(places)
-        .size(ControlSize::Sm)
-        .on_change({
-            let this = this.clone();
-            move |value, _, cx| {
-                let value = value as f32;
-                set(&this, format!("{field} is {value}"), cx, |settings| {
-                    put(settings, value)
-                })
-            }
-        })
-}
-
 fn switch(
     this: &Entity<SettingsWindow>,
     field: &'static str,
@@ -232,7 +238,7 @@ fn switch(
     Switch::new(field, on).on_change(move |on, _, cx| {
         set(&this, format!("{field} is {on}"), cx, |settings| {
             put(settings, on)
-        })
+        });
     })
 }
 
@@ -263,7 +269,7 @@ fn grid(
                     } else {
                         settings.theme = id.into();
                     }
-                })
+                });
             })
     });
     let rem = window.rem_size();

src/term/mod.rs

@@ -18,4 +18,4 @@ mod unix;
 mod view;
 
 pub use pty::{Events, Launch, Listener, Pty, PtyEvent, config, fit, spawn};
-pub use view::{BLINK, ICONS, MONO, TerminalEvent, TerminalView};
+pub use view::{BLINK, ICONS, MONO, TerminalEvent, TerminalView, load_font};

src/term/view.rs

@@ -9,9 +9,9 @@ use alacritty_terminal::{
 use ely_gpui_component::theme::ActiveTheme;
 use futures::StreamExt;
 use gpui::{
-    Bounds, Context, EventEmitter, FocusHandle, Focusable, Font, FontFeatures, FontStyle,
+    Bounds, Context, EventEmitter, FocusHandle, Focusable, Font, FontFeatures, FontId, FontStyle,
     FontWeight, IntoElement, MouseButton, ParentElement, Pixels, Render, Styled, Subscription,
-    Task, Window, div, prelude::*, px, size,
+    Task, TextSystem, Window, div, prelude::*, px, size,
 };
 
 use super::{
@@ -254,6 +254,18 @@ impl TerminalView {
     }
 }
 
+/// `font` as gpui loads it; gpui falls back to another font without a word, and loads none without an `m`.
+pub fn load_font(font: &Font, system: &TextSystem) -> anyhow::Result<FontId> {
+    let id = system.resolve_font(font);
+    let loaded = system.get_font_for_id(id);
+    anyhow::ensure!(
+        loaded.is_some_and(|loaded| loaded.family == font.family),
+        "font_family is {:?}, which the terminal cannot load",
+        font.family
+    );
+    Ok(id)
+}
+
 /// The font, cell and metrics the settings give at this window's scale.
 fn look(settings: &Settings, ink: &Ink, window: &Window) -> anyhow::Result<Look> {
     let font = Font {
@@ -269,7 +281,7 @@ fn look(settings: &Settings, ink: &Ink, window: &Window) -> anyhow::Result<Look>
     };
     let text = px(settings.font_size);
     let system = window.text_system();
-    let id = system.resolve_font(&font);
+    let id = load_font(&font, system)?;
     let advance = system
         .advance(id, text, 'M')
         .map_err(|error| anyhow::anyhow!("{} cannot measure M: {error}", settings.font_family))?;