feat: report non-fatal import diagnostics - #439
Merged
Merged
Conversation
The generated core parsers repair values silently: an unknown enum literal falls back to the first variant, numbers are clamped into range, unreadable text becomes a default, and a missing lyric-language/@xml:lang becomes "und". Carry a ParseContext through every generated parser and report those repairs with their XML path. Value types gain a parse overload that says whether the text was invalid or adjusted, so valid alternate spellings are not reported. MusicXml::fromFile and fromStream take an optional Diagnostics that the same collector can then pass to getScore. Parsing produces the same document either way.
Contributor
gen-quality
|
core::parse and every generated parser now take the context, with no
overload that constructs a silent one. Callers that want no reports
pass ParseContext{} explicitly: the defaults probe, the core tests and
the corert runner.
SilentParseProducesTheSameDocument now also checks that the reporting
parse saw repairs, so its comparison with ParseContext{} is meaningful.
Contributor
gen-quality
|
Name-token attributes now go through the reporting helper. A repaired xs:ID also warns that it may duplicate another ID in the document, since the repair can merge distinct IDs. Surrounding whitespace is a valid spelling and is not reported. Mark the unique ID exception with TODOs at getCoreDocument() and beside the claims that invalid MusicXML is unrepresentable.
Contributor
gen-quality
|
This was referenced Sep 17, 2026
Color, font-family, SMuFL glyph names, time-only, yyyy-mm-dd and ending-number values gain parse(text, outcome), which reports invalid when the strict parse rejects the text. parseValue no longer has a silent branch, so a value type without the outcome form fails to compile.
DiagnosticCode gains droppedData, for data that could not be read or written and was left out. The read context now reaches the direction, note, ornament and layout readers and the timewise conversion, and the direction readers take a MeasureCursor so reports carry the part and measure.
getScore now reports the data it changes, defaults or leaves out: unmatched or duplicate parts, unbalanced part groups, rounded divisions and durations, a clamped backup, out-of-range staff numbers, snapped octave-shift sizes and tremolos, dropped degrees, staff details and repeated time signatures, and more. A non-numeric part-group number reads as unspecified and a clef number above the staff count adds staves, where both failed the read. Divisions that round to 0 read as 1, and a least common multiple of the divisions that overflows falls back to the largest divisions.
The write context now reaches the properties, direction and dynamics writers, the encoding and defaults functions and the timewise conversion. reportAdjusted reports when a core value built to write differs from the api value it came from. An Id remembers the text it scrubbed, so writing it can report the change.
fromScore now reports scrubbed ids, clamped values, defaulted required values, dropped marks, curves, keys and part groups, duplicate part ids and unmatched spanners.
Contributor
gen-quality
|
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.
Human Summary
This commit threads a
ParseContextobject through themx::coredocument parsing and updates the public interface as necessary to report coercions out to the client application asDiagnostics.Summary
The generated
mx::coreparsers are strict about structure but lenient about values: an unknownenum literal falls back to the first variant, a number is clamped into its schema range, malformed
text becomes a default, and a missing
lyric-language/@xml:langis filled in withund. Until nowall of that happened silently.
MusicXml::fromFileandfromStreamgainDiagnostics &overloads, so the collector from #437 canbe handed to the parse and then on to
getScore, gathering both layers' reports in one place. Theexisting overloads are unchanged and stay silent.
Inside core, a new
ParseContextis a required parameter ofcore::parseand every generatedparser, with no overload that fills one in. A caller that wants no reports passes
ParseContext{},as the defaults probe, the core tests and the corert runner do. Core keeps its own
Diagnostictypewith an XML path, and
mx::apimaps it to the public one, so core still does not depend onmx::api. Value types gain aparse(text, outcome)overload that says whether the text wasunreadable or was adjusted. That is what lets
+01or1pass quietly while12for an octaveis reported as adjusted to
9. Two new public codes,invalidValueandmissingValueDefaulted,join the existing
valueAdjusted. All import reports are warnings.TokenandNameTokenattributes report their repairs too. A repairedxs:IDalso says it mayduplicate another ID in the document, because the repair can merge distinct IDs:
"1"and"2"both become
"X". Checking ID uniqueness itself is left for later, and the docs ofgetCoreDocument()and of the valid-by-construction claims carry TODOs for that exception.Serialization is untouched, and parsing produces the same document with or without a collector.
Not yet reported: the bespoke value types (color, yyyy-mm-dd, comma-separated text and font family,
time-only, ending number, the SMuFL glyph names) still repair silently. They can be added as report
sites later using the same
parse(text, outcome)shape.Testing
fromFile,fromStream, a collection reused acrossfromStreamand
getScore, identical output without a collector, and a throwing handler becominginternalErrormake test-allmake wasm-testmake gen-test,make gen-quality,make gen-lintmake fmtReferences
mx::core. #397