feat: ending numbers and display text in mx::api

data/api.features.xml

@@ -297,7 +297,8 @@
       <notes>EncodingData.encodingDescription; EncodingFunctions.cpp.</notes>
     </feature>
     <feature name="ending" support="full">
-      <notes>BarlineData ending (EndingType via endingMap).</notes>
+      <notes>BarlineData.ending -&gt; EndingData: type (EndingType via endingMap), the full number
+      list, and the display text.</notes>
     </feature>
     <feature name="ensemble" support="full">
       <notes>InstrumentData.soloOrEnsemble=ensemble.</notes>

src/include/mx/api/BarlineData.h

@@ -7,6 +7,10 @@
 #include "mx/api/ApiCommon.h"
 #include "mx/api/PositionData.h"
 
+#include <optional>
+#include <string>
+#include <vector>
+
 namespace mx
 {
 namespace api
@@ -29,14 +33,45 @@ enum class BarlineType
     short_
 };
 
+// Where a barline sits in an ending (volta) bracket. start opens the bracket on the left barline
+// of the ending's first measure; stop closes it with a downward jog, as a first ending usually
+// does; discontinue closes it without a jog, as a final ending usually does.
 enum class EndingType
 {
-    none,
     start,
     stop,
     discontinue
 };
 
+// An ending (volta) bracket: the numbered bracket drawn over the measures played on a particular
+// pass through a repeat.
+class EndingData
+{
+  public:
+    EndingType type;
+
+    // The passes on which this ending is played, in the order they are written: an ending marked
+    // "1, 2, 3" is played on passes 1, 2, and 3. Numbering starts at 1; a value below 1 is written
+    // as 1. Leave this empty for a blank ending -- the way MusicXML says that a bracket is here
+    // but which passes it covers is unknown.
+    std::vector<int> numbers;
+
+    // The label drawn under the bracket. Leave it empty to have the numbers themselves displayed;
+    // set it only when the label differs from them, as when passes 1, 2, and 3 are labeled "1-3".
+    std::string text;
+
+    EndingData() : type{EndingType::start}, numbers{}, text{}
+    {
+    }
+};
+
+MXAPI_EQUALS_BEGIN(EndingData)
+MXAPI_EQUALS_MEMBER(type)
+MXAPI_EQUALS_MEMBER(numbers)
+MXAPI_EQUALS_MEMBER(text)
+MXAPI_EQUALS_END;
+MXAPI_NOT_EQUALS_AND_VECTORS(EndingData);
+
 // Whether a repeat mark faces forward (the start of a repeated section, drawn |:) or backward
 // (the end, drawn :|).
 enum class RepeatDirection
@@ -63,11 +98,11 @@ class BarlineData
   public:
     int tickTimePosition;
     BarlineType barlineType;
-    EndingType endingType;
-    int endingNumber;
+    // The ending (volta) bracket this barline starts or ends, if any.
+    std::optional<EndingData> ending;
     bool repeat;
     // Number of times a backward repeat is played (the repeat's `times` attribute). 0 = not
-    // specified, mirroring endingNumber above.
+    // specified.
     int repeatTimes;
     // Whether the repeat faces forward (start of a repeated section) or backward (end). Leave
     // unspecified to let mx infer it from the barline's position -- a repeat on a left or
@@ -82,19 +117,17 @@ class BarlineData
     HorizontalAlignment location;
 
     BarlineData()
-        : tickTimePosition{0}, barlineType{BarlineType::normal}, endingType{EndingType::none}, endingNumber{0},
-          repeat{false}, repeatTimes{0}, repeatDirection{RepeatDirection::unspecified},
-          repeatAfterJump{Bool::unspecified}, repeatWinged{RepeatWinged::unspecified},
-          location{HorizontalAlignment::unspecified}
+        : tickTimePosition{0}, barlineType{BarlineType::normal}, ending{}, repeat{false}, repeatTimes{0},
+          repeatDirection{RepeatDirection::unspecified}, repeatAfterJump{Bool::unspecified},
+          repeatWinged{RepeatWinged::unspecified}, location{HorizontalAlignment::unspecified}
     {
     }
 };
 
 MXAPI_EQUALS_BEGIN(BarlineData)
 MXAPI_EQUALS_MEMBER(tickTimePosition)
 MXAPI_EQUALS_MEMBER(barlineType)
