add analogous fallible font fetching methods#62
Conversation
src/font_family.rs
Outdated
| .unwrap() | ||
| } | ||
|
|
||
| pub fn try_get_first_matching_font( |
There was a problem hiding this comment.
Since Rust idiomatically doesn't use the word get_ in method names, I wonder if a way to do this would be to make the fallible ones names like first_matching_font and then to deprecate the non-fallible versions. That way we could gradually transition to a more idiomatically-Rust-like interface. What do you think?
There was a problem hiding this comment.
Sounds good to me! Will update.
There was a problem hiding this comment.
Ok I've updated the names and #[deprecated] the old methods.
| self.font(index).unwrap() | ||
| } | ||
|
|
||
| pub fn font(&self, index: u32) -> Result<Font, HRESULT> { |
There was a problem hiding this comment.
I'm a little unsure about the name font for this method. Any suggestions? font_from_index maybe?
There was a problem hiding this comment.
font_at_index or font_from_index both sound good to me.
mrobinson
left a comment
There was a problem hiding this comment.
Great! I think this looks good. I do agree that font_at_index/font_from_index might be a better name, but this could probably land without that as well.
|
@acarl005 Thanks for the change! Let me know if you like to rename |
|
@mrobinson nah, I like the name actually. |
This closes #61
Some of the methods in
font_family::FontFamilycan cause a panic. I have added analogoustry_methods for each which will return aResultinstead in order to let the caller handle the error.The next step would be to start migrating callers to use these new methods, e.g.
https://github.com/servo/font-kit/blob/d49041ca57da4e9b412951f96f74cb34e3b6324f/src/sources/directwrite.rs#L44