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.

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

Fonts with comma in filename break on CLI #38

Closed
1 task done
SvanteRichter opened this issue Dec 16, 2024 · 3 comments · Fixed by #39
Closed
1 task done

Fonts with comma in filename break on CLI #38

SvanteRichter opened this issue Dec 16, 2024 · 3 comments · Fixed by #39

Comments

@SvanteRichter
Copy link

SvanteRichter commented Dec 16, 2024

Hello!

I was using the CLI to convert google fonts and it seems like it cannot handle files with commas like for example the upstream google font "NotoSans[wdth,wght].ttf". It seems like the web version works fine.

If I just rename the font to not include a comma it works.

Thanks!

How to reproduce

Adding ./NotoSans[wdth
terminate called after throwing an instance of 'std::runtime_error'
  what():  Could not open font face
Aborted (core dumped)

Attachments

@bdon
Copy link
Collaborator

bdon commented Dec 19, 2024

Looks like a limitation of cxxopts: jarro2783/cxxopts#228

You can add #define CXXOPTS_VECTOR_DELIMITER '\0' to the top of main.cpp as a workaround

Or maybe we should just require the input doesn't have commas?

@SvanteRichter
Copy link
Author

I think it would be nice to have a specific error thrown/logged, I spent quite a while trying to figure out if my font was broken. On the other hand if I understand correctly the CLI is a bit semi-official, so maybe I'm just an outlier for both using the CLI and using the raw font names from google fonts.

Either way, thanks so much for the tool :)

bdon added a commit that referenced this issue Dec 20, 2024
@bdon bdon mentioned this issue Dec 20, 2024
@bdon bdon closed this as completed in #39 Dec 20, 2024
bdon added a commit that referenced this issue Dec 20, 2024
@bdon
Copy link
Collaborator

bdon commented Dec 20, 2024

Should be fixed in #39 , thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants