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

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

plugins/executebrowser/browserappjob.cpp (Diff revision 1)
void BrowserAppJob::start()
77
      proc.execute();
77
      proc.startDetached();
makes absolutely sense, but please commit separately.

plugins/executebrowser/iexecutebrowserplugin.h (Diff revision 1)
public:
51
    // second param overrides the url set in the launch configuration, e.g. with additional parameters
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


On April 13th, 2012, 11:29 p.m., Dominik Schmidt wrote:

Review request for KDevelop and Quanta.
By Dominik Schmidt.

Updated April 13, 2012, 11:29 p.m.

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 :-)

Testing

It works ... ;-)

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)

View Diff