Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add fallible font methods to FontCollection #63

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

acarl005
Copy link

Add more fallible getter methods a. la #62.

self.font_family_by_name(family_name).unwrap()
}

pub fn font_family_by_name(&self, family_name: &str) -> Result<Option<FontFamily>, HRESULT> {
Copy link
Author

@acarl005 acarl005 Feb 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that we have a Result<Option<FontFamily>, HRESULT> instead of just a Result<FontFamily, HRESULT>. I think this makes sense, as there may be no font family with the given name, so Ok(None) is returned to designate that the name doesn't exist, and no errors occurred.

An alternative I considered was creating a new error enum, e.g.

enum GetError {
    Win32Error(HRESULT),
    NotFound
}

And we could return Result<FontFamily, GetError> instead.

self.font_from_descriptor(desc).unwrap()
}

// Find a font matching the given font descriptor in this font collection.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Find a font matching the given font descriptor in this font collection.
/// Find a font matching the given font descriptor in this font collection.

@mrobinson
Copy link
Member

@acarl005 I'm not able to modify your branch. Please apply the following diff and I will land this one:

diff --git a/src/font_collection.rs b/src/font_collection.rs
index 3e4974f..2154b28 100644
--- a/src/font_collection.rs
+++ b/src/font_collection.rs
@@ -114,6 +114,7 @@ impl FontCollection {
         self.font_family(index).unwrap()
     }
 
+    /// Returns the [`FontFamily`] at the given index.
     pub fn font_family(&self, index: u32) -> Result<FontFamily, HRESULT> {
         let mut family: *mut IDWriteFontFamily = ptr::null_mut();
         unsafe {
@@ -130,7 +131,7 @@ impl FontCollection {
         self.font_from_descriptor(desc).unwrap()
     }
 
-    // Find a font matching the given font descriptor in this font collection.
+    /// Find a font matching the given font descriptor in this [`FontCollection`].
     pub fn font_from_descriptor(&self, desc: &FontDescriptor) -> Result<Option<Font>, HRESULT> {
         if let Some(family) = self.font_family_by_name(&desc.family_name)? {
             let font = family.first_matching_font(desc.weight, desc.stretch, desc.style)?;
@@ -151,6 +152,7 @@ impl FontCollection {
         self.font_from_face(face).ok()
     }
 
+    /// Get a [`Font`] from the given [`FontFace`].
     pub fn font_from_face(&self, face: &FontFace) -> Result<Font, HRESULT> {
         let mut font: *mut IDWriteFont = ptr::null_mut();
         unsafe {
@@ -167,6 +169,8 @@ impl FontCollection {
         self.font_family_by_name(family_name).unwrap()
     }
 
+    /// Find a [`FontFamily`] with the given name. Returns `None` if no family
+    /// with that name is found.
     pub fn font_family_by_name(&self, family_name: &str) -> Result<Option<FontFamily>, HRESULT> {
         let mut index: u32 = 0;
         let mut exists: BOOL = FALSE;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants