Fix the fonts panic and completions in the settings (#4205)

Kirill Bulatov created 2 years ago

* Do not add fonts' `font_family` into the font completion list, if
there's no corresponding attribute in the font data
* Remove `font_style_name` and `font_name` attribute from the completion
options, since those bring `-Italic`, `-Bold` variants that are not
family names (which we use to load the fonts, so they are useless)

Release Notes:

- Improved font family names completion in the settings

Change summary

crates/gpui/src/platform/mac/text_system.rs | 45 ++++++++++++++++++++--
1 file changed, 41 insertions(+), 4 deletions(-)

Detailed changes

crates/gpui/src/platform/mac/text_system.rs

@@ -81,13 +81,11 @@ impl PlatformTextSystem for MacTextSystem {
     fn all_font_names(&self) -> Vec<String> {
         let collection = core_text::font_collection::create_for_all_families();
         let Some(descriptors) = collection.get_descriptors() else {
-            return vec![];
+            return Vec::new();
         };
         let mut names = BTreeSet::new();
         for descriptor in descriptors.into_iter() {
-            names.insert(descriptor.font_name());
-            names.insert(descriptor.family_name());
-            names.insert(descriptor.style_name());
+            names.extend(lenient_font_attributes::family_name(&descriptor));
         }
         if let Ok(fonts_in_memory) = self.0.read().memory_source.all_families() {
             names.extend(fonts_in_memory);
@@ -612,6 +610,45 @@ impl From<FontStyle> for FontkitStyle {
     }
 }
 
+// Some fonts may have no attributest despite `core_text` requiring them (and panicking).
+// This is the same version as `core_text` has without `expect` calls.
+mod lenient_font_attributes {
+    use core_foundation::{
+        base::{CFRetain, CFType, TCFType},
+        string::{CFString, CFStringRef},
+    };
+    use core_text::font_descriptor::{
+        kCTFontFamilyNameAttribute, CTFontDescriptor, CTFontDescriptorCopyAttribute,
+    };
+
+    pub fn family_name(descriptor: &CTFontDescriptor) -> Option<String> {
+        unsafe { get_string_attribute(descriptor, kCTFontFamilyNameAttribute) }
+    }
+
+    fn get_string_attribute(
+        descriptor: &CTFontDescriptor,
+        attribute: CFStringRef,
+    ) -> Option<String> {
+        unsafe {
+            let value = CTFontDescriptorCopyAttribute(descriptor.as_concrete_TypeRef(), attribute);
+            if value.is_null() {
+                return None;
+            }
+
+            let value = CFType::wrap_under_create_rule(value);
+            assert!(value.instance_of::<CFString>());
+            let s = wrap_under_get_rule(value.as_CFTypeRef() as CFStringRef);
+            Some(s.to_string())
+        }
+    }
+
+    unsafe fn wrap_under_get_rule(reference: CFStringRef) -> CFString {
+        assert!(!reference.is_null(), "Attempted to create a NULL object.");
+        let reference = CFRetain(reference as *const ::std::os::raw::c_void) as CFStringRef;
+        TCFType::wrap_under_create_rule(reference)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::{font, px, FontRun, MacTextSystem, PlatformTextSystem};