source-foundry / ufofmt

A fast, flexible UFO source file formatter based on the Rust Norad library
Apache License 2.0
7 stars 1 forks source link

Evaluate UFO source format diffs across commonly used UFO serializers #4

Open chrissimpkins opened 3 years ago

chrissimpkins commented 3 years ago

https://github.com/chrissimpkins/ufo-format-diff

chrissimpkins commented 3 years ago

Norad v0.4.0 https://github.com/chrissimpkins/ufo-format-diff/pull/1

chrissimpkins commented 3 years ago

ufonormalizer v0.5.4 https://github.com/chrissimpkins/ufo-format-diff/pull/3

chrissimpkins commented 3 years ago

ufoLib2 v0.11.1 https://github.com/chrissimpkins/ufo-format-diff/pull/5

chrissimpkins commented 3 years ago

norad v0.4.0 after ufonormalizer v0.5.4 (shows diff between ufofmt/norad and ufonormalizer formatted sources)

https://github.com/chrissimpkins/ufo-format-diff/pull/7

chrissimpkins commented 3 years ago

defcon v0.8.1 https://github.com/chrissimpkins/ufo-format-diff/pull/9

cmyr commented 3 years ago

So... funny question. What is the expected behaviour around line endings? Should the tool use the appropriate line ending for the current target platform, or should it (say) always use unix-style line endings?

This has come up because I noticed that one of the test files in norad, sample_period_normalized.glif, has windows-style line endings. I believe this file was created using by @madig, using ufonormalizeron windows.

If this is a convention we are expected to respect, things will be slightly more complicated...

chrissimpkins commented 3 years ago

Good question. I didn't notice that. IMO it would be best to stick with a single approach that does not introduce a platform-specific diff with norad writes on different platforms. I think that Win and *nix users of Runebender would appreciate that in collaborative projects. This seems to make sense for cross-platform UFO formatting with an autoformatter on sources created by other tools too.

@madig @belluzj @anthrotype do you know if ufonormalizer writes platform-specific line endings? If so, is this intentional/desirable? A bug?

chrissimpkins commented 3 years ago

It looks like the intent may be to write with unix style newline char everywhere?

https://github.com/unified-font-object/ufoNormalizer/blob/186dbc1f49962c7147ac09fc7d91725734f5ff4b/src/ufonormalizer/__init__.py#L1586-L1589

cmyr commented 3 years ago

Okay, there may have been something else going on in that test file, I'm not sure of its full origin story.

madig commented 3 years ago

It does indeed enforce Unix line endings.

chrissimpkins commented 3 years ago

https://github.com/fonttools/fonttools/issues/2383

cmyr commented 3 years ago

I am irrationally annoyed about another behaviour of ufonormalizer: it modifies whitespace to in the <note> field, which is a destructive operation; e.g. if you have some valid python code in a note for a glyph, ufonormalizer will helpfully fix it for you.

cmyr commented 3 years ago

I've opened an issue there (https://github.com/unified-font-object/ufoNormalizer/issues/84) to see if there's something here that I'm overlooking.

chrissimpkins commented 3 years ago

Are you explicitly trimming those <note> field strings in norad? Or is this automated by your lib dependencies?

cmyr commented 3 years ago

I'm not sure I understand. Norad (currently) serializes the contents of notes directly, and does not modify whitespace at all; we have a commented out test that compares against the output of ufonormalizer, and there I noticed that ufonormalizer was adding whitespace.

chrissimpkins commented 3 years ago

The glif <note> section seems to have newlines trimmed by norad. See:

madig commented 3 years ago
chrissimpkins commented 3 years ago

IMO the trimmed format is an improvement in the above example note. But I think that Colin makes a good point that there may be formatting requirements that should be preserved in freeform notes included by designers. I haven't come across any note use in the past to judge what designers put in there. What have you seen that section used to document Niko?

madig commented 3 years ago

I have only ever seen garbage in there or one-line comments.

cmyr commented 3 years ago

Yea, I don't think this matters a ton in practice but I just think it's bad form to go about inserting and deleting stuff that has been input by the user. changing leading/trailing whitespace is maybe-annoying-but-mostly-probably-fine, but inserting or removing whitespace in the middle of the text just feels wrong.

madig commented 3 years ago

Yeah, I also don't like changing anything other than trimming start and ending. The XML format is, uh, weird about this, though...