Skip to content

Disallow empty font lists#6049

Merged
laurmaedje merged 5 commits intotypst:mainfrom
MDLC01:text-font-tweaks
Apr 2, 2025
Merged

Disallow empty font lists#6049
laurmaedje merged 5 commits intotypst:mainfrom
MDLC01:text-font-tweaks

Conversation

@MDLC01
Copy link
Collaborator

@MDLC01 MDLC01 commented Mar 12, 2025

This PR disallows passing an empty array as text.font. I also improved the documentation for text.fallback: with the default text font (Libertinus Serif), there are no tofu glyphs.

@laurmaedje
Copy link
Member

#set text(fallback: false)
\u{1234}

gives a tofu

@MDLC01
Copy link
Collaborator Author

MDLC01 commented Mar 12, 2025

Oh wait, i might have confused the default font with no fonts (i.e., the thing this PR disallows). But still, I'm unsure what happens when no font in the fallback chain has a .notdef glyph.

@laurmaedje
Copy link
Member

IIRC OpenType fonts must always have a notdef glyph (though it may have empty outlines)

@MDLC01
Copy link
Collaborator Author

MDLC01 commented Mar 13, 2025

Then I suppose it's fine to leave the documentation as it was before

@PgBiel
Copy link
Contributor

PgBiel commented Mar 13, 2025

I wonder if #set text(font: ()) could actually be useful in an environment without fonts? E.g. using Typst to draw shapes without any text.

Maybe it could be changed to bail if there's any text, rather than bailing on the set rule itself.

@laurmaedje
Copy link
Member

If there's no text, the set text wouldn't really have an effect, right? But what is kinda missing from the PR is at least one sentence motivating this change so that we are all on the same page on what the intent is.

@MDLC01
Copy link
Collaborator Author

MDLC01 commented Mar 14, 2025

It's true that I should have given a motivation in my original comment, sorry for that.

I feel like specifying an empty font list is significantly more likely to be a mistake than not: If you have text in your document, why would you want to have an empty font list? And if you don't, why would you want to change a property that affects only text?

@laurmaedje
Copy link
Member

I tend to agree, though I wonder whether a warning would be a better or worse choice.

@laurmaedje laurmaedje added interface PRs that add to or change Typst's user-facing interface as opposed to internals or docs changes. diagnostics Improvements to compiler errors labels Mar 24, 2025
@MDLC01
Copy link
Collaborator Author

MDLC01 commented Mar 25, 2025

The advantage of an error is that the font list is now guaranteed to be non-empty, which I suppose may be useful for some packages (although I don't have any specific example).

@MDLC01 MDLC01 force-pushed the text-font-tweaks branch from 10d9f47 to d54e0e1 Compare April 2, 2025 11:43
@laurmaedje laurmaedje enabled auto-merge April 2, 2025 11:44
@laurmaedje
Copy link
Member

Thanks!

@laurmaedje laurmaedje added this pull request to the merge queue Apr 2, 2025
Merged via the queue into typst:main with commit ed2106e Apr 2, 2025
7 checks passed
@MDLC01 MDLC01 deleted the text-font-tweaks branch April 2, 2025 11:52
hongjr03 pushed a commit to hongjr03/typst that referenced this pull request Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

diagnostics Improvements to compiler errors interface PRs that add to or change Typst's user-facing interface as opposed to internals or docs changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants