api: fix mx::api enum data-loss bugs#198
Merged
Merged
Conversation
## Summary - core::DynamicsChoice::Kind has n, pf, and sfzp (MusicXML 3.1), but the api::MarkType dynamics section stopped at fz and dynamicsMap omitted the three rows, so these dynamics fell back to MarkType::unspecified on read and were lost. - Adds the three api::MarkType members, the three dynamicsMap rows, the isMarkDynamic predicate entries, and the dynamicsKindToString cases so the values read and write back correctly. ## Test plan - New MarkRoundTrip tests DynamicsN/DynamicsPf/DynamicsSfzp round-trip each dynamic through serialize -> deserialize and assert the mark survives. - make check and make test pass.
## Summary - core::ArticulationsChoice::Kind::softAccent (MusicXML 3.1) had no api::MarkType member and articulationsMap omitted the row, so a <soft-accent/> articulation fell back to MarkType::unspecified on read and was lost. The reader/writer already handled the core kind. - Adds the api::MarkType::softAccent member, the articulationsMap row, the isMarkArticulation predicate entry, and the Converter::isArticulation predicate entry so it round-trips. ## Test plan - New MarkRoundTrip test SoftAccent round-trips the mark and asserts it survives. - make check and make test pass.
…#195) ## Summary - core::OrnamentsGroupChoice::Kind has haydn and invertedVerticalTurn (MusicXML 3.1), but api::MarkType lacked the members and ornamentsMap omitted the rows, so they fell back to MarkType::unspecified on read and were lost. The reader (OrnamentsFunctions) and writer (NotationsWriter) already handled the core kinds. - Adds the two api::MarkType members, the two ornamentsMap rows, and the isMarkOrnament predicate entries so they round-trip. ## Test plan - New MarkRoundTrip tests OrnamentHaydn/OrnamentInvertedVerticalTurn round-trip each ornament and assert it survives. - make check and make test pass.
…n-mute golpe silently dropped (#196) ## Summary - core::TechnicalChoice::Kind has brassBend, flip, smear, open, halfMuted, harmonMute, and golpe (MusicXML 3.1-4.0), but api::MarkType lacked the members and technicalMarkMap omitted the rows, so they fell back to MarkType::unspecified and were lost. These technical marks carry no payload. - Adds the seven api::MarkType members, the seven technicalMarkMap rows, the isMarkTechnical predicate entries, the reader cases in TechnicalFunctions, and the writer cases in NotationsWriter so they round-trip. ## Test plan - New MarkRoundTrip tests TechnicalBrassBend/Flip/Smear/Open/HalfMuted/ HarmonMute/Golpe round-trip each mark and assert it survives. - make check and make test pass.
## Summary - core::FermataShape has double-angled, double-square, double-dot, half-curve, and curlew (MusicXML 4.0), but api::MarkType's fermata section stopped at square and fermataMap omitted the rows, so these fermata shapes fell back to MarkType::unspecified on read and were lost. - Adds five api::MarkType members (base shapes, mirroring the existing fermata structure), the five fermataMap rows, the isMarkFermata predicate entries, the reader branches in FermataFunctions, and the writer cases in NotationsWriter so they round-trip. ## Test plan - New MarkRoundTrip tests FermataDoubleAngled/DoubleSquare/DoubleDot/ HalfCurve/Curlew round-trip each shape and assert it survives. - make check and make test pass.
…opped (#182) ## Summary - core::AccidentalValue has double-sharp-down, double-sharp-up, flat-flat-down, flat-flat-up, arrow-down, and arrow-up, but they were missing from both api::Accidental and the accidentalMap, so a note carrying one read back as Accidental::none (visible data loss). The accidentalMarkMap (and the parallel api::MarkType accidental-mark members) had the same gap. - Adds the six api::Accidental members and accidentalMap rows, plus the six api::MarkType accidental-mark members and accidentalMarkMap rows, so the values read and write back correctly. ## Test plan - New PitchData test MicrotonalArrowAndDoubleVariants round-trips a note for each of the six accidental variants and asserts the value is preserved. - make check and make test pass.
## Summary - core::NoteheadValue has circled (MusicXML 4.0) and the other catch-all, but noteheadMap omitted them and api::Notehead lacked the members, so a note with either notehead read back as Notehead::none (data loss). - Adds the two api::Notehead members and the two noteheadMap rows so they round-trip. ## Test plan - New NoteData tests noteheadCircledRoundtrip and noteheadOtherRoundtrip round-trip a note with each notehead and assert the value is preserved. - make check and make test pass.
## Summary - core::SoundID has the MusicXML 4.0 ids drum-tabor, drum-tamborim, pitched-percussion-handchimes, pluck-cavaquinho, wind-flutes-whistle-tin-c, wind-reed-clarinet-d, and wind-reed-clarinet-g, but they were missing from api::SoundID and instrumentMap, so a part with one read back as SoundID::unspecified (data loss). - Adds the seven api::SoundID members and the seven instrumentMap rows so they read and write back correctly. ## Test plan - New ConverterSoundID tests soundId40Additions_coreToApi and soundId40Additions_apiToCore assert both conversion directions for all seven ids. - make check and make test pass.
gen-quality
|
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.9% | 28539 / 36619 |
| Functions | 74.4% | 6360 / 8547 |
| Branches | 50.7% | 22672 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 69.9% | 4895 / 7001 |
| Functions | 55.3% | 1511 / 2733 |
| Branches | 41.3% | 4011 / 9703 |
Core HTML report | API HTML report
Commit a41fc61d9b0c77e97624b30531d944a226a0de7e.
webern
changed the title
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
1 participant
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.
Fixes the audit section 1b enum data-loss bugs, where a
coreenum value is silently dropped because theConverter::EnumMaptable and/or parallelmx::apienum lacked the member. One commit per issue.Each fix updates the
Convertermap, the parallelmx::apienum member, the relevantisMark*predicate / reader / writer path as needed, adds a regression round-trip test, and marks the audit tracker row.Test plan
MarkRoundTripTest.cpp, plus additions toPitchDataTest.cpp,NoteDataTest.cpp,ConverterSoundIDTest.cpp)make testgreen (245 cases, 3970 assertions)make check(fmt-check) greenReferences
n,pf,sfzpsoft-accenthaydn,inverted-vertical-turnbrass-bend/flip/smear/open/half-muted/harmon-mute/golpedouble-angled,double-square,double-dot,half-curve,curlew)circledandother