Fonts: search for fonts in system/user paths#2424
Fonts: search for fonts in system/user paths#2424muesli wants to merge 1 commit intoInfiniTimeOrg:mainfrom
Conversation
Local copies of fonts will still be prioritized, but this fallback allows us to eventually remove a few TrueType fonts from the repo and rely on system packages for fonts instead.
|
Build size and comparison to main:
|
|
I'm bit hesitant on this change. Relying on systems from providing the required fonts seems like potential for problems. What if they're not there (e.g. in minimal build environments). What if they get updated or otherwise change? Basically I feel relying on resources outside of the repository is a unwise in terms of build stability and reproduceability I get that it makes prototyping much more convenient, but I'm just not sure it's a worthwhile change for fonts since we almost never want to change them |
mark9064
added
enhancement
|
I totally respect that and therefore didn't dare to propose removing the fonts from the repo just yet. I entertained the thought however, as I suspected the Docker build to be the official source of release builds, in which case the fonts could still be pinned to a specific version by the Dockerfile. If we just keep the existing fonts in the repo this change won't affect common builds or releases. It still has the added convenience of wider font availability during development however, which might be worth considering. It actually only exists because I was playing with various fonts while I was working on my own custom watch face and it became a bit tedious to copy fonts back and forth. |
|
I think you're right about the docker, but also if someone installs the same toolchain/sdk on any linux we also support that as an "official" build configuration. It'd be a waste not to have this as an option though. Maybe some CMake flag like |
| for root, _, files in os.walk(font_dir): | ||
| for f in files: | ||
| if f.lower() == name.lower(): | ||
| found = os.path.join(root, f) |
There was a problem hiding this comment.
Nested function that you return from might be nicer than this break pattern
There was a problem hiding this comment.
Doesn't need to be nested really actually, see what works
Also breaking out the font search to another function or method is probably a good idea structurally
| import argparse | ||
| import subprocess | ||
|
|
||
| # Common system font locations to try when a font file is not found locally |
There was a problem hiding this comment.
Not ideal that these files are so similar. Might not be worth refactoring though
Sounds good to me, I like that. I'll update the PR! |
2 participants
Local copies of fonts will still be prioritized, but this fallback allows us to eventually remove a few TrueType fonts from the repo and rely on system packages for fonts instead.