From fdda34ab1c87f7259c888cea7cef1405a743fd8c Mon Sep 17 00:00:00 2001 From: hhdebb Date: Sun, 13 Sep 2026 17:21:39 +0800 Subject: [PATCH] fix(terminal): reread the font fallback chain instead of cloning it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `with_terminal` builds the chain once, out of `font_family` and `font_fallbacks`, and from then on it is only ever copied: `set_font_family` took it off the font it was replacing, and `alt_font` takes it off the regular face when it builds bold and italic. Nothing reread it. So a `font_fallbacks` edit had no live path at all — only panes opened afterwards saw it. Changing `font_family` and changing it back did not help either, because that path cloned the chain too. Carrying the chain across a family change is also wrong on its own terms. `fallback_chain` decides the pins from the family it is handed: it skips pinning a last-resort face that the family already is, and pins the bundled Hack otherwise. The chain built for `Hack` therefore has no Hack in it, and reusing it after a switch away from Hack leaves the anchor missing. `set_font_family` now rebuilds from the config, `reload_from_config` watches `font_fallbacks` and pushes a rebuild into every open pane, and the rebuild writes all three faces rather than the regular one alone — bold and italic carry no chain of their own, so skipping them would strand two thirds of the text on the old one. --- src/terminal/view.rs | 68 ++++++++++++++++++++++++++++++++++++++++++-- src/ui/app.rs | 22 +++++++++++++- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 56555d5d..1cf69399 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -1208,6 +1208,27 @@ fn fallback_chain(family: &str, configured: &[String]) -> Vec { chain } +/// Put `chain` on the regular face and on the bold and italic ones. +/// +/// Bold and italic never carry a chain of their own — `alt_font` copies theirs +/// off the regular face when they are built — so a rebuild that skipped them +/// would leave two of the three faces resolving against the old chain. +fn apply_fallback_chain( + chain: Vec, + font: &mut Font, + bold: &mut Option, + italic: &mut Option, +) { + let fallbacks = Some(gpui::FontFallbacks::from_fonts(chain)); + font.fallbacks = fallbacks.clone(); + if let Some(font) = bold { + font.fallbacks = fallbacks.clone(); + } + if let Some(font) = italic { + font.fallbacks = fallbacks; + } +} + impl TerminalView { pub fn spawn_shell_terminal_in( workspace: Option, @@ -3460,16 +3481,32 @@ impl TerminalView { } pub fn set_font_family(&mut self, family: String, cx: &mut Context) { - let fallbacks = self.font.fallbacks.clone(); let mut font = gpui::font(family); - font.fallbacks = fallbacks; if let Some(features) = &self.font_features { font.features = features.clone(); } self.font = font; + // Rebuild rather than carry the chain over: `fallback_chain` skips + // pinning a last-resort face that the family already is, so the chain + // that went with the old family can be missing a pin the new one needs. + self.reread_fallback_chain(cx); cx.notify(); } + /// Rebuild the fallback chain from the config and put it on all three faces. + /// + /// The chain is built once in `with_terminal` and then only ever cloned + /// around, so a `font_fallbacks` edit reaches new panes and no one else. + pub fn reread_fallback_chain(&mut self, cx: &mut Context) { + let chain = fallback_chain(&self.font.family, &cx.global::().font_fallbacks); + apply_fallback_chain( + chain, + &mut self.font, + &mut self.font_bold, + &mut self.font_italic, + ); + } + pub fn set_font_family_bold(&mut self, family: Option, cx: &mut Context) { self.font_bold = self.alt_font(family); cx.notify(); @@ -8579,6 +8616,33 @@ mod tests { ); } + #[test] + fn apply_fallback_chain_reaches_every_face_a_view_has() { + // Bold and italic are the ones at risk: they hold a copy taken off the + // regular face when `alt_font` built them, so a rebuild that wrote only + // the regular face would strand them on the chain it replaced. + let mut font = gpui::font("Hack"); + let mut bold = Some(gpui::font("Hack Bold")); + let mut italic = Some(gpui::font("Hack Italic")); + + super::apply_fallback_chain(vec!["Menlo".to_string()], &mut font, &mut bold, &mut italic); + + for face in [&font, bold.as_ref().unwrap(), italic.as_ref().unwrap()] { + assert_eq!(face.fallbacks.as_ref().unwrap().fallback_list(), ["Menlo"]); + } + + // Neither is configured by default, and a view carries `None` for one + // it was never given. + let (mut none_bold, mut none_italic) = (None, None); + super::apply_fallback_chain( + vec!["Menlo".to_string()], + &mut font, + &mut none_bold, + &mut none_italic, + ); + assert_eq!(font.fallbacks.unwrap().fallback_list(), ["Menlo"]); + } + #[test] fn fallback_chain_appends_platform_stock_faces() { let stock = crate::core::config::platform_last_resort_fallbacks(); diff --git a/src/ui/app.rs b/src/ui/app.rs index 8409a7c5..96f97ead 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -852,6 +852,7 @@ pub struct Tty7App { pub(crate) font_family: String, pub(crate) font_family_bold: Option, pub(crate) font_family_italic: Option, + pub(crate) font_fallbacks: Vec, pub(crate) font_features: Option, terminal_cursor_style: ConfigCursorStyle, terminal_scrollback_limit: usize, @@ -1323,6 +1324,7 @@ impl Tty7App { font_family, font_family_bold, font_family_italic, + font_fallbacks, font_features, terminal_cursor_style, terminal_scrollback_limit, @@ -1334,6 +1336,7 @@ impl Tty7App { cfg.font_family.clone(), cfg.font_family_bold.clone(), cfg.font_family_italic.clone(), + cfg.font_fallbacks.clone(), cfg.font_features .as_ref() .map(crate::core::config::gpui_font_features), @@ -1469,6 +1472,7 @@ impl Tty7App { font_family, font_family_bold, font_family_italic, + font_fallbacks, font_features, terminal_cursor_style, terminal_scrollback_limit, @@ -6336,12 +6340,13 @@ impl Tty7App { self.terminal_scrollback_limit = config.scrollback_limit; self.apply_terminal_config_to_panes(&config, cx); } - let (font_size, line_height, font_family, font_features) = { + let (font_size, line_height, font_family, font_fallbacks, font_features) = { let cfg = cx.global::(); ( cfg.font_size, cfg.line_height, cfg.font_family.clone(), + cfg.font_fallbacks.clone(), cfg.font_features .as_ref() .map(crate::core::config::gpui_font_features), @@ -6384,6 +6389,21 @@ impl Tty7App { } } } + // A `font_fallbacks` edit on its own reached no live pane at all: the + // chain is built once per view and from then on only cloned around. + // `set_font_family` rereads it too, so an edit that moves both ends up + // building the same chain twice rather than disagreeing about it. + if font_fallbacks != self.font_fallbacks { + self.font_fallbacks = font_fallbacks; + for tab in &self.tabs { + for leaf in tab.pane.terminals() { + leaf.update(cx, |v, cx| { + v.reread_fallback_chain(cx); + cx.notify(); + }); + } + } + } if font_features != self.font_features { self.font_features = font_features.clone(); for tab in &self.tabs {