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 a sort_fonts function that internally calls FCFontSort. #39

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

alokedesai
Copy link
Contributor

  • This PR adds a sort_fonts function (which internally calls FCFontSort) and an add_integer function (which internally calls FCPatternAddInteger).
  • The former is especially useful for font fallback support as the information returned by list_fonts does not allow for sorting by closeness or trimming results from the font set if the an earlier font in the list already covers the same unicode range as a given font.

@alokedesai
Copy link
Contributor Author

Hey @wezm 👋🏽 Thanks for maintaining such a fantastic crate. Would love your thoughts on this and whether this is a change you'd accept. Please let me know if you have any questions on the approach!

Copy link
Contributor

@wezm wezm left a comment

Choose a reason for hiding this comment

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

Overall the changes look good. Some minor tweaks suggested. Also please make sure to run cargo fmt.

@@ -511,6 +529,17 @@ pub fn list_fonts<'fc>(pattern: &Pattern<'fc>, objects: Option<&ObjectSet>) -> F
}
}

/// Returns a `FontSet` containing fonts sorted by closeness to the supplied `pattern`. If `trim` is true, elements in
/// the list which don't include Unicode coverage not provided by earlier elements in the list are elided.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

fontconfig/src/lib.rs Outdated Show resolved Hide resolved
@alokedesai
Copy link
Contributor Author

Thanks for the review, PTAL.

@wezm wezm merged commit 42e049c into yeslogic:master Jan 18, 2024
2 checks passed
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