fontdb icon indicating copy to clipboard operation
fontdb copied to clipboard

expand home paths in fontconfig

Open snoyer opened this issue 2 years ago • 2 comments

Paths retrieved from fontconfig may contain ~ and need to be expanded. There is probably a prettier/better way to do this but I've never looked at any Rust code before so I wouldn't know, feel free to treat this as a bug report rather that a PR and fix it the proper way if needed.

snoyer avatar Sep 14 '22 12:09 snoyer

I'm not sure how much error handling and edge cases you want to cover. Assuming doing the minimum of trying to expand ~/ and silently ignoring the case where HOME is not set (leaving ~/ in and letting load_fonts_dir() deal with non-existing path) maybe having a function is simpler?

            fn _expand_home<P: AsRef<std::path::Path>>(path: P) -> std::path::PathBuf {
                if let Ok(ref suffix) = path.as_ref().strip_prefix("~/") {
                    if let Ok(ref home) = std::env::var("HOME") {
                        let home_path = std::path::Path::new(home);
                        return home_path.join(suffix);
                    }
                }
                return path.as_ref().to_path_buf();
            }

            #[cfg(feature = "fontconfig")]
            {
                let mut fontconfig = fontconfig_parser::FontConfig::default();
                match fontconfig.merge_config("/etc/fonts/fonts.conf") {
                    Ok(_) => {
                        for dir in fontconfig.dirs {
                            self.load_fonts_dir(_expand_home(dir.path));
                        }

                        // Yes, stop here. No need to load fonts from hardcoded paths.
                        return;
                    }
                    Err(err) => {
                        // Fontconfig parsing failed? Try to load fonts from hardcoded paths then.
                        log::warn!("Failed to parse fontconfig because: {}", err);
                    }
                }
            }

            self.load_fonts_dir("/usr/share/fonts/");
            self.load_fonts_dir("/usr/local/share/fonts/");
            self.load_fonts_dir(_expand_home("~/.fonts"));
            self.load_fonts_dir(_expand_home("~/.local/share/fonts"));

That's pretty much the limit of my understanding of the language and of the codebase though :(

snoyer avatar Sep 15 '22 03:09 snoyer

How about just:

                        for dir in fontconfig.dirs {
                            let path = if dir.path.starts_with("~") {
                                if let Ok(ref home) = std::env::var("HOME") {
                                    let home_path = std::path::Path::new(home);
                                    home_path.join(dir.path.strip_prefix("~").unwrap())
                                } else {
                                    continue;
                                }
                            } else {
                                dir.path
                            };
                            self.load_fonts_dir(path);
                        }

this is right after match fontconfig.merge_config("/etc/fonts/fonts.conf") {

This should be enough.

RazrFalcon avatar Sep 15 '22 03:09 RazrFalcon

Done.

RazrFalcon avatar Oct 22 '22 16:10 RazrFalcon