fix: round-trip part-name/print-object instead of forcing print-object="no"#233
Merged
Merged
Conversation
webern
added
bug
Claude
gen-quality
|
…t="no" (#227) mx force-wrote print-object="no" onto every <part-name>/<part-abbreviation>, silently hiding otherwise-visible part names. The 2016 code justified this with the MusicXML 2.0 deprecation note, but that note deprecates only the name's *formatting* attributes (font, color, justify, print-style position) in favor of the *-display elements -- it says nothing about print-object, which is a separate, non-deprecated attribute controlling visibility. - model print-object on PartData as a tri-state (namePrintObject / abbreviationPrintObject) and round-trip it faithfully - stop force-writing print-object="no" - on read, migrate any deprecated <part-name> formatting into the display model; on write, emit name formatting only at the non-deprecated *-display elements and never back onto <part-name> (a present *-display wins) - add PartData::effectiveName()/effectiveAbbreviation() so an empty <part-name> with a <part-name-display> still yields a usable name without mutating the round-tripped fields - document the "two places, one model" reasoning in api/PartData.h - grow the pinned api round-trip pass-list from 4 to 32 files The three file-based ApiTester baselines whose sources carry print-object="no" now set namePrintObject to match the (now faithfully modeled) data.
webern
force-pushed
the
claude/fix-227-part-name-print-object
branch
from
a4b9cec to
7c18907
Compare
Coverage reportCore-dev coverage
|
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 77.9% | 28539 / 36624 |
| Functions | 74.4% | 6360 / 8550 |
| Branches | 50.7% | 22672 / 44725 |
API coverage src/private/mx/{api,impl,utility}/
| Metric | Coverage | Covered / Total |
|---|---|---|
| Lines | 75.2% | 5644 / 7509 |
| Functions | 60.1% | 1897 / 3159 |
| Branches | 45.5% | 4736 / 10419 |
Core HTML report | API HTML report
Commit 943a54c8887de498f272bde5c31c9f92372eac89.
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
mx force-wrote
print-object="no"onto every<part-name>/<part-abbreviation>, silently hiding otherwise-visible part names. The 2016 code blamed the MusicXML 2.0 deprecation note, but that note deprecates only the name's formatting attributes (font, color, justify, print-style position) in favor of the*-displayelements — not print-object, which is a separate, non-deprecated visibility attribute.This models name/abbreviation visibility faithfully and handles the deprecation the way mx::api wants it: one model, written to the modern location.
PartDataas a tri-state (namePrintObject/abbreviationPrintObject) and round-tripped faithfully; the forced"no"is gone.<part-name>formatting is migrated into the display model on read and emitted only at the non-deprecated*-displayelements on write (never back onto<part-name>); a present*-displaywins.PartData::effectiveName()/effectiveAbbreviation()give a usable name when<part-name>is empty but a<part-name-display>is present, without mutating the round-tripped fields.api/PartData.h.Grows the pinned api round-trip pass-list from 4 to 32 files.
Incidental finding: the generic
impl::setPrintObjecttemplate is broken (inverted has-check and noapi::Bool→YesNoconversion, so it always writes"yes"). It is unused for writing elsewhere; this PR uses the manual converter idiom and leaves the helper alone.Testing
mxtest-api-roundtrip regressionover the pinned baseline: 32 passed, 0 failedly41i) and the visible-name case (ly01b)mxtestsuite: 4126 assertions in 274 cases, all passclang-format --Werrorclean on the changed sourcesReferences