Review Request: Add direct support for remote URLs to previewjob

[David Faure]

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

Ship it!


Looks good, the code is much simpler now. Just two small things, then I'm OK with this being committed to 4.7 branch -- won't affect existing thumbnail plugins.


kio/kio/previewjob.cpp
<http://git.reviewboard.kde.org/r/102929/#comment6638>

    Remove this if(), it's not needed. The foreach will do nothing if the list is empty, anyway.



kio/kio/previewjob.cpp
<http://git.reviewboard.kde.org/r/102929/#comment6639>

    The const ref here is not necessary [I hear the compilers make it work, but it "reads wrong", e.g. it points to the temporary QStringList() if the protocol wasn't found...].
    
    Anyway, why use a local variable for this case and not for the "KIO" case? This is harder to read because not symmetric.
    I guess making it a single line, just like for KIO, would be okay.


- David Faure


On Oct. 22, 2011, 12:55 a.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102929/
> -----------------------------------------------------------
> 
> (Updated Oct. 22, 2011, 12:55 a.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Description
> -------
> 
> This patch allows previewjobs to take a URL in case they understand how to deal with that. preview plugis can specify a list of protocols which they support, this is then matched with the scheme of the URL and if supported the URL is passed directly into the plugin. Otherwise, the file is downloaded and passed as local file (this part hasn't changed).
> 
> A suitable preview plugin that does the job will be submitted separately to kde-runtime.
> 
> Is this definitively Frameworks 5 material, or would you be OK with adding it to the 4.7 branch? (The latter would mean, we could use vanilla kdelibs for Plasma Active 2 in december.)
> 
> 
> Diffs
> -----
> 
>   kdecore/services/kservice.h 040507c 
>   kio/kio/previewjob.cpp ff6d340 
> 
> Diff: http://git.reviewboard.kde.org/r/102929/diff/diff
> 
> 
> Testing
> -------
> 
> Generating preview for HTTP URLs, for local files and for remote files without a suitable plugin.
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20111028/4b687bc1/attachment.htm>