feat: extend direction text formatting in mx::api and unify the enclosure enum - #378
Open
rpatters1 wants to merge 1 commit into
Open
feat: extend direction text formatting in mx::api and unify the enclosure enum#378rpatters1 wants to merge 1 commit into
rpatters1 wants to merge 1 commit into
Conversation
- 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.
5 tasks
Contributor
Author
|
This is the same as #374 but based on the correct source branch. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human Summary
Extends direction text formatting with
justifyandsystemRelationadded where available in the musicxml spec. Also unified theEnclosureenum 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:
RehearsalEnclosureandPercussionEnclosureare removed rather than aliased. The vocabularies are unchanged, so the downstream fix is a mechanical rename toEnclosure. Otherwise there is no structural API change; the additions are new fields on existing types with defaults that preserve current behavior.Testing
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.