fix: round-trip part-name/print-object instead of forcing print-object="no"

src/include/mx/api/PartData.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include "mx/api/ApiCommon.h"
 #include "mx/api/MeasureData.h"
 #include "mx/api/SoundID.h"
 #include "mx/api/TransposeData.h"
@@ -83,23 +84,62 @@ class PartData
   public:
     std::string uniqueId;
 
-    // because the MuscXML specification says this "Formatting
-    // attributes for the part-name element are deprecated in
-    // Version 2.0 in favor of the new part-name-display and
-    // part-abbreviation-display elements" the name will always
-    // be written with 'print-object="no". You must populate
-    // the displayName field in order to have a name displayed.
+    // ---------------------------------------------------------------------
+    // Part name / abbreviation, and the MusicXML "two places" problem
+    // ---------------------------------------------------------------------
+    //
+    // MusicXML can express how a part is named in two overlapping ways, and
+    // that overlap is a perennial source of confusion. mx::api deliberately
+    // collapses it into a single, simpler model. The reasoning:
+    //
+    //   1. <part-name> (and <part-abbreviation>) carry the canonical, plain-
+    //      text name. <part-name> is REQUIRED by the schema, so mx always
+    //      writes it (even when empty). Its *formatting* attributes -- font,
+    //      color, justify and the print-style position attributes -- were
+    //      deprecated in MusicXML 2.0 in favor of the *-display elements.
+    //
+    //   2. <part-name-display> (and <part-abbreviation-display>) are the
+    //      modern, non-deprecated home for a formatted rendering of the name.
+    //
+    // So the same *formatting* has two possible homes. mx::api keeps exactly
+    // one -- the display* fields below -- and follows these rules:
+    //
+    //   * On read, formatting is taken from wherever it lives. Formatting
+    //     found on the deprecated <part-name> attributes is migrated into the
+    //     display* model. If a *-display element is also present it WINS over
+    //     the deprecated attributes (the non-deprecated location is canonical).
+    //   * On write, formatting is emitted ONLY to the non-deprecated *-display
+    //     elements. mx never writes the deprecated formatting attributes back
+    //     onto <part-name>. A document that used the deprecated attributes is
+    //     therefore normalized to the modern form; by design this does not
+    //     round-trip byte-for-byte.
+    //
+    // This is purely about *formatting*. The two name strings are not pure
+    // duplicates: <part-name> is the canonical text, while <part-name-display>
+    // may hold a different, richer rendering (e.g. with accidental glyphs).
+    // That is why both `name` and `displayName` are retained.
+
+    // The canonical part name: the text of the required <part-name> element.
+    // May be empty; see effectiveName() for the "fall back to the display
+    // name when this is empty" convenience.
     std::string name;
 
-    // because the MuscXML specification says this "Formatting
-    // attributes for the part-name element are deprecated in
-    // Version 2.0 in favor of the new part-name-display and
-    // part-abbreviation-display elements" the abbreviation
-    // will always be written with 'print-object="no". You
-    // must populate the displayAbbreviation field in order to
-    // have an abbreviation displayed.
+    // The canonical part abbreviation: the text of <part-abbreviation>. The
+    // element is optional, so this is only written when non-empty.
     std::string abbreviation;
 
+    // Visibility of <part-name> / <part-abbreviation> via the print-object
+    // attribute. print-object is NOT part of the 2.0 formatting deprecation:
+    // it is an independent property controlling whether the name is rendered,
+    // and mx round-trips it faithfully:
+    //   unspecified -> no print-object attribute is written (defaults to shown)
+    //   yes / no    -> print-object="yes" / "no" is written verbatim
+    // (Historically mx force-wrote print-object="no" on every part, silently
+    // hiding otherwise-visible names. That conflated print-object with the
+    // formatting deprecation; it was incorrect and has been removed.)
+    Bool namePrintObject = Bool::unspecified;
+    Bool abbreviationPrintObject = Bool::unspecified;
+
     std::string displayName;
     PrintData displayNamePrintData;
     PositionData displayNamePositionData;
@@ -119,6 +159,22 @@ class PartData
 
     std::vector<MeasureData> measures;
 
