Skip to content

Commit 1651816

Browse files
simoncozenssimoncozens
committed
Tidy lots of things up, allow pluggable file types
1 parent f97a39a commit 1651816

File tree

16 files changed

+180
-116
lines changed

16 files changed

+180
-116
lines changed

‎fontspector-checkapi/Cargo.toml

+3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ edition = "2021"
88
skrifa = "0.15.0"
99
read-fonts = "0.15.0"
1010

11+
# Filetype
12+
glob-match = "0.2.1"
13+
1114
# Needed so that we can refer to status codes on the command line
1215
clap = {version = "3.2.5", features = ["derive"]}
1316

‎fontspector-checkapi/src/check.rs

+11-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::{font::FontCollection, Status, StatusList, TestFont};
1+
use crate::{font::FontCollection, Registry, Status, StatusList, Testable};
22

33
pub type CheckId = String;
44

@@ -8,8 +8,9 @@ pub struct Check<'a> {
88
pub title: &'a str,
99
pub rationale: Option<&'a str>,
1010
pub proposal: Option<&'a str>,
11-
pub check_one: Option<&'a dyn Fn(&TestFont) -> StatusList>,
11+
pub check_one: Option<&'a dyn Fn(&Testable) -> StatusList>,
1212
pub check_all: Option<&'a dyn Fn(&FontCollection) -> StatusList>,
13+
pub applies_to: &'a str,
1314
}
1415

1516
pub struct CheckResult {
@@ -21,7 +22,14 @@ pub struct CheckResult {
2122
}
2223

2324
impl<'a> Check<'a> {
24-
pub fn run_one(&'a self, f: &'a TestFont) -> Box<dyn Iterator<Item = CheckResult> + 'a> {
25+
pub fn applies(&self, f: &'a Testable, registry: &Registry) -> bool {
26+
registry
27+
.filetypes
28+
.get(self.applies_to)
29+
.map_or(false, |ft| ft.applies(f))
30+
}
31+
32+
pub fn run_one(&'a self, f: &'a Testable) -> Box<dyn Iterator<Item = CheckResult> + 'a> {
2533
if let Some(check_one) = self.check_one {
2634
return Box::new(check_one(f).map(|r| CheckResult {
2735
status: r,

‎fontspector-checkapi/src/filetype.rs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
use crate::Testable;
2+
use glob_match::glob_match;
3+
pub struct FileType<'a> {
4+
pub pattern: &'a str,
5+
}
6+
impl FileType<'_> {
7+
pub fn new(pattern: &str) -> FileType {
8+
FileType { pattern }
9+
}
10+
11+
pub fn applies(&self, file: &Testable) -> bool {
12+
glob_match(self.pattern, &file.filename)
13+
}
14+
}
15+
16+
pub trait FileTypeConvert<T> {
17+
#[allow(clippy::wrong_self_convention)]
18+
fn from_testable(&self, t: &Testable) -> Option<T>;
19+
}

‎fontspector-checkapi/src/font.rs

+13-11
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::constants::RIBBI_STYLE_NAMES;
1+
use crate::{constants::RIBBI_STYLE_NAMES, filetype::FileTypeConvert, FileType, Testable};
22
use read_fonts::{tables::os2::SelectionFlags, TableProvider};
33
use skrifa::{
44
font::FontRef,
@@ -8,21 +8,23 @@ use skrifa::{
88
use std::error::Error;
99

1010
pub struct TestFont {
11-
pub filename: String,
12-
// pub font: FontRef<'a>,
1311
font_data: Vec<u8>,
1412
}
1513

16-
impl TestFont {
17-
pub fn new(filename: &str) -> Result<Self, Box<dyn Error>> {
18-
let font_data = std::fs::read(filename).expect("Couldn't open file");
19-
Ok(Self {
20-
filename: filename.to_owned(),
21-
// font: FontRef::new(&font_data)?,
22-
font_data,
23-
})
14+
pub const TTF: FileType = FileType { pattern: "*.ttf" };
15+
16+
impl<'a> FileTypeConvert<TestFont> for FileType<'a> {
17+
fn from_testable(&self, t: &Testable) -> Option<TestFont> {
18+
if self.applies(t) {
19+
let font_data = std::fs::read(&t.filename).expect("Couldn't open file");
20+
Some(TestFont { font_data })
21+
} else {
22+
None
23+
}
2424
}
25+
}
2526

27+
impl TestFont {
2628
pub fn font(&self) -> FontRef {
2729
FontRef::new(&self.font_data).expect("Can't parse font")
2830
}

‎fontspector-checkapi/src/lib.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
mod check;
22
mod constants;
3+
mod filetype;
34
mod font;
45
mod profile;
56
mod registry;
67
mod status;
8+
mod testable;
79
pub use check::{return_result, Check, CheckId, CheckResult};
8-
pub use font::{FontCollection, TestFont};
10+
pub use filetype::{FileType, FileTypeConvert};
11+
pub use font::{FontCollection, TestFont, TTF};
912
pub use profile::{Override, Profile};
1013
pub use registry::Registry;
1114
pub use status::{Status, StatusCode, StatusList};
15+
pub use testable::Testable;
1216

1317
pub trait Plugin {
1418
fn register(&self, cr: &mut Registry);

‎fontspector-checkapi/src/profile.rs

+9-1
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,17 @@ impl Profile {
4040
}
4141
}
4242
}
43-
if missing_checks.len() > 0 {
43+
if !missing_checks.is_empty() {
4444
return Err(format!("Missing checks: {}", missing_checks.join(", ")));
4545
}
46+
for check in registry.checks.iter() {
47+
if !registry.filetypes.contains_key(check.applies_to) {
48+
return Err(format!(
49+
"Check {} applies to unknown filetype {}",
50+
check.id, check.applies_to
51+
));
52+
}
53+
}
4654
Ok(())
4755
}
4856
}

‎fontspector-checkapi/src/registry.rs

+11-4
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,27 @@
11
use std::collections::HashMap;
22

3-
use crate::{Check, Profile};
3+
use crate::{Check, FileType, Profile, TTF};
44

55
#[derive(Default)]
66
pub struct Registry<'a> {
77
pub checks: Vec<Check<'a>>,
88
profiles: HashMap<String, Profile>,
9+
pub(crate) filetypes: HashMap<String, FileType<'a>>,
910
}
1011

11-
impl Registry<'_> {
12+
impl<'a> Registry<'a> {
1213
pub fn new() -> Registry<'static> {
13-
Registry::default()
14+
let mut reg = Registry::default();
15+
reg.register_filetype("TTF", TTF);
16+
reg
1417
}
1518

1619
pub fn iter(&self) -> impl Iterator<Item = &Check> {
1720
self.checks.iter()
1821
}
1922

2023
pub fn load_plugin(&mut self, plugin_path: &str) {
21-
let plugin = unsafe { crate::load_plugin(&plugin_path) }.unwrap_or_else(|e| {
24+
let plugin = unsafe { crate::load_plugin(plugin_path) }.unwrap_or_else(|e| {
2225
panic!("Could not load plugin {:?}: {:?}", plugin_path, e);
2326
});
2427
plugin.register(self);
@@ -31,4 +34,8 @@ impl Registry<'_> {
3134
pub fn get_profile(&self, name: &str) -> Option<&Profile> {
3235
self.profiles.get(name)
3336
}
37+
38+
pub fn register_filetype(&mut self, name: &str, filetype: FileType<'a>) {
39+
self.filetypes.insert(name.to_string(), filetype);
40+
}
3441
}

‎fontspector-checkapi/src/testable.rs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
pub struct Testable {
2+
pub filename: String,
3+
pub source: Option<String>,
4+
}
5+
6+
impl Testable {
7+
pub fn new(filename: &str) -> Self {
8+
Self {
9+
filename: filename.to_owned(),
10+
source: None,
11+
}
12+
}
13+
14+
pub fn new_with_source(filename: &str, source: &str) -> Self {
15+
Self {
16+
filename: filename.to_owned(),
17+
source: Some(source.to_owned()),
18+
}
19+
}
20+
}

‎fontspector-cli/src/main.rs

+15-21
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22
use std::collections::HashMap;
33

44
use clap::Parser;
5-
use fontspector_checkapi::{
6-
Check, CheckResult, FontCollection, Plugin, Registry, StatusCode, TestFont,
7-
};
5+
use fontspector_checkapi::{Check, CheckResult, Plugin, Registry, StatusCode, Testable};
86
use itertools::iproduct;
97
// use rayon::prelude::*;
108

@@ -55,22 +53,17 @@ fn main() {
5553
}
5654

5755
// Load the relevant profile
58-
let profile = registry
59-
.get_profile(&args.profile)
60-
.expect("Could not load profile");
56+
let profile = registry.get_profile(&args.profile).unwrap_or_else(|| {
57+
log::error!("Could not find profile {:}", args.profile);
58+
std::process::exit(1);
59+
});
6160
if let Err(fail) = profile.validate(&registry) {
62-
println!("Profile validation failed: {:}", fail);
61+
log::error!("Profile validation failed: {:}", fail);
6362
std::process::exit(1);
6463
}
6564

66-
let testables: Vec<TestFont> = args
67-
.inputs
68-
.iter()
69-
.filter(|x| x.ends_with(".ttf"))
70-
.map(|x| TestFont::new(x).unwrap_or_else(|_| panic!("Could not load font {:}", x)))
71-
.collect();
72-
let thing: Vec<&TestFont> = testables.iter().collect();
73-
let collection = FontCollection(thing);
65+
let testables: Vec<Testable> = args.inputs.iter().map(|x| Testable::new(x)).collect();
66+
// let collection = FontCollection(thing);
7467
let checkmap: HashMap<&str, &Check<'_>> = registry.checks.iter().map(|c| (c.id, c)).collect();
7568

7669
for (sectionname, checknames) in profile.sections.iter() {
@@ -83,14 +76,15 @@ fn main() {
8376
})
8477
.collect();
8578

86-
let results_all: Vec<CheckResult> = checks
87-
.iter()
88-
.flat_map(|check| check.run_all(&collection))
89-
.collect();
79+
let results_all = [];
80+
// let results_all: Vec<CheckResult> = checks
81+
// .iter()
82+
// .flat_map(|check| check.run_all(&collection))
83+
// .collect();
9084

9185
let results_one: Vec<CheckResult> = iproduct!(checks.iter(), testables.iter())
92-
.map(|(check, file)| check.run_one(file))
93-
.flatten()
86+
.filter(|(check, file)| check.applies(file, &registry))
87+
.flat_map(|(check, file)| check.run_one(file))
9488
.collect();
9589

9690
for result in results_all

‎profile-testplugin/src/lib.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
use fontspector_checkapi::{return_result, Check, Profile, Registry, StatusList, TestFont};
1+
use fontspector_checkapi::{return_result, Check, Profile, Registry, StatusList, Testable};
22

33
struct Test;
44

5-
fn say_hello(_c: &TestFont) -> StatusList {
5+
fn say_hello(_c: &Testable) -> StatusList {
66
println!("Hello from the test plugin!");
77
return_result(vec![])
88
}
@@ -14,6 +14,7 @@ pub const SAY_HELLO: Check = Check {
1414
proposal: None,
1515
check_all: None,
1616
check_one: Some(&say_hello),
17+
applies_to: "TTF",
1718
};
1819

1920
impl fontspector_checkapi::Plugin for Test {

‎profile-universal/src/checks/bold_italic_unique.rs

+47-47
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,49 @@
1-
use std::collections::HashSet;
1+
// use std::collections::HashSet;
22

3-
use fontspector_checkapi::{return_result, Check, FontCollection, Status, StatusCode, StatusList};
4-
use read_fonts::tables::os2::SelectionFlags;
5-
use skrifa::string::StringId;
3+
// use fontspector_checkapi::{return_result, Check, FontCollection, Status, StatusCode, StatusList};
4+
// use read_fonts::tables::os2::SelectionFlags;
5+
// use skrifa::string::StringId;
66

7-
fn bold_italic_unique(c: &FontCollection) -> StatusList {
8-
let ribbi = c.ribbi_fonts();
9-
let mut problems = vec![];
10-
let mut flags: HashSet<(bool, bool)> = HashSet::new();
11-
for font in ribbi.iter() {
12-
let _names_list = font.get_name_entry_strings(StringId::FAMILY_NAME);
13-
match font.get_os2_fsselection() {
14-
Ok(fsselection) => {
15-
let val = (
16-
fsselection.intersects(SelectionFlags::BOLD),
17-
fsselection.intersects(SelectionFlags::ITALIC),
18-
);
19-
if flags.contains(&val) {
20-
problems.push(Status {
21-
message: Some(format!(
22-
"Font {} has the same selection flags ({}{}{}) as another font",
23-
font.filename,
24-
if val.0 { "bold" } else { "" },
25-
if val.0 && val.1 { " & " } else { "" },
26-
if val.1 { "italic" } else { "" }
27-
)),
28-
code: StatusCode::Error,
29-
});
30-
} else {
31-
flags.insert(val);
32-
}
33-
}
34-
Err(_e) => problems.push(Status {
35-
message: Some(format!("Font {} had no OS2 table", font.filename)),
36-
code: StatusCode::Error,
37-
}),
38-
}
39-
}
40-
return_result(problems)
41-
}
42-
pub const BOLD_ITALIC_UNIQUE_CHECK: Check = Check {
43-
id: "com.adobe.fonts/check/family/bold_italic_unique_for_nameid1",
44-
title: "Check that OS/2.fsSelection bold & italic settings are unique for each NameID1",
45-
rationale: None,
46-
proposal: Some("https://github.com/googlefonts/fontbakery/pull/2388"),
47-
check_all: Some(&bold_italic_unique),
48-
check_one: None,
49-
};
7+
// fn bold_italic_unique(c: &FontCollection) -> StatusList {
8+
// let ribbi = c.ribbi_fonts();
9+
// let mut problems = vec![];
10+
// let mut flags: HashSet<(bool, bool)> = HashSet::new();
11+
// for font in ribbi.iter() {
12+
// let _names_list = font.get_name_entry_strings(StringId::FAMILY_NAME);
13+
// match font.get_os2_fsselection() {
14+
// Ok(fsselection) => {
15+
// let val = (
16+
// fsselection.intersects(SelectionFlags::BOLD),
17+
// fsselection.intersects(SelectionFlags::ITALIC),
18+
// );
19+
// if flags.contains(&val) {
20+
// problems.push(Status {
21+
// message: Some(format!(
22+
// "Font {} has the same selection flags ({}{}{}) as another font",
23+
// font.filename,
24+
// if val.0 { "bold" } else { "" },
25+
// if val.0 && val.1 { " & " } else { "" },
26+
// if val.1 { "italic" } else { "" }
27+
// )),
28+
// code: StatusCode::Error,
29+
// });
30+
// } else {
31+
// flags.insert(val);
32+
// }
33+
// }
34+
// Err(_e) => problems.push(Status {
35+
// message: Some(format!("Font {} had no OS2 table", font.filename)),
36+
// code: StatusCode::Error,
37+
// }),
38+
// }
39+
// }
40+
// return_result(problems)
41+
// }
42+
// pub const BOLD_ITALIC_UNIQUE_CHECK: Check = Check {
43+
// id: "com.adobe.fonts/check/family/bold_italic_unique_for_nameid1",
44+
// title: "Check that OS/2.fsSelection bold & italic settings are unique for each NameID1",
45+
// rationale: None,
46+
// proposal: Some("https://github.com/googlefonts/fontbakery/pull/2388"),
47+
// check_all: Some(&bold_italic_unique),
48+
// check_one: None,
49+
// };

‎profile-universal/src/checks/mod.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
mod bold_italic_unique;
22
mod name_trailing_spaces;
3-
mod unwanted_tables;
43
mod required_tables;
5-
pub use bold_italic_unique::BOLD_ITALIC_UNIQUE_CHECK;
4+
mod unwanted_tables;
5+
// pub use bold_italic_unique::BOLD_ITALIC_UNIQUE_CHECK;
66
pub use name_trailing_spaces::NAME_TRAILING_SPACES_CHECK;
7-
pub use unwanted_tables::UNWANTED_TABLES_CHECK;
87
pub use required_tables::REQUIRED_TABLES_CHECK;
8+
pub use unwanted_tables::UNWANTED_TABLES_CHECK;

0 commit comments

Comments
 (0)