<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/124200/">https://git.reviewboard.kde.org/r/124200/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On Lipiec 16th, 2015, 12:27 rano CEST, <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;">can you add a multi-file unit test for this as well?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">in general though, I do wonder whether this shows some serious issues with the clang API for refactoring. Renaming a variable based on the data we store in the DUChain cache could be done much quicker. Of course that cache must be up2date, but that's the same for you here... Maybe what we have had in oldcpp (minus the horrible code) wasn't that bad after all. For these "simple" rename refactorings at least, the old architecture could serve us quite well. What I'd be interested in seeing next is other refactorings which we so far never had. Like negating conditionals, putting code into a function, wrapping string literals, ... for that one really needs a proper AST access and probably libTooling. And it's purely local thus does not require all other files to be updated.</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;">I agree. This tool is not breathtaking. But it has its advantage - it showed a few big issues with Clang tooling.
It's vary true - renaming could (and should) be done using cached informations about code. This is different approach (match-and-replace), very expensive...</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You are right about these local refactorings, they can be easier to apply (and less expensive) but on the other hand refactoring tools seems to be the one solution to refactor big pieces of code. Everybody can negate conditionals (its tedious, but possible). The other situation is with applying some (small) changes to almost any source file in project. It must be done automatically...
But I see the point, I also noticed that Clang would be much better (to use) at these "local" changes than "whole project". Maybe some arrangements should be changed to see more "local" refactorings instead of some "global" (which concerns almost exclusively refactoring of "interface").</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">PS I can add more test and will do it. I couldn't add them in this change pack - UTF is "a few" (10?) commits ahead. Tests are coming separately..</p></pre>
<br />
<p>- Maciej</p>
<br />
<p>On Lipiec 14th, 2015, 1:44 po południu CEST, 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 Lip 14, 2015, 1:44 po południu</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;">Introduced rename variable refactoring.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch provides preliminary support for variable rename refactoring.
Currently only one project is supported.
Currently it is required to save all opened documents (cache is not ready).
Regeneration of compile_commands.json needs to be polished. How to get appropriate notification (IProjectBuilder::configured signal)? As a workaround i simply restarted KDevelop to get this regenerated.
To use this refactoring right click on variable declaration (definition) and select "rename" refactoring.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Full renaming support will consist of a few parts - one of them is renaming around VarDecl (this patch).
Quite early, but i wanted to see that this work is going into some direction after month of coding of pure interfaces/details.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">How to split diffs? I noticed that RBTool created diff consisting of all changes ignoring other pending request. I wanted only the last one, because earlier commits are subject of other review.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">NOTE: This is very conservative refactoring. It runs ClangTool on every TU defined in compile_commands.json. Such refactorings are very slow. I will have to work more on it (make some heuristics reducing set of TUs which need processing).</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 on very small code bases as cache and compile_commands.json regeneration is not fully functional.</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/utils.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/utils.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamevardeclrefactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/renamevardeclrefactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringmanager.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/kdevrefactorings.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/debug.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/debug.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/CMakeLists.txt <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124200/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>