Repository navigation
feat: harmony print-frame and empty kind text in mx::api - #467
Merged
Merged
Conversation
This was referenced Oct 28, 2026
webern
marked this pull request as ready for review
October 29, 2026 07:06
webern
changed the title
webern
changed the title
<harmony print-frame> had no api home, so a chord symbol whose diagram was
hidden lost that instruction on write. <kind text=""> (the kind spelled with
nothing) lost its empty text, because an empty string was treated as absent.
ChordData gains printFrame, a Bool that applies to the whole harmony. The
writer emits it from the first chord that sets it, with or without frame
data. ChordData::text becomes std::optional<std::string> so an empty string
is different from an absent one.
This is a source-breaking change to ChordData::text. Callers that read it as
a string now need text.value_or(""). Assigning "" now writes text=""; leave
text as std::nullopt to omit the attribute.
Six more corpus files now round-trip and are pinned in the baseline.
webern
force-pushed
the
m/mxdev-harmframe
branch
from
October 30, 2026 08:15
fbcfccd to
ed5b346
Compare
webern
added a commit
that referenced
this pull request
Nov 1, 2026
## Summary `<key print-object="no">` (a key signature that applies to the music but is not drawn) had no api home, so it was written back as a plain `<key>` and the signature became visible. `KeyData` gains `printObject`, a tri-state `Bool` that mirrors `ClefData::printObject`. The reader fills it for traditional and non-traditional keys; `PropertiesWriter::writeKey`, the one place a `<key>` is built, emits it when it is not unspecified. The default leaves output unchanged. `data/api.features.xml` marks key `print-object` as supported. `musuite/testInvisibleElements.xml` now round-trips and is pinned in the baseline. Not addressed here: the non-traditional key reader branch still does not set `staffIndex` or `tickTimePosition`, and other `<key>` attributes (color, position, font) are still dropped. ## Testing - [x] New `PrintObject*` cases in `KeyDataTest.cpp`: no, yes, unspecified (omitted), non-traditional, per-staff, and equality - [x] `make api-test` passes (6141 assertions in 710 test cases) - [x] `make api-roundtrip` passes, 442 of 442 pinned files - [x] `make api-roundtrip-discover`: 442 pass, no file that passed before now fails - [x] `make fmt-check` passes ## References - Stacked on #467
webern
added a commit
that referenced
this pull request
Nov 1, 2026
## Summary The barline documentation in the schema (complexType `barline`) says the location attribute must match where the barline occurs in the musical data, and that a left barline "should be the first element in the measure, aside from the print, bookmark, and link elements." `MeasureWriter` broke both halves of that: - A left barline was written ahead of the measure's `<print>`. It now follows the `<print>` (and still precedes `<attributes>`). - A `location="middle"` barline was written at the end of the measure. It is now written when the write position reaches its tick: before a `<backup>`/`<forward>` that moves past it, and after one that lands on it. A middle barline whose tick no note reaches is still written at the end. Right barlines and barlines with no location are written where they were before. No api change. This also resolves the old TODO in `MeasureWriter.cpp` about barlines landing in the wrong place. Two corpus files now round-trip and are pinned: - lysuite/ly46b_MidmeasureBarline.xml - musetrainer/Bella_Ciao.xml (also relies on the harmony print-frame change in #467) The left-barline change removes a reorder from 7 of the 16 corpus files that have `<print>` followed by a left barline; the rest still fail for other reasons. Known limits, none of which occur in the corpus: a source that puts a middle barline after a `<backup>` gets it written in the earlier voice at the same tick (valid, not byte-identical), and a middle barline at the very end of a multi-voice measure is written before the `<backup>`. ## Testing - [x] New `MeasureWriterTest` cases: left barline after print, left barline without print, middle barline at its tick, after a forward, before a backup, unspecified location stays last, and a middle barline no note reaches is written once at the end - [x] `make api-test` passes (6212 assertions in 723 test cases) - [x] `make api-roundtrip` passes, 447 of 447 pinned files - [x] `make api-roundtrip-discover`: 447 pass, against 445 on the commit below; no file lost its pass - [x] `make fmt-check` passes ## References - Stacked on #469 - Related to #208 (api round-trip pass-list coverage)
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.
Summary
Two harmony attributes were lost on a round trip through
mx::api.<harmony print-frame="no">had no api home, so a chord symbol whose fretboard diagram is meantto be hidden came back without that instruction.
ChordDatagainsprintFrame, a tri-stateBoolthat applies to the whole<harmony>. The reader copies it onto every chord in theharmony; the writer emits it from the first chord that sets it, whether or not the chord has
frame data.
<kind text="">major</kind>(the kind spelled with nothing, so C major prints as just "C")lost its empty text, because an empty string was treated as absent.
ChordData::textbecomesstd::optional<std::string>, so empty and absent are different.The second change is source-breaking for code that reads
ChordData::textas a string; suchcallers need
text.value_or(""). Assigning""now writestext=""; leavetextasstd::nulloptto omit the attribute. Default-constructed chords behave as before.Six more corpus files round-trip and are pinned in the baseline:
Testing
HarmonyExtrasApiTest.cppcover print-frame yes/no/unspecified, print-framewithout frame data, multiple chords in one harmony, and empty vs absent kind text
make api-testpasses (6102 assertions in 704 test cases)make api-roundtrippasses, 441 of 441 pinned filesmake api-roundtrip-discover: 441 pass (was 435), no file that passed before now failsmake fmt-checkpassesReferences
mx::api#406 (harmony and frameidattributes)ChordData::text(e.g. Add emscripted build to CI #386) need thevalue_or("")change