Separate multiple values in basic tag with " / " instead of " "

taglib/ape/apetag.cpp

@@ -103,31 +103,31 @@ ByteVector APE::Tag::fileIdentifier()
 String APE::Tag::title() const
 {
   Item value = d->itemListMap.value("TITLE");
-  return value.isEmpty() ? String() : value.values().toString();
+  return value.isEmpty() ? String() : joinTagValues(value.values());
 }
 
 String APE::Tag::artist() const
 {
   Item value = d->itemListMap.value("ARTIST");
-  return value.isEmpty() ? String() : value.values().toString();
+  return value.isEmpty() ? String() : joinTagValues(value.values());
 }
 
 String APE::Tag::album() const
 {
   Item value = d->itemListMap.value("ALBUM");
-  return value.isEmpty() ? String() : value.values().toString();
+  return value.isEmpty() ? String() : joinTagValues(value.values());
 }
 
 String APE::Tag::comment() const
 {
   Item value = d->itemListMap.value("COMMENT");
-  return value.isEmpty() ? String() : value.values().toString();
+  return value.isEmpty() ? String() : joinTagValues(value.values());
 }
 
 String APE::Tag::genre() const
 {
   Item value = d->itemListMap.value("GENRE");
-  return value.isEmpty() ? String() : value.values().toString();
+  return value.isEmpty() ? String() : joinTagValues(value.values());
 }
 
 unsigned int APE::Tag::year() const

taglib/asf/asftag.cpp

@@ -34,6 +34,18 @@
 
 using namespace TagLib;
 
+namespace
+{
+  StringList attributeListToStringList(const ASF::AttributeList &attributes)
+  {
+    StringList strs;
+    for(const auto &attribute : attributes) {
+      strs.append(attribute.toString());
+    }
+    return strs;
+  }
+}  // namespace
+
 class ASF::Tag::TagPrivate
 {
 public:
@@ -65,7 +77,8 @@ String ASF::Tag::artist() const
 String ASF::Tag::album() const
 {
   if(d->attributeListMap.contains("WM/AlbumTitle"))
-    return d->attributeListMap["WM/AlbumTitle"][0].toString();
+    return joinTagValues(
+      attributeListToStringList(d->attributeListMap.value("WM/AlbumTitle")));
   return String();
 }
 
@@ -107,7 +120,8 @@ unsigned int ASF::Tag::track() const
 String ASF::Tag::genre() const
 {
   if(d->attributeListMap.contains("WM/Genre"))
-    return d->attributeListMap["WM/Genre"][0].toString();
+    return joinTagValues(
+      attributeListToStringList(d->attributeListMap.value("WM/Genre")));
   return String();
 }
 

taglib/mpeg/id3v2/frames/attachedpictureframe.cpp

@@ -66,6 +66,11 @@ String AttachedPictureFrame::toString() const
   return d->description.isEmpty() ? s : d->description + " " + s;
 }
 
+StringList AttachedPictureFrame::toStringList() const
+{
+  return {d->description, d->mimeType};
+}
+
 String::Type AttachedPictureFrame::textEncoding() const
 {
   return d->textEncoding;

taglib/mpeg/id3v2/frames/attachedpictureframe.h

@@ -79,6 +79,11 @@ namespace TagLib {
        */
       String toString() const override;
 
+      /*!
+       * Returns a string list containing the description and mime-type.
+       */
+      StringList toStringList() const override;
+
       /*!
        * Returns the text encoding used for the description.
        *

taglib/mpeg/id3v2/frames/generalencapsulatedobjectframe.cpp

@@ -76,6 +76,11 @@ String GeneralEncapsulatedObjectFrame::toString() const
   return text;
 }
 
+StringList GeneralEncapsulatedObjectFrame::toStringList() const
+{
+  return {d->description, d->fileName, d->mimeType};
+}
+
 String::Type GeneralEncapsulatedObjectFrame::textEncoding() const
 {
   return d->textEncoding;

taglib/mpeg/id3v2/frames/generalencapsulatedobjectframe.h

@@ -82,6 +82,11 @@ namespace TagLib {
        */
       String toString() const override;
 
+      /*!
+       * Returns a string list containing the description, file name and mime-type.
+       */
+      StringList toStringList() const override;
+
       /*!
        * Returns the text encoding used for the description and file name.
        *

taglib/mpeg/id3v2/frames/ownershipframe.cpp

@@ -65,6 +65,11 @@ String OwnershipFrame::toString() const
   return "pricePaid=" + d->pricePaid + " datePurchased=" + d->datePurchased + " seller=" + d->seller;
 }
 
+StringList OwnershipFrame::toStringList() const
+{
+  return {d->pricePaid, d->datePurchased, d->seller};
+}
+
 String OwnershipFrame::pricePaid() const
 {
   return d->pricePaid;

taglib/mpeg/id3v2/frames/ownershipframe.h

@@ -64,12 +64,17 @@ namespace TagLib {
       OwnershipFrame &operator=(const OwnershipFrame &) = delete;
 
       /*!
-       * Returns the text of this popularimeter.
+       * Returns price paid, date purchased and seller.
        *
        * \see text()
        */
       String toString() const override;
 
+      /*!
+       * Returns price paid, date purchased and seller.
+       */
+      StringList toStringList() const override;
+
       /*!
        * Returns the date purchased.
        *

taglib/mpeg/id3v2/frames/popularimeterframe.cpp

@@ -24,6 +24,7 @@
  ***************************************************************************/
 
 #include "popularimeterframe.h"
+#include "tstringlist.h"
 
 using namespace TagLib;
 using namespace ID3v2;
@@ -60,6 +61,11 @@ String PopularimeterFrame::toString() const
   return d->email + " rating=" + String::number(d->rating) + " counter=" + String::number(d->counter);
 }
 
+StringList PopularimeterFrame::toStringList() const
+{
+  return {d->email, String::number(d->rating), String::number(d->counter)};
+}
+
 String PopularimeterFrame::email() const
 {
   return d->email;

taglib/mpeg/id3v2/frames/popularimeterframe.h

@@ -71,6 +71,11 @@ namespace TagLib {
        */
       String toString() const override;
 
+      /*!
+       * Returns email, rating and counter.
+       */
+      StringList toStringList() const override;
+
       /*!
        * Returns the email.
        *

taglib/mpeg/id3v2/frames/textidentificationframe.cpp

@@ -110,6 +110,11 @@ String TextIdentificationFrame::toString() const
   return d->fieldList.toString();
 }
 
+StringList TextIdentificationFrame::toStringList() const
+{
+  return d->fieldList;
+}
+
 StringList TextIdentificationFrame::fieldList() const
 {
   return d->fieldList;

taglib/mpeg/id3v2/frames/textidentificationframe.h

@@ -162,6 +162,7 @@ namespace TagLib {
 
       void setText(const String &s) override;
       String toString() const override;
+      StringList toStringList() const override;
 
       /*!
        * Returns the text encoding that will be used in rendering this frame.

taglib/mpeg/id3v2/id3v2frame.cpp

@@ -122,6 +122,11 @@ void Frame::setText(const String &)
 {
 }
 
+StringList Frame::toStringList() const
+{
+  return toString();
+}
+
 ByteVector Frame::render() const
 {
   ByteVector fieldData = renderFields();

taglib/mpeg/id3v2/id3v2frame.h

@@ -108,6 +108,14 @@ namespace TagLib {
        */
       virtual String toString() const = 0;
 
+      /*!
+       * This returns the textual representation of the data in the frame.
+       * Subclasses can reimplement this method to provide a string list
+       * representation of the frame's data.  The default implementation
+       * returns the single string representation from toString().
+       */
+      virtual StringList toStringList() const;
+
       /*!
        * Render the frame back to its binary format in a ByteVector.
        */

taglib/mpeg/id3v2/id3v2tag.cpp

@@ -121,21 +121,21 @@ ID3v2::Tag::~Tag() = default;
 String ID3v2::Tag::title() const
 {
   if(!d->frameListMap["TIT2"].isEmpty())
-    return d->frameListMap["TIT2"].front()->toString();
+    return joinTagValues(d->frameListMap["TIT2"].front()->toStringList());
   return String();
 }
 
 String ID3v2::Tag::artist() const
 {
   if(!d->frameListMap["TPE1"].isEmpty())
-    return d->frameListMap["TPE1"].front()->toString();
+    return joinTagValues(d->frameListMap["TPE1"].front()->toStringList());
   return String();
 }
 
 String ID3v2::Tag::album() const
 {
   if(!d->frameListMap["TALB"].isEmpty())
-    return d->frameListMap["TALB"].front()->toString();
+    return joinTagValues(d->frameListMap["TALB"].front()->toStringList());
   return String();
 }
 
@@ -157,10 +157,6 @@ String ID3v2::Tag::comment() const
 
 String ID3v2::Tag::genre() const
 {
-  // TODO: In the next major version (TagLib 2.0) a list of multiple genres
-  // should be separated by " / " instead of " ".  For the moment to keep
-  // the behavior the same as released versions it is being left with " ".
-
   const FrameList &tconFrames = d->frameListMap["TCON"];
   if(tconFrames.isEmpty())
   {
@@ -196,7 +192,7 @@ String ID3v2::Tag::genre() const
       genres.append(field);
   }
 
-  return genres.toString();
+  return joinTagValues(genres);
 }
 
 unsigned int ID3v2::Tag::year() const

taglib/ogg/xiphcomment.cpp

@@ -66,19 +66,19 @@ Ogg::XiphComment::~XiphComment() = default;
 String Ogg::XiphComment::title() const
 {
   StringList value = d->fieldListMap.value("TITLE");
-  return value.isEmpty() ? String() : value.toString();
+  return value.isEmpty() ? String() : joinTagValues(value);
 }
 
 String Ogg::XiphComment::artist() const
 {
   StringList value = d->fieldListMap.value("ARTIST");
-  return value.isEmpty() ? String() : value.toString();
+  return value.isEmpty() ? String() : joinTagValues(value);
 }
 
 String Ogg::XiphComment::album() const
 {
   StringList value = d->fieldListMap.value("ALBUM");
-  return value.isEmpty() ? String() : value.toString();
+  return value.isEmpty() ? String() : joinTagValues(value);
 }
 
 String Ogg::XiphComment::comment() const
@@ -86,13 +86,13 @@ String Ogg::XiphComment::comment() const
   StringList value = d->fieldListMap.value("DESCRIPTION");
   if(!value.isEmpty()) {
     d->commentField = "DESCRIPTION";
-    return value.toString();
+    return joinTagValues(value);
   }
 
   value = d->fieldListMap.value("COMMENT");
   if(!value.isEmpty()) {
     d->commentField = "COMMENT";
-    return value.toString();
+    return joinTagValues(value);
   }
 
   return String();
@@ -101,7 +101,7 @@ String Ogg::XiphComment::comment() const
 String Ogg::XiphComment::genre() const
 {
   StringList value = d->fieldListMap.value("GENRE");
-  return value.isEmpty() ? String() : value.toString();
+  return value.isEmpty() ? String() : joinTagValues(value);
 }
 
 unsigned int Ogg::XiphComment::year() const

taglib/tag.cpp

@@ -189,3 +189,8 @@ void Tag::duplicate(const Tag *source, Tag *target, bool overwrite) // static
       target->setTrack(source->track());
   }
 }
+
+String Tag::joinTagValues(const StringList &l)
+{
+  return l.toString(" / ");
+}

taglib/tag.h

@@ -227,6 +227,14 @@ namespace TagLib {
      */
     static void duplicate(const Tag *source, Tag *target, bool overwrite = true);
 
+    /*!
+     * Join the \a values of a tag to a single string separated by " / ".
+     * If the tag implementation can have multiple values for a basic tag
+     * (e.g. artist), they can be combined to a single string for the basic
+     * tag getters (e.g. artist()).
+     */
+    static String joinTagValues(const StringList &values);
+
   protected:
     /*!
      * Construct a Tag.  This is protected since tags should only be instantiated

tests/test_apetag.cpp

@@ -80,7 +80,7 @@ public:
     tag.setProperties(dict);
     CPPUNIT_ASSERT_EQUAL(String("17"), tag.itemListMap()["TRACK"].values()[0]);
     CPPUNIT_ASSERT_EQUAL(2u, tag.itemListMap()["ARTIST"].values().size());
-    CPPUNIT_ASSERT_EQUAL(String("artist 1 artist 2"), tag.artist());
+    CPPUNIT_ASSERT_EQUAL(String("artist 1 / artist 2"), tag.artist());
     CPPUNIT_ASSERT_EQUAL(17u, tag.track());
     const APE::Item &textItem = tag.itemListMap()["TRACK"];
     CPPUNIT_ASSERT_EQUAL(APE::Item::Text, textItem.type());

tests/test_id3v2.cpp

@@ -957,7 +957,7 @@ public:
 
     ID3v2::Tag tag;
     tag.addFrame(frame);
-    CPPUNIT_ASSERT_EQUAL(String("Disco Eurodisco"), tag.genre());
+    CPPUNIT_ASSERT_EQUAL(String("Disco / Eurodisco"), tag.genre());
   }
 
   void testUpdateGenre23_3()
@@ -980,7 +980,7 @@ public:
 
     ID3v2::Tag tag;
     tag.addFrame(frame);
-    CPPUNIT_ASSERT_EQUAL(String("Metal Black Metal Viking Metal"), tag.genre());
+    CPPUNIT_ASSERT_EQUAL(String("Metal / Black Metal / Viking Metal"), tag.genre());
   }
 
   void testUpdateGenre24()
@@ -1000,7 +1000,7 @@ public:
 
     ID3v2::Tag tag;
     tag.addFrame(frame);
-    CPPUNIT_ASSERT_EQUAL(String("R&B Eurodisco"), tag.genre());
+    CPPUNIT_ASSERT_EQUAL(String("R&B / Eurodisco"), tag.genre());
   }
 
   void testUpdateDate22()

tests/test_tag_c.cpp

@@ -181,7 +181,7 @@ public:
 
       TagLib_Tag *tag = taglib_file_tag(file);
       CPPUNIT_ASSERT_EQUAL("Quod Libet Test Data"s, std::string(taglib_tag_album(tag)));
-      CPPUNIT_ASSERT_EQUAL("piman jzig"s, std::string(taglib_tag_artist(tag)));
+      CPPUNIT_ASSERT_EQUAL("piman / jzig"s, std::string(taglib_tag_artist(tag)));
       CPPUNIT_ASSERT_EQUAL("Silence"s, std::string(taglib_tag_genre(tag)));
       CPPUNIT_ASSERT_EQUAL(""s, std::string(taglib_tag_comment(tag)));
       CPPUNIT_ASSERT_EQUAL("Silence"s, std::string(taglib_tag_title(tag)));

tests/test_xiphcomment.cpp

@@ -144,15 +144,15 @@ public:
     f.tag()->addField("TITLE", "Title3", false);
     f.tag()->addField("artist", "Artist1");
     f.tag()->addField("ARTIST", "Artist2", false);
-    CPPUNIT_ASSERT_EQUAL(String("Title1 Title1 Title2 Title3"), f.tag()->title());
-    CPPUNIT_ASSERT_EQUAL(String("Artist1 Artist2"), f.tag()->artist());
+    CPPUNIT_ASSERT_EQUAL(String("Title1 / Title1 / Title2 / Title3"), f.tag()->title());
+    CPPUNIT_ASSERT_EQUAL(String("Artist1 / Artist2"), f.tag()->artist());
 
     f.tag()->removeFields("title", "Title1");
-    CPPUNIT_ASSERT_EQUAL(String("Title2 Title3"), f.tag()->title());
-    CPPUNIT_ASSERT_EQUAL(String("Artist1 Artist2"), f.tag()->artist());
+    CPPUNIT_ASSERT_EQUAL(String("Title2 / Title3"), f.tag()->title());
+    CPPUNIT_ASSERT_EQUAL(String("Artist1 / Artist2"), f.tag()->artist());
 
     f.tag()->removeFields("Artist");
-    CPPUNIT_ASSERT_EQUAL(String("Title2 Title3"), f.tag()->title());
+    CPPUNIT_ASSERT_EQUAL(String("Title2 / Title3"), f.tag()->title());
     CPPUNIT_ASSERT(f.tag()->artist().isEmpty());
 
     f.tag()->removeAllFields();