Add alternative methods to XiphComment::removeField().

Using XiphComment::removeField() may lead to a linkage error, however we can't fix the method itself without breaking the ABI or changing its behavior.
So we added some alternative method and marked the old one deprecated.

diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp
index e5b2fc8e370dcbade77ef06b4c1295a25fa867f6..a36862dfc2ed64dcaaffd5ec89bf5a0181931fbe 100644 (file)
--- a/taglib/ogg/xiphcomment.cpp
+++ b/taglib/ogg/xiphcomment.cpp
@@ -264,22 +264,36 @@ void Ogg::XiphComment::addField(const String &key, const String &value, bool rep
 
 void Ogg::XiphComment::removeField(const String &key, const String &value)
 {
-  if(!value.isNull()) {
-    StringList::Iterator it = d->fieldListMap[key].begin();
-    while(it != d->fieldListMap[key].end()) {
-      if(value == *it)
-        it = d->fieldListMap[key].erase(it);
-      else
-        it++;
-    }
-  }
+  if(!value.isNull())
+    removeFields(key, value);
   else
-    d->fieldListMap.erase(key);
+    removeFields(key);
+}
+
+void Ogg::XiphComment::removeFields(const String &key)
+{
+  d->fieldListMap.erase(key.upper());
+}
+
+void Ogg::XiphComment::removeFields(const String &key, const String &value)
+{
+  StringList &fields = d->fieldListMap[key.upper()];
+  for(StringList::Iterator it = fields.begin(); it != fields.end(); ) {
+    if(*it == value)
+      it = fields.erase(it);
+    else
+      ++it;
+  }
+}
+
+void Ogg::XiphComment::removeAllFields()
+{
+  d->fieldListMap.clear();
 }
 
 bool Ogg::XiphComment::contains(const String &key) const
 {
-  return d->fieldListMap.contains(key) && !d->fieldListMap[key].isEmpty();
+  return !d->fieldListMap[key.upper()].isEmpty();
 }
 
 ByteVector Ogg::XiphComment::render() const

diff --git a/taglib/ogg/xiphcomment.h b/taglib/ogg/xiphcomment.h
index 54f3070bf39de621bf8fcdeafb91e867829cbd5e..8914ec07052b068b3734911fd1254c37c759ac0d 100644 (file)
--- a/taglib/ogg/xiphcomment.h
+++ b/taglib/ogg/xiphcomment.h
@@ -181,9 +181,33 @@ namespace TagLib {
       /*!
        * Remove the field specified by \a key with the data \a value.  If
        * \a value is null, all of the fields with the given key will be removed.
+       *
+       * \deprecated Using this method may lead to a linkage error.
        */
+      // BIC: remove and merge with below
       void removeField(const String &key, const String &value = String::null);
 
+      /*!
+       * Remove all the fields specified by \a key.
+       *
+       * \see removeAllFields()
+       */
+      void removeFields(const String &key);
+
+      /*!
+       * Remove all the fields specified by \a key with the data \a value.
+       *
+       * \see removeAllFields()
+       */
+      void removeFields(const String &key, const String &value);
+
+      /*!
+       * Remove all the fields in the comment.
+       *
+       * \see removeFields()
+       */
+      void removeAllFields();
+
       /*!
        * Returns true if the field is contained within the comment.
        *

diff --git a/tests/test_xiphcomment.cpp b/tests/test_xiphcomment.cpp
index e235833387f279b81c1fc5dce8ef24e27527a826..617bd7653f12653bd242e0fc30f4010d4b97d3a6 100644 (file)
--- a/tests/test_xiphcomment.cpp
+++ b/tests/test_xiphcomment.cpp
@@ -1,7 +1,7 @@
 #include <string>
 #include <stdio.h>
-#include <flacfile.h>
 #include <xiphcomment.h>
+#include <vorbisfile.h>
 #include <tpropertymap.h>
 #include <tdebug.h>
 #include <cppunit/extensions/HelperMacros.h>
@@ -19,6 +19,7 @@ class TestXiphComment : public CppUnit::TestFixture
   CPPUNIT_TEST(testSetTrack);
   CPPUNIT_TEST(testInvalidKeys);
   CPPUNIT_TEST(testClearComment);
+  CPPUNIT_TEST(testRemoveFields);
   CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -78,20 +79,46 @@ public:
 
   void testClearComment()
   {
-    ScopedFileCopy copy("no-tags", ".flac");
+    ScopedFileCopy copy("empty", ".ogg");
 
     {
-      FLAC::File f(copy.fileName().c_str());
-      f.xiphComment()->addField("COMMENT", "Comment1");
+      Ogg::Vorbis::File f(copy.fileName().c_str());
+      f.tag()->addField("COMMENT", "Comment1");
       f.save();
     }
     {
-      FLAC::File f(copy.fileName().c_str());
-      f.xiphComment()->setComment("");
-      CPPUNIT_ASSERT_EQUAL(String(""), f.xiphComment()->comment());
+      Ogg::Vorbis::File f(copy.fileName().c_str());
+      f.tag()->setComment("");
+      CPPUNIT_ASSERT_EQUAL(String(""), f.tag()->comment());
     }
   }
 
+  void testRemoveFields()
+  {
+    Ogg::Vorbis::File f(TEST_FILE_PATH_C("empty.ogg"));
+    f.tag()->addField("title", "Title1");
+    f.tag()->addField("Title", "Title1", false);
+    f.tag()->addField("titlE", "Title2", false);
+    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());
+
+    f.tag()->removeFields("title", "Title1");
+    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(f.tag()->artist().isEmpty());
+
+    f.tag()->removeAllFields();
+    CPPUNIT_ASSERT(f.tag()->title().isEmpty());
+    CPPUNIT_ASSERT(f.tag()->artist().isEmpty());
+    CPPUNIT_ASSERT_EQUAL(String("Xiph.Org libVorbis I 20050304"), f.tag()->vendorID());
+  }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestXiphComment);