<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/103817/">http://git.reviewboard.kde.org/r/103817/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 21st, 2012, 10:33 p.m., <b>Milian Wolff</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;">could you please elaborate on the "assistant stacking"?
and this code really could need some love, style-wise. Please, I'd really appreciate if you could clean the code up while you are at it. Also some more comments would certainly help! I don't understand e.g. what the three new functions in IAssistant are supposed to do... And please please please, don't inline all code, separate it cleanly between header (with apidox please?) and implementation...</pre>
</blockquote>
<p>On February 21st, 2012, 10:39 p.m., <b>Milian Wolff</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;">oh and please - could you add unit tests? this should be possible I think...</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">"Assistant Stacking":
Currently if a sig assist shows, and then a rename assist is shown, the sig assist is destroyed (you'll never see it). A simple solution would be to hide the sig assist and show it when the rename assist is acknowledged. In any case, I removed it, because it'll take some work.
The only thing this patch changes is
1. "If changing a function declaration, the signature assistant renames the uses in the definition"
"Love, style-wise":
On one hand I do feel like I deserve a beating for throwing up a diff with as many ugly green lines as this one, but on the other, I really just left the code exactly as it is. You can see the ugly stuff supposedly added in parseJobFinished is the same as the removed stuff (perhaps it was the commented out code I removed).
"Three new functions":
the getRenameActions function gets the rename actions.
The other two functions are just refactoring stuff out of the rather large parseJobFinished function so it's not a PITA to work with. Let me know what you want on that.
"don't inline all code":
The RenameAction class used to be in the RenameAssistant's .cpp, exactly as it is, in like manner to the SignatureAssistant's AdaptSignatureAction before it. Only now it's included in two .cpp files, again for minimal changes. I'll change that if there's no reason for it to be inlined like that.
"tests":
I'm pretty sure there aren't any code assistant tests...?
In summary:
This is a small feature, I think I only improved the surrounding code when adding it, but there's certainly plenty to do in the SigAssist. If you don't want this area touched until the surrounding area is cleaned up, I'll have to discard this until I have time to do so.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 21st, 2012, 10:33 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/103817/diff/3/?file=49576#file49576line50" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/codegen/renameaction.h</a>
<span style="font-weight: normal;">
(Diff revision 3)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><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">50</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KDevelop</span><span class="o">::</span><span class="n">DUChainReadLocker</span> <span class="n">lock</span><span class="p">(</span><span class="n">KDevelop</span><span class="o">::</span><span class="n">DUChain</span><span class="o">::</span><span class="n">lock</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;">new code shouldn't pass anything to the lock's ctor. i.e.:
DUChainReadLocker lock;</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;">This isn't, strictly speaking, new code. It's been moved from a .cpp file as-is. An easy change though.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 21st, 2012, 10:33 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/103817/diff/3/?file=49579#file49579line301" style="color: black; font-weight: bold; text-decoration: underline;">languages/cpp/codegen/signatureassistant.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">class AdaptSignatureAction : public KDevelop::IAssistantAction {</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">301</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">while</span> <span class="p">(</span><span class="n">m_renameActions</span><span class="p">.</span><span class="n">size</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;">could it happen that this is never called? yes, or - if the assistant is not applied? then we'd leak the rename actions.
so add a qDeleteAll to the ctor of IAssistantAction</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;">*facepalm*</pre>
<br />
<p>- Olivier Jean de</p>
<br />
<p>On February 17th, 2012, 10:50 a.m., Olivier Jean de Gaalon 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.</div>
<div>By Olivier Jean de Gaalon.</div>
<p style="color: grey;"><i>Updated Feb. 17, 2012, 10:50 a.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;">After editing function declaration, when updating the definition to match, rename uses of renamed declarations.
Note:
Removed the assistant stacking since the sig assist depends on being destroyed (and it was hacking where change is needed). Shouldn't be much work to fix that, but time I don't have now...</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;">Using the signature assistant.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=255706">255706</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>languages/cpp/codegen/renameaction.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>languages/cpp/codegen/renameassistant.cpp <span style="color: grey">(9bbdce7)</span></li>
<li>languages/cpp/codegen/signatureassistant.h <span style="color: grey">(2c6853d)</span></li>
<li>languages/cpp/codegen/signatureassistant.cpp <span style="color: grey">(0231e90)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/103817/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>