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.

Sign up for GitHub

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

Jump to bottom

Do not conflate Archivo Narrow and Archivo (Black) fonts #4478

Merged
merged 1 commit into from
Jul 14, 2024
Merged

Do not conflate Archivo Narrow and Archivo (Black) fonts #4478

merged 1 commit into from
Jul 14, 2024

Conversation

matze
Copy link
Contributor

@matze matze commented Jun 29, 2024

Archivo Narrow is a distinct typeface and not to be conflated with Archivo (Black). This fixes the issue not being able to select Archivo and Archivo Narrow and being stuck with one.

@matze
Do not conflate Archivo Narrow and Archivo (Black) fonts
fbae57f 
Archivo Narrow is a distinct typeface and not to be conflated with
Archivo (Black). This fixes the issue not being able to select Archivo
and Archivo Narrow and being stuck with one.
@laurmaedje
Copy link
Member

I downloaded Archivo Narrow from Google Fonts and it looks like it also has semibold variants which wouldn't be covered by this.

This PR really shows again that the style trimming we currently do is really problematic:

typst/crates/typst/src/text/font/book.rs

Lines 339 to 399 in 2df138a

fn typographic_family(mut family: &str) -> &str {
// Separators between names, modifiers and styles.
const SEPARATORS: [char; 3] = [' ', '-', '_'];
// Modifiers that can appear in combination with suffixes.
const MODIFIERS: &[&str] =
&["extra", "ext", "ex", "x", "semi", "sem", "sm", "demi", "dem", "ultra"];
// Style suffixes.
#[rustfmt::skip]
const SUFFIXES: &[&str] = &[
"normal", "italic", "oblique", "slanted",
"thin", "th", "hairline", "light", "lt", "regular", "medium", "med",
"md", "bold", "bd", "demi", "extb", "black", "blk", "bk", "heavy",
"narrow", "condensed", "cond", "cn", "cd", "compressed", "expanded", "exp"
];
// Trim spacing and weird leading dots in Apple fonts.
family = family.trim().trim_start_matches('.');
// Lowercase the string so that the suffixes match case-insensitively.
let lower = family.to_ascii_lowercase();
let mut len = usize::MAX;
let mut trimmed = lower.as_str();
// Trim style suffixes repeatedly.
while trimmed.len() < len {
len = trimmed.len();
// Find style suffix.
let mut t = trimmed;
let mut shortened = false;
while let Some(s) = SUFFIXES.iter().find_map(|s| t.strip_suffix(s)) {
shortened = true;
t = s;
}
if !shortened {
break;
}
// Strip optional separator.
if let Some(s) = t.strip_suffix(SEPARATORS) {
trimmed = s;
t = s;
}
// Also allow an extra modifier, but apply it only if it is separated it
// from the text before it (to prevent false positives).
if let Some(t) = MODIFIERS.iter().find_map(|s| t.strip_suffix(s)) {
if let Some(stripped) = t.strip_suffix(SEPARATORS) {
trimmed = stripped;
}
}
}
// Apply style suffix trimming.
family = &family[..len];
family
}

The exceptions were mostly intended for fixing fonts, not Typst :/

@matze
Copy link
Contributor Author

matze commented Jul 8, 2024

So, how do we go forward? Close this and somehow solve the style trimming issue more holistically? As a workaround, I removed one of the fonts but it's annoying to say the least.

@laurmaedje
Copy link
Member

I honestly don't know. :/

@laurmaedje laurmaedje added this pull request to the merge queue Jul 14, 2024
@laurmaedje
Copy link
Member

We can revisit this later and merge the fix for now. Thanks!

Merged via the queue into typst:main with commit 5c71ad7 Jul 14, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@matze @laurmaedje