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

whitespace and empty tags #53

Open
schriftgestalt opened this issue Jan 28, 2019 · 8 comments
Open

whitespace and empty tags #53

schriftgestalt opened this issue Jan 28, 2019 · 8 comments

Comments

@schriftgestalt
Copy link

How did you decide how the intendation and empty elements are handled.

I compared the .plist files generated by the Normalizer and what macOS (Xcode) would produce and found significantly differences. RoboFont agrees in this points with macOS.

  1. The top level dict element is indented in the normalised file and not in the file written by Xcode.
    from the Normalizer:
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
	<dict>
		<key>ascender</key>
		<integer>1491</integer>

from Xcode

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
	<key>ascender</key>
	<integer>1491</integer>
  1. empty elements:
    from the Normalizer:
		<key>openTypeOS2Selection</key>
		<array>
		</array>

from Xcode:

 	<key>openTypeOS2Selection</key>
	<array/>

I tested this in Xcode on 10.9 and 10.13.

@benkiel
Copy link
Contributor

benkiel commented Jan 28, 2019

I believe that the whitespace preferences came from the older version of plistlib that the ufoLib used to use. However, this is the reason for the normalizer, so that the writing libs can change, but you still get meaningful diffs (see the optional use of lxml writer in UFOlib now, etc). It serves as a benchmark, not the only way to write out XML, but one that can be relied on over time to produce the same result, no matter the tool, so the whitespace and empty element preferences are set as they are.

@schriftgestalt
Copy link
Author

so the whitespace and empty element preferences are set as they are

But how did you pick that particular preference? Wouldn’t it make sense to follow the dtd?

Why deviate from several reference implementations?

@benkiel
Copy link
Contributor

benkiel commented Jan 28, 2019

@schriftgestalt To be very clear, I did not write the code for this. My best recollection/guess is that the preference was set by how ufoLib wrote out UFOs in 2016, which is a reference standard and implementation.

I don't think that there is much value is arguing about whitespace and empty elements: many folks use the normalizer in production workflows, so changing the preference now would mean a lot of noise in version control, the very thing that the normalizer is made to avoid.

@anthrotype
Copy link
Member

the formatting is based upon what ElementTree does (as that is the library that the normalizer uses to write XML)

actually, ufoNormalizer uses its own XMLWriter class, ElementTree is only used for parsing.

class XMLWriter(object):

@benkiel
Copy link
Contributor

benkiel commented Jan 28, 2019

@anthrotype my bad, memory failed me and I didn't double check closely enough.

@miguelsousa
Copy link
Member

Regardless, you provided an excellent description of the main goal of the normalizer:

I don't think that there is much value is arguing about whitespace and empty elements: many folks use the normalizer in production workflows, so changing the preference now would mean a lot of noise in version control, the very thing that the normalizer is made to avoid.

I'm inclined to close this issue.

@Alhadis
Copy link

Alhadis commented Nov 2, 2019

so changing the preference now would mean a lot of noise in version control

That's probably the same reason <array>\n</array> is used instead of <array/>. Compare:

 	<array>
+		<string>New entry</string>
 	</array>

… as opposed to:

-	<array/>
+	<array>
+		<string>New entry</string>
+	</array>

@schriftgestalt
Copy link
Author

Maybe adding a second profile that collects all the cleanup an synchronizations. So people can choose when or if they switch.

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

5 participants