[prev in list] [next in list] [prev in thread] [next in thread]
List: quanta-devel
Subject: Re: [quanta-devel] Review Request: Actually use ExecuteBrowserPlugin in XDebugJob
From: "Milian Wolff" <mail () milianw ! de>
Date: 2012-04-17 12:42:43
Message-ID: 20120417124243.12346.16761 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> 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
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/104595/">http://git.reviewboard.kde.org/r/104595/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 15th, 2012, 8:47 p.m., <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: \
collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: \
4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/104595/diff/1/?file=56596#file56596line297" style="color: black; \
font-weight: bold; text-decoration: underline;">debuggers/xdebug/debugjob.cpp</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void \
XDebugBrowserJob::start()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">297</font></th> <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: \
140%; margin: 0; "> <span class="n">kWarning</span><span class="p">()</span> <span \
class="o"><<</span> <span class="s">"openUrl failed, something went wrong when creating the \
job"</span><span class="p">;</span></pre></td> <th bgcolor="#e9eaa8" style="border-left: 1px solid \
#C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">297</font></th> <td \
bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span \
class="n">IExecuteBrowserPlugin</span><span class="o">*</span> <span class="n">iface</span> <span \
class="o">=</span> <span class="n">KDevelop</span><span class="o">::</span><span \
class="n">ICore</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span \
class="o">-></span><span class="n">pluginController</span><span class="p">()</span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;">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</pre> </blockquote>
<p>On April 17th, 2012, 12:01 a.m., <b>Dominik Schmidt</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;">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? ;)</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;">yeah just add the X-KDevelop-Required thingy to the \
*.desktop file of this plugin</pre> <br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 15th, 2012, 8:47 p.m., <b>Milian Wolff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: \
collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: \
4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/104595/diff/1/?file=56601#file56601line52" style="color: black; \
font-weight: bold; text-decoration: underline;">plugins/executebrowser/iexecutebrowserplugin.h</a> <span \
style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font \
size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; \
margin: 0; "></pre></td> <th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px \
solid #C0C0C0;" align="right"><font size="2">51</font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="c1">// second param overrides the \
url set in the launch configuration, e.g. with additional parameters</span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;">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?</pre> \
</blockquote>
<p>On April 17th, 2012, 12:01 a.m., <b>Dominik Schmidt</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;">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"
?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; \
white-space: -o-pre-wrap; word-wrap: break-word;">yeah ok</pre> <br />
<p>- Milian</p>
<br />
<p>On April 13th, 2012, 11:29 p.m., Dominik Schmidt wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: \
url('http://git.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: \
left top; background-repeat: repeat-x; border: 1px black solid;"> <tr>
<td>
<div>Review request for KDevelop and Quanta.</div>
<div>By Dominik Schmidt.</div>
<p style="color: grey;"><i>Updated April 13, 2012, 11:29 p.m.</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid \
#b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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 :-)</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">It works ... ;-)</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>debuggers/xdebug/debugjob.h <span style="color: grey">(9925733)</span></li>
<li>debuggers/xdebug/debugjob.cpp <span style="color: grey">(0f04914)</span></li>
<li>plugins/executebrowser/browserappjob.h <span style="color: grey">(37ff700)</span></li>
<li>plugins/executebrowser/browserappjob.cpp <span style="color: grey">(a211205)</span></li>
<li>plugins/executebrowser/executebrowserplugin.h <span style="color: grey">(7c78733)</span></li>
<li>plugins/executebrowser/executebrowserplugin.cpp <span style="color: grey">(921142f)</span></li>
<li>plugins/executebrowser/iexecutebrowserplugin.h <span style="color: grey">(f786622)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/104595/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
quanta-devel mailing list
quanta-devel@kde.org
https://mail.kde.org/mailman/listinfo/quanta-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic