Skip to content

fix: write arranger and publisher, add page-text enclosure - #390

Merged
webern merged 1 commit into
webern:mainfrom
rpatters1:api-metadata-and-credit-enclosure
Aug 7, 2026
Merged

fix: write arranger and publisher, add page-text enclosure#390
webern merged 1 commit into
webern:mainfrom
rpatters1:api-metadata-and-credit-enclosure

Conversation

@rpatters1

@rpatters1 rpatters1 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Human Summary

This is a omnibus PR that cleans up some minor issues.

  • clean up stale comments
  • add enclosure to PageTextData
  • correctly read/write lyricist, arranger, and publisher credits

Summary

Three small gaps in mx::api, collected into one fix-up PR. Each is confined to mx::impl plus a single new field; none needs a new API model.

Arranger and publisher were accepted but never written. ScoreData::arranger and ScoreData::publisher have existed, with equality members, but nothing in mx::impl ever touched them -- a scan of every MXAPI_EQUALS_MEMBER name against src/private/mx/impl/ showed these two as the only dead public fields. Worse than dead: ScoreReader read an incoming <creator type="arranger"> into ScoreData::lyricist, so a file carrying both an arranger and a lyricist lost the lyricist, and ScoreWriter then re-emitted the arranger's text mislabelled as type="lyricist". Both now read into their own field and are written beside composer and lyricist. Where a file has several creators of one type (recsuite/Echigo_Jishi.xml has two arrangers) the first wins, since the api has one slot.

Before, on musuite/testMetaData.xml: lyricist == "MetaArranger", arranger == "", and the round trip emitted <creator type="lyricist">MetaArranger</creator>. After: composer, lyricist and arranger each land in their own field and are rewritten with the right type.

Page text carried no enclosure. <credit-words> is type formatted-text-id, the same core class as <words> and <rehearsal>, so it carries the whole text-formatting attribute group. core::FormattedTextID already exposes enclosure()/setEnclosure(), and Converter already has both directions from #378, so only the api field was missing. PageTextData gains Enclosure enclosure, converted in both directions in PageTextFunctions.cpp beside the existing justify handling -- the same shape #273 used for justify.

A stale comment about <pedal>. SpannerNumberResolver asserted in two places that <pedal> has no number attribute. True through MusicXML 3.0; 3.1 added it and core::Pedal supports it. Pedals still stay out of number resolution, because mx::api does not model the attribute -- behaviour unchanged, only the stated reason corrected.

Testing

  • Two new DocumentManager tests: all four creator types survive a round trip independently and serialize with the right type, and a read of musuite/testMetaData.xml keeps composer, lyricist and arranger apart. The second fails before the fix (lyricist comes back as MetaArranger).
  • Two new ROUND_TRIP_TEST_SCALAR entries for arranger and publisher
  • Three new creditRoundTrip tests: enclosure="rectangle" survives, an explicit none survives, and an unset enclosure writes no attribute and reads back unspecified
  • Full api suite passes (5922 assertions in 528 test cases)
  • make api-roundtrip passes (363 of 363 pinned)
  • make api-roundtrip-discover still shows 363 PASS -- no file changed status, so the baseline is untouched
  • make core-roundtrip-test passes (839 test cases)
  • make core-unit passes (212 assertions in 41 test cases)
  • make fmt-check clean on touched files

No corpus files added or removed, so no pinned-count bump and no audit regeneration.

References

@rpatters1
rpatters1 force-pushed the api-metadata-and-credit-enclosure branch from ad41a10 to b29df45 Compare August 7, 2026 03:00
Three small gaps from Denigma's mx::api gap list, each confined to mx::impl
plus one new field.

ScoreData::arranger and ScoreData::publisher existed with equality members but
nothing in mx::impl ever touched them. ScoreReader read an incoming
<creator type="arranger"> into ScoreData::lyricist, so a file carrying both an
arranger and a lyricist lost the lyricist, and ScoreWriter then re-emitted the
arranger's text mislabelled as type="lyricist". Both now read into their own
field (first of each type wins, as the api has one slot) and are written on the
same path as composer and lyricist.

<credit-words> is type formatted-text-id, the same core class as <words> and
<rehearsal>, so it carries the whole text-formatting group. core::FormattedTextID
already exposes enclosure()/setEnclosure() and Converter already has both
directions; only the api field was missing. PageTextData gains Enclosure
enclosure, converted in both directions in PageTextFunctions.cpp beside the
existing justify handling.

SpannerNumberResolver's comments claimed <pedal> has no number attribute. That
was true through MusicXML 3.0; 3.1 added it. Pedals stay out of number
resolution because mx::api does not model the attribute -- behaviour unchanged,
only the stated reason corrected.
@rpatters1
rpatters1 force-pushed the api-metadata-and-credit-enclosure branch from b29df45 to 8816a0a Compare August 7, 2026 03:04
@webern
webern merged commit 4c0c694 into webern:main Aug 7, 2026
7 checks passed
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