<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="https://git.reviewboard.kde.org/r/117922/">https://git.reviewboard.kde.org/r/117922/</a>
     </td>
    </tr>
   </table>
   <br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On May 2nd, 2014, 8:31 p.m. UTC, <b>Olivier Jean de Gaalon</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;">Hmm, this doesn't do parameter change tracking, which means the default argument handling can easily do bad things (it also means you can't rename). If you prefer not to do change tracking with this initial patch, just make the default arguments very conservative (only assign when types and param count didn't change), and we can port the old logic over later.

(and yes, change tracking cannot be foolproof, but at least it could pass the existing tests...)</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;">But hmm, regarding tests, I'm not sure it's useful to add tests depending on how much they need to be tied to the ClangCodeAssistant. Perhaps it makes sense to go without for now... Milian?</pre>
<br />










<p>- Olivier Jean de</p>


<br />
<p>On May 2nd, 2014, 7:56 p.m. UTC, David Stevens wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for KDevelop.</div>
<div>By David Stevens.</div>


<p style="color: grey;"><i>Updated May 2, 2014, 7:56 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;">This is a preliminary implementation of the adjust signature helper. It works fairly well under normal usage, but it runs into problems if the user starts trying to use it at a high frequency. There are some rather fundamental issues both due to the different information organization (files vs translation units) and due to the discrepencies between what KDevelop has in the text editor and what clang has in the translation units. Hopefully I can get some feedback on the implementation.</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;">Manual. Any unit tests would be deeply tied into the GUI, so I'm not sure how to go about writing those. The old c++ plugin unfortunately doesn't have any tests to work off of.</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>clangsupport.h <span style="color: grey">(1316f88)</span></li>

 <li>clangsupport.cpp <span style="color: grey">(5e3f464)</span></li>

 <li>codecompletion/completionhelper.cpp <span style="color: grey">(dfaf6b3)</span></li>

 <li>codegen/CMakeLists.txt <span style="color: grey">(0b913bd)</span></li>

 <li>codegen/clangsignatureassistant.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>codegen/clangsignatureassistant.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>codegen/codeassistant.h <span style="color: grey">(PRE-CREATION)</span></li>

 <li>codegen/codeassistant.cpp <span style="color: grey">(PRE-CREATION)</span></li>

 <li>duchain/clangtypes.h <span style="color: grey">(c7293a9)</span></li>

 <li>duchain/clangtypes.cpp <span style="color: grey">(11760c8)</span></li>

 <li>tests/CMakeLists.txt <span style="color: grey">(c2fd36d)</span></li>

 <li>util/clangutils.h <span style="color: grey">(3516821)</span></li>

 <li>util/clangutils.cpp <span style="color: grey">(a74017c)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/117922/diff/" style="margin-left: 3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>