<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/106581/">http://git.reviewboard.kde.org/r/106581/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 2nd, 2012, 10:55 a.m., <b>David Faure</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/106581/diff/3/?file=88080#file88080line1875" style="color: black; font-weight: bold; text-decoration: underline;">kfile/kfilewidget.cpp</a>
<span style="font-weight: normal;">
(Diff revision 3)
</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; ">void KFileWidgetPrivate::writeConfig(KConfigGroup &configGroup)</pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void KFileWidgetPrivate::writeViewConfig()</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1872</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">KFileWidgetPrivate</span><span class="o">::</span><span class="n">writeConfig</span><span class="p">(</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">configGroup</span></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">1857</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">KFileWidgetPrivate</span><span class="o">::</span><span class="n">write<span class="hl">View</span>Config</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;">OK, I was about to react against the move to a member variable configGroup (which basically means "accept() will use the last group that was passed to the public readConfig method"), but in fact I found the commit log for the addition of the public readConfig, and that's apparently what was wanted...
commit 245888935048691e65a89ad8786b896d7bb3be7d
Author: Andreas Pakulat <apaku@gmx.de>
Date: Tue Jun 30 20:35:49 2009 +0000
Provide a readConfig method so one can let the widget read its config from a
custom config group. Useful when embedding this widget somewhere else than the
kfiledialog.
OK then. But maybe Apaku should review this too :)</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;">Since the code comes from Apaku I realized it must be related to the open project dialog in KDevelop. It is indeed used here: http://lxr.kde.org/source/extragear/kdevelop/kdevplatform/shell/openprojectpage.cpp#70
With the changes I propose, view settings are always saved to kdeglobals/[KFileDialog Settings], even if KFileWidget::readConfig() was called. Would it make more sense to save view settings to the custom config group in this case?</pre>
<br />
<p>- Aurélien</p>
<br />
<p>On October 2nd, 2012, 10:55 a.m., Aurélien Gâteau 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 kdelibs and Andreas Pakulat.</div>
<div>By Aurélien Gâteau.</div>
<p style="color: grey;"><i>Updated Oct. 2, 2012, 10:55 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 makes KFileDialog remember settings such as which view mode is selected and whether the places sidebar should be visible.
Original code tried to save those to kdeglobals so that changes would be shared among all applications but it did so the wrong way. The patch writes the configuration to kdeglobals correctly, but saves the KDirOperator to the application config file (KDirOperator configuration settings are sort settings, show preview, show hidden files, view style (icon, detail, treeview))
There are two reasons for not saving KDirOperator config to kdeglobals:
1. It is right now not possible to tell KDirOperator::writeConfig() to save to kdeglobals. It could be done by adding a new version of writeConfig() which would accept a KConfigBase::WriteFlags argument though.
2. It probably would not be a good idea to remember KDirOperator settings globally anyway because depending on the application one may want to use different settings.
For example if user wants to select images or videos he might set the file dialog to show big icons and the preview pane (so that videos can be played). This setup would however not be adapted in an application where one wants to select a text file.</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;">Tested with two different KDE applications. Settings are correctly remembered.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=139475">139475</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kfile/kfilewidget.cpp <span style="color: grey">(8e2f967)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/106581/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>