+    // MusicXML requires <part-name>, but real files often leave it empty and
+    // place the human-readable name only in <part-name-display>. These helpers
+    // return the name to actually show: the canonical `name` when present,
+    // otherwise the `displayName` (and likewise for the abbreviation). The
+    // stored fields are left untouched, so the original document still
+    // round-trips.
+    inline const std::string &effectiveName() const
+    {
+        return !name.empty() ? name : displayName;
+    }
+
+    inline const std::string &effectiveAbbreviation() const
+    {
+        return !abbreviation.empty() ? abbreviation : displayAbbreviation;
+    }
+
     inline int getNumStaves() const
     {
         int numStaves = 0;
@@ -190,6 +246,8 @@ MXAPI_EQUALS_BEGIN(PartData)
 MXAPI_EQUALS_MEMBER(uniqueId)
 MXAPI_EQUALS_MEMBER(name)
 MXAPI_EQUALS_MEMBER(abbreviation)
+MXAPI_EQUALS_MEMBER(namePrintObject)
+MXAPI_EQUALS_MEMBER(abbreviationPrintObject)
 MXAPI_EQUALS_MEMBER(displayName)
 MXAPI_EQUALS_MEMBER(displayNamePrintData)
 MXAPI_EQUALS_MEMBER(displayNamePositionData)

src/private/mx/impl/NameDisplayFunctions.cpp

@@ -8,6 +8,8 @@
 #include "mx/core/generated/FormattedText.h"
 #include "mx/core/generated/NameDisplay.h"
 #include "mx/core/generated/NameDisplayChoice.h"
+#include "mx/impl/PositionFunctions.h"
+#include "mx/impl/PrintFunctions.h"
 
 #include <sstream>
 
@@ -39,11 +41,38 @@ std::string extractDisplayText(const core::NameDisplay &nameDisplay)
     return ss.str();
 }
 
+void extractDisplayFormatting(const core::NameDisplay &nameDisplay, api::PrintData &outPrintData,
+                              api::PositionData &outPositionData)
+{
+    for (const auto &c : nameDisplay.choice())
+    {
+        if (c.isDisplayText())
+        {
+            const auto &ft = c.asDisplayText();
+            outPrintData = getPrintData(ft);
+            outPositionData = getPositionData(ft);
+            return; // the api keeps one run's formatting; the first run wins
+        }
+    }
+}
+
 core::NameDisplay makeNameDisplay(const std::string &text)
+{
+    return makeNameDisplay(text, api::PrintData{}, api::PositionData{});
+}
+
+core::NameDisplay makeNameDisplay(const std::string &text, const api::PrintData &printData,
+                                  const api::PositionData &positionData)
 {
     core::NameDisplay nameDisplay{};
     core::FormattedText ft{};
     ft.setValue(text);
+    // Emit font/color and the print-style position attributes onto the
+    // <display-text> run -- the modern, non-deprecated home for this
+    // formatting. (print-object is not a <display-text> attribute, so the
+    // PrintData::printObject field is intentionally not applied here.)
+    setAttributesFromPrintData(printData, ft);
+    setAttributesFromPositionData(positionData, ft);
     nameDisplay.addChoice(core::NameDisplayChoice::displayText(ft));
     return nameDisplay;
 }

src/private/mx/impl/NameDisplayFunctions.h

@@ -4,6 +4,9 @@
 
 #pragma once
 
+#include "mx/api/PositionData.h"
+#include "mx/api/PrintData.h"
+
 #include <string>
 
 namespace mx
@@ -20,12 +23,25 @@ namespace impl
 {
 // Best-effort plain-text extraction from a MusicXML name-display element
 // (<part-name-display>, <group-name-display>, etc.): the concatenated
-// <display-text> runs, with <accidental-text> rendered as "b"/"#". Formatting
-// attributes are not preserved -- the api models display names as plain text.
+// <display-text> runs, with <accidental-text> rendered as "b"/"#".
 std::string extractDisplayText(const core::NameDisplay &nameDisplay);
 
-// Build a minimal name-display carrying a single <display-text> run. The
-// inverse of extractDisplayText for plain-text display names.
+// Reads the formatting (font, color, and print-style position attributes) of a
+// name-display element from its first <display-text> run. The api models a
+// display name as a single formatted string, so a multi-run display collapses
+// to the formatting of its first run.
+void extractDisplayFormatting(const core::NameDisplay &nameDisplay, api::PrintData &outPrintData,
+                              api::PositionData &outPositionData);
+
+// Build a minimal name-display carrying a single, unformatted <display-text>
+// run. The inverse of extractDisplayText for plain-text display names.
 core::NameDisplay makeNameDisplay(const std::string &text);
+
+// As makeNameDisplay(text), but also applies the given formatting to the
+// <display-text> run. This is how mx emits name formatting at the modern,
+// non-deprecated *-display location instead of on the deprecated <part-name>
+// attributes (see api/PartData.h for the full rationale).
+core::NameDisplay makeNameDisplay(const std::string &text, const api::PrintData &printData,
+                                  const api::PositionData &positionData);
 } // namespace impl
 } // namespace mx

src/private/mx/impl/PartReader.cpp

@@ -29,6 +29,7 @@
 #include "mx/impl/Converter.h"
 #include "mx/impl/MeasureReader.h"
 #include "mx/impl/NameDisplayFunctions.h"
+#include "mx/impl/PositionFunctions.h"
 #include "mx/impl/PrintFunctions.h"
 #include "mx/utility/Throw.h"
 #include "mx/utility/Unused.h"
@@ -39,6 +40,42 @@ namespace mx
 {
 namespace impl
 {
+namespace
+{
+// True when a <part-name>/<part-abbreviation> carries any of the formatting
+// attributes that MusicXML 2.0 deprecated in favor of the *-display elements.
+bool nameHasDeprecatedFormatting(const core::PartName &n)
+{
+    return n.fontFamily().has_value() || n.fontStyle().has_value() || n.fontSize().has_value() ||
+           n.fontWeight().has_value() || n.color().has_value() || n.defaultX().has_value() ||
+           n.defaultY().has_value() || n.relativeX().has_value() || n.relativeY().has_value() ||
+           n.justify().has_value();
+}
+
+// Reads a name/abbreviation's display text and formatting into the api's single
+// display model, following the deprecation rules documented in api/PartData.h:
+// a present *-display element is canonical and wins; otherwise any deprecated
+// formatting on the name element itself is migrated into the display model so it
+// is re-emitted at the modern location.
+void readNameDisplay(const core::PartName &nameElement, const std::optional<core::NameDisplay> &display,
+                     std::string &outText, api::PrintData &outPrintData, api::PositionData &outPositionData)
+{
+    if (display.has_value())
+    {
+        outText = extractDisplayText(*display);
+        extractDisplayFormatting(*display, outPrintData, outPositionData);
+    }
+    else if (nameHasDeprecatedFormatting(nameElement))
+    {
+        outText = nameElement.value();
+        outPrintData = getPrintData(nameElement);
+        // print-object belongs to the name element, not the display run.
+        outPrintData.printObject = api::Bool::unspecified;
+        outPositionData = getPositionData(nameElement);
+    }
+}
+} // namespace
+
 PartReader::PartReader(const core::ScorePart &inScorePart, const core::PartwisePart &inPartwisePartRef,
                        int globalTicksPerMeasure, const core::ScorePartwise &inScore, int inDivisionsValue)
     : myPartwisePart{inPartwisePartRef}, myScorePart{inScorePart}, myNumStaves{-1},
@@ -164,21 +201,26 @@ int PartReader::calculateNumStaves() const
 void PartReader::parseScorePart() const
 {
     myOutPartData.uniqueId = myScorePart.id().value();
-    myOutPartData.name = myScorePart.partName().value();
 
-    if (myScorePart.partNameDisplay().has_value())
-    {
-        myOutPartData.displayName = extractDisplayText(*myScorePart.partNameDisplay());
-    }
+    const auto &corePartName = myScorePart.partName();
+    myOutPartData.name = corePartName.value();
+    myOutPartData.namePrintObject = getPrintObject(corePartName);
+    readNameDisplay(corePartName, myScorePart.partNameDisplay(), myOutPartData.displayName,
+                    myOutPartData.displayNamePrintData, myOutPartData.displayNamePositionData);
 
     if (myScorePart.partAbbreviation().has_value())
     {
-        myOutPartData.abbreviation = myScorePart.partAbbreviation()->value();
+        const auto &coreAbbreviation = *myScorePart.partAbbreviation();
+        myOutPartData.abbreviation = coreAbbreviation.value();
+        myOutPartData.abbreviationPrintObject = getPrintObject(coreAbbreviation);
+        readNameDisplay(coreAbbreviation, myScorePart.partAbbreviationDisplay(), myOutPartData.displayAbbreviation,
+                        myOutPartData.displayAbbreviationPrintData, myOutPartData.displayAbbreviationPositionData);
     }
-
-    if (myScorePart.partAbbreviationDisplay().has_value())
+    else if (myScorePart.partAbbreviationDisplay().has_value())
     {
         myOutPartData.displayAbbreviation = extractDisplayText(*myScorePart.partAbbreviationDisplay());
+        extractDisplayFormatting(*myScorePart.partAbbreviationDisplay(), myOutPartData.displayAbbreviationPrintData,
+                                 myOutPartData.displayAbbreviationPositionData);
     }
 
     if (!myScorePart.scoreInstrument().empty())

src/private/mx/impl/PartWriter.cpp

@@ -42,6 +42,21 @@ namespace mx
 {
 namespace impl
 {
+namespace
+{
+// Writes the print-object attribute onto a <part-name>/<part-abbreviation> only
+// when the api specifies it; api::Bool::unspecified leaves the attribute off so
+// the element defaults to shown.
+void applyPrintObject(api::Bool printObject, core::PartName &out)
+{
+    if (printObject != api::Bool::unspecified)
+    {
+        Converter converter;
+        out.setPrintObject(converter.convert(printObject));
+    }
+}
+} // namespace
+
 PartWriter::PartWriter(const api::PartData &inPartData, int inPartIndex, int inTicksPerQuarter,
                        const ScoreWriter &inScoreWriter)
     : myPartData{inPartData}, myPartIndex{inPartIndex}, myTicksPerQuarter{inTicksPerQuarter}, myMutex{},
@@ -54,27 +69,33 @@ core::ScorePart PartWriter::getScorePart() const
     core::ScorePart scorePart{};
     scorePart.setID(core::Token{myPartData.uniqueId});
 
+    // <part-name> is required, so always write it. print-object is round-
+    // tripped from the model (not force-hidden); deprecated formatting is never
+    // written here -- it goes to <part-name-display> below. See api/PartData.h.
     core::PartName partName{};
     partName.setValue(myPartData.name);
-    partName.setPrintObject(core::YesNo::no());
+    applyPrintObject(myPartData.namePrintObject, partName);
     scorePart.setPartName(partName);
 
     if (myPartData.abbreviation.size() > 0)
     {
         core::PartName abbrev{};
         abbrev.setValue(myPartData.abbreviation);
-        abbrev.setPrintObject(core::YesNo::no());
+        applyPrintObject(myPartData.abbreviationPrintObject, abbrev);
         scorePart.setPartAbbreviation(abbrev);
     }
 
     if (myPartData.displayName.size() > 0)
     {
-        scorePart.setPartNameDisplay(makeNameDisplay(myPartData.displayName));
+        scorePart.setPartNameDisplay(makeNameDisplay(myPartData.displayName, myPartData.displayNamePrintData,
+                                                     myPartData.displayNamePositionData));
     }
 
     if (myPartData.displayAbbreviation.size() > 0)
     {
-        scorePart.setPartAbbreviationDisplay(makeNameDisplay(myPartData.displayAbbreviation));
+        scorePart.setPartAbbreviationDisplay(makeNameDisplay(myPartData.displayAbbreviation,
+                                                             myPartData.displayAbbreviationPrintData,
+                                                             myPartData.displayAbbreviationPositionData));
     }
 
     core::ScoreInstrument scoreInstrument{};

src/private/mxtest/api/ApiK009bSlurScoreData.h

@@ -24,6 +24,7 @@ inline mx::api::ScoreData apiK009bSlurAttributesScoreData()
     auto &part = score.parts.front();
     part.uniqueId = "P1";
     part.name = "MusicXML Part";
+    part.namePrintObject = Bool::no; // the source has <part-name print-object="no">
 
     // 1
     part.measures.emplace_back(MeasureData{});

src/private/mxtest/api/ApiK014aFermatasScoreData.h

@@ -20,6 +20,7 @@ inline mx::api::ScoreData apiK014aFermatasScoreData()
     auto &part = score.parts.front();
     part.uniqueId = "P1";
     part.name = "MusicXML Part";
+    part.namePrintObject = Bool::no; // the source has <part-name print-object="no">
 
     // 1
     part.measures.emplace_back(MeasureData{});

src/private/mxtest/api/ApiK016aMiscScoreData.h

@@ -20,6 +20,7 @@ inline mx::api::ScoreData apiK016aMiscScoreData()
     scoreData.parts.emplace_back(PartData{});
     auto &part = scoreData.parts.back();
     part.name = "hello world";
+    part.namePrintObject = Bool::no; // the source has <part-name print-object="no">
     part.uniqueId = "ID";
     part.measures.emplace_back(MeasureData{});
     auto &measure = part.measures.back();