Skip to content

feat: extend direction text formatting in mx::api and unify the enclosure enum - #378

Open
rpatters1 wants to merge 1 commit into
webern:mainfrom
rpatters1:direction-attributes
Open

feat: extend direction text formatting in mx::api and unify the enclosure enum#378
rpatters1 wants to merge 1 commit into
webern:mainfrom
rpatters1:direction-attributes

Conversation

@rpatters1

Copy link
Copy Markdown
Contributor

Human Summary

Extends direction text formatting with justify and systemRelation added where available in the musicxml spec. Also unified the Enclosure enum which resulted in a breaking change, but it seems like the best approach moving forward. (See below for more details.)

Summary

Four MusicXML attributes that mx::api could not express are now modeled, and the two duplicate enclosure enums are merged into one.

justify on words, symbol and rehearsal. All three elements carry both justify and halign, which MusicXML treats as different things: halign says which edge of the text the position refers to, justify says how the lines sit relative to each other when the text runs to more than one line. halign was already reaching the api through positionData.horizontalAlignment; justify was dropped. PageTextData and TempoData already had a justify field, so the new ones follow that precedent. Elements carrying justify without halign (lyric, part-name, group-name) are deliberately left alone -- the spec says justify doubles as alignment there, which is how the lyric reader already treats it.

systemRelation on DirectionData, mapping the system attribute of both direction and harmony. The api::SystemRelation enum already existed for measure numbering. The direction vocabulary is narrower (only-top, also-top, none), so the api-to-core conversion returns an optional and writes no attribute for the two measure-numbering-only values instead of coercing them into something wrong.

The enclosure vocabulary is now complete. RehearsalEnclosure was missing invertedBracket and pentagon through decagon, while PercussionEnclosure already had all fifteen. Since both described the same MusicXML enclosure-shape type with identical values, they are replaced by a single Enclosure in ApiCommon.h. That also collapses three parallel conversion paths -- two hand-written switches in DirectionReader and DirectionWriter plus the Converter table -- into the Converter table alone, which is where the drift came from in the first place.

Breaking change: RehearsalEnclosure and PercussionEnclosure are removed rather than aliased. The vocabularies are unchanged, so the downstream fix is a mechanical rename to Enclosure. Otherwise there is no structural API change; the additions are new fields on existing types with defaults that preserve current behavior.

Testing

  • 11 new tests across DirectionMarksRoundTripTest, DirectionDataTest and HarmonyExtrasApiTest, covering each attribute set and left unspecified, the unwritable system values, and every shape in the enclosure vocabulary
  • make api-test: 5367 assertions in 474 test cases
  • make api-roundtrip: 295 passed, 0 failed of 295 pinned
  • make core-roundtrip-test: 837 test cases
  • Unity build (CMAKE_UNITY_BUILD_BATCH_SIZE=0), since two file-local functions were removed

No corpus fixture was pinned to roundtrip-baseline.txt: the files exercising these attributes (synthetic/direction.4.0.xml, synthetic/words.3.x.xml) still fail the api round trip on unrelated gaps, directive on direction and dir on words.

- WordsData, SymbolData and RehearsalData gain justify, mapping the attribute of the same name on <words>, <symbol> and <rehearsal>.
- DirectionData gains systemRelation, mapping the system attribute on <direction> and <harmony>.
- Enclosure in ApiCommon.h replaces RehearsalEnclosure and PercussionEnclosure, completing the enclosure-shape vocabulary and
  collapsing three conversion paths into one Converter table. The two old enum names are removed; the vocabularies are unchanged.
@rpatters1

Copy link
Copy Markdown
Contributor Author

This is the same as #374 but based on the correct source branch.

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.

1 participant