fix: suppress spurious implied-default element injection in api round-trip (#228)#241
Merged
Conversation
Add DurationData::isDurationNameSpecified (default true). The reader sets it false when the source note omits <type>; the writer emits <type> only when the flag is set. The existing derive-on-read (for consumer convenience) is preserved -- callers still get a durationName even when the source had none -- but it is no longer re-emitted on the write path. Pin 4 newly passing corpus files.
gen-quality
|
gen-quality
|
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.8% | 28487 / 36624 |
| Functions | 74.3% | 6349 / 8550 |
| Branches | 50.6% | 22632 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 78.2% | 5887 / 7529 |
| Functions | 62.1% | 1962 / 3159 |
| Branches | 47.6% | 4965 / 10441 |
Core HTML report | API HTML report
Commit e826966bebc917ac65c04c83fae65dea5bffaf28.
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.
Summary
Three commits, one per element family from #228.
add:type—DurationData::isDurationNameSpecifiedThe writer was always emitting
<type>even when the source note had none. Addedbool isDurationNameSpecified(defaulttrue) toDurationData. The reader sets itfalsewhen the source lacks<type>; the writer skips the element whenfalse.durationNameis still derived from tick length on read so consumers always have something to work with — the flag only controls write-side emission.add:line—ClefData::isLineSpecifiedMeasureReader::importClefwas fabricating a default line number from the clef symbol when<line>was absent. Addedbool isLineSpecified(defaulttrue) toClefData. The reader sets itfalsein the else-branch;PropertiesWriter::writeClefnow gates<line>emission on the flag instead of>= 0.add:voice—NoteWriter::setStaffAndVoiceThe writer was unconditionally emitting
<voice>whenevermyCursor.voiceIndex >= 0, which is always true. The fix honors the existing TODO comment ("only show voice number if it is needed") by tracking voice count per staff (numVoices, passed fromMeasureWriter::writeVoices). Voice is now emitted when any of these is true:<voice>(userRequestedVoiceNumber != -1), orvoiceIndex > 0), ornumVoices > 1) — labels all voices unambiguously for authored multi-voice scores.Testing
add:type:ly41d,ly41f,ly45d,ly45eadd:line:ly12aadd:voice:ChordDirectionPlacement,hello_timewise,key-accidental.3.0References
<type><line><voice><staff>#228