<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/124369/">https://git.reviewboard.kde.org/r/124369/</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 24th, 2015, 11:15 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;">the dialog UI needs to be revamped, the functionality and code is nice though - good work!</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">a) the line edit for the field to encapsulate, why is it a line edit? or is it read only? what happens if the user changes the contents?
b) the dialog structure is really awkward, I have to say. please use a form layout with labels left and widgets right. Insteaqd of the radio boxes, use combo boxes, and don't use a sentence structure to layout the widgets, that will break horribly once the dialog is translated (think about RTL languages!). And maybe, it's best to let the user configure the signature in pure line edits? Adding combo boxes for noexcept etc. pp. will never scale. Or maybe even a text edit so the bodies can be edited directly as well? My current approach would be something like this: http://i.imgur.com/wWHOIHd.png the sources you can find at https://paste.kde.org/pdrz1iqo6 . I'm only unsure whether we can easily get the name of the setter argument to create the body. And should we also allow configuration about inlining the bodies? Probably a good idea for the future?</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;">ad a) line edit is read only. it is information for user which field is going to be encapsulated. user cannot change contents.
ad b) i'm very bad GUI designer, that's fact... radio boxes were used because need one click instead of two in combo boxes. scaling is an issue, i like idea to provide text edits with bodies (prefilled with default generated body). Currently implementation is placed in (all) RecordDecl (class or structure). I'm still thinking how to find appropriate place in appropriate implementation file. I can get "path" of record decls up to TU record decl (root), but then how to find the same scope (place) in another TU (and prove that this TU will be available available after compilation to all uses)?</p></pre>
<br />
<p>- Maciej</p>
<br />
<p>On Lipiec 23rd, 2015, 4:07 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 23, 2015, 4:07 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;">Encapsulate field refactoring</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Displays dialog</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Transforms "use" site - replaces direct uses of encapsulated variable by calls to getter and setter method/function.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">Generates getter and setter implementation</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Attached image presents GUI with its default content (prefilled by heuristics on DeclaratorDecl AST node).</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, unit testing</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/encapsulatefielddialog.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/encapsulatefielddialog.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/encapsulatefielddialog.ui <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/encapsulatefieldrefactoring.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/encapsulatefieldrefactoring.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/encapsulatefieldrefactoring_changepack.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/encapsulatefieldrefactoring_changepack.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/redeclarationchain.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/refactoringmanager.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/tudecldispatcher.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>refactoring/tudecldispatcher.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>
<li>tests/CMakeLists.txt <span style="color: grey">(20d17efae9a2a77cd7ef76bf7484ccfcf12e4cd8)</span></li>
<li>tests/test_encapsulatefield.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>tests/test_encapsulatefield.cpp <span style="color: grey">(PRE-CREATION)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/124369/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/15/ea15c55b-cc4d-4dfa-ac34-8c568a93701d__snapshot3.png">Encapsulate field dialog</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>