<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/124300/">https://git.reviewboard.kde.org/r/124300/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 25th, 2015, 1:28 p.m. UTC, <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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">hm have you ever tried out oldcpp's "change signature" feature? That one was <em style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">much</em> better useability wise. You simply edited the signature, and an assistant asked you whether you want to copy the changes to the corresponding define/declaration. That would be much better to copy. Having an editor dialog like you propose here is going to be cumbersome and I don't see myself using that I'm afraid to admit.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What do you say? Can you reuse this code to implement the oldcpp behavior for changing a signature?</p></pre>
</blockquote>
<p>On July 25th, 2015, 5:01 p.m. UTC, <b>Maciej Poleski</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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You mean "adapt signature" (after change of declaration in source code)?
It is something different. I don't know how to track changes in source code so gracefully, but the main issue here is that i wan't to track changes very precisely so that I preserve as much original information as possible. The most important is not changing signature (in modern code bases we have (one forward declaration +) one definition - at most 2 entities), but adapting uses (arbitrary number of function calls and references). If we only change order of parameters (or remove some) - we can automatically adapt all uses so that code remians semantically equal (change of function name doesn't matter - we always can handle that). Only introduction of new parameters breaks code. This a bit strange dialog (QTableView + 4 QToolButton to be precise) has one important task: collect information in form of binding from original signature to new one.
As I previously mentioned I'm not a good GUI designer. I can give features, usabilitity is always an issue. But I can hardly find necessary informations in "adapt signature" style (even if i forget that i don't know how to handle direct changes in source code (how to use Clang to handle them, to be precise)).</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Ah ok, and the adapt signature assistant is of course already implemented as well, my bad. Question is, if we could eventually reuse this code from there as well. Anyhow, I'll review the code now, and consider my statement above invalidated.</p></pre>
<br />
<p>- Milian</p>
<br />
<p>On July 13th, 2015, 9:34 p.m. UTC, Maciej Poleski wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for KDevelop.</div>
<div>By Maciej Poleski.</div>
<p style="color: grey;"><i>Updated July 13, 2015, 9:34 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdev-clang
</div>
<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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Change signature refactoring</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I decided not to implement rename function refactoring. Change signature will provide this functionality.
We can rename all functions except constructors, destructors, conversion operators, operators.
Operators must preserve number of parameters (imagine 2+2 and operator+).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This functionality allows to:
- change return type
- change function name
- change parameter types (preserving identity, tracking of this is not perfect - we lose this information)
- change order of parameters
- introduce new parameters
- remove existing parameters</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Somewhere here should be screenshoot of this dialog...</p></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;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Manual testing of GUI. No transformation tests yet...</p></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>refactoring/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturedialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturedialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturedialog.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturerefactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturerefactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturerefactoringchangepack.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturerefactoringchangepack.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturerefactoringinfopack.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/changesignaturerefactoringinfopack.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/debug.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringmanager.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/utils.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/utils.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124300/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/07/08/e8a8db09-92f9-4fb6-ac4a-3ab2a5051033__snapshot1.png">snapshot</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>