Review Request: KIO::PreviewJob: Respect the enabled plugins from "PreviewSettings"

David Faure faure at kde.org
Wed Feb 16 11:00:23 GMT 2011


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

Ship it!


Looks good, nice cleanup and fix, thanks.
Long ago the idea was just "if the app doesn't specify plugins, then use them all". But if it's configurable nowadays, then better respect the user's wishes indeed.


kio/kio/previewjob.h
<http://git.reviewboard.kde.org/r/100578/#comment1217>

    I think this has to be KIO_EXPORT_DEPRECATED instead (for Windows in particular).


- David


On Feb. 5, 2011, 7:53 p.m., Peter Penz wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100578/
> -----------------------------------------------------------
> 
> (Updated Feb. 5, 2011, 7:53 p.m.)
> 
> 
> Review request for kdelibs and David Faure.
> 
> 
> Summary
> -------
> 
> Currently KIO::PreviewJob respects the settings "MaximumSize" and "MaximumRemoteSize" from the KConfigGroup "PreviewSettings", but completely ignores the "Plugins" settings. Because of this in the following locations the plugins are manually read and passed to KIO::PreviewJob:
> - Tooltips in Dolphin/Konqueror
> - Information Panel in Dolphin
> - KFilePreviewGenerator in kdelibs
> 
> Recently it turned out that the file-open-dialog ignores the "Plugins" too. Before adding some code again I've provided this patch for KIO::PreviewJob. Changing the current behavior of KIO::PreviewJob cannot be done, so a new constructor has been added. Following Qt's constructor-pattern the new constructor is quite minimal and setter/getter-methods have been added for the missing parameters.
> 
> The changed behavior of the new constructor is that if enabledPlugins is zero the plugins from "PreviewSettings" are used now.
> 
> If it is OK to merge this patch, I'd take care to replace the calls to the deprecated API within kdelibs + kdebase.
> 
> Also it might be useful to change the implementation to use PreviewJob::ScaleType internally instead of the two bools bScale and bSave (the ScaleType has been introduced because of the KDE5 comment). But I'd do this in a second commit as it won't change any behavior.
> 
> 
> Diffs
> -----
> 
>   kio/kio/previewjob.cpp 96e5b27 
>   kio/kio/previewjob.h b86fc9b 
> 
> Diff: http://git.reviewboard.kde.org/r/100578/diff
> 
> 
> Testing
> -------
> 
> Temporary adjusted KFilePreviewGenerator for testing.
> 
> 
> Thanks,
> 
> Peter
> 
>

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


More information about the kde-core-devel mailing list