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

On April 15th, 2012, 8:47 p.m., Milian Wolff wrote:

debuggers/xdebug/debugjob.cpp (Diff revision 1)
void XDebugBrowserJob::start()
297
            kWarning() << "openUrl failed, something went wrong when creating the job";
297
        IExecuteBrowserPlugin* iface = KDevelop::ICore::self()->pluginController()
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

On April 17th, 2012, 12:01 a.m., Dominik Schmidt wrote:

Good hint!

It doesn't do that so far, but it perfectly makes sense:
a) the settings are already stored in an executebrowserplugin instance
b) you'd need to implement fallback code in the xdebug plugin which is already inside the EBP
c) if not for this, what is the EBP good for then? ;)
yeah just add the X-KDevelop-Required thingy to the *.desktop file of this plugin

On April 15th, 2012, 8:47 p.m., Milian Wolff wrote:

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

On April 17th, 2012, 12:01 a.m., Dominik Schmidt wrote:

Sure.

How about ..

"// if you provide a valid url as second parameter that one is opened instead of the url stored in the launch configuration
 // e.g. this is used in xdebug plugin to add additional GET parameters to the configured url"

?
yeah ok

- Milian


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