Skip to content

feat: clef additional and after-barline in mx::api - #375

Open
rpatters1 wants to merge 1 commit into
webern:mainfrom
rpatters1:clef-additional-attribute
Open

feat: clef additional and after-barline in mx::api#375
rpatters1 wants to merge 1 commit into
webern:mainfrom
rpatters1:clef-additional-attribute

Conversation

@rpatters1

Copy link
Copy Markdown
Contributor

Human Summary

Add Bool ClefData::additional and map MusicXML's clef@additional in both directions. unspecified (the default) omits the attribute; yes/no are
written verbatim.

Summary

Also fix a write-side gap found while adding it: ClefData::location was
already populated by MeasureReader from clef@after-barline, but
PropertiesWriter never emitted the attribute, so the value was silently
lost on write. afterBarline and beforeBarline now write after-barline
yes/no; unspecified and midMeasure emit nothing, since MusicXML ignores the attribute on mid-measure clefs. Document ClefLocation, which had no comment.

Tests cover yes/no/omitted for both attributes, plus an all-fields
round-trip over ClefData that pins the remaining eight fields. That test
records one deliberate normalization: writeStaffNumber=no on a
single-staff part reads back as unspecified, because the knob records
only a divergence from the automatic rule and the automatic rule already
omits the number there. The emitted XML is identical either way.

data/synthetic/clef.3.0.xml and clef.3.1.xml still fail the api
roundtrip on unmodeled position, font, color, and id attributes, so
neither is pinned. The discovery pass-list is unchanged at 297.

ClefData::location was read from clef@after-barline but never written.
Adds Bool ClefData::additional and maps both attributes in both directions.

@webern webern left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This one is pretty straightforward and looks good. Easy to approve from the phone. 👍

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.

2 participants