feat: add measure text and measure-numbering staff to mx::api - #373
Open
rpatters1 wants to merge 1 commit into
Open
feat: add measure text and measure-numbering staff to mx::api#373rpatters1 wants to merge 1 commit into
rpatters1 wants to merge 1 commit into
Conversation
…"N">` to `MeasureData`.
webern
approved these changes
Jul 30, 2026
webern
left a comment
Owner
There was a problem hiding this comment.
Nice. I agree with a quicker fix now instead of redesigning or refactoring right this moment.
| SystemRelation measureNumberingSystemRelation; | ||
|
|
||
| // Which staff of the part the measure number is vertically positioned against, zero-based from | ||
| // the top staff. Meaningful only when measureNumbering != unspecified. Absent means the top staff. |
Owner
There was a problem hiding this comment.
Meaningful only when measureNumber != unspecified is not ideal. But there are plenty of patterns already like this and it's easier/faster to just do this than to redesign it now. So I agree with the choice.
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
Add fields for
<measure text="...">and<measure-numbering staff="N">toMeasureData. With the number of measure-number-related fields up the 3 inMeasureDatait may make sense to refactor them into dedicatedMeasureNumberDataclass. This would be a breaking change, so I did not include that in this PR. I could rework it that way, though, if it makes sense to do so.Summary
Two
<measure>-scoped attributes were being dropped on import and never written on export. Bothare now exposed on
MeasureDataand wired symmetrically throughmx::impl.displayedNumber(std::optional<std::string>) carries<measure text="...">: the measurenumber as printed, when that differs from the number that identifies the measure. It is fully
independent of
MeasureData::number—number's existing "empty means index + 1" normalizationdoes not touch it. An empty string is not emitted, since
text=""is not legal MusicXML andcore::MeasureTextwould repair it to"-"; suppressing the printed number is whatimplicitis for.
measureNumberingStaffIndex(std::optional<int>) carries<measure-numbering staff="N">, thestaff used as the reference point for vertical positioning of the number. Zero-based, converted
across the impl boundary, matching the sibling
measureNumbering*fields. Absent means the topstaff.
Both fields got
MXAPI_EQUALS_MEMBERlines.No
roundtrip-baseline.txtentries added. Discovery showssynthetic/measure.3.1.xmlmoved fromfailing on
textto failing on the still-unmodeled<measure id=...>, andsynthetic/measure-numbering.4.0.xmlstill fails on the unmodeled print-style-align attributes.Neither pass is available yet, so neither is attributable to this change.
Testing
MeasureDatacases cover round-trip, omission when absent, and empty-stringsuppression:
displayedNumberRoundTrip,displayedNumberAbsentOmitsAttribute,emptyDisplayedNumberOmitsAttribute,measureNumberingStaffRoundTrip,measureNumberingStaffAbsentOmitsAttributemake api-testpasses (5237 assertions in 468 test cases)make api-roundtrippasses (295 of 295 pinned files)make fmt-checkpasses