Skip to content

Add the default for fontconfig cachedir #265

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

declantsien
Copy link

@declantsien declantsien commented Feb 2, 2025

I have a fontconfig configuration file without cachedir.
Without this change Collection.family_names() is empty.
According to FontConfig User Documation, the default value for cachedir is $XDG_CACHE_HOME/fontconfig

FYI, This PR add an extra dep (xdg) for fontique on linux.

@declantsien declantsien force-pushed the fontconfig_xdg_cachedir_with_a_default branch from 028e7bf to ead42bf Compare February 3, 2025 00:47
.as_path(),
&mut config,
);
let xdg_dirs = xdg::BaseDirectories::with_prefix("fontconfig").unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

Is unwrapping the right behaviour here? In case we couldn't determine $HOME previously, it looks like we just didn't parse this file?

Copy link
Author

Choose a reason for hiding this comment

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

Removed the xdg dep

impl Config {
pub fn cache_dirs(&self) -> Vec<PathBuf> {
if self.cache_dirs.is_empty() {
let xdg_dirs = xdg::BaseDirectories::with_prefix("fontconfig").unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

This does an awful lot of allocation, given that we only use one item from it.

Copy link
Author

Choose a reason for hiding this comment

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

Removed

@declantsien declantsien force-pushed the fontconfig_xdg_cachedir_with_a_default branch 3 times, most recently from 3f6b28a to 1c73527 Compare February 3, 2025 13:48
@declantsien
Copy link
Author

@@ -30,6 +30,7 @@ pub fn parse_config(path: &Path, sink: &mut impl ParserSink) {
if root.tag_name().name() != "fontconfig" {
return;
}
let mut cachedir_count = 0;
Copy link
Member

Choose a reason for hiding this comment

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

I'd probably just use a boolean here, since the actual count doesn't matter.

Copy link
Author

Choose a reason for hiding this comment

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

Updated

@tomcur
Copy link
Member

tomcur commented Feb 3, 2025

It looks like the test failure is hitting against the TODO in parley/src/layout/cluster.rs line 477.

// TODO: Use a test font

@declantsien declantsien force-pushed the fontconfig_xdg_cachedir_with_a_default branch from 1c73527 to b26737a Compare February 3, 2025 14:15
@tomcur
Copy link
Member

tomcur commented Feb 3, 2025

I believe this PR isn't the cause of the test failure, but simply uncovered an issue where the test isn't explicitly using a known font. I'll likely have time to look at it, but perhaps not today.

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.

3 participants