-MXAPI_EQUALS_MEMBER(endingType)
-MXAPI_EQUALS_MEMBER(endingNumber)
+MXAPI_EQUALS_MEMBER(ending)
 MXAPI_EQUALS_MEMBER(repeat)
 MXAPI_EQUALS_MEMBER(repeatTimes)
 MXAPI_EQUALS_MEMBER(repeatDirection)

src/private/mx/impl/Converter.cpp

@@ -1893,6 +1893,11 @@ core::StartStopDiscontinue Converter::convert(api::EndingType value) const
     return findCoreItem(endingMap, core::StartStopDiscontinue::start(), value);
 }
 
+api::EndingType Converter::convert(core::StartStopDiscontinue value) const
+{
+    return findApiItem(endingMap, api::EndingType::start, value);
+}
+
 core::BackwardForward Converter::convert(api::RepeatDirection value) const
 {
     return findCoreItem(repeatDirectionMap, core::BackwardForward::backward(), value);

src/private/mx/impl/Converter.h

@@ -169,6 +169,7 @@ class Converter
     api::BarlineType convert(core::BarStyle value) const;
 
     core::StartStopDiscontinue convert(api::EndingType value) const;
+    api::EndingType convert(core::StartStopDiscontinue value) const;
 
     core::BackwardForward convert(api::RepeatDirection value) const;
     api::RepeatDirection convert(core::BackwardForward value) const;

src/private/mx/impl/MeasureReader.cpp

@@ -914,8 +914,6 @@ void MeasureReader::parseBarline(const core::Barline &inMxBarline) const
     auto barline = api::BarlineData{};
     auto loc = api::HorizontalAlignment::unspecified;
     auto style = api::BarlineType::unspecified;
-    auto endingType = api::EndingType::none;
-    auto endingNumber = 0;
     auto repeat = false;
     auto repeatTimes = 0;
     auto repeatDirection = api::RepeatDirection::unspecified;
@@ -944,28 +942,15 @@ void MeasureReader::parseBarline(const core::Barline &inMxBarline) const
 
     if (inMxBarline.ending().has_value())
     {
-        const auto &ending = *inMxBarline.ending();
+        const auto &mxEnding = *inMxBarline.ending();
+        auto endingData = api::EndingData{};
+        endingData.type = myConverter.convert(mxEnding.type());
 
-        switch (ending.type().tag())
-        {
-        case core::StartStopDiscontinue::Tag::start:
-            endingType = api::EndingType::start;
-            break;
-
-        case core::StartStopDiscontinue::Tag::stop:
-            endingType = api::EndingType::stop;
-            break;
+        const auto numValues = mxEnding.number().values();
+        endingData.numbers.assign(numValues.begin(), numValues.end());
+        endingData.text = mxEnding.value();
 
-        case core::StartStopDiscontinue::Tag::discontinue:
-            endingType = api::EndingType::discontinue;
-            break;
-        };
-
-        const auto &numValues = ending.number().values();
-        if (!numValues.empty())
-        {
-            endingNumber = numValues.front();
-        }
+        barline.ending = std::move(endingData);
     }
 
     if (inMxBarline.repeat().has_value())
@@ -988,8 +973,6 @@ void MeasureReader::parseBarline(const core::Barline &inMxBarline) const
 
     barline.barlineType = style;
     barline.location = loc;
-    barline.endingType = endingType;
-    barline.endingNumber = endingNumber;
     barline.repeat = repeat;
     barline.repeatTimes = repeatTimes;
     barline.repeatDirection = repeatDirection;

src/private/mx/impl/MeasureWriter.cpp

@@ -847,16 +847,21 @@ void MeasureWriter::writeBarlines(int tickTimePosition)
             barlineElement.setBarStyle(bsc);
         }
 
