Review Request: Fix and cleanup of AdiumXtra styles installer

[Daniele Elmo Domenichelli]

> On March 21, 2012, 3:50 p.m., Dominik Schmidt wrote:
> > adiumxtra-protocol-handler/main.cpp, line 55
> > <http://git.reviewboard.kde.org/r/104355/diff/1/?file=54061#file54061line55>
> >
> >     Why don't you invoke it directly but through the event loop?

The problem is that the KApplication won't quit if I call it directly, the QObject must be in the event loop and should start when the app.exec() is called.
I made several tests, and this is the only way to do it, but if you have any suggestion...


> On March 21, 2012, 3:50 p.m., Dominik Schmidt wrote:
> > adiumxtra-protocol-handler/chat-style-installer.cpp, line 159
> > <http://git.reviewboard.kde.org/r/104355/diff/1/?file=54059#file54059line159>
> >
> >     Can't this be done more elegantly? Fixing race conditions by just waiting a fixed time seems utterly wrong

Yes, I found the way
ChatWindowStyleManager should not call loadStyles(), the apps who need to load the styles should call it directly (the ktp_kcm_appearance does it already)
In this way the KDirLister is not created and there is no race condition


> On March 21, 2012, 3:50 p.m., Dominik Schmidt wrote:
> > lib/chat-window-style-manager.cpp, line 196
> > <http://git.reviewboard.kde.org/r/104355/diff/1/?file=54062#file54062line196>
> >
> >     Aren't those required by the spec?
> >     Are we providing defaults when they are not around?

Those are optional in the specs, we have a "fallback" mechanism that will load the correct resource if those files are not in the theme


On March 21, 2012, 3:50 p.m., Daniele Elmo Domenichelli wrote:
> > Thanks for taking care, it's nice to see the code is not completely abandoned! :-)

The installer is very useful, the plan is to let users install themes from here: http://community.kde.org/Real-Time_Communication_and_Collaboration/Components/Chat_Window/Themes :)


- Daniele Elmo


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


On March 21, 2012, 2:53 p.m., Daniele Elmo Domenichelli wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104355/
> -----------------------------------------------------------
> 
> (Updated March 21, 2012, 2:53 p.m.)
> 
> 
> Review request for Telepathy, David Edmundson and Dominik Schmidt.
> 
> 
> Description
> -------
> 
> AdimuXtra is currently broken, this set of patches fixes the installation of styles, I didn't try the installation of emoticons though
> 
> Branch here: http://quickgit.kde.org/index.php?p=clones%2Fktp-text-ui%2Fddomenichelli%2Fktp-text-ui.git&a=shortlog&h=refs/heads/adiumxtra
> 
> 
> This addresses bug 282519.
>     http://bugs.kde.org/show_bug.cgi?id=282519
> 
> 
> Diffs
> -----
> 
>   adiumxtra-protocol-handler/adiumxtra-protocol-handler.h 9b58210ad257d6ad8b802957cb7bcc66c0c5c2a9 
>   adiumxtra-protocol-handler/adiumxtra-protocol-handler.cpp 728a3722b9556c9c21196f8f8e8bad5c0f03dcb8 
>   adiumxtra-protocol-handler/chat-style-installer.cpp 1b2a1c95fc468d0c80007b057997ef4e91a0d306 
>   adiumxtra-protocol-handler/emoticon-set-installer.cpp e324df022300ec97dca9bbcb1193c5f5f89c8269 
>   adiumxtra-protocol-handler/main.cpp e1ed40abad717475800451236a5295e1520d3881 
>   lib/chat-window-style-manager.cpp 235fc5361f916ab0f1c1a05ec4e662232a917804 
> 
> Diff: http://git.reviewboard.kde.org/r/104355/diff/
> 
> 
> Testing
> -------
> 
> Installing themes works!
> 
> 
> Thanks,
> 
> Daniele Elmo Domenichelli
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120321/56db9a44/attachment.html>