search

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

Add optional XML declaration single quote replacement support #25

Closed chrissimpkins closed 3 years ago

chrissimpkins commented 3 years ago

Implementation of optional XML declaration quote style customization. Default is double quotes on declaration attributes. This PR adds a new --singlequotes option to the executable with format style defined by the new WriteOptions configuration in the norad lib.

chrissimpkins commented 3 years ago

MacBook Pro (15-inch, 2017)

3 trials with the new parallel quote formatting on eight Noto Sans Thai master sources as of https://github.com/source-foundry/ufofmt/pull/25/commits/d123d42824853b2138209614ce1e3e04b6e3bd45

without single quote formatting

real         0.45
user         0.29
sys          0.61
real         0.38
user         0.30
sys          0.58
real         0.43
user         0.27
sys          0.54

with single quote formatting

real         0.39
user         0.28
sys          0.84
real         0.42
user         0.27
sys          0.88
real         0.44
user         0.28
sys          0.88

I find it hard to believe that we are doing a full second R/W pass with edits through plist and glif files in nearly the same time, but the files are changing format with each run... 🤷

❤️ rayon

madig commented 3 years ago

Rust is faster than fonts.

Random thought: make norad serialise to bytes so one can replace the xml declaration and write the files just once.

chrissimpkins commented 3 years ago

Rust is faster than fonts.

Random thought: make norad serialise to bytes so one can replace the xml declaration and write the files just once.

👍 https://github.com/source-foundry/ufofmt/issues/19#issuecomment-890050322

chrissimpkins commented 3 years ago

Refactored to use upstream norad implementation of XML declaration single quote formatting added in https://github.com/linebender/norad/pull/157 as of https://github.com/source-foundry/ufofmt/pull/25/commits/35e51effb2071462094a6d44399ada6f7ab79382

chrissimpkins commented 3 years ago

Will add tests then this is ready for merge IMO. Let me know if you have any suggestions/concerns about the approach.

chrissimpkins commented 3 years ago

Rebased on the new Win and macOS CI runners in main. Tests fail b/c there are different line endings on unix vs. win platforms.

chrissimpkins commented 3 years ago

Odd. I think that it may be git that is leading to the line ending discrepancies. If I push a file with line feed line endings, it is read in by Rust with line feeds on Unix platforms, but with carriage return + line feed on Windows. Something is changing the expected values. The tests now pass across platforms with valid expected line ending values using hard coded expected strings in the Rust source in place of expected value file reads.