Make validation optional

[benkiel]

A pass at making the validation optional for ufoLib. No speedup testing has been done with this yet. Read is set to have validation off by default, Write is set to have it on by default. One can turn on/off validation on each method as wanted, or just set the global class option and let it be.

There is one thing I couldn't untangle, the _validateAndMassagePointStructures method. It does things that are needed, and it does validation. Where one starts and the other ends, I wasn't 100% on, so I left this alone for now.

[typesupply]

I think we should look at things like this:

    for attr in component.attrib.keys():
        if attr not in componentAttributesFormat2:
            raise GlifLibError("Unknown attribute in component element: %s" % attr)

Should this test only be performed is validation is requested? If no, should all potential errors be silently passed along to the caller? I'm trying to figure out where we draw the line. I suppose that all of the validation is more or less like this so maybe we should skip all of these tests when validation is off.

[benkiel]

Yes, was thinking the same earlier; will take a pass through and do so

[benkiel]

@typesupply I put those in and got the errors down to six in the test cases. Couldn't see why they are failing right now.

[anthrotype]

thank you guys for working on this. I'll take a look soon

[benkiel]

@anthrotype we're working on lxml in the validation branch, have you worked out how to get it to not self close empty tags? i.e., <contour></contour instead of <contour />?

[anthrotype]

lxml support transforming xml with XSLT language (https://www.w3schools.com/xml/xsl_intro.asp). @moyogo once worked on a xslt script to normalize UFO plist and glif xml to be used with lxml. I remember it looked fairly complicated. It would be great if he could to share it with us :)

[typesupply]

I think I have a solution. In the glyph writer I can test for children of elements (specifically, outline and contour) and if there are none then I can set the element text to "\n". This prevents the self-closing tag. Though, I do wonder if forcing "\n" is right or if I should get the linesep from lxml somehow...

Anyway, this self-closing issue goes way back to fontTools.xmlWriter. That didn't do self closing on empty elements, so <foo></foo> became the "standard" without us even making the choice. I carried this over to the UFO Normalizer. If we didn't have such a history with it, I'd say, "Eh, we're doing self-closing now. Version control systems can handle the diffs." but, this change will probably cause many, many, many diffs.

[chrissimpkins]

@benkiel @typesupply confirming that you will be opening a new pull request or creating an issue report / updating an existing IR for this before it hits master branch? I will need to update ufolint to continue to support our CI testing once you make these changes and just want to make sure that I know when it occurs. thx

[benkiel]

@chrissimpkins Yes, of course. See discussion here: https://github.com/fonttools/fonttools/issues/1095#issuecomment-397458994. We're waiting a bit for any more comments before opening the PR.

[chrissimpkins]

@benkiel great thanks Ben. I appreciate it!