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

List:       kde-commits
Subject:    Re: [Amarok] 1c52f3e: Fix playlist filenames when saving
From:       Bart Cerneels <bart.cerneels () kde ! org>
Date:       2010-12-13 14:31:17
Message-ID: AANLkTimB0Q8re1pH_2Xh3EcrZPJ7gNuDL7zx=cTtm18F () mail ! gmail ! com
[Download RAW message or body]

On Mon, Dec 13, 2010 at 02:34, Rick W.Chen <stuffcorpse@archlinux.us> wrote:
> commit 1c52f3e3e197e03590d2c000ce8011252a9a562e
> branch master
> Author: Rick W. Chen <stuffcorpse@archlinux.us>
> Date:   Mon Dec 13 04:48:16 2010 +1300
> 
> Fix playlist filenames when saving
> 
> o when saving from the playlist dock, a generated date/time is used as the
> name. if slashes are used in the user's date format settings, saving will
> fail.
> o only save as xspf if not using any other playlist type. this fixes
> 717daa7ef840036892f90a8d696fc16d176f58b9 where everything was being saved
> as xspf.
> 
> diff --git a/src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp \
> b/src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp index e0727c0..75da8ef \
>                 100644
> --- a/src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp
> +++ b/src/core-impl/playlists/types/file/xspf/XSPFPlaylist.cpp
> @@ -16,6 +16,8 @@
> * this program.  If not, see <http://www.gnu.org/licenses/>.                        \
>                 *
> ****************************************************************************************/
>  
> +#define DEBUG_PREFIX "XSPFPlaylist"
> +
> #include "core-impl/playlists/types/file/xspf/XSPFPlaylist.h"
> 
> #include "core/support/Debug.h"
> @@ -147,7 +149,7 @@ XSPFPlaylist::save( const KUrl &location, bool relative )
> 
> if( !file.open( QIODevice::WriteOnly ) )
> {
> -        warning() << QString( "Cannot write playlist (%1)." ).arg( file.fileName() \
> ); +        warning() << QString( "Cannot write playlist (%1)." ).arg( \
> file.fileName() ) << file.errorString(); 
> return false;
> }
> diff --git a/src/playlistmanager/file/PlaylistFileProvider.cpp \
> b/src/playlistmanager/file/PlaylistFileProvider.cpp index dceeb28..e836eb5 100644
> --- a/src/playlistmanager/file/PlaylistFileProvider.cpp
> +++ b/src/playlistmanager/file/PlaylistFileProvider.cpp
> @@ -213,17 +213,32 @@ Playlists::PlaylistPtr
> PlaylistFileProvider::save( const Meta::TrackList &tracks, const QString &name )
> {
> DEBUG_BLOCK
> +    if( name.isEmpty() )
> +    {
> +        error() << "trying to save playlist without a name";
> +        return Playlists::PlaylistPtr();
> +    }
> +
> +    QString filename = QString( name ).replace( QLatin1Char('/'), QLatin1Char('-') \
> ); KUrl path( Amarok::saveLocation( "playlists" ) );
> -    path.addPath( Amarok::vfatPath( name ) + ".xspf" );
> +    path.addPath( Amarok::vfatPath( filename ) );
> if( QFileInfo( path.toLocalFile() ).exists() )
> {
> //TODO:request overwrite
> return Playlists::PlaylistPtr();
> }
> -    QString ext = Amarok::extension( path.fileName() );
> +
> Playlists::PlaylistFormat format = m_defaultFormat;
> -    if( !name.isNull() && !ext.isEmpty() )
> +    QString ext = Amarok::extension( path.fileName() );
> +    if( ext.isEmpty() )
> +    {
> +        ext = QLatin1String("xspf");
> +        path.setFileName( QString("%1.%2").arg(Amarok::vfatPath(filename), ext) );
> +    }
> +    else
> +    {
> format = Playlists::getFormat( path );
> +    }
> 
> Playlists::PlaylistFile *playlistFile = 0;
> switch( format )
> @@ -238,10 +253,10 @@ PlaylistFileProvider::save( const Meta::TrackList &tracks, \
> const QString &name ) playlistFile = new Playlists::XSPFPlaylist( tracks );
> break;
> default:
> -            debug() << QString("Do not support filetype with extension \
> \"%1!\"").arg( ext ); +            error() << QString("Do not support filetype with \
> extension \"%1!\"").arg( ext ); return Playlists::PlaylistPtr();
> }
> -    playlistFile->setName( name );
> +    playlistFile->setName( filename );
> debug() << "Forcing save of playlist!";
> playlistFile->save( path, true );
> playlistFile->setProvider( this );
> 

Ah, I forgot about the session playlist. Didn't realize I broke it.

The name.isEmpty() case shouldn't be handled like this though, not
saving is not an option. Instead I proposed to use
QDateTime::currentDateTime().toString( "ddd MMMM d yy hh-mm") ).

Bart


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