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.

Sign up for GitHub

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

Jump to bottom

Add XML declaration custom single/double quote style support in glif and *.plist files #157

Merged
merged 16 commits into from
Aug 15, 2021
Merged

Add XML declaration custom single/double quote style support in glif and *.plist files #157

merged 16 commits into from
Aug 15, 2021

Conversation

chrissimpkins
Copy link
Collaborator

Closes #156

@chrissimpkins chrissimpkins mentioned this pull request Aug 6, 2021
Closed
3 tasks
chrissimpkins
chrissimpkins commented Aug 6, 2021
View reviewed changes
src/layer.rs Outdated Show resolved Hide resolved
chrissimpkins
chrissimpkins commented Aug 6, 2021
View reviewed changes
src/write.rs Outdated Show resolved Hide resolved
@chrissimpkins chrissimpkins marked this pull request as ready for review August 12, 2021 01:45
@chrissimpkins
Copy link
Collaborator Author

Ready for review. I'll begin working on tests.

cmyr
cmyr reviewed Aug 12, 2021
View reviewed changes
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Cool, this looks good. I think just rewriting the file before closing the file handle is a reasonable approach, and the code seems nice and compact.

One naming nit, and then I'm going to patch that double write separately and investigate the performance a bit more.

src/write.rs Outdated Show resolved Hide resolved
cmyr added a commit that referenced this pull request Aug 12, 2021
@cmyr
Fix bug where glyphs were saved twice
bcb1ddf 
As pointed out in #157.
@cmyr cmyr mentioned this pull request Aug 12, 2021
Merged
@chrissimpkins
Copy link
Collaborator Author

Do we want to add Win CI testing since there are platform-specific approaches added here?

@madig
Copy link
Collaborator

madig commented Aug 12, 2021

I think so!

@chrissimpkins chrissimpkins changed the title Add XML declaration custom single/double quote style support Add XML declaration custom single/double quote style support in glif and *.plist files Aug 12, 2021
@chrissimpkins chrissimpkins mentioned this pull request Aug 13, 2021
Merged
@chrissimpkins
Copy link
Collaborator Author

chrissimpkins commented Aug 13, 2021 •
edited
Loading

Do we want to add Win CI testing since there are platform-specific approaches added here?

I think so!

#161

@chrissimpkins
Copy link
Collaborator Author

Tests added

cmyr added a commit that referenced this pull request Aug 13, 2021
@cmyr
Fix bug where glyphs were saved twice
7fa20bc 
As pointed out in #157.
chrissimpkins and others added 15 commits August 14, 2021 18:52
@chrissimpkins
[write] add WriteOptions XML decl quote style support
2dfa075
@chrissimpkins
[glyph/serialize] add glif serialization custom XML declaration singl…
4297d0a 
…e / double quote style support
@chrissimpkins
expose write::QuoteStyle enum as top-level re-export
f08788a
@chrissimpkins
[write] doc updates
d384187
@chrissimpkins
add support for optional plist file XML declaration single quote form…
34c1df8 
…atting style
@chrissimpkins
[write] sync after all buffer edits
f348a64
@chrissimpkins
[write] refactor duplicate code to a function
be57f56
@chrissimpkins @cmyr
QuoteStyle -> QuoteChar
6497336 
Co-authored-by: Colin Rofls <[email protected]>
@chrissimpkins
refactor enum QuoteStyle to enum QuoteChar
4cdf7d7
@chrissimpkins
[write] clippy lint: remove reference that is immediately dereferenced
f80645f
@chrissimpkins
add plist file WriteOptions unit tests
94e489f
@chrissimpkins
add glif file custom XML declaration quote formatting unit test
51454d9
@chrissimpkins
clippy lints: convert to char
0a4ca11
@chrissimpkins
moar clippy...
d9d2758
@chrissimpkins
[glyph::tests] add single quote xml decl without custom spacing test
8169c76
cmyr
cmyr approved these changes Aug 15, 2021
View reviewed changes
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Looks good, one tiny fixup, I'll apply that and merge. :)

src/write.rs Outdated Show resolved Hide resolved
@cmyr cmyr merged commit 1a20be6 into linebender:master Aug 15, 2021
@chrissimpkins chrissimpkins deleted the custom-xml-decl-quotes branch August 16, 2021 04:38
chrissimpkins added a commit to source-foundry/ufofmt that referenced this pull request Aug 16, 2021
@chrissimpkins
use norad master branch revision following linebender/norad#157
f07bd6b
@chrissimpkins chrissimpkins mentioned this pull request Aug 16, 2021
Merged
chrissimpkins added a commit to source-foundry/ufofmt that referenced this pull request Aug 17, 2021
@chrissimpkins
use norad master branch revision following linebender/norad#157
f0c52f4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmyr cmyr cmyr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow customizing quote char in xml header
3 participants
@chrissimpkins @madig @cmyr