Panics · P01 Primitives: init returns an error, a missing app icon logs and shows a broken picture

src/lib.rs

@@ -47,19 +47,23 @@ pub mod typography;
 
 pub use assets::Assets;
 
+use anyhow::Context as _;
 use gpui::{App, KeyBinding};
 
 use crate::primitives::{FocusNext, FocusPrev, IconName};
 
 /// Loads fonts and the theme, binds Tab and text keys. Call once, first.
-pub fn init(cx: &mut App) {
+pub fn init(cx: &mut App) -> anyhow::Result<()> {
     match cx.asset_source().load(IconName::Check.path()) {
         Ok(Some(_)) => {}
-        Ok(None) => panic!("ely: pass `ely_gpui_component::Assets` to `Application::with_assets`"),
-        Err(error) => panic!("ely: asset source failed: {error:#}"),
+        Ok(None) => {
+            anyhow::bail!("ely: pass `ely_gpui_component::Assets` to `Application::with_assets`")
+        }
+        Err(error) => return Err(error.context("ely: asset source failed")),
     }
-    assets::load_fonts(cx).expect("ely: embedded fonts failed to register");
+    assets::load_fonts(cx).context("ely: embedded fonts failed to register")?;
     setup(cx);
+    Ok(())
 }
 
 /// `init` without assets or fonts, for app tests that render.

src/primitives/icon.rs

@@ -1,4 +1,4 @@
-use std::collections::HashSet;
+use std::collections::HashMap;
 
 use gpui::{
     App, AssetSource, Global, Hsla, IntoElement, Radians, RenderOnce, SharedString, Transformation,
@@ -362,7 +362,7 @@ impl Icon {
         }
     }
 
-    /// Draws an SVG supplied by the application's asset source; a missing one fails at render.
+    /// Draws an SVG supplied by the application's asset source; a missing one logs and shows `broken`.
     pub fn from_path(path: impl Into<SharedString>) -> Self {
         Self {
             path: path.into(),
@@ -414,80 +414,67 @@ impl From<IconName> for Icon {
     }
 }
 
-/// App paths already found in the asset source.
+/// App paths already looked up, and whether the source held them.
 #[derive(Default)]
-struct FoundIcons(HashSet<SharedString>);
+struct FoundIcons(HashMap<SharedString, bool>);
 
 impl Global for FoundIcons {}
 
-fn ensure_found(source: &dyn AssetSource, path: &str) {
+fn found(source: &dyn AssetSource, path: &str) -> bool {
     match source.load(path) {
-        Ok(Some(_)) => {}
-        Ok(None) => panic!("ely: icon {path} is not in the app's asset source"),
-        Err(error) => panic!("ely: icon {path} failed to load: {error:#}"),
+        Ok(Some(_)) => true,
+        Ok(None) => {
+            log::error!("ely: icon {path} is not in the app's asset source");
+            false
+        }
+        Err(error) => {
+            log::error!("ely: icon {path} failed to load: {error:#}");
+            false
+        }
+    }
+}
+
+impl Icon {
+    /// What a missing app icon shows: a broken picture in danger.
+    fn broken(self, cx: &App) -> Self {
+        Self {
+            path: IconName::ImageOff.path().into(),
+            bundled: true,
+            color: Some(cx.theme().colors.danger),
+            hover: None,
+            ..self
+        }
     }
 }
 
 impl RenderOnce for Icon {
     fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement {
-        if !self.bundled && !cx.default_global::<FoundIcons>().0.contains(&self.path) {
-            ensure_found(cx.asset_source().as_ref(), &self.path);
-            cx.default_global::<FoundIcons>()
+        let source = cx.asset_source().clone();
+        let missing = !self.bundled
+            && !*cx
+                .default_global::<FoundIcons>()
                 .0
-                .insert(self.path.clone());
-        }
+                .entry(self.path.clone())
+                .or_insert_with(|| found(source.as_ref(), &self.path));
+        let icon = if missing { self.broken(cx) } else { self };
         let theme = cx.theme();
-        let color = self.color.unwrap_or(theme.colors.fg);
+        let color = icon.color.unwrap_or(theme.colors.fg);
         svg()
-            .path(self.path)
-            .size(theme.icon_size(self.size))
+            .path(icon.path)
+            .size(theme.icon_size(icon.size))
             .flex_none()
             .text_color(color)
-            .when_some(self.hover, |svg, (group, hover)| {
+            .when_some(icon.hover, |svg, (group, hover)| {
                 svg.group_hover(group, |style| style.text_color(hover))
             })
-            .when(self.rotation.is_some() || self.scale != 1.0, |svg| {
+            .when(icon.rotation.is_some() || icon.scale != 1.0, |svg| {
                 svg.with_transformation(
-                    Transformation::rotate(self.rotation.unwrap_or(Radians(0.0)))
-                        .with_scaling(size(self.scale, self.scale)),
+                    Transformation::rotate(icon.rotation.unwrap_or(Radians(0.0)))
+                        .with_scaling(size(icon.scale, icon.scale)),
                 )
             })
     }
 }
 
 #[cfg(test)]
-mod tests {
-    use super::{Icon, IconName, ensure_found};
-
-    #[test]
-    fn every_icon_ships_in_the_bundle() {
-        for icon in IconName::ALL {
-            assert!(
-                crate::Assets::get(icon.path()).is_some(),
-                "{icon:?} missing at {}",
-                icon.path()
-            );
-        }
-    }
-
-    #[test]
-    fn an_app_path_draws_as_given_and_a_name_as_its_bundled_file() {
-        let custom = Icon::from_path("app-icons/mark.svg");
-        assert_eq!(custom.path.as_ref(), "app-icons/mark.svg");
-        assert!(!custom.bundled);
-        let named = Icon::new(IconName::Check);
-        assert_eq!(named.path.as_ref(), IconName::Check.path());
-        assert!(named.bundled);
-    }
-
-    #[test]
-    fn a_path_the_source_holds_is_found() {
-        ensure_found(&crate::Assets, IconName::Check.path());
-    }
-
-    #[test]
-    #[should_panic(expected = "ely: icon app-icons/missing.svg is not in the app's asset source")]
-    fn a_path_the_source_lacks_fails() {
-        ensure_found(&crate::Assets, "app-icons/missing.svg");
-    }
-}
+mod tests;

src/primitives/icon/tests.rs

@@ -0,0 +1,70 @@
+use gpui::{Context, IntoElement, Render, TestAppContext, Window, black};
+
+use super::{FoundIcons, Icon, IconName, found};
+use crate::theme::{ActiveTheme, Theme};
+
+#[test]
+fn every_icon_ships_in_the_bundle() {
+    for icon in IconName::ALL {
+        assert!(
+            crate::Assets::get(icon.path()).is_some(),
+            "{icon:?} missing at {}",
+            icon.path()
+        );
+    }
+}
+
+#[test]
+fn an_app_path_draws_as_given_and_a_name_as_its_bundled_file() {
+    let custom = Icon::from_path("app-icons/mark.svg");
+    assert_eq!(custom.path.as_ref(), "app-icons/mark.svg");
+    assert!(!custom.bundled);
+    let named = Icon::new(IconName::Check);
+    assert_eq!(named.path.as_ref(), IconName::Check.path());
+    assert!(named.bundled);
+}
+
+#[test]
+fn the_source_answers_whether_it_holds_a_path() {
+    assert!(found(&crate::Assets, IconName::Check.path()));
+    assert!(!found(&crate::Assets, "app-icons/missing.svg"));
+}
+
+struct Missing;
+
+impl Render for Missing {
+    fn render(&mut self, _: &mut Window, _: &mut Context<Self>) -> impl IntoElement {
+        Icon::from_path("app-icons/missing.svg")
+    }
+}
+
+#[gpui::test]
+fn a_missing_app_icon_draws_its_mark(cx: &mut TestAppContext) {
+    cx.update(Theme::init);
+    let (_, cx) = cx.add_window_view(|_, _| Missing);
+    cx.run_until_parked();
+    let seen = cx.update(|_, cx| cx.global::<FoundIcons>().0.clone());
+    assert_eq!(seen.get("app-icons/missing.svg"), Some(&false));
+}
+
+#[gpui::test]
+fn a_broken_icon_shows_a_broken_picture_in_danger(cx: &mut TestAppContext) {
+    cx.update(|cx| {
+        Theme::init(cx);
+        let icon = Icon::from_path("app-icons/missing.svg")
+            .color(black())
+            .group_hover_color("row", black())
+            .broken(cx);
+        assert_eq!(icon.path.as_ref(), IconName::ImageOff.path());
+        assert_eq!(icon.color, Some(cx.theme().colors.danger));
+        assert!(icon.hover.is_none(), "hover keeps the danger color");
+    });
+}
+
+#[gpui::test]
+fn init_without_ely_assets_returns_an_error(cx: &mut TestAppContext) {
+    let error = cx
+        .update(crate::init)
+        .expect_err("the test source holds no Ely assets");
+    assert!(format!("{error:#}").contains("pass `ely_gpui_component::Assets`"));
+}