Skip to content

feat: add measure text and measure-numbering staff to mx::api - #373

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

feat: add measure text and measure-numbering staff to mx::api#373
rpatters1 wants to merge 1 commit into
webern:mainfrom
rpatters1:measnums-missing-attributes

Conversation

@rpatters1

Copy link
Copy Markdown
Contributor

Human Summary

Add fields for <measure text="..."> and <measure-numbering staff="N"> to MeasureData. With the number of measure-number-related fields up the 3 in MeasureData it may make sense to refactor them into dedicated MeasureNumberData class. 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. Both
are now exposed on MeasureData and wired symmetrically through mx::impl.

displayedNumber (std::optional<std::string>) carries <measure text="...">: the measure
number 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" normalization
does not touch it. An empty string is not emitted, since text="" is not legal MusicXML and
core::MeasureText would repair it to "-"; suppressing the printed number is what implicit
is for.

measureNumberingStaffIndex (std::optional<int>) carries <measure-numbering staff="N">, the
staff 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 top
staff.

Both fields got MXAPI_EQUALS_MEMBER lines.

No roundtrip-baseline.txt entries added. Discovery shows synthetic/measure.3.1.xml moved from
failing on text to failing on the still-unmodeled <measure id=...>, and
synthetic/measure-numbering.4.0.xml still fails on the unmodeled print-style-align attributes.
Neither pass is available yet, so neither is attributable to this change.

Testing

  • Five new MeasureData cases cover round-trip, omission when absent, and empty-string
    suppression: displayedNumberRoundTrip, displayedNumberAbsentOmitsAttribute,
    emptyDisplayedNumberOmitsAttribute, measureNumberingStaffRoundTrip,
    measureNumberingStaffAbsentOmitsAttribute
  • make api-test passes (5237 assertions in 468 test cases)
  • make api-roundtrip passes (295 of 295 pinned files)
  • make fmt-check passes

@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.

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.

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.

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.

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