-        if (myBarlinesIter->endingType != api::EndingType::none)
+        if (myBarlinesIter->ending.has_value())
         {
+            const auto &endingData = *myBarlinesIter->ending;
             core::Ending ending{};
-            ending.setType(myConverter.convert(myBarlinesIter->endingType));
+            ending.setType(myConverter.convert(endingData.type));
 
-            if (myBarlinesIter->endingNumber > 0)
+            // number is a required attribute; an empty list serializes as number="", which is
+            // MusicXML's blank ending.
+            ending.setNumber(core::EndingNumber{endingData.numbers});
+
+            // The text is written only when the author supplied one. Left empty, the ending
+            // displays its numbers.
+            if (!endingData.text.empty())
             {
-                core::EndingNumber en{};
-                en.addValue(myBarlinesIter->endingNumber);
-                ending.setNumber(en);
+                ending.setValue(endingData.text);
             }
 
             barlineElement.setEnding(ending);

src/private/mxtest/api/RepeatApiTest.cpp

@@ -144,4 +144,178 @@ TEST(repeatOmittedAttributesAreUnspecified, Repeat)
 
 T_END;
 
+// Wrap an <ending> element in the smallest score that can carry one.
+static std::string endingXmlForRepeat(const std::string &inEndingElement)
+{
+    return R"(<?xml version="1.0" encoding="UTF-8" standalone="no"?>
+<score-partwise version="4.0">
+  <part-list>
+    <score-part id="id1">
+      <part-name>x</part-name>
+    </score-part>
+  </part-list>
+  <part id="id1">
+    <measure number="1">
+      <barline location="left">
+        )" +
+           inEndingElement +
+           R"(
+      </barline>
+    </measure>
+  </part>
+</score-partwise>)";
+}
+
+// An ending played on several passes keeps every pass number, and its display label survives
+// alongside them. This is the "1, 2, 3" ending labeled "1-3" case.
+TEST(endingMultipleNumbersAndTextRoundTrip, Repeat)
+{
+    auto score = makeScoreWithBarlineForRepeat();
+    auto &barline = score.parts.back().measures.back().barlines.back();
+    barline.location = HorizontalAlignment::left;
+    EndingData ending;
+    ending.type = EndingType::start;
+    ending.numbers = {1, 2, 3};
+    ending.text = "1-3";
+    barline.ending = ending;
+
+    const auto out = roundTrip(score);
+
+    const auto &obarlines = out.parts.back().measures.back().barlines;
+    REQUIRE(obarlines.size() == 1);
+    const auto &ob = obarlines.front();
+    REQUIRE(ob.ending.has_value());
+    CHECK(EndingType::start == ob.ending->type);
+    REQUIRE(ob.ending->numbers.size() == 3);
+    CHECK_EQUAL(1, ob.ending->numbers.at(0));
+    CHECK_EQUAL(2, ob.ending->numbers.at(1));
+    CHECK_EQUAL(3, ob.ending->numbers.at(2));
+    CHECK_EQUAL(std::string{"1-3"}, ob.ending->text);
+}
+
+T_END;
+
+// The serialized MusicXML must spell the number list the way the spec does and carry the label as
+// the element's text.
+TEST(endingSerializesNumberListAndText, Repeat)
+{
+    auto score = makeScoreWithBarlineForRepeat();
+    auto &barline = score.parts.back().measures.back().barlines.back();
+    barline.location = HorizontalAlignment::left;
+    EndingData ending;
+    ending.type = EndingType::start;
+    ending.numbers = {1, 2, 3};
+    ending.text = "1-3";
+    barline.ending = ending;
+
+    const auto xml = toXml(score);
+
+    CHECK(xml.find(R"(number="1, 2, 3")") != std::string::npos);
+    CHECK(xml.find(R"(type="start")") != std::string::npos);
+    CHECK(xml.find(">1-3</ending>") != std::string::npos);
+}
+
+T_END;
+
+// The reader must surface the whole number list, not just the first pass, along with the label.
+TEST(endingMultipleNumbersAndTextReadFromXml, Repeat)
+{
+    const auto score = fromXml(endingXmlForRepeat(R"(<ending number="1, 2, 3" type="start">1-3</ending>)"));
+
+    REQUIRE(score.parts.size() == 1);
+    const auto &barlines = score.parts.back().measures.back().barlines;
+    REQUIRE(barlines.size() == 1);
+    const auto &ob = barlines.front();
+    REQUIRE(ob.ending.has_value());
+    CHECK(EndingType::start == ob.ending->type);
+    REQUIRE(ob.ending->numbers.size() == 3);
+    CHECK_EQUAL(1, ob.ending->numbers.at(0));
+    CHECK_EQUAL(2, ob.ending->numbers.at(1));
+    CHECK_EQUAL(3, ob.ending->numbers.at(2));
+    CHECK_EQUAL(std::string{"1-3"}, ob.ending->text);
+}
+
+T_END;
+
+// An ending with no label reads back with empty text, and writing one emits no text node -- the
+// numbers are what gets displayed.
+TEST(endingWithoutTextOmitsTextNode, Repeat)
+{
+    const auto score = fromXml(endingXmlForRepeat(R"(<ending number="1" type="stop"/>)"));
+    const auto &ob = score.parts.back().measures.back().barlines.front();
+    REQUIRE(ob.ending.has_value());
+    CHECK(EndingType::stop == ob.ending->type);
+    REQUIRE(ob.ending->numbers.size() == 1);
+    CHECK_EQUAL(1, ob.ending->numbers.at(0));
+    CHECK(ob.ending->text.empty());
+
+    const auto xml = toXml(score);
+    CHECK(xml.find("</ending>") == std::string::npos);
+    CHECK(xml.find(R"(number="1")") != std::string::npos);
+}
+
+T_END;
+
+// A blank ending -- a bracket whose passes are unknown -- is an ending with no numbers, and writes
+// as number="".
+TEST(endingBlankNumberRoundTrip, Repeat)
+{
+    auto score = makeScoreWithBarlineForRepeat();
+    auto &barline = score.parts.back().measures.back().barlines.back();
+    barline.location = HorizontalAlignment::left;
+    EndingData ending;
+    ending.type = EndingType::start;
+    barline.ending = ending;
+
+    const auto xml = toXml(score);
+    CHECK(xml.find(R"(number="")") != std::string::npos);
+
+    const auto out = roundTrip(score);
+    const auto &ob = out.parts.back().measures.back().barlines.front();
+    REQUIRE(ob.ending.has_value());
+    CHECK(EndingType::start == ob.ending->type);
+    CHECK(ob.ending->numbers.empty());
+    CHECK(ob.ending->text.empty());
+}
+
+T_END;
+
+// A barline that carries no ending must report none, and must not emit an <ending> element.
+TEST(barlineWithoutEndingIsAbsent, Repeat)
+{
+    auto score = makeScoreWithBarlineForRepeat();
+    auto &barline = score.parts.back().measures.back().barlines.back();
+    barline.barlineType = BarlineType::lightHeavy;
+    barline.location = HorizontalAlignment::right;
+
+    const auto xml = toXml(score);
+    CHECK(xml.find("<ending") == std::string::npos);
+
+    const auto out = roundTrip(score);
+    const auto &ob = out.parts.back().measures.back().barlines.front();
+    CHECK(!ob.ending.has_value());
+}
+
+T_END;
+
+// The core -> api read path against the shape of data/synthetic/ending.3.0.xml, whose ending
+// carries a text body ("x") that differs from its number attribute ("1"). The attributes mx::api
+// does not model are ignored; the number and the label must both arrive.
+TEST(endingTextDiffersFromNumberReadFromXml, Repeat)
+{
+    const auto score = fromXml(
+        endingXmlForRepeat(R"(<ending default-x="1" default-y="1" relative-x="1" relative-y="1" font-family="x" )"
+                           R"(font-style="normal" font-size="1" font-weight="normal" color="#FF000000" number="1" )"
+                           R"(type="start" print-object="yes" end-length="1" text-x="1" text-y="1">x</ending>)"));
+
+    const auto &ob = score.parts.back().measures.back().barlines.front();
+    REQUIRE(ob.ending.has_value());
+    CHECK(EndingType::start == ob.ending->type);
+    REQUIRE(ob.ending->numbers.size() == 1);
+    CHECK_EQUAL(1, ob.ending->numbers.at(0));
+    CHECK_EQUAL(std::string{"x"}, ob.ending->text);
+}
+
+T_END;
+
 #endif