Review Request: fix path for storing viewpropertysettings on Windows

[Peter Penz]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104313/#review11532
-----------------------------------------------------------

Ship it!


Thanks for the patch! As you say it is "not a beauty" but as it is a one-liner encapsulated within a define I'd say it is ok.

> Theoretically using KUrl::path() would fix that, but that doesn't work for the rest of the code.

You mean this does not work under Windows (I cannot test it)?
#ifdef Q_OS_WIN
    m_filePath = destinationDir("local") + url.path();
#else 
    m_filePath = destinationDir("local") + m_filePath;
#endif

I would slightly prefer the code above if it works, but please feel free to commit your patch 1:1 if I missed something in my suggestion.


- Peter Penz


On March 17, 2012, 5:51 p.m., Patrick Spendrin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104313/
> -----------------------------------------------------------
> 
> (Updated March 17, 2012, 5:51 p.m.)
> 
> 
> Review request for Dolphin and Peter Penz.
> 
> 
> Description
> -------
> 
> On Windows you cannot simply concatenate two paths because the : character is forbidden except after the drive letter. The second issue I am addressing with this patch is that you use the local file name e.g. C:/kde/ which doesn't start with a / so the resulting path is something like this C:/Users/Patrick/.kde/.../localC:/kde. Theoretically using KUrl::path() would fix that, but that doesn't work for the rest of the code.
> 
> I know that the patch is a bit ugly, if you can think of a better solution, please just let me know.
> 
> 
> This addresses bug 287360.
>     http://bugs.kde.org/show_bug.cgi?id=287360
> 
> 
> Diffs
> -----
> 
>   dolphin/src/views/viewproperties.cpp 36ec6bd 
> 
> Diff: http://git.reviewboard.kde.org/r/104313/diff/
> 
> 
> Testing
> -------
> 
> Windows only, the bug 287360 is fixed.
> 
> 
> Thanks,
> 
> Patrick Spendrin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20120318/0b461f92/attachment.htm>