On 20. 6. 2012 Sven Krohlas wrote:
> Git commit 04dd8cfb4c2767f9c9c4b2f0b087c16b2a2421de by Sven Krohlas.
>
> Don't hardcode / as directory separator
>
> Might have caused problems on Windows. I don't know, I don't have a Windows
> machine to test. While we are at it:
>
> diff --git a/src/playlistmanager/file/PlaylistFileProvider.cpp
> b/src/playlistmanager/file/PlaylistFileProvider.cpp index 9510268..f364984
> 100644
> --- a/src/playlistmanager/file/PlaylistFileProvider.cpp
> +++ b/src/playlistmanager/file/PlaylistFileProvider.cpp
> @@ -129,7 +129,7 @@ PlaylistFileProvider::save( const Meta::TrackList
> &tracks, const QString &name ) {
> DEBUG_BLOCK
> QString filename = name.isEmpty() ?
> QDateTime::currentDateTime().toString( "ddd MMMM d yy hh-mm") : name; -
> filename.replace( QLatin1Char('/'), QLatin1Char('-') );
> + filename.replace( QDir::separator(), QLatin1Char('-') );
Sven, your fix is actually incorrect. From QDir::separator() documentation:
> Returns the native directory separator: "/" under Unix (including Mac OS X)
> and "\" under Windows. You do not need to use this function to build file
> paths. If you always use "/", Qt will translate your paths to conform to the
> underlying operating system.
Imagine a Windows user tries to save a playlist called "play/list". As only \
would be filtered, the path passes and then Qt translates the / to native
Windows separator. To be really safe, I would filter-out both "/" and
QDir::separator(), no-op on Unix and safe on Windoze. As a bonus, it would be
very nice if this was tested in TestPlaylistFileProvider. ;)
Kudos to you, Sven, for cleaning up our tests, this is much needed.
Matěj
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
