Improve fonts generation: clean up font build scripts #56
Conversation
The KaTeX test results for recents PRs including this are available at KaTeX/KaTeX#1699. There are no visible changes, but only four subpixel changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ylemkimon sorry for the delay in reviewing this. I merged in master
and re-ran sh buildFonts.sh
and committed the new files. It looks like KaTeX/KaTeX#1888 is the replacement for KaTeX/KaTeX#1699. I'm going to update that PR as well to double check that nothing's changed with the fonts besides the removal of Math_Regular
.
exit 1 | ||
fi | ||
tar cf "$FILE" ../package.json Makefile default.cfg fonts/OTF/TeX | ||
tar cfP "$FILE" ../package.json Makefile default.cfg fonts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to add a P
here b/c tar
was complaining about the leading ../
in package.json. The P
option is intended to allow absolute paths so this resolution seems a bit wonky to me, but that's what was suggested in https://askubuntu.com/questions/568858/tar-removing-leading-from-member-names. I'm open to other suggestions.
@@ -74,11 +73,11 @@ fi | |||
|
|||
CMDS="set -ex | |||
export SOURCE_DATE_EPOCH=${LAST_COMMIT_DATE} | |||
tar xf MathJax-dev.tar.gz | |||
tar xfP MathJax-dev.tar.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added P
here as well.
# echo "eusm10" | ||
# $(PYTHON) $(MFTRACE_MODIFIED) --magnification 1000 --simplify eusm10 | ||
# echo "eusb10" | ||
# $(PYTHON) $(MFTRACE_MODIFIED) --magnification 1000 --simplify eusb10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that most of the other lines in this file use tab this change makes sense.
if [[ ! -f fonts/OTF/TeX/Makefile ]]; then | ||
echo "src does not look like MathJax-dev" >&2 | ||
if [[ ! -f fonts/Makefile ]]; then | ||
echo "src does not look like katex-fonts" >&2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call.
|
||
.PHONY: fonts | ||
fonts: ff | ||
mkdir -p ttf woff woff2 | ||
rm -f ttf/* woff/* woff2/* | ||
|
||
# Handle the KaTeX_Size4 font first as it used by other fonts. | ||
@echo "KaTeX_Size4.ff" | ||
$(FONTFORGE) -lang=ff -script ff/KaTeX_Size4.ff |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this no longer necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kevinbarabash Some glyphs were dependent on the Size4 font. I've removed their dependency and they can now be compiled independently.
@kevinbarabash Sorry for the late response. I somehow missed the notification. Thank you for the review! |
I wonder if there's some way we can use GitHub actions to generate the fonts and update PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a while since I looked at this, but it looks like I updated KaTeX/KaTeX#1888 and the tests passed so this should be good to go after regenerating the font files.
@kevinbarabash Thanks again for the review. I've updated KaTeX/KaTeX#2155. |
This is final PR before moving to JS build system or possibly back to the main repo.