unified-font-object / ufoLib

A low-level UFO reader and writer.
Other
37 stars 19 forks source link

Nicer, more detailed error messages #10

Open adrientetar opened 8 years ago

adrientetar commented 8 years ago

I'm thinking of GlifLibError specifically.

In trufont/trufont#155, there was an error: ufoLib.glifLib.GlifLibError: line can not have an offcurve.

Right now we display message with code like this:

try:
    ...
except Exception as e:
    title = e.__class__.__name__
    QMessageBox.critical(self, title, str(e))

which displays:

glif

So here, the message could be capitalized, and include an information about the offending glyph etc. Maybe the offending glyph (and any other accessible parameter like the file name and line where the error occurs) could be stored as an attribute of the GlifLibError object such that caller can format it however they want.

Edit: regarding capitalization, maybe str.title() is in-order an on the command-line the title and content are on a single sentence.

madig commented 6 years ago

I stumbled across the following while trying to fix my corrupted .glyphs file (https://github.com/googlei18n/glyphsLib/issues/282):

INFO:fontmake.font_project:Building master UFOs and designspace from Glyphs source
INFO:glyphsLib.parser:Parsing .glyphs file
INFO:glyphsLib:Loading to UFOs
INFO:glyphsLib.util:Writing master_ufo/Cantarell-Light.ufo
INFO:glyphsLib.util:Writing master_ufo/Cantarell-Regular.ufo
INFO:glyphsLib.util:Writing master_ufo/Cantarell-Bold.ufo
Traceback (most recent call last):
  File "/Users/nw/Library/Python/3.6/bin/fontmake", line 11, in <module>
    sys.exit(main())
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/fontmake/__main__.py", line 183, in main
    project.run_from_glyphs(glyphs_path, **args)
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/fontmake/font_project.py", line 378, in run_from_glyphs
    designspace_path, instance_data=instance_data, **kwargs)
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/fontmake/font_project.py", line 416, in run_from_designspace
    reader = DesignSpaceDocumentReader(designspace_path, ufoVersion=3)
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/mutatorMath/ufo/document.py", line 409, in __init__
    self.readSources()
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/mutatorMath/ufo/document.py", line 522, in readSources
    sourceObject = self._instantiateFont(sourcePath)
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/mutatorMath/ufo/document.py", line 877, in _instantiateFont
    glyphAnchorClass=self._glyphAnchorClass)
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/defcon/objects/font.py", line 141, in __init__
    layerNames = reader.getLayerNames()
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/ufoLib/__init__.py", line 427, in getLayerNames
    layerContents = self._readLayerContents()
  File "/Users/nw/Library/Python/3.6/lib/python/site-packages/ufoLib/__init__.py", line 420, in _readLayerContents
    raise UFOLibError(error)
ufoLib.UFOLibError: Empty layer name in layercontents.plist.

At first I thought this was about the bold master, an edit to the error message to include the UFO path revealed it was about the regular master.

Given that ufoLib is used by other libs, a much more detailed error object of some sort would be appreciated.

MrBrezina commented 6 years ago

I had the same problem. It is because I copied some glyphs from another font with more masters and the extra layers got preserved. It is not just layers without names, but also others which should not be there.

This Glyphs.app script helps to clean things up. (Run it multiple times. That’s because I am lazy to figure out how to delete all phantom layer systematically in one run.)

import GlyphsApp

font = Glyphs.font
real_layers = [m.id for m in font.masters]

for g in font.glyphs:
    for l in g.layers:
        if l.layerId not in real_layers:
            del(font.glyphs[g.name].layers[l.layerId])
            print "Deleting layer from", g.name
madig commented 6 years ago

(Yeah, that was the problem. I used the "Delete all non-master layers" script by mekkablue to kill and destroy everything.)

anthrotype commented 6 years ago

I agree, we need to break up the two overly generic UFOLibError and GlifLibError into more specific errors (subclasses), that also carry more rich data than just the error message string. Another issue to move on to the future fontTools.ufoLib.

madig commented 6 years ago

I think one important thing is to except and reraise (i.e. chain exceptions) in more strategic positions, so you can infer from the traceback which glyph in which layer in which UFO is to blame.

anthrotype commented 6 years ago

Now that ufoLib requires pyfilesystem2, since the latter in turn requires six, we could make use of py2.py3 compatible functions six.raise_from and six.reraise.