<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, 11:51 a.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;">Current master requires KDE 4.6 already I believe (if not it should due to the huge amount of fixes related to kate).
I didn't look at the patch itself yet, but how does your code handle the case of two project views being restored? As far as I recall there's no API for plugins/views to find out which kconfig group to use to restore view-state from since this is completely hidden in sublime/shell code.
Apart from that IMHO storing this in the project configuration is wrong. The view state has nothing to do with the project itself, its about view-state and view-state should go into the session configuration file. The project-configuration file is for project-specific data, which expansion state is not, since I can load the same project into two different sessions and would not want one session to overwrite the view-state of another session for that project.</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;">CMakeLists still has set(KDE_MIN_VERSION "4.5.0") I can bump that if you like, but that doesn't belong in this patch considering the remove of the #if/#else (below).
Re: multiple project views. Until now I was not aware that this was even possible. I will address that in my updated patch (which will be hopefully tomorrow).</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>