Bug 185397 - JJ: Saved playlist default name could be smarter
Summary: JJ: Saved playlist default name could be smarter
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Playlists/Saved Playlists (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords: junior-jobs
: 256054 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-02-24 00:24 UTC by Jisakiel
Modified: 2010-12-01 08:10 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.4
Sentry Crash Report:


Attachments
Patch for implementing this wishlist item (3.33 KB, patch)
2010-11-24 23:26 UTC, Dennis
Details
A more robust version of the previous patch (2.99 KB, patch)
2010-11-25 02:04 UTC, Dennis
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jisakiel 2009-02-24 00:24:29 UTC
Version:           2.0.1.1-1 (using KDE 4.2.0)
OS:                Linux
Installed from:    Debian testing/unstable Packages

As the known Apple player does, the default name for the playlists could be way smarter, which would help usability of amarok. Instead of "new playlist", for instance: 

- If the playlist contains a single artist and album, default name could well be "Artist - Album". No need to allow configuration here (after all, it can be edited, it is just the suggestion amarok would give). 
- If the album contains several artists, default name could be "Album" or "Various - Album" or even "Album - by Various". iTunes uses the first option, IIRC. 
- If the playlist contains just one artist but different albums, default name could be "Artist" or "Artist - Various". 

This small improvement would do a lot to help organizing the library.
Comment 1 Bart Cerneels 2010-07-15 10:46:52 UTC
At the moment we use the date and time. That ensures uniqueness of the name and could help the user remember why that playlist was saved.

I'll mark this as a junior job.
Comment 2 Myriam Schweingruber 2010-11-04 22:48:33 UTC
*** Bug 256054 has been marked as a duplicate of this bug. ***
Comment 3 karaluh 2010-11-05 09:39:10 UTC
How about "save as" option?
Comment 4 Dennis 2010-11-24 23:26:22 UTC
Created attachment 53711 [details]
Patch for implementing this wishlist item

Here is a patch that attempts to implement this wishlist item. It is also posted on the reviewboard : http://git.reviewboard.kde.org/r/100168
Comment 5 Dennis 2010-11-25 02:04:08 UTC
Created attachment 53713 [details]
A more robust version of the previous patch
Comment 6 Bart Cerneels 2010-11-29 18:45:14 UTC
commit 1ceee4c6ff6fa3a184ca2ed442bea31ef4f14466
branch master
Author: Bart Cerneels <bart.cerneels@kde.org>
Date:   Mon Nov 29 18:43:44 2010 +0100

    Give saved playlists a sensible default name.
    
    Patch by Dennis Francis.
    
    BUG:185397

diff --git a/src/playlist/PlaylistDock.cpp b/src/playlist/PlaylistDock.cpp
index 6170f78..91d4b74 100644
--- a/src/playlist/PlaylistDock.cpp
+++ b/src/playlist/PlaylistDock.cpp
@@ -257,7 +257,8 @@ Playlist::Dock::slotSaveCurrentPlaylist()
     QWeakPointer<Playlists::UserPlaylistProvider> pointer = action->data().value< QWeakPointer<Playlists::UserPlaylistProvider> >();
     Playlists::UserPlaylistProvider* provider = pointer.data();
 
-    The::playlistManager()->save( The::playlist()->tracks(), QString(), provider );
+    The::playlistManager()->save( The::playlist()->tracks(),
+                                  Playlist::ModelStack::instance()->bottom()->generatePlaylistName(), provider );
 }
 
 void
diff --git a/src/playlist/PlaylistModel.cpp b/src/playlist/PlaylistModel.cpp
index 0162321..94e97ae 100644
--- a/src/playlist/PlaylistModel.cpp
+++ b/src/playlist/PlaylistModel.cpp
@@ -4,6 +4,7 @@
  * Copyright (c) 2008 Seb Ruiz <ruiz@kde.org>                                           *
  * Copyright (c) 2008 Soren Harward <stharward@gmail.com>                               *
  * Copyright (c) 2010 Nanno Langstraat <langstr@gmail.com>                              *
+ * Copyright (c) 2010 Dennis Francis <dennisfrancis.in@gmail.com>                       *
  *                                                                                      *
  * This program is free software; you can redistribute it and/or modify it under        *
  * the terms of the GNU General Public License as published by the Free Software        *
@@ -817,6 +818,74 @@ Playlist::Model::tracks() const
 }
 
 QString
+Playlist::Model::generatePlaylistName() const
+{
+    QString datePart = KGlobal::locale()->formatDateTime( QDateTime::currentDateTime(),
+                                                          KLocale::ShortDate, true );
+    Meta::TrackList trackList = tracks();
+
+    if( trackList.count() == 0 )
+    {
+        return i18nc( "A saved playlist with the current time (KLocale::Shortdate) added between \
+                      the parentheses",
+                      "Empty Playlist (%1)").arg( datePart );
+    }
+
+    bool singleArtist = true;
+    bool singleAlbum = true;
+
+    Meta::ArtistPtr artist = trackList.first()->artist();
+    Meta::AlbumPtr album = trackList.first()->album();
+
+    QString artistPart;
+    QString albumPart;
+
+    foreach( const Meta::TrackPtr track, trackList )
+    {
+        if( artist != track->artist() )
+            singleArtist = false;
+
+        if( album != track->album() )
+            singleAlbum = false;
+
+        if ( !singleArtist && !singleAlbum )
+            break;
+    }
+
+    if( ( !singleArtist && !singleAlbum ) ||
+        ( !artist && !album ) )
+        return i18nc( "A saved playlist with the current time (KLocale::Shortdate) added between \
+                      the parentheses",
+                      "Various Tracks (%1)" ).arg( datePart );
+
+    if( singleArtist )
+    {
+        if( artist )
+            artistPart = artist->prettyName();
+        else
+            artistPart = i18n( "Unknown Artist(s)" );
+    }
+    else if( album && album->hasAlbumArtist() && singleAlbum )
+        artistPart = album->albumArtist()->prettyName();
+    else
+        artistPart = i18n( "Various Artists" );
+
+    if( singleAlbum )
+    {
+        if( album )
+            albumPart = album->prettyName();
+        else
+            albumPart = i18n( "Unknown Album(s)" );
+    }
+    else
+        albumPart = i18n( "Various Albums" );
+
+    return i18nc( "A saved playlist titled <artist> - <album>", "%1 - %2")
+            .arg( artistPart, albumPart );
+
+}
+
+QString
 Playlist::Model::prettyColumnName( Column index ) //static
 {
     switch ( index )
diff --git a/src/playlist/PlaylistModel.h b/src/playlist/PlaylistModel.h
index 7b7633f..e99abbc 100644
--- a/src/playlist/PlaylistModel.h
+++ b/src/playlist/PlaylistModel.h
@@ -94,6 +94,7 @@ class AMAROK_EXPORT Model : public QAbstractListModel, public Meta::Observer, pu
         Meta::TrackPtr trackAt( int row ) const;
         Meta::TrackPtr trackForId( const quint64 id ) const;
         virtual Meta::TrackList tracks() const;
+        virtual QString generatePlaylistName() const;
 
         // Inherited from Meta::Observer
         using Observer::metadataChanged;