fix: write arranger and publisher, add page-text enclosure

src/include/mx/api/ApiCommon.h

@@ -87,10 +87,10 @@ enum class SystemRelation
 };
 
 // The shape drawn around a piece of text or a symbol -- MusicXML's enclosure attribute, carried by
-// RehearsalData, WordsData, SymbolData and PercussionData. unspecified means the attribute is
-// absent, which draws no enclosure; none states explicitly that there is none. A bracket is a
-// rectangle with the bottom line missing, as is common in jazz notation, and an invertedBracket is
-// one with the top line missing.
+// RehearsalData, WordsData, SymbolData, PercussionData and PageTextData. unspecified means the
+// attribute is absent, which draws no enclosure; none states explicitly that there is none. A
+// bracket is a rectangle with the bottom line missing, as is common in jazz notation, and an
+// invertedBracket is one with the top line missing.
 enum class Enclosure
 {
     unspecified,

src/include/mx/api/PageTextData.h

@@ -40,6 +40,10 @@ class PageTextData
     // carried in `positionData.horizontalAlignment`; MusicXML defines both
     // attributes on `<credit-words>`.
     HorizontalAlignment justify = HorizontalAlignment::unspecified;
+
+    // A shape drawn around the text. Enclosure::unspecified draws no enclosure; Enclosure::none
+    // states explicitly that there is none.
+    Enclosure enclosure = Enclosure::unspecified;
 };
 
 MXAPI_EQUALS_BEGIN(PageTextData)
@@ -54,6 +58,7 @@ if (!areVectorsEqual(lhs.creditTypes, rhs.creditTypes))
     return false;
 }
 MXAPI_EQUALS_MEMBER(justify)
+MXAPI_EQUALS_MEMBER(enclosure)
 MXAPI_EQUALS_END;
 MXAPI_NOT_EQUALS_AND_VECTORS(PageTextData);
 } // namespace api

src/private/mx/impl/PageTextFunctions.cpp

@@ -91,6 +91,12 @@ void createCredits(const api::ScoreData &inScoreData, core::ScoreHeaderGroup &ou
                 words.setJustify(converter.convert(p.justify));
             }
 
+            if (p.enclosure != api::Enclosure::unspecified)
+            {
+                const Converter converter;
+                words.setEnclosure(converter.convert(p.enclosure));
+            }
+
             core::CreditChoiceGroupChoice groupChoice = core::CreditChoiceGroupChoice::creditWords(words);
             core::CreditChoiceGroup group;
             group.setChoice(groupChoice);
@@ -164,6 +170,12 @@ void createCredits(const core::ScoreHeaderGroup &inHeader, api::ScoreData &outSc
                 const Converter converter;
                 pageText.justify = converter.convert(*words.justify());
             }
+
+            if (words.enclosure().has_value())
+            {
+                const Converter converter;
+                pageText.enclosure = converter.convert(*words.enclosure());
+            }
         }
 
         for (const auto &ct : c.creditType())

src/private/mx/impl/ScoreReader.cpp

@@ -171,13 +171,15 @@ api::ScoreData ScoreReader::getScoreData() const
         myOutScoreData.movementNumber = *myHeaderGroup.movementNumber();
     }
 
-    bool isComposerFound = false;
-    bool isCopyrightFound = false;
-
     if (myHeaderGroup.identification().has_value())
     {
         const auto &ident = *myHeaderGroup.identification();
 
+        bool isComposerFound = false;
+        bool isArrangerFound = false;
+        bool isPublisherFound = false;
+        bool isCopyrightFound = false;
+
         for (const auto &i : ident.creator())
         {
             const bool hasType = i.type().has_value();
@@ -199,17 +201,18 @@ api::ScoreData ScoreReader::getScoreData() const
                 myOutScoreData.lyricist = i.value();
             }
 
-            // TODO: arranger/publisher overwrite lyricist (ScoreData has no fields for them),
-            // and ScoreWriter re-emits the value as type="lyricist" -- lossy and mislabeled.
-            // Issue: add arranger/publisher fields to ScoreData.
-            if (typeStr == "arranger")
+            // ScoreData has one slot each for arranger and publisher, but MusicXML permits any
+            // number of <creator> elements of a given type. Keep the first of each.
+            if (typeStr == "arranger" && !isArrangerFound)
             {
-                myOutScoreData.lyricist = i.value();
+                myOutScoreData.arranger = i.value();
+                isArrangerFound = true;
             }
 
-            if (typeStr == "publisher")
+            if (typeStr == "publisher" && !isPublisherFound)
             {
-                myOutScoreData.lyricist = i.value();
+                myOutScoreData.publisher = i.value();
+                isPublisherFound = true;
             }
         }
 

src/private/mx/impl/ScoreWriter.cpp

@@ -117,6 +117,24 @@ core::ScorePartwise ScoreWriter::getScorePartwise() const
         hasIdentification = true;
     }
 
+    if (!myScoreData.arranger.empty())
+    {
+        core::TypedText arranger{};
+        arranger.setType(std::string{"arranger"});
+        arranger.setValue(myScoreData.arranger);
+        identification.addCreator(arranger);
+        hasIdentification = true;
+    }
+
+    if (!myScoreData.publisher.empty())
+    {
+        core::TypedText publisher{};
+        publisher.setType(std::string{"publisher"});
+        publisher.setValue(myScoreData.publisher);
+        identification.addCreator(publisher);
+        hasIdentification = true;
+    }
+
     if (!myScoreData.copyright.empty())
     {
         core::TypedText copyright{};

src/private/mx/impl/SpannerNumberResolver.cpp

@@ -85,7 +85,7 @@ class SpannerNumberEventCollector
     // Mirrors DirectionWriter::emitDirectionTypes: one pass over the ordered direction-type
     // content, registering each spanner event with the address of its DirectionChoice -- the
     // same identity the writer presents when it asks for the emitted number. Pedals are
-    // skipped: <pedal> has no number attribute.
+    // skipped: mx::api does not model <pedal>'s number attribute.
     void addDirection(const api::DirectionData &inDirection)
     {
         for (const auto &choice : inDirection.directionTypes)

src/private/mx/impl/SpannerNumberResolver.h

@@ -40,8 +40,9 @@ namespace impl
 //
 // Identity ids are scoped per part and per spanner class: events in the same
 // part sharing a class and id are one logical spanner, even across staves.
-// Pedal starts/stops carry SpannerNumber but the <pedal> element has no
-// number attribute, so pedals are ignored here.
+// Pedal starts/stops carry SpannerNumber, but mx::api does not model the
+// <pedal> number attribute (added in MusicXML 3.1), so pedals are ignored
+// here and no number is ever emitted for one.
 //
 // If more than 16 spanners of one class are open at once in a part (which no
 // real score approaches), resolution fails loudly with an exception rather

src/private/mxtest/api/CreditRoundTripTest.cpp

@@ -149,4 +149,59 @@ TEST(creditRoundTrip, justifyAbsentStaysAbsent)
     CHECK(HorizontalAlignment::unspecified == out.pageTextItems.at(0).justify);
 }
 
+TEST(creditRoundTrip, enclosureSurvives)
+{
+    auto in = makeMinimalScore();
+    PageTextData credit{};
+    credit.text = "Framed title";
+    credit.pageNumber = 1;
+    credit.enclosure = Enclosure::rectangle;
+    in.pageTextItems.push_back(credit);
+
+    const auto xml = mxtest::toXml(in);
+    CHECK(xml.find("enclosure=\"rectangle\"") != std::string::npos);
+
+    const auto out = mxtest::roundTrip(in);
+
+    REQUIRE(out.pageTextItems.size() == 1);
+    const auto &got = out.pageTextItems.at(0);
+    CHECK_EQUAL("Framed title", got.text);
+    CHECK(Enclosure::rectangle == got.enclosure);
+}
+
+TEST(creditRoundTrip, enclosureNoneIsExplicit)
+{
+    auto in = makeMinimalScore();
+    PageTextData credit{};
+    credit.text = "Deliberately unframed";
+    credit.pageNumber = 1;
+    credit.enclosure = Enclosure::none;
+    in.pageTextItems.push_back(credit);
+
+    const auto xml = mxtest::toXml(in);
+    CHECK(xml.find("enclosure=\"none\"") != std::string::npos);
+
+    const auto out = mxtest::roundTrip(in);
+
+    REQUIRE(out.pageTextItems.size() == 1);
+    CHECK(Enclosure::none == out.pageTextItems.at(0).enclosure);
+}
+
+TEST(creditRoundTrip, enclosureAbsentStaysAbsent)
+{
+    auto in = makeMinimalScore();
+    PageTextData credit{};
+    credit.text = "no enclosure here";
+    credit.pageNumber = 1;
+    in.pageTextItems.push_back(credit);
+
+    const auto xml = mxtest::toXml(in);
+    CHECK(xml.find("enclosure=") == std::string::npos);
+
+    const auto out = mxtest::roundTrip(in);
+
+    REQUIRE(out.pageTextItems.size() == 1);
+    CHECK(Enclosure::unspecified == out.pageTextItems.at(0).enclosure);
+}
+
 #endif

src/private/mxtest/api/DocumentManagerTest.cpp

@@ -261,13 +261,75 @@ ROUND_TRIP_TEST_SCALAR(std::string, movementTitle, movementTitle, "value", 0);
 ROUND_TRIP_TEST_SCALAR(std::string, movementNumber, movementNumber, "value", 0);
 ROUND_TRIP_TEST_SCALAR(std::string, composer, composer, "value", 0);
 ROUND_TRIP_TEST_SCALAR(std::string, lyricist, lyricist, "value", 0);
+ROUND_TRIP_TEST_SCALAR(std::string, arranger, arranger, "value", 0);
+ROUND_TRIP_TEST_SCALAR(std::string, publisher, publisher, "value", 0);
 ROUND_TRIP_TEST_SCALAR(std::string, copyright, copyright, "value", 0);
 ROUND_TRIP_TEST_SCALAR(std::string, encoding.encoder, encoder, "value", 0);
 ROUND_TRIP_TEST_SCALAR(std::string, encoding.encodingDescription, encodingDescription, "value", 0);
 ROUND_TRIP_TEST_SCALAR(int, encoding.encodingDate.year, year, 2016, 0);
 ROUND_TRIP_TEST_SCALAR(int, encoding.encodingDate.month, month, 9, 0);
 ROUND_TRIP_TEST_SCALAR(int, encoding.encodingDate.day, day, 12, 0);
 
+// The four <creator> types mx::api models are independent slots. The arranger used to be read
+// into ScoreData::lyricist and rewritten as type="lyricist", so a file carrying both lost the
+// lyricist; the publisher was dropped on the way out entirely.
+
+TEST(RoundTrip_AllCreatorTypes, DocumentManager)
+{
+    auto input = ScoreData{};
+    input.composer = "MetaComposer";
+    input.lyricist = "MetaLyricist";
+    input.arranger = "MetaArranger";
+    input.publisher = "MetaPublisher";
+
+    auto &docMngr = DocumentManager::getInstance();
+    const auto createResult = docMngr.createFromScore(input);
+    REQUIRE(createResult.ok());
+    const int writeId = createResult.value();
+    std::ostringstream oss;
+    const auto writeResult = docMngr.writeToStream(writeId, oss);
+    REQUIRE(writeResult.ok());
+    docMngr.destroyDocument(writeId);
+
+    const auto xml = oss.str();
+    CHECK(xml.find("<creator type=\"composer\">MetaComposer</creator>") != std::string::npos);
+    CHECK(xml.find("<creator type=\"lyricist\">MetaLyricist</creator>") != std::string::npos);
+    CHECK(xml.find("<creator type=\"arranger\">MetaArranger</creator>") != std::string::npos);
+    CHECK(xml.find("<creator type=\"publisher\">MetaPublisher</creator>") != std::string::npos);
+
+    const auto output = roundTripScore(input);
+    CHECK_EQUAL("MetaComposer", output.composer);
+    CHECK_EQUAL("MetaLyricist", output.lyricist);
+    CHECK_EQUAL("MetaArranger", output.arranger);
+    CHECK_EQUAL("MetaPublisher", output.publisher);
+}
+
+T_END
+
+// Reading the same shape from a real file: musuite/testMetaData.xml carries an arranger, a
+// composer and a lyricist, plus creator types mx::api does not model, which must not disturb
+// the ones it does.
+
+TEST(ReadAllCreatorTypes, DocumentManager)
+{
+    auto &docMngr = DocumentManager::getInstance();
+    const auto createResult = docMngr.createFromFile(std::string{mxtest::getResourcesDirectoryPath()} +
+                                                     std::string{"/musuite/testMetaData.xml"});
+    REQUIRE(createResult.ok());
+    const int documentId = createResult.value();
+    const auto dataResult = docMngr.getData(documentId);
+    REQUIRE(dataResult.ok());
+    const auto score = dataResult.value();
+    docMngr.destroyDocument(documentId);
+
+    CHECK_EQUAL("MetaComposer", score.composer);
+    CHECK_EQUAL("MetaLyricist", score.lyricist);
+    CHECK_EQUAL("MetaArranger", score.arranger);
+    CHECK_EQUAL("", score.publisher);
+}
+
+T_END
+
 // --- Page margin coalescing -------------------------------------------------
 // Equal odd/even margins collapse to a single <page-margins type="both">;
 // unequal margins emit separate odd and even entries. This rule is exercised