Review Request 125675: Use the new KOverlayIconPlugin interface from KIO::Widgets

[Emmanuel Pescosta]

> On Oct. 19, 2015, 3:46 p.m., Emmanuel Pescosta wrote:
> > src/kitemviews/kfileitemmodelrolesupdater.cpp, lines 140-150
> > <https://git.reviewboard.kde.org/r/125675/diff/2/?file=411729#file411729line140>
> >
> >     IMHO this and all the foreach overlay plugin loops should also go into KIO hidden behind
> >     a nice service api (getOverlays and overlaysChanged signal). Clients are usually not interested where the overlays come from and where the necessary plugins are located ...
> >     
> >     This avoids duplicated code when other clients want to make use of the overlay infrastructure and makes it easier to extend in future.
> 
> Olivier Goffart wrote:
>     This just converts a QList<QObject*> to a QList<KOverlayIconPlugin*>.   (maybe KPluginLoader::instentiatePlugin could take a template parametter to do that?)
>     But it also connects signals to dolphin itself
> 
> Emmanuel Pescosta wrote:
>     > maybe KPluginLoader::instentiatePlugin could take a template parametter to do that?
>     
>     Good idea :)
>     
>     But I meant more the hidding of implementation details of the whole overlay functionality:
>     
>     OverlayService {
>       get()
>       signal changed()
>     }
>     
>     The client has one method to get overlays and one changed signal to connect with, completely irrelevant
>     if the overlays come from plugins or anywhere else. So we can easily extend it in future (e.g. implementing
>     enabling/disabling of plugins, ...) without adjusting all users of the overlay infrastructure (only Dolphin atm
>     but I think it would also be interesting for the folder view applet ;) ).
> 
> Olivier Goffart wrote:
>     The other plugins in KIO::Widgets such as the actions plugin don't have that.  Also, so far, dolphin is the only user of this.

See KFileItemActions for example, there is basically the same pattern. KFileItemActions hiddes away all the desktop file handling, authorization, ... and makes it possible to extend it without changing anything on the client side. The client can use a nice api which takes the selected items and adds all the actions.

The same would be nice for overlay plugins to e.g. add code to enable/disable plugins in future (like the version control plugins)
It is also easier for lib users to use a well definded and easy api instead of loading and managing all the stuff itself.

But ok, ... if you don't see the advantage feel free to ship it, so that we get it in before the freeze ;)


- Emmanuel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125675/#review87070
-----------------------------------------------------------


On Nov. 3, 2015, 4:44 p.m., Olivier Goffart wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125675/
> -----------------------------------------------------------
> 
> (Updated Nov. 3, 2015, 4:44 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: dolphin
> 
> 
> Description
> -------
> 
> Depends on, https://git.reviewboard.kde.org/r/125436
> 
> Replaces https://git.reviewboard.kde.org/r/125136/
> 
> 
> Diffs
> -----
> 
>   src/kitemviews/kfileitemmodelrolesupdater.h 6e7559f 
>   src/kitemviews/kfileitemmodelrolesupdater.cpp 14e3015 
>   CMakeLists.txt 8f1e283 
> 
> Diff: https://git.reviewboard.kde.org/r/125675/diff/
> 
> 
> Testing
> -------
> 
> Tested with owncloud plugin.
> 
> 
> Thanks,
> 
> Olivier Goffart
> 
>

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