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





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 5th, 2012, 4:17 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;">I agree with Andreas, this should be per-session.</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;">I'm actually very happy to hear you both say that as that's where I initially felt it belonged.  I'll gladly switch it over.</pre>
<br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 5th, 2012, 4:17 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/103619/diff/1/?file=45327#file45327line329" style="color: black; font-weight: bold; text-decoration: underline;">plugins/projectmanagerview/projecttreeview.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </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; ">void ProjectTreeView::saveState(IProject *project)</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">329</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#if KDE_IS_VERSION( 4, 6, 0 )</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;">I wonder about this, as it is a ABI/source-incompatible change. How could this happen with kdelibs code?

Also: I don't see setTreeView here:

http://api.kde.org/4.5-api/kdelibs-apidocs/kdeui/html/kviewstatesaver_8h_source.html

while the apidox in here:

http://api.kde.org/4.5-api/kdelibs-apidocs/kdeui/html/classKViewStateSaver.html#8ea1426a563a88733f26f55d837946ee

do refer to a "setTreeView", it is not actually listed as a method there either.</pre>
 </blockquote>



 <p>On January 5th, 2012, 4:53 p.m., <b>Andreas Pakulat</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;">It seems like Andrew got confused by the api docs and didn't really compile against a 4.5 kdelibs. Both 4.5 and 4.6 have the setView function in the header so apparently no breakage of ABI/API or source-compatibility, just a typo/oversight in the documentation (note the typo is also in both versions of the docs)</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;">I read the API documentation for 4.5 here: http://www.purinchu.net/kdelibs-apidocs/kdeui/html/classKViewStateSaver.html which now I see has a different interface than the one at api.kde.org.

This means the #if/#else is not needed even for 4.5.  It also means I should never rely on a mirror and always use api.kde.org.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 5th, 2012, 4:17 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/103619/diff/1/?file=45330#file45330line38" style="color: black; font-weight: bold; text-decoration: underline;">project/projectmodelsaver.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 1)

    </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; ">QModelIndex ProjectModelSaver::indexFromConfigString(const QAbstractItemModel *model, const QString &key) const</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">38</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="k">if</span><span class="p">(</span> <span class="o">!</span><span class="n">proxy</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;">I'd prefer {} even for one-liner conditionals.

furthermore please stay consistent with spaces, here you do:

if( a )

further down you do

if (a)</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;">A man after my own heart!

For the future, are {} also preferred when updating an existing file that does not use them for single-line conditionals?  (ie: which is more important, consistency within a file, or transitioning to always use {}'s?)</pre>
<br />




<p>- Andrew</p>


<br />
<p>On January 3rd, 2012, 6:29 a.m., Andrew Fuller 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 Andrew Fuller.</div>


<p style="color: grey;"><i>Updated Jan. 3, 2012, 6:29 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;">Save/Restore the ProjectTreeView.  This enables a more seamless transition when returning to a project that was closed.

I'm using KViewStateSaver to do the actual save/restore which unfortunately has an interface change between KDE 4.5 and 4.6, so there's a small #if #else (twice) in the code.  This can be removed when KDE 4.5 is no longer supported.

I was directed in #kdevelop to store this in the project config files (as done in this patch).  There are two implications of storing it there since the tree view combines all the projects together.  The first is that when multiple projects are open, then the stored values may contain data for other projects (in practice this is irrelevant, it just feels impure).  The other is that because it gets restored piecemeal that the scrollbar is incorrect.  This is also irrelevant.  Just imperfect.

This would also work very well stored in the session config.  To do this I would only need to add a signal for when the session is about to be closed.  Currently the QuitSession signal triggers all the cleanup and I would need to guarantee that the settings are saved before the model is emptied.

Finally, I did a best-guess to follow the coding style within kdevplatform, but some files were conflicting and http://www.kdevelop.org/mediawiki/index.php/Coding_Guidelines (from Architecture.dox) is MIA.  So please point out any style errors as well.</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;">Load a session, Exit, and return.  VoilĂ !  The tree is right where you left it.</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>plugins/projectmanagerview/projecttreeview.h <span style="color: grey">(16893f0713b83a78a5b025a55fcfa082265d4419)</span></li>

 <li>plugins/projectmanagerview/projecttreeview.cpp <span style="color: grey">(8237af513627c0459d571de2bccc275e8a4b0ae9)</span></li>

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

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

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

</ul>

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




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








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