<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/101589/">http://git.reviewboard.kde.org/r/101589/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">generally looks good to me, but there are a few questions (see below). Esp. some comments on the new code would help me tremendously :)</pre>
<br />
<div>
<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/101589/diff/1/?file=24319#file24319line73" style="color: black; font-weight: bold; text-decoration: underline;">shell/workingsets/workingset.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">WorkingSet::WorkingSet(QString id, QString icon)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">73</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">WorkingSet</span><span class="o">::</span><span class="n">saveFromArea</span><span class="p">(</span><span class="n">Sublime</span><span class="o">::</span><span class="n">Area</span><span class="o">*</span> <span class="n">a</span><span class="p">,</span> <span class="n">Sublime</span><span class="o">::</span><span class="n">AreaIndex</span> <span class="o">*</span> <span class="n">area</span><span class="p">,</span> <span class="n">KConfigGroup</span> <span class="o">&</span><span class="hl"> </span><span class="n"><span class="hl">g</span>roup</span><span class="p">)</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">73</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">WorkingSet</span><span class="o">::</span><span class="n">saveFromArea</span><span class="p">(</span> <span class="n">Sublime</span><span class="o">::</span><span class="n">Area</span><span class="o">*</span> <span class="n">a</span><span class="p">,</span> <span class="n">Sublime</span><span class="o">::</span><span class="n">AreaIndex</span> <span class="o">*</span> <span class="n">area</span><span class="p">,</span> <span class="n">KConfigGroup</span> <span class="o">&</span><span class="n"><span class="hl">wsG</span>roup</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">KConfigGroup</span></span><span class="hl"> </span><span class="o"><span class="hl">&</span></span><span class="n"><span class="hl">areaGroup</span></span><span class="hl"> </span><span class="p">)</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">maybe use setGroup instead of wsGroup? just a personal style question though, ignore me if you don't like it :)</pre>
</div>
<br />
<div>
<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/101589/diff/1/?file=24319#file24319line80" style="color: black; font-weight: bold; text-decoration: underline;">shell/workingsets/workingset.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">WorkingSet::WorkingSet(QString id, QString icon)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">80</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">s</span>ubgroup</span><span class="p"><span class="hl">.</span></span><span class="n"><span class="hl">delete</span>Group</span><span class="p"><span class="hl">(</span>);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">80</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">KConfigGroup</span></span><span class="hl"> </span><span class="n"><span class="hl">areaS</span>ubgroup</span><span class="p"><span class="hl">(</span></span><span class="o"><span class="hl">&</span></span><span class="n"><span class="hl">area</span>Group</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="s"><span class="hl">"0"</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">where has the deleteGroup gone to? is this intended?</pre>
</div>
<br />
<div>
<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/101589/diff/1/?file=24319#file24319line86" style="color: black; font-weight: bold; text-decoration: underline;">shell/workingsets/workingset.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">WorkingSet::WorkingSet(QString id, QString icon)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">86</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">s</span>ubgroup</span><span class="p"><span class="hl">.</span></span><span class="n"><span class="hl">delete</span>Group</span><span class="p"><span class="hl">(</span>);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">86</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">KConfigGroup</span></span><span class="hl"> </span><span class="n"><span class="hl">areaS</span>ubgroup</span><span class="p"><span class="hl">(</span></span><span class="o"><span class="hl">&</span></span><span class="n"><span class="hl">area</span>Group</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="s"><span class="hl">"1"</span></span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">where has the deleteGroup gone to? is this intended?</pre>
</div>
<br />
<div>
<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/101589/diff/1/?file=24319#file24319line206" style="color: black; font-weight: bold; text-decoration: underline;">shell/workingsets/workingset.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void WorkingSet::loadToArea(Sublime::Area* area, Sublime::AreaIndex* areaIndex, bool clear) {</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">199</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KConfigGroup</span> <span class="n">areaGroup</span> <span class="o">=</span> <span class="n">setConfig</span><span class="p">.</span><span class="n">group</span><span class="p">(</span><span class="n">m_id</span> <span class="o">+</span> <span class="n">area</span><span class="o">-></span><span class="n">title</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">a unique separator between id and title would maybe be good to make sure? something like '|' ?! or even a subgroup?</pre>
</div>
<br />
<div>
<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/101589/diff/1/?file=24319#file24319line212" style="color: black; font-weight: bold; text-decoration: underline;">shell/workingsets/workingset.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void WorkingSet::loadToArea(Sublime::Area* area, Sublime::AreaIndex* areaIndex, bool clear) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">211</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">foreach</span><span class="p">(</span><span class="n">Sublime</span><span class="o">::</span><span class="n">MainWindow</span><span class="o">*</span> <span class="n">window</span><span class="p">,</span> <span class="n">Core</span><span class="o">::</span><span class="n">self</span><span class="p">()</span><span class="o">-></span><span class="n">uiControllerInternal</span><span class="p">()</span><span class="o">-></span><span class="n">mainWindows</span><span class="p">())</span> <span class="p">{</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">good cleanup :)</pre>
</div>
<br />
<div>
<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/101589/diff/1/?file=24319#file24319line251" style="color: black; font-weight: bold; text-decoration: underline;">shell/workingsets/workingset.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void WorkingSet::loadToArea(Sublime::Area* area, Sublime::AreaIndex* areaIndex, bool clear) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">250</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">areaIndex</span></span><span class="hl"> </span><span class="o"><span class="hl">=</span></span><span class="hl"> </span><span class="n">areaIndex</span><span class="o">-></span><span class="n">first</span><span class="p">();</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">234</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">Q_ASSERT</span></span><span class="p"><span class="hl">(</span></span><span class="n">areaIndex</span><span class="o">-></span><span class="n">first</span><span class="p">()<span class="hl">)</span>;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">this looks unrelated?</pre>
</div>
<br />
<div>
<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/101589/diff/1/?file=24319#file24319line256" style="color: black; font-weight: bold; text-decoration: underline;">shell/workingsets/workingset.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void WorkingSet::loadToArea(Sublime::Area* area, Sublime::AreaIndex* areaIndex, bool clear) {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">255</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="kt">int</span> <span class="n">viewCount</span> <span class="o">=</span> <span class="n">group</span><span class="p">.</span><span class="n">readEntry</span><span class="p">(</span><span class="s">"View Count"</span><span class="p">,</span> <span class="mi">0</span><span class="p">);</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">238</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QHash</span><span class="o"><</span><span class="n">QString</span><span class="p">,</span> <span class="n">Sublime</span><span class="o">::</span><span class="n">View</span><span class="o">*></span> <span class="n">viewsBySpec</span><span class="p">;</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">some documents would be cool on your new code here, I know the old code is not documented either, but you now seem to have understood it to some degree. please help us other poor souls ;-)</pre>
</div>
<br />
<p>- Milian</p>
<br />
<p>On June 12th, 2011, 3:01 a.m., Olivier Jean de Gaalon 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 Olivier Jean de Gaalon.</div>
<p style="color: grey;"><i>Updated June 12, 2011, 3:01 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;">This patch saves and loads certain area-specific states for each working set, in particular, it saves
*The document state (cursor position, or selection)
*The currently active document
*The order of the documents
For each area, so that when you reopen KDevelop, all is as you left it, in each area.
Also, this fixes a bug where when you split a view and then unsplit it (close the split off view), when you restarted KDevelop it would come back (unless you closed the entire working set or closed the split view in *all* the areas).
If this fixes any reported bugs, let me know what they are.
It's possible this isn't the best way to do this as I'm not familiar with this area at all, but it's unlikely to break horribly, and is even backwards compatible (ie, you won't lose your current workingsets).</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;">Starting and stopping kdevelop, splitting and unsplitting, restarting kdevelop, reordering documents, changing cursor positions, restarting kdevelop...</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>shell/workingsets/workingset.h <span style="color: grey">(512c5cb)</span></li>
<li>shell/workingsets/workingset.cpp <span style="color: grey">(c494e51)</span></li>
<li>sublime/areaindex.h <span style="color: grey">(7dfc13f)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/101589/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>