Review Request: Actually use ExecuteBrowserPlugin in XDebugJob

[Milian Wolff]

> On April 15, 2012, 8:47 p.m., Milian Wolff wrote:
> > debuggers/xdebug/debugjob.cpp, line 297
> > <http://git.reviewboard.kde.org/r/104595/diff/1/?file=56596#file56596line297>
> >
> >     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
> 
> 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 15, 2012, 8:47 p.m., Milian Wolff wrote:
> > plugins/executebrowser/iexecutebrowserplugin.h, line 52
> > <http://git.reviewboard.kde.org/r/104595/diff/1/?file=56601#file56601line52>
> >
> >     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?
> 
> 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


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


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/20120417/d11608fd/attachment.html>