<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://svn.reviewboard.kde.org/r/5451/">http://svn.reviewboard.kde.org/r/5451/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 26th, 2010, 8:22 p.m., <b>Aaron Seigo</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="/r/5451/diff/1/?file=38404#file38404line340" style="color: black; font-weight: bold; text-decoration: underline;">trunk/KDE/kdelibs/plasma/corona.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 Corona::saveLayout(const QString &configName) 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">340</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">void</span> <span class="n">Corona</span><span class="o">::</span><span class="n">exportLayout</span><span class="p">(</span><span class="n">KConfigBase</span> <span class="o">&</span><span class="n">config</span><span class="p">,</span> <span class="n">QList</span><span class="o"><</span><span class="n">Containment</span><span class="o">*></span> <span class="n">containments</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;">this should probably take a KConfigGroup, since KConfigGroup can refer to the top level item, e.g. the whole config file, using the magic QString() name for the group.
unfortunately, Corona::importLayout() requires a KConfigBase which would make this assymetrical. iirc that bit of the API was written before i was aware of that KConfigGroup trick (which was actually undocumented until i found out about it :).
perhaps we should add an importLayout that takes a KConfigGroup, mark the importLayout which takes a KConfigBase as deprecaton and have it call the new one.
then we have a nice api with symmetry?</pre>
</blockquote>
<p>On September 27th, 2010, 7:01 a.m., <b>Chani Armitage</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;">so... void Corona::exportLayout(KConfigGroup *config, QList<Containment*> containments)
and void Corona::importLayout(KConfigGroup *config)</pre>
</blockquote>
<p>On September 27th, 2010, 8:49 a.m., <b>Aaron Seigo</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;">KConfigGroup &, but otherwise, yes...</pre>
</blockquote>
<p>On September 27th, 2010, 8:59 a.m., <b>Chani Armitage</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 thought it was bad form to use & without const...?</pre>
</blockquote>
<p>On September 27th, 2010, 9:14 a.m., <b>Chani Armitage</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;">oh, for import is would be const KConfigGroup &, duhh. :)
but for export, KConfigGroup* is more correct, right?</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;">it's usually considered bad form, but in the case it's fine and prevents having to beware of null pointers in the lib code and keeps the API as symmetric as possible. it's also how we've handled KConfigGroup elsewhere in libplasma, and there are other areas of kde that do similarly. for KConfigGroup it simpler and sensible to pass it around by reference in these cases.</pre>
<br />
<p>- Aaron</p>
<br />
<p>On September 27th, 2010, 2:38 p.m., Chani Armitage wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 Plasma.</div>
<div>By Chani Armitage.</div>
<p style="color: grey;"><i>Updated 2010-09-27 14:38:42</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 adds exportLayout to corona, which saves a group of containments to a config file and deletes them from the main config.
Activity::close() becomes a lot shorter by calling it, like this: m_corona->exportLayout(external, m_containments.values());
I feel a bit out of it today though, so tell me if I've missed anything obvious...</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;">closing an activity while locked is perfectly safe now :)</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>trunk/KDE/kdelibs/plasma/corona.h <span style="color: grey">(1179499)</span></li>
<li>trunk/KDE/kdelibs/plasma/corona.cpp <span style="color: grey">(1179499)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5451/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>