[amarok] /: MountPointManager: fix inability to deselect "Use MusicLocation"

Matěj Laitl matej at laitl.cz
Sun May 12 20:05:40 UTC 2013


Git commit bbbf5abc82b3b470d92b78b591487732310f8428 by Matěj Laitl.
Committed on 12/05/2013 at 21:24.
Pushed by laitl into branch 'master'.

MountPointManager: fix inability to deselect "Use MusicLocation"

...and also inability to clear the Local Collection if collection
folders are cleared.

BUGFIXES:
 * Fix inability to reverse "Use Music Location" decision and inability
   to clear database once all collection directories have been unset.

This patch is a candidate for inclusion into 2.7.1

BUG: 316216
FIXED-IN: 2.8
CCMAIL: amarok-devel at kde.org

M  +2    -0    ChangeLog
M  +16   -20   src/App.cpp
M  +31   -19   src/core-impl/collections/db/MountPointManager.cpp
M  +7    -0    src/core-impl/collections/db/MountPointManager.h
M  +2    -1    src/scanner/GenericScanManager.cpp

http://commits.kde.org/amarok/bbbf5abc82b3b470d92b78b591487732310f8428

diff --git a/ChangeLog b/ChangeLog
index f244a82..0f4a2af 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -52,6 +52,8 @@ VERSION 2.8-Beta 1
      not to fool users.
 
   BUGFIXES:
+   * Fix inability to reverse "Use Music Location" decision and inability to clear
+     database once all collection directories have been unset. (BR 316216)
    * Fix `amarok --play file.mp3` option didn't actually start playback.
    * Fix frequent crashes on Linux when starting to play a track. (BR 319371)
    * Reason why a particular track is not playable is now shown in playlist tooltip; patch
diff --git a/src/App.cpp b/src/App.cpp
index 36500ea..4e96d3b 100644
--- a/src/App.cpp
+++ b/src/App.cpp
@@ -650,33 +650,29 @@ void App::handleFirstRun()
     int result = KMessageBox::No;
     if( dir.exists() && dir.isReadable() )
     {
-        result = KMessageBox::questionYesNoCancel(
-            mainWindow(),
-            i18n( "A music path, %1, is set in System Settings.\nWould you like to use that as a collection folder?", musicDir )
-            );
+        result = KMessageBox::questionYesNoCancel( mainWindow(), i18n( "A music path, "
+                "%1, is set in System Settings.\nWould you like to use that as a "
+                "collection folder?", musicDir ) );
     }
 
-    KConfigGroup folderConf = Amarok::config( "Collection Folders" );
-    bool useMusicLocation( false );
     switch( result )
     {
-    case KMessageBox::Yes:
-        if( CollectionManager::instance()->primaryCollection() )
+        case KMessageBox::Yes:
         {
-            CollectionManager::instance()->primaryCollection()->setProperty( "collectionFolders", QStringList() << musicDir );
-            CollectionManager::instance()->startFullScan();
-            useMusicLocation = true;
+            Collections::Collection *coll = CollectionManager::instance()->primaryCollection();
+            if( coll )
+            {
+                coll->setProperty( "collectionFolders", QStringList() << musicDir );
+                CollectionManager::instance()->startFullScan();
+            }
+            break;
         }
-        break;
-
-    case KMessageBox::No:
-        slotConfigAmarok( "CollectionConfig" );
-        break;
-
-    default:
-        break;
+        case KMessageBox::No:
+            slotConfigAmarok( "CollectionConfig" );
+            break;
+        default:
+            break;
     }
-    folderConf.writeEntry( "Use MusicLocation", useMusicLocation );
     config.writeEntry( "First Run", false );
 }
 
diff --git a/src/core-impl/collections/db/MountPointManager.cpp b/src/core-impl/collections/db/MountPointManager.cpp
index caaa484..a64da1e 100644
--- a/src/core-impl/collections/db/MountPointManager.cpp
+++ b/src/core-impl/collections/db/MountPointManager.cpp
@@ -49,6 +49,7 @@ MountPointManager::MountPointManager( QObject *parent, SqlStorage *storage )
     {
         debug() << "Dynamic Collection deactivated in amarokrc, not loading plugins, not connecting signals";
         m_ready = true;
+        handleMusicLocation();
         return;
     }
 
@@ -58,6 +59,34 @@ MountPointManager::MountPointManager( QObject *parent, SqlStorage *storage )
     createDeviceFactories();
 }
 
+void
+MountPointManager::handleMusicLocation()
+{
+    // For users who were using QDesktopServices::MusicLocation exclusively up
+    // to v2.2.2, which did not store the location into config.
+    // and also for versions up to 2.7-git that did wrote the Use MusicLocation entry
+
+    KConfigGroup folders = Amarok::config( "Collection Folders" );
+    const QString entryKey( "Use MusicLocation" );
+    if( !folders.hasKey( entryKey ) )
+        return; // good, already solved, nothing to do
+
+    // write the music location as another collection folder in this case
+    if( folders.readEntry( entryKey, false ) )
+    {
+        const KUrl musicUrl = QDesktopServices::storageLocation( QDesktopServices::MusicLocation );
+        const QString musicDir = musicUrl.toLocalFile( KUrl::RemoveTrailingSlash );
+        const QDir dir( musicDir );
+        if( dir.exists() && dir.isReadable() )
+        {
+            QStringList currentFolders = collectionFolders();
+            if( !currentFolders.contains( musicDir ) )
+                setCollectionFolders( currentFolders << musicDir );
+        }
+    }
+
+    folders.deleteEntry( entryKey ); // get rid of it for good
+}
 
 MountPointManager::~MountPointManager()
 {
@@ -96,6 +125,7 @@ MountPointManager::createDeviceFactories()
             createHandlerFromDevice( device, device.udi() );
     }
     m_ready = true;
+    handleMusicLocation();
 }
 
 int
@@ -240,7 +270,7 @@ MountPointManager::getMountedDeviceIds() const
 QStringList
 MountPointManager::collectionFolders() const
 {
-    if (!m_ready)
+    if( !m_ready )
     {
         debug() << "requested collectionFolders from MountPointManager that is not yet ready";
         return QStringList();
@@ -263,24 +293,6 @@ MountPointManager::collectionFolders() const
         }
     }
 
-    // For users who were using QDesktopServices::MusicLocation exclusively up
-    // to v2.2.2, which did not store the location into config.
-    const KConfigGroup generalConfig = KGlobal::config()->group( "General" );
-    if( result.isEmpty() && folders.readEntry( "Use MusicLocation", true )
-                         && !generalConfig.readEntry( "First Run", true ) )
-    {
-        const KUrl musicUrl = QDesktopServices::storageLocation( QDesktopServices::MusicLocation );
-        const QString musicDir = musicUrl.toLocalFile( KUrl::RemoveTrailingSlash );
-        const QDir dir( musicDir );
-        bool useMusicLocation( false );
-        if( dir.exists() && dir.isReadable() )
-        {
-            result << musicDir;
-            const_cast<MountPointManager*>(this)->setCollectionFolders( result );
-            useMusicLocation = true;
-        }
-        folders.writeEntry( "Use MusicLocation", useMusicLocation );
-    }
     return result;
 }
 
diff --git a/src/core-impl/collections/db/MountPointManager.h b/src/core-impl/collections/db/MountPointManager.h
index 8c6bf95..4e588d7 100644
--- a/src/core-impl/collections/db/MountPointManager.h
+++ b/src/core-impl/collections/db/MountPointManager.h
@@ -191,6 +191,13 @@ private:
     void createDeviceFactories();
 
     /**
+     * Old Amarok versions used to have "Use MusicLocation" config option which caused
+     * problems (bug 316216). This method converts out of it and needs to be run after
+     * MountPointManager has initialized.
+     */
+    void handleMusicLocation();
+
+    /**
      * checks whether a medium identified by its unique database id is currently mounted.
      * Note: does not handle deviceId = -1! It only checks real devices
      * @param deviceId the mediums unique id
diff --git a/src/scanner/GenericScanManager.cpp b/src/scanner/GenericScanManager.cpp
index eb13de3..b668ac9 100644
--- a/src/scanner/GenericScanManager.cpp
+++ b/src/scanner/GenericScanManager.cpp
@@ -89,7 +89,8 @@ GenericScanManager::requestScan( QList<KUrl> directories, ScanType type )
         scanDirsSet << path;
     }
 
-    if( scanDirsSet.isEmpty() )
+    // we cannot skip the scan even for empty scanDirsSet and non-partial scan, bug 316216
+    if( scanDirsSet.isEmpty() && type == PartialUpdateScan )
         return; // nothing to do
 
     m_scannerJob = new GenericScannerJob( this, scanDirsSet.toList(), type );


More information about the Amarok-devel mailing list