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

ttx: is creating empty XML files necessary? #972

Open
frankrolf opened this issue May 22, 2017 · 11 comments
Open

ttx: is creating empty XML files necessary? #972

frankrolf opened this issue May 22, 2017 · 11 comments
Assignees

Comments

@frankrolf
Copy link

frankrolf commented May 22, 2017

I often (sometimes) run into the following scenario:

$ ttx -t os/2 font.otf
Dumping "font.otf" to "font.ttx"...
No 'os/2' table found.
$ ttx -t os_2 font.otf
Dumping "font.otf" to "font#1.ttx"...
No 'os_2' table found.
$ ttx -t OS/2 font.otf
Dumping "font.otf" to "font#2.ttx"...
Dumping 'OS/2' table...

In total, three files are created, two of them empty. This is of course a result of user error – but I wonder if it would make sense to just not create a ttx file in scenarios 1 & 2?

@justvanrossum
Copy link
Collaborator

Possibly even better: to raise an error if a table is requested that is not in the font.

@anthrotype
Copy link
Member

I agree with @justvanrossum, we should raise a TTLibError exception in _tableToXML function (used by TTFont.saveXML). Then clients can decide how to handle that (e.g. print the error message and continue or exit early).
And yeah, we shouldn't leave behind an empty ttx file when the tables turns out to not be there. The reason it's doing that is that when we make the output file name, we 'touch' the file (create an empty one) to avoid race conditions in choosing the output filename.
I'll see what I can do.

@behdad
Copy link
Member

behdad commented May 23, 2017

Making ttx by default err on missing tables would be a change of behavior though. For example, if I'm doing:

for x in *.ttf; do ttx -tGDEF -tGSUB -tGPOS "$x"; done

I'd be really pissed if some of those ttx commands failed because the font (legitimately) didn't have GDEF.

@anthrotype
Copy link
Member

True.. I think we should keep the existing ttx behavior of passing on missing tables (with a message); but still a TTLibError could make more sense when one calls TTFont.saveXML from python code and the tables list some which are missing; ttx could intercept that exception and handle it the same way it currently does, plus cleaning up the empty .ttx file it has created.

@behdad
Copy link
Member

behdad commented May 23, 2017

I suppose we can make saveXML "return" (not throw) an exception. Caller can raise it if they want.

You definitely can't / shouldn't just raise exception when first missing table is encountered as that interrupts the normal flow of the code and we end up not saving the remaining tables.

@anthrotype
Copy link
Member

yeah, I like this idea of returning an exception object, containing the list of missing tables. If all the requested tables turn out to be missing, then ttx should not leave behind the empty file.

@anthrotype anthrotype self-assigned this May 23, 2017
@behdad
Copy link
Member

behdad commented May 23, 2017

If all the requested tables turn out to be missing, then ttx should not leave behind the empty file.

I'm not so sure of that. These are, command-line tools after all. They are designed to be scripted together. How is a script to detect this? Or why? This sounds like convenience for user typing commands, but definitely not for scripts. I don't have a strong opinion here. And I'm NOT suggesting adding more commandline options for these tiny nuances...

@justvanrossum
Copy link
Collaborator

Perhaps just issue a warning for missing tables?

@anthrotype
Copy link
Member

I don't have a strong opinion either, as you can see from my swinging... 😅
maybe it's best to leave it as is for now, and just elevate the logging message from INFO to WARNING.

@frankrolf
Copy link
Author

ttx -t name Charter.ttc
Dumping "Charter.ttc" to "Charter.ttx"...
ERROR: specify a font number between 0 and 5 (inclusive)

Why does this command need to create a zero-kB file?

@behdad
Copy link
Member

behdad commented Aug 19, 2022

Best to delete the output file upon error.

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

No branches or pull requests

4 participants