[amarok] src/playlistmanager/file: Don't hardcode / as directory separator

[Mathias Panzenböck]

I'm fairly sure that Windows eats both \ and / (actually I think even DOS >=2.0). So using / as 
directory separator should work anywhere. Though it is not nice/clean for Windows users and there 
might be some Windows Programs that choke on it.

On 06/20/2012 04:00 PM, Matěj Laitl wrote:
> 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 at kde.org
> https://mail.kde.org/mailman/listinfo/amarok-devel