Skip to content
This repository has been archived by the owner on Jan 28, 2019. It is now read-only.

Nicer, more detailed error messages #10

Open
adrientetar opened this issue Jan 9, 2016 · 6 comments
Open

Nicer, more detailed error messages #10

adrientetar opened this issue Jan 9, 2016 · 6 comments

Comments

@adrientetar
Copy link
Collaborator

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
Copy link

madig commented Nov 12, 2017

I stumbled across the following while trying to fix my corrupted .glyphs file (googlefonts/glyphsLib#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
Copy link

MrBrezina commented Nov 13, 2017

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
Copy link

madig commented Nov 13, 2017

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

@anthrotype
Copy link
Member

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
Copy link

madig commented Oct 10, 2018

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
Copy link
Member

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.

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

No branches or pull requests

4 participants