Review Request: Actually use ExecuteBrowserPlugin in XDebugJob

[Niko Sams]

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


looks good, but please adapt to moved repositories. (now kdev-xdebug and kdev-executebrowser repositories)


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

    makes absolutely sense, but please commit separately.



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

    hm, I don't like this overriding of url, the url is *the* thing of the launch - and you can override it.
    
    what about a QMap<QString, QString> queryItems parameter? (or just QString?)


- Niko Sams


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/20120501/7d8262bb/attachment.html>