[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    kdemultimedia/juk
From:       Michael Pyne <michael.pyne () kdemail ! net>
Date:       2005-05-01 5:59:21
Message-ID: 20050501055921.A1C72635 () office ! kde ! org
[Download RAW message or body]

CVS commit by mpyne: 

Fix bug 104889 (Cover art doesn't show for tracks with no album), by not even
bothering to set the cover art in the first place.  It doesn't work too well for the
way we do it currently to set cover art for tracks that don't have both their Artist
and Album tags filled out.

What I've done instead is notified the user if they try to assign cover art to tracks that
don't support it.  If the user selects a mix of tracks that can and cannot be assigned cover
art, everything proceeds as normal, and then JuK will inform the user that some tracks were
skipped.

This is kind of suboptimal, but fixing it will have to wait until we re-think how we store
the cover information.

BUG:104889
GUI:


  M +50 -4     playlist.cpp   1.302
  M +7 -0      playlist.h   1.158


--- kdemultimedia/juk/playlist.h  #1.157:1.158
@@ -538,4 +538,11 @@ private:
     void refreshAlbum(const QString &artist, const QString &album);
 
+    /**
+     * Returns the number of PlaylistItems in @p items that can be assigned a
+     * cover.  Used to avoid wasting the users' time setting the cover for 20
+     * items when none are eligible.
+     */
+    int eligibleCoverItems(const PlaylistItemList &items);
+
     void updatePlaying() const;
 

--- kdemultimedia/juk/playlist.cpp  #1.301:1.302
@@ -801,4 +801,20 @@ void Playlist::slotRemoveCover()
 }
 
+int Playlist::eligibleCoverItems(const PlaylistItemList &items)
+{
+    PlaylistItemList::ConstIterator it = items.begin();
+    int numEligible = 0;
+
+    for(; it != items.end(); ++it) {
+        if(!(*it)->file().tag()->artist().isEmpty() &&
+           !(*it)->file().tag()->album().isEmpty())
+        {
+            ++numEligible;
+        }
+    }
+
+    return numEligible;
+}
+
 void Playlist::slotAddCover(bool retrieveLocal)
 {
@@ -808,4 +824,15 @@ void Playlist::slotAddCover(bool retriev
         return;
 
+    if(eligibleCoverItems(items) == 0) {
+        // No items in the list can be assigned a cover, inform the user and
+        // bail.
+
+        KMessageBox::sorry(this, i18n("None of the items you have selected can "
+                    "be assigned a cover.  A track must have both the Artist "
+                    "and Album tags set to be assigned a cover."));
+
+        return;
+    }
+
     QImage image;
 
@@ -1234,11 +1261,24 @@ void Playlist::refreshAlbums(const Playl
 {
     QValueList< QPair<QString, QString> > albums;
+    QStringList ignoredItems;
 
     for(PlaylistItemList::ConstIterator it = items.begin(); it != items.end(); ++it) {
-        if(albums.find(qMakePair((*it)->file().tag()->artist(),
-                                 (*it)->file().tag()->album())) == albums.end())
+        QString artist = (*it)->file().tag()->artist();
+        QString album = (*it)->file().tag()->album();
+
+        // Right now JuK requires that the artist and album tags be assigned
+        // for the cover art, double check here.
+
+        if(!image.isNull() && (artist.isEmpty() || album.isEmpty())) {
+            QString title = (*it)->file().tag()->title();
+            QString mid = title.isEmpty() || artist.isEmpty() ? "" : " - ";
+
+            ignoredItems.append(artist + mid + title);
+            continue;
+        }
+
+        if(albums.find(qMakePair(artist, album)) == albums.end())
         {
-            albums.append(qMakePair((*it)->file().tag()->artist(),
-                                    (*it)->file().tag()->album()));                                 
+            albums.append(qMakePair(artist, album));
             if(image.isNull())
                 (*it)->file().coverInfo()->clearCover();
@@ -1255,4 +1295,10 @@ void Playlist::refreshAlbums(const Playl
         refreshAlbum((*it).first, (*it).second);
     }
+
+    if(!ignoredItems.isEmpty()) {
+        KMessageBox::informationList(this, i18n("The following songs were "
+                    "ignored because they don't have both their Artist and "
+                    "Album tags filled in."), ignoredItems);
+    }
 }
 


[prev in list] [next in list] [prev in thread] [next in thread]