Skip to content

Commit 8dea632

Browse files
ollimeierollimeier
committed
feat(fontwerk/name_consistency): Refactor code
1 parent edff19f commit 8dea632

File tree

1 file changed

+76
-127
lines changed
  • profile-fontwerk/src/checks/fontwerk

1 file changed

+76
-127
lines changed

‎profile-fontwerk/src/checks/fontwerk/names.rs‎

Lines changed: 76 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ fn name_entries(f: &Testable, context: &Context) -> CheckFnResult {
8181
id = "fontwerk/name_consistency",
8282
rationale = "
8383
Check if names are consistently written throughout the name table:
84-
1 + 2 == 16 + 17 == 21 + 22
84+
1 + 2 == 4 == 16 + 17 == 21 + 22
8585
('Regular' will be ignored, because it may be elidable)
8686
",
8787
proposal = "https://github.com/ollimeier/fontspector/issues/2",
@@ -93,100 +93,66 @@ fn name_consistency(t: &Testable, context: &Context) -> CheckFnResult {
9393
let mut bad_names: Vec<String> = vec![];
9494

9595
let name_ids: Vec<(StringId, Option<StringId>)> = vec![
96-
(StringId::FAMILY_NAME, Some(StringId::SUBFAMILY_NAME)), // NID 1 + 2, required for Fontwerk
97-
(StringId::FULL_NAME, None), // NID 4, required for Fontwerk
98-
(StringId::TYPOGRAPHIC_FAMILY_NAME, Some(StringId::TYPOGRAPHIC_SUBFAMILY_NAME)), // required for Fontwerk
99-
(StringId::WWS_FAMILY_NAME, Some(StringId::WWS_SUBFAMILY_NAME)), // not required
96+
(StringId::FAMILY_NAME, Some(StringId::SUBFAMILY_NAME)),
97+
(StringId::FULL_NAME, None),
98+
(StringId::TYPOGRAPHIC_FAMILY_NAME, Some(StringId::TYPOGRAPHIC_SUBFAMILY_NAME)),
99+
(StringId::WWS_FAMILY_NAME, Some(StringId::WWS_SUBFAMILY_NAME)),
100100
];
101-
// Get the name table of font.font()
102-
let name = font.font().name().unwrap();
103-
let records: Vec<NameRecord> = name
104-
.name_record()
105-
.iter()
106-
.map(|r| {
107-
#[allow(clippy::unwrap_used)]
108-
NameRecord::new(
109-
r.platform_id(),
110-
r.encoding_id(),
111-
r.language_id(),
112-
r.name_id(),
113-
r.string(name.string_data())
114-
.unwrap()
115-
.chars()
116-
.collect::<String>()
117-
.to_string()
118-
.into(),
119-
)
120-
})
121-
.collect();
122-
let name_table = Name::new(records);
123-
let name_codes = get_name_PEL_codes(name_table);
124-
for code in name_codes.as_ref().unwrap() {
125-
let mut name_strings: Vec<String> = vec![];
126-
for (platform, encoding, language) in code.iter() {
127-
for (fam_id, sub_id) in name_ids.iter() {
128-
let mut full_name = String::new();
129-
let mut pair = vec![];
130-
if let Some(fam_string) = get_name_entry_string(&font.font(),
131-
*platform,
132-
*encoding,
133-
*language,
134-
*fam_id,
135-
) {
136-
pair.push(true);
137-
full_name.push_str(&fam_string.to_string());
138-
full_name.push(' ');
139-
} else {
140-
if *fam_id == StringId::WWS_FAMILY_NAME {
141-
// WWS_FAMILY_NAME is optional, so we don't fail if it's missing.
142-
} else {
143-
bad_names.push(format!("Missing required name table entry: {fam_id}"));
144-
continue;
101+
102+
let name_PEL_codes = get_name_PEL_codes(font.font());
103+
if let Some(PEL_codes) = name_PEL_codes {
104+
for code in PEL_codes {
105+
let mut name_strings: Vec<(String, String)> = vec![];
106+
for (platform, encoding, language) in code.iter() {
107+
for (i, name_id_pair) in name_ids.iter().enumerate() {
108+
let mut full_name = String::new();
109+
let mut pair = vec![];
110+
let mut id_pair = vec![name_id_pair.0, ];
111+
if let Some(sub_name_id) = name_id_pair.1 {
112+
id_pair.push(sub_name_id);
145113
}
146-
}
147-
if let Some(sub_id_val) = sub_id {
148-
if let Some(sub_string) = get_name_entry_string(&font.font(),
149-
*platform,
150-
*encoding,
151-
*language,
152-
*sub_id_val,
153-
) {
154-
pair.push(true);
155-
full_name.push_str(&sub_string.to_string());
156-
} else {
157-
if *sub_id_val == StringId::WWS_SUBFAMILY_NAME {
158-
// WWS_FAMILY_NAME is optional, so we don't fail if it's missing.
159-
} else {
160-
bad_names.push(format!("Missing required name table entry: {sub_id_val}"));
161-
continue;
114+
for name_id in id_pair.iter() {
115+
if let Some(name_string) = get_name_entry_string(&font.font(),
116+
*platform,
117+
*encoding,
118+
*language,
119+
*name_id,
120+
) {
121+
pair.push(true);
122+
full_name.push_str(&name_string.to_string());
123+
full_name.push(' ');
162124
}
163125
}
126+
if pair.is_empty() {
127+
// Skip if no name entries were found
128+
continue;
129+
}
130+
// Normalize the full name by removing 'Regular' and trimming whitespace
131+
let trimmed = full_name.trim();
132+
let replaced = trimmed.replace("Regular", "");
133+
let normalized_full_name = replaced.trim();
134+
let pair_info = if i == 0 {
135+
"1 + 2".to_string()
136+
} else if i == 1 {
137+
"4".to_string()
138+
} else if i == 2 {
139+
"16 + 17".to_string()
140+
} else {
141+
"21 + 22".to_string()
142+
};
143+
name_strings.push((normalized_full_name.to_string(), pair_info));
164144
}
165-
if pair.is_empty() {
166-
// Skip if no name entries were found for full name comparison
167-
continue;
168-
}
169-
// Normalize the full name by removing 'Regular' and trimming whitespace
170-
let trimmed = full_name.trim();
171-
let replaced = trimmed.replace("Regular", "");
172-
let normalized_full_name = replaced.trim();
173-
name_strings.push(normalized_full_name.to_string());
174145
}
175-
}
176-
// We only check for consistency if we have more than one name string
177-
if name_strings.len() > 1 {
178-
let first = &name_strings[0];
179-
for (i, name) in name_strings[1..].iter().enumerate() {
180-
let name_id_info = if i == 1 {
181-
"16 + 17".to_string()
182-
} else {
183-
"21 + 22".to_string()
184-
};
185-
if first != name {
186-
bad_names.push(format!(
187-
"Inconsistent names: {} (1 + 2) != {} ({})",
188-
first, name, name_id_info
189-
));
146+
// We only check for consistency if we have more than one name string
147+
if name_strings.len() > 1 {
148+
let first = &name_strings[0];
149+
for name in name_strings[1..].iter() {
150+
if first.0 != name.0 {
151+
bad_names.push(format!(
152+
"Inconsistent names {:?}: {} ({}) != {} ({})",
153+
code, first.0, first.1, name.0, name.1
154+
));
155+
}
190156
}
191157
}
192158
}
@@ -195,7 +161,7 @@ fn name_consistency(t: &Testable, context: &Context) -> CheckFnResult {
195161
Status::just_one_pass()
196162
} else {
197163
Status::just_one_fail(
198-
"bad-name-table-entries",
164+
"bad-name-consistency",
199165
&format!(
200166
"The following issues have been found:\n\n{}",
201167
bullet_list(context, bad_names)
@@ -269,10 +235,12 @@ fn get_string_id_from_string(name_id_string: &str) -> Option<StringId> {
269235
}
270236

271237

272-
fn get_name_PEL_codes(name_table: Name) -> Option<Vec<Vec<(u16, u16, u16)>>> {
238+
fn get_name_PEL_codes(font: FontRef) -> Option<Vec<Vec<(u16, u16, u16)>>> {
239+
let name_table = font.name().ok()?;
240+
273241
let mut codes: HashMap<(u16, u16, u16), Vec<(u16, u16, u16)>> = HashMap::new();
274-
for rec in &name_table.name_record {
275-
let code = (rec.platform_id, rec.encoding_id, rec.language_id);
242+
for rec in name_table.name_record().iter() {
243+
let code = (rec.platform_id(), rec.encoding_id(), rec.language_id());
276244
codes.entry(code).or_default().push(code);
277245
}
278246
// Remove duplicates by converting to a HashSet
@@ -281,9 +249,9 @@ fn get_name_PEL_codes(name_table: Name) -> Option<Vec<Vec<(u16, u16, u16)>>> {
281249
unique_codes.insert(*code);
282250
}
283251
// Convert HashSet back to Vec and sort it
284-
let mut codes_vec = vec![unique_codes.into_iter().collect::<Vec<(u16, u16, u16)>>()];
285-
codes_vec.iter_mut().for_each(|v| v.sort());
286-
Some(codes_vec)
252+
let mut name_PEL_codes = vec![unique_codes.into_iter().collect::<Vec<(u16, u16, u16)>>()];
253+
name_PEL_codes.iter_mut().for_each(|v| v.sort());
254+
Some(name_PEL_codes)
287255
}
288256

289257

@@ -310,29 +278,14 @@ mod tests {
310278

311279
#[test]
312280
fn test_get_name_PEL_codes() {
313-
let mut tests: Vec<(Vec<(u16, u16, u16, u16, &str)>, Vec<(u16, u16, u16)>)> = Vec::new();
314-
tests.push(([(3, 1, 1033, 0, "Copyright"), (3, 1, 1033, 11, "https://fontwerk.com")].to_vec(), [(3, 1, 1033),].to_vec()));
315-
tests.push(([(1, 1, 1033, 0, "Copyright"), (3, 1, 1033, 0, "Copyright")].to_vec(), [ (1, 1, 1033), (3, 1, 1033)].to_vec()));
316-
tests.push(([].to_vec(), [].to_vec()));
317-
318-
for (name_recs, expected_codes) in tests.iter() {
319-
let mut name_table = Name::default();
320-
let mut new_records = Vec::new();
321-
for (platform_id, encoding_id, language_id, name_id, name_string) in name_recs.iter() {
322-
let name_rec = NameRecord::new(
323-
*platform_id,
324-
*encoding_id,
325-
*language_id,
326-
NameId::new(*name_id),
327-
String::from(*name_string).into(),
328-
);
329-
new_records.push(name_rec);
330-
}
331-
new_records.sort();
332-
name_table.name_record = new_records;
333-
let name_codes = get_name_PEL_codes(name_table);
334-
assert_eq!(name_codes, Some(vec![expected_codes.clone()]));
335-
}
281+
let contents = include_bytes!(
282+
"../../../../fontspector-py/data/test/montserrat/Montserrat-Regular.ttf"
283+
);
284+
let font = FontRef::new(contents)
285+
.expect("Failed to create FontRef from contents");
286+
let expected_codes = vec![(1, 0, 0), (3, 1, 1033)];
287+
let name_PEL_codes = get_name_PEL_codes(font);
288+
assert_eq!(name_PEL_codes, Some(vec![expected_codes.clone()]));
336289
}
337290

338291
#[test]
@@ -419,14 +372,10 @@ mod tests {
419372
#[test]
420373
fn test_name_consistency() {
421374
let mut tests = Vec::new();
422-
tests.push((StatusCode::Pass, None, [(1, "Family Name"), (2, "Bold"), (4, "Family Name Bold"), (16, "Family Name"), (17, "Bold")].to_vec()));
423-
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Missing required name table entry: FULL_NAME".to_string()), [(1, "Family Name"), (2, "Bold"), (16, "Family Name"), (17, "Bold")].to_vec()));
424-
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Inconsistent names: Family Name Bold (1 + 2) != Family Name Medium (16 + 17)".to_string()), [(1, "Family Name"), (2, "Bold"), (4, "Family Name Bold"), (16, "Family Name"), (17, "Medium")].to_vec()));
425-
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Missing required name table entry: TYPOGRAPHIC_SUBFAMILY_NAME".to_string()), [(1, "Family Name"), (2, "Bold"), (4, "Family Name Bold"), (16, "Family Name")].to_vec()));
426-
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Missing required name table entry: TYPOGRAPHIC_FAMILY_NAME".to_string()), [(1, "Family Name"), (2, "Bold"), (4, "Family Name Bold"), (17, "Bold")].to_vec()));
427-
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Missing required name table entry: SUBFAMILY_NAME".to_string()), [(1, "Family Name"), (4, "Family Name Bold"), (16, "Family Name"), (17, "Bold")].to_vec()));
428-
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Missing required name table entry: FAMILY_NAME".to_string()), [(2, "Bold"), (4, "Family Name Bold"), (16, "Family Name"), (17, "Bold")].to_vec()));
429-
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Inconsistent names: Family Name Condensed Bold (1 + 2) != Family Name Cond Bold (21 + 22)".to_string()), [(1, "Family Name Condensed"), (2, "Bold"), (4, "Family Name Condensed Bold"), (16, "Family Name Condensed"), (17, "Bold"), (21, "Family Name"), (22, "Cond Bold")].to_vec()));
375+
tests.push((StatusCode::Pass, None, [(1, "Family Name"), (2, "Bold"), (4, "Family Name Bold"), (16, "Family Name"), (17, "Bold"), (21, "Family Name"), (22, "Bold")].to_vec()));
376+
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Inconsistent names [(3, 1, 1033)]: Family Name Bold (1 + 2) != Family Name Medium (4)".to_string()), [(1, "Family Name"), (2, "Bold"), (4, "Family Name Medium")].to_vec()));
377+
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Inconsistent names [(3, 1, 1033)]: Family Name Bold (1 + 2) != Family Name Medium (16 + 17)".to_string()), [(1, "Family Name"), (2, "Bold"), (16, "Family Name"), (17, "Medium")].to_vec()));
378+
tests.push((StatusCode::Fail, Some("The following issues have been found:\n\n* Inconsistent names [(3, 1, 1033)]: Family Name Condensed Bold (1 + 2) != Family Name Cond Bold (21 + 22)".to_string()), [(1, "Family Name Condensed"), (2, "Bold"), (21, "Family Name"), (22, "Cond Bold")].to_vec()));
430379
for (expected_severity, expected_message, records) in tests.iter(){
431380
let mut builder = FontBuilder::new();
432381
builder.add_table(&Maxp::default()).unwrap();

0 commit comments

Comments
 (0)