Review Request: Actually use ExecuteBrowserPlugin in XDebugJob

[Milian Wolff]

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



debuggers/xdebug/debugjob.cpp
<http://git.reviewboard.kde.org/r/104595/#comment9799>

    this is unstable, it will crash as soon as the executebrowserplugin is disabled, or is that a requirement of the xdebug plugin? hm would make sense - just make sure that this is so in the *.desktop file please



plugins/executebrowser/browserappjob.cpp
<http://git.reviewboard.kde.org/r/104595/#comment9802>

    is it ok to emit the result directly? or should one wait for the "started" signal (that should exist afaik)? that would also allow one to properly handle error cases, i.e. if. m_browser points to a non-existing browser-app or if something else goes wrong, or?



plugins/executebrowser/iexecutebrowserplugin.h
<http://git.reviewboard.kde.org/r/104595/#comment9801>

    I don't understand this comment after the e.g. part - could you rephrase this? or give a proper example? what happens by default, i.e. if the url is empty?


- Milian Wolff


On April 13, 2012, 11:29 p.m., Dominik Schmidt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104595/
> -----------------------------------------------------------
> 
> (Updated April 13, 2012, 11:29 p.m.)
> 
> 
> Review request for KDevelop and Quanta.
> 
> 
> Description
> -------
> 
> Currently the XDebugJob always uses QDesktopServices::openUrl() to open the debug page in the browser, this patch makes use of the ExecuteBrowserPlugin instance that is already present in XDebug job so the configured browser is launched.
> 
> Also it makes BrowserAppJob launch the external browser KProc with .startDetached() instead of .execute() to prevent freezing of the KDevelop GUI.
> 
> 
> Arguments are currently ignored still, a patch for that is following
> 
> Feel free to nitpick, I haven't done any KDE coding in a while and would like to hear any suggestion for improvements :-)
> 
> 
> Diffs
> -----
> 
>   debuggers/xdebug/debugjob.h 9925733 
>   debuggers/xdebug/debugjob.cpp 0f04914 
>   plugins/executebrowser/browserappjob.h 37ff700 
>   plugins/executebrowser/browserappjob.cpp a211205 
>   plugins/executebrowser/executebrowserplugin.h 7c78733 
>   plugins/executebrowser/executebrowserplugin.cpp 921142f 
>   plugins/executebrowser/iexecutebrowserplugin.h f786622 
> 
> Diff: http://git.reviewboard.kde.org/r/104595/diff/
> 
> 
> Testing
> -------
> 
> It works ... ;-)
> 
> 
> Thanks,
> 
> Dominik Schmidt
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120415/9e8e7517/attachment.html>