Review Request 120342: Give Stage- and KoPA-specific tools own service types (fixes crash in Braindump and for me Flow)

[Friedrich W. H. Kossebau]

> On Sept. 25, 2014, 2:03 p.m., Boudewijn Rempt wrote:
> > It looks fine to me, though I'm not the right maintainer :-)

Thanks Boud. Listed you in cc: because of your experience with that stuff and because I followed Krita's KisFactory2 code. So was looking forward to your comment :)
If noone else comments until October 1st, I will go one and ship it :)


- Friedrich W. H.


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


On Sept. 24, 2014, 9:24 p.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/120342/
> -----------------------------------------------------------
> 
> (Updated Sept. 24, 2014, 9:24 p.m.)
> 
> 
> Review request for Calligra, Cyrille Berger Skott, Yue Liu, Boudewijn Rempt, and Thorsten Zachmann.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Currently blacklisting is done for the two Stage- and KoPA-specific tools calligrastagetoolanimation and kopabackgroundtool in all apps (not done for Braindump because there is no default braindumprc with an "ToolPluginsDisabled" entry).
> Actually these two tools will even crash other apps because they expect the passed objects, like the canvas, to be of certain classes.
> Als does the current blacklisting have a problem: if the entry "ToolPluginsDisabled" is written to the personal copy of the rc file, installing a new. Which happened to me, because I have a very old flowrc file in personal config dir with only "ToolPluginsDisabled=kopabackgroundtool", so the newer entry in the newer installed flowrc was no longer read, resulting in not blacklisting calligrastagetoolanimation for me in recent calligra versions. And trying that tool made me think Flow is buggy ;)
> 
> So given that those tools are not compatible with all apps, I propose to instead give them own service types:
> "CalligraPageApp/Tool" and "CalligraStage/Tool".
> So only programs which explicitely demand services of these types get them, and the rest no longer has to protect against them.
> 
> This also helps with potential 3rd-party KoPA-specific tool plugins, as they would not be catched by the existing blacklisting.
> 
> Only disadvantage: programs which want them have to explicitely demand them. But that seems okay to me.
> 
> Followed the example of Krita's KisFactory2 code how to do refcount-guarded loading of the plugins. Not sure that is perfect. But at least consistent :)
> 
> 
> Diffs
> -----
> 
>   karbon/data/karbonrc 33ac9b3 
>   krita/animator/kritaanimationrc 60036d9 
>   krita/data/kritarc 9ffcae4 
>   krita/gemini/kritageminirc 354c741 
>   krita/sketch/kritasketchrc f3efba6 
>   libs/kopageapp/tools/CMakeLists.txt 3dea584 
>   libs/kopageapp/tools/backgroundTool/kopabackgroundtool.desktop 1b8683a 
>   libs/kopageapp/tools/kopa_tool.desktop PRE-CREATION 
>   sheets/sheetsrc 9b622fb 
>   stage/data/CMakeLists.txt 50e6efa 
>   stage/data/kpr_tool.desktop PRE-CREATION 
>   stage/part/KPrFactory.cpp 848b15c 
>   stage/part/tools/animationtool/calligrastagetoolanimation.desktop ac2737e 
>   words/part/author/authorrc 2537c68 
>   words/part/wordsrc 4cd2801 
>   flow/part/FlowFactory.cpp 7f14fb0 
>   flow/part/flowrc 1d1c12f 
> 
> Diff: https://git.reviewboard.kde.org/r/120342/diff/
> 
> 
> Testing
> -------
> 
> calligrastagetoolanimation and kopabackgroundtool now longer show up were they should not show up.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20140925/c504193d/attachment.htm>