google / fonts

Font files available from Google Fonts, and a public issue tracker for all things Google Fonts
https://fonts.google.com
17.84k stars 2.6k forks source link

Add `primary_language` field to Chinese fonts; Add languages to new Chinese fonts; #7792

Closed laubonghaudoi closed 1 month ago

laubonghaudoi commented 1 month ago

Current Traditional Chinese fonts don't have a primary language setting in their meta data, resulting in the default sample text to be in wuu_Hant which has the highest population. This PR specifies the primary_language fields of them.

The PR also adds the Chinese languages to the metadata of new Chinese fonts.

chrissimpkins commented 1 month ago

Ty!

@simoncozens Mind reviewing this PR when you have an opportunity?

chrissimpkins commented 1 month ago

cc @nathan-williams re: incoming changes to front end specimens

laubonghaudoi commented 1 month ago

One of the tests is failing:

CactusClassicalSerif-Regular.ttf: 
com.google.fonts/check/metadata/unreachable_subsetting: FAIL [unparsable-metadata]
/opt/hostedtoolcache/Python/3.10.14/x64/lib/python3.10/site-packages/fontbakery/checkrunner.py:144: FutureWarning: The behavior of this method will change in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
  val = bool(self._get(name, identity.iterargs, condition=True))

summary:

ERROR: 0 FATAL: 1 FAIL: 3 WARN: 14 INFO: 9 SKIP: 110 PASS: 114
A report in markdown format has been saved to "out/cactusclassicalserif/Fontbakery/report.md"
Cannot post Fontbakery report!
Most likely, the repository may lack a GH_TOKEN secret, or the pull request has come from a forked repo which is not allowed to access the repo's secrets for security reasons. Full traceback:
403 Client Error: Forbidden for url: https://api.github.com/repos/google/fonts/issues/7792/comments
Fontbakery has raised a fatal error. Please fix!
Traceback (most recent call last):
Checking modified family metadata: ofl/cactusclassicalserif
  File "/home/runner/work/fonts/fonts/.ci/run.py", line 148, in <module>
    main()
  File "/home/runner/work/fonts/fonts/.ci/run.py", line 137, in main
    subprocess.run(qa_cmd_prefix + ["--fontbakery", "-o", out], check=True)
  File "/opt/hostedtoolcache/Python/3.10.14/x64/lib/python3.10/subprocess.py", line 526, in run
    raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['gftools', 'qa', '-f', 'ofl/cactusclassicalserif/CactusClassicalSerif-Regular.ttf', '-o', 'out/cactusclassicalserif', '--out-url', 'https://www.github.com/google/fonts/pull/[77](https://github.com/google/fonts/actions/runs/9309734568/job/25655076453?pr=7792#step:6:78)92', '--fontbakery', '-o', 'out/cactusclassicalserif']' returned non-zero exit status 1.
Error: Process completed with exit code 1.

so it seems like the metadata of cactus serif has parsing error. I checked ofl/cactusclassicalserif/METADATA.pb and it al LGTM. What might be the issue here?

simoncozens commented 1 month ago

The problem here is that fontbakery's version of the protobuf headers doesn't have a primary_language field, so it's not being parsed correctly there. I'll fix in fontbakery and try again.

laubonghaudoi commented 1 month ago

I see. But if I remove all languages in the non-Noto families, how to specify the default displayed sample text for them? Currently they are all showing Shanghainese texts by default which is what this PR is trying to fix.

laubonghaudoi commented 1 month ago

I just updated the PR and removed the languages and primary_language for all non-Noto fonts. I see that https://github.com/fonttools/fontbakery/pull/4756 is working in progress, IIUC I should make another PR to add the languages back once that PR is merged and rolled out, so that the default display language issue can be fixed. Is this correct?

laubonghaudoi commented 1 month ago

Okay done, thanks for letting me know!