Bug 231437 - Implement support to bookmark folders
Summary: Implement support to bookmark folders
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Tools/Bookmark Manager (show other bugs)
Version: 2.3.0
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-20 17:35 UTC by Oleg Atamanenko
Modified: 2010-03-21 14:23 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In: 2.3.1


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Oleg Atamanenko 2010-03-20 17:35:42 UTC
Version:           2.3.0 (using 4.4.1 (KDE 4.4.1), Chakra)
Compiler:          gcc
OS:                Linux (i686) release 2.6.32-ARCH

Could you please implement support to bookmark folders as it was in Amarok 1.x?
Comment 1 Sven Krohlas 2010-03-20 19:28:05 UTC
You are talking about bookmarks in the file browser?
Comment 2 Oleg Atamanenko 2010-03-20 20:03:11 UTC
Yes, I'm talking about bookmarks in file browser.

Actually, when I click on 'bookmark media sources view' in File browser amarok creates bookmark but url is strange: amarok://navigate/files, so actual path is not saved and this feature is unuseful.
Comment 3 Sven Krohlas 2010-03-21 12:30:11 UTC
Some progress: git master as of now has Places support in the file browser.
Comment 4 Nikolaj Hald Nielsen 2010-03-21 14:18:00 UTC
commit 2cfbcaed74346500f6e5c9dd901b9f8c41f722c7
Author: Nikolaj Hald Nielsen <nhn@kde.org>
Date:   Sun Mar 21 14:07:29 2010 +0100

    Make bookmarking the file browser also store the patch it is showing.
    Also fix the name of the bookmarks to be "files" instead of just "/"
    
    BUG: 231437

diff --git a/src/amarokurls/NavigationUrlGenerator.cpp b/src/amarokurls/NavigationUrlGenerator.cpp
index e38a947..39f1a92 100644
--- a/src/amarokurls/NavigationUrlGenerator.cpp
+++ b/src/amarokurls/NavigationUrlGenerator.cpp
@@ -25,6 +25,7 @@
 #include "browsers/CollectionTreeItemModelBase.h"
 #include "browsers/collectionbrowser/CollectionWidget.h"
 #include "browsers/playlistbrowser/PlaylistBrowser.h"
+#include "browsers/filebrowser/FileBrowser.h"
 #include "collection/sqlcollection/SqlMeta.h"
 #include "PlaylistManager.h"
 
@@ -120,9 +121,23 @@ AmarokUrl NavigationUrlGenerator::CreateAmarokUrl()
             url.appendArg( "show_years", "false" );
     }
 
-
     //come up with a default name for this url..
     QString name = The::mainWindow()->browserWidget()->list()->activeCategoryRecursive()->prettyName();
+
+    //if in the file browser, also store the file path
+    if( url.path().endsWith( "files", Qt::CaseInsensitive ) )
+    {
+
+        //Give a proper name since it will return "/" as that is what is used in the breadcrumb.
+        name = i18n( "Files" );
+
+        FileBrowser * fileBrowser = dynamic_cast<FileBrowser *>( The::mainWindow()->browserWidget()->list()->activeCategory() );
+        if( fileBrowser )
+        {
+            url.appendArg( "path", fileBrowser->currentDir() );
+        }
+    }
+
     url.setName( name );
     
     return url;
diff --git a/src/amarokurls/NavigationUrlRunner.cpp b/src/amarokurls/NavigationUrlRunner.cpp
index cff35b6..169a749 100644
--- a/src/amarokurls/NavigationUrlRunner.cpp
+++ b/src/amarokurls/NavigationUrlRunner.cpp
@@ -25,6 +25,7 @@
 #include "PlaylistManager.h"
 #include "browsers/CollectionTreeItemModelBase.h"
 #include "browsers/collectionbrowser/CollectionWidget.h"
+#include "browsers/filebrowser/FileBrowser.h"
 #include "browsers/playlistbrowser/PlaylistBrowser.h"
 #include "browsers/servicebrowser/ServiceBrowser.h"
 #include "services/ServiceBase.h"
@@ -78,7 +79,7 @@ NavigationUrlRunner::run( AmarokUrl url )
 
 
     //if we are activating the local collection, check if we need to restore "show cover" and "show year"
-        //if in the local collection view, also store "show covers" and "show years"
+    //if in the local collection view, also store "show covers" and "show years"
     if( url.path().endsWith( "collections", Qt::CaseInsensitive ) )
     {
         if ( args.keys().contains( "show_cover" ) )
@@ -98,6 +99,19 @@ NavigationUrlRunner::run( AmarokUrl url )
         }
     }
 
+    //also set the correct path if we are navigating to the file browser
+    if( url.path().endsWith( "files", Qt::CaseInsensitive ) )
+    {
+        FileBrowser * fileBrowser = dynamic_cast<FileBrowser *>( The::mainWindow()->browserWidget()->list()->activeCategory() );
+        if( fileBrowser )
+        {
+            if( args.keys().contains( "path" ) )
+            {
+                fileBrowser->setDir( args.value( "path" ) );
+            }
+        }
+    }
+
     if ( args.keys().contains( "filter" ) )
         active->setFilter( args.value( "filter" ) );
 
diff --git a/src/browsers/filebrowser/FileBrowser.cpp b/src/browsers/filebrowser/FileBrowser.cpp
index 8bf2e15..bf93554 100644
--- a/src/browsers/filebrowser/FileBrowser.cpp
+++ b/src/browsers/filebrowser/FileBrowser.cpp
@@ -134,6 +134,15 @@ FileBrowser::polish()
     setupAddItems();
 }
 
+QString
+FileBrowser::currentDir()
+{
+    if( m_showingPlaces )
+        return "places:";
+    else
+        return m_currentPath;
+}
+
 void
 FileBrowser::itemActivated( const QModelIndex &index )
 {
@@ -359,8 +368,11 @@ FileBrowser::prettyName() const
 void
 FileBrowser::setDir( const QString &dir )
 {
-    //This function just happens to do exactly what we need
-    addItemActivated( dir );
+
+    if( dir == "places:" )
+        showPlaces();
+    else
+       addItemActivated( dir );  //This function just happens to do exactly what we need
 }
 
 void
diff --git a/src/browsers/filebrowser/FileBrowser.h b/src/browsers/filebrowser/FileBrowser.h
index 6326bf9..1f7950c 100644
--- a/src/browsers/filebrowser/FileBrowser.h
+++ b/src/browsers/filebrowser/FileBrowser.h
@@ -45,6 +45,11 @@ public:
     */
     void setDir( const QString &dir );
 
+    /**
+     * Return the path of the currently shown dir.
+     */
+    QString currentDir();
+
 protected slots:
     void itemActivated( const QModelIndex &index );