Skip to content

Preserve newlines around notes - #402

Merged
cmyr merged 3 commits into
mainfrom
notes-should-have-newlines
Jun 24, 2026
Merged

Preserve newlines around notes#402
cmyr merged 3 commits into
mainfrom
notes-should-have-newlines

Conversation

@madig

@madig madig commented May 13, 2026

Copy link
Copy Markdown
Collaborator

@Hoolean found an unfortunate roundtripping hindrance:

  1. ufoLib adds newlines around notes
  2. ufonormalizer preserves them
  3. Norad strips them
  4. ufonormalizer preserves that, too.

Norad trims all text in the .glif XML on read:

reader.config_mut().trim_text(true);

Maybe it should be more selective in its trimming, see https://docs.rs/quick-xml/latest/quick_xml/reader/struct.Config.html#method.trim_text.

@RickyDaMa

Copy link
Copy Markdown
Collaborator

It sounds reasonable to me to have norad do the same \n{note}\n wrapping that the Python does, preserving the stripping on reading

@cmyr

cmyr commented May 13, 2026

Copy link
Copy Markdown
Member

so this does raise lots of questions like "is it okay to insert a newline if there wasn't one there when we loaded", but... maybe we don't need to overthink this? I think it's reasonable if we add them on save and strip them on load.

@madig
madig marked this pull request as ready for review June 12, 2026 17:18
Comment thread src/glyph/tests.rs Outdated
Co-authored-by: Ricky <ricky.atkins@daltonmaag.com>

@cmyr cmyr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@cmyr
cmyr merged commit 4ee08b7 into main Jun 24, 2026
5 checks passed
@cmyr
cmyr deleted the notes-should-have-newlines branch June 24, 2026 16:03
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

Successfully merging this pull request may close these issues.

3 participants