Repository navigation
feat: expose musicxml id attributes in mx::api - #401
Merged
Merged
Conversation
Beginning with MusicXML 3.1 most elements can carry an id attribute that names that one element within the document. mx::api dropped nearly all of them. Add an optional id member to every api type that maps one-to-one onto an element with an id, and read and write it in mx::impl. New id fields: NoteData, MeasureData, ClefData, KeyData, TimeChoice, TransposeData, BarlineData, LyricData, DirectionData, WordsData, SymbolData, RehearsalData, WedgeStart, WedgeStop, SpannerStart, SpannerStop, PedalLineData, CurveStart, CurveContinue, CurveStop, TieLetRing, TupletStart, TupletStop, SoundData and FiguredBassData. A new IdFunctions.h holds the two impl helpers that read and write the attribute. ApiCommon.h explains what ids are for and the uniqueness rule that mx does not enforce. Left out are the elements where placing the id needs a design decision: the grouping elements mx::api does not model (direction-type, notations, articulations, ornaments, technical, beam), the marks folded into MarkData (dynamics, fermata, accidental-mark), harmony and frame (one DirectionData holds several ChordData that serialize into a single harmony), credit (both credit and credit-words carry an id and PageTextData models the pair), and the elements mx::api does not model at all (print, measure-style, grouping, for-part, glissando, slide). synthetic/bracket.3.1.xml and synthetic/dashes.3.1.xml now survive the strict api round trip and are pinned in roundtrip-baseline.txt.
webern
added
feature
Claude
Merges into #401, not into `main`. This is the "what would it look like" experiment for replacing the bare `std::optional<std::string> id` with a type. ## The problem An id attribute is not any old string. MusicXML requires it to be an XML name: a letter or an underscore first, then letters, digits, dots, hyphens, or underscores. On #401 as it stands, mx enforces that at the last possible moment. `mx::impl` hands the caller's text to `core::Token` on the way out, `core::Token` scrubs it, and the file gets an id the caller never asked for and never hears about: ```cpp note.id = "3 blind mice"; // the file gets id="blindmice" ``` ## The change Every id member in `mx::api` becomes a `std::optional<Id>`. `api::Id` holds a `core::Token` behind a pimpl. That is the point of the design: the text is scrubbed once, when the `Id` is built, and from then on `mx::impl` hands `mx::core` the token itself. A `core::Token` cannot hold a malformed id, so there is no second conversion left to make. `Id.cpp` is now the only place in mx where an api id string becomes a token. The public header names no core type. `IdAccess`, in `src/private/mx/api/IdAccess.h`, is the only door between an `Id` and its token, and it is not part of the public api. ## What a caller sees Scrubbing is silent, so `Id` documents the check rather than inventing an error channel: ```cpp const auto id = Id{myText}; if (id.value() != myText) { // myText was not a legal id; decide what to do about it } ``` Uniqueness is unchanged. It is a property of the whole score rather than of one name, so `Id` cannot check it (#397). ## Value semantics `Id` copies, assigns, compares, sorts, and hashes like any other value. `std::hash` is specialized for it, so an `Id` works as a key in both `std::map` and `std::unordered_map`. Copies share one immutable token instead of allocating another, and a moved-from `Id` still holds a token, so reading one is harmless rather than undefined. ## Scope All 43 id members are converted, so the api has one way to say this and not three: - the 25 added by #401; - the 16 that already shipped as `std::optional<std::string>` (`DampData`, `EyeglassesData`, `HarpPedalsData`, `ImageData`, `MarkDataChoice`, `OtherDirectionData`, `PercussionData`, `PrincipalVoiceData`, `ScordaturaData`, `StaffDivideData`, `StringMuteData`, `TempoData`, `AccordionRegistrationData`); - `SegnoData` and `CodaData`, which held a `std::string id` plus an `isIdSpecified` flag. Both flags are gone, along with their equality-block lines. The reader and writer sites that hand-rolled the id attribute now go through `getId` / `setId` alongside the rest. `PartData::uniqueId` and `InstrumentData::uniqueId` are left alone. They are the required part-id join between `<score-part>` and `<part>`, not the `optional-unique-id` attribute this change is about. ## Two things worth a look - **`Id{""}` is `"X"`.** There is no nearest legal id for empty text, so it gets `core::Token`'s natural zero. To write no id attribute, leave the `std::optional` empty. This is not new behavior — #401 already writes `id="X"` for an unusable string — but the type makes it visible. - **The reader no longer reports what the file literally said.** A file containing `id="3 mice"` reaches the api as `mice`. By doctrine that is right (an invalid document should be hard to express, and this is documented normalization), but it is a real change and worth naming. ## Gates Run natively with `MX_RUNNING_IN_DOCKER=1`; the Docker daemon is not available in this environment. | Gate | Result | |---|---| | `make fmt-check` | passed | | `make api-test` | 6050 assertions in 546 test cases | | `make core-roundtrip-test` | 839 test cases | | unit tests | 221 assertions in 44 test cases | | `make api-roundtrip` | 366 passed, 0 failed (of 366 pinned) | | unity build (`BATCH_SIZE=0`) | built | `make wasm-test` was not run locally (no Emscripten here); CI covers it. New tests in `IdAttributeApiTest.cpp` cover scrubbing, the compare-your-input check, a scrubbed id surviving a round trip, and the value semantics including both map kinds. The existing `segnoAndCodaRoundTrip` test in `DirectionWriterTest.cpp` compares whole objects, so it covers the two converted ids through the equality block.
webern
commented
Aug 23, 2026
| LineData lineData; | ||
|
|
||
| SpannerStart() : number{}, tickTimePosition{0}, positionData{}, printData{}, lineData{} | ||
| // The id attribute of the element this spanner end is written as -- <bracket>, |
Owner
Author
There was a problem hiding this comment.
End? This is a spammer start.
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
Model ID for those cases where the mx::api data model is close enough the MusicXML data model to make that simple.
Caveats:
Summary
Starting with MusicXML 3.1 most elements can carry an
idattribute naming that one element within the document.mx::apidropped nearly all of them (only a handful of recently-added direction types had one). This adds an optionalidmember to every api type that maps one-to-one onto an element with an id, and reads and writes it inmx::impl.Types that gained an
id:NoteData,LyricDataMeasureData,BarlineDataClefData,KeyData,TimeChoice,TransposeDataDirectionData,SoundDataWordsData,SymbolData,RehearsalDataWedgeStart,WedgeStopSpannerStart,SpannerStopPedalLineDataCurveStart,CurveContinue,CurveStop,TieLetRingTupletStart,TupletStopFiguredBassDataSupporting changes:
src/private/mx/impl/IdFunctions.hwith the two helpers (getId,setId) the readers and writers call.ApiCommon.hexplains what ids are for, and that mx repairs a malformed id but does not check uniqueness (see ID uniqueness is not constrained bymx::core. #397).<time>element carries it.SoundData::isSpecifiednow counts an id, so a<sound>that has nothing but an id is not dropped.Left out, because placing the id would need a design decision:
mx::apidoes not model as their own type: direction-type, notations, articulations, ornaments, technical, beam.MarkData: dynamics, fermata, accidental-mark. These would need newMarkDataChoicealternatives.DirectionDataholds severalChordDatathat serialize into a single<harmony>, so it is not clear which chord owns the id.<credit>and<credit-words>have an id andPageTextDatamodels the pair.mx::apidoes not model at all: print, measure-style, grouping, for-part, glissando, slide (see api: model note-attached paired spanners (glissando, slide, wavy-line) #389).Testing
src/private/mxtest/api/IdAttributeApiTest.cpp: 7 cases, 53 assertions, covering every new field throughroundTrip, plus one case reading ids straight off source XML and one pinning the implicit-time-signature rulemake test-allgreen: core roundtrip (839 cases), core unit (44 cases), api/impl suite (6018 assertions in 542 cases), api roundtrip (366 pinned, 0 failed)synthetic/bracket.3.1.xmlandsynthetic/dashes.3.1.xmlnewly pass because of the bracket and dashes ids, and are pinned inroundtrip-baseline.txt. No file regressed.make fmt-checkpassesReferences
mx::api. #399mx::core. #397 (id uniqueness is not constrained bymx::core)