<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/110674/">http://git.reviewboard.kde.org/r/110674/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On May 27th, 2013, 11:02 p.m. UTC, <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/110674/diff/2/?file=146722#file146722line31" style="color: black; font-weight: bold; text-decoration: underline;">staging/kde4support/src/kdeui/ktoolbarspaceraction.h</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<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">31</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> *</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;">@deprecated since 5.0, use XXX instead.
What should the k3b, amarok, and kopete people use instead? They need to know...
QToolBar::addSeparator? It doesn't have width/minWidth/maxWidth stuff.
Hmm, OK, going through the uses of KToolBarSpacerAction in lxr.kde.org seems to indicate that none of the apps set these things anyway, so maybe addSeparator is enough indeed.
Anyway - please always add @deprecated since 5.0 and the suggested replacement, for the benefit of people porting the code.</pre>
</blockquote>
<p>On May 29th, 2013, 1:01 p.m. UTC, <b>Aleix Pol Gonzalez</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;">This is tricky, QToolBar does a different thing, it adds a visible separator in the toolbar instead of adding a space as KToolBarSpacerAction is doing.
Maybe this class should stay?</pre>
</blockquote>
<p>On June 3rd, 2013, 10:23 a.m. UTC, <b>David Faure</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'm not sure it's worth it. If it is, the right place would be in Qt, but... hard to justify, possibly?
I think the best thing to do would be to talk to the k3b developers (they seem to be the main users of that class; otherwise amarok and kopete), and see if they really think this should exist (in KF5 or Qt). It could be that this predates QToolBar::addSeparator or that they didn't see it because they looked in kdelibs first.</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;">** See screenshots **
As far as I can see, QToolBar::addSeparator and the KToolBarSpacerAction do different things. We can argue that the KToolBarSpacerAction is not needed. For example in k3b, the spacer is used to push the configuration button to the right corner. It's arguable that it's desirable, but they do different things.</pre>
<br />
<p>- Aleix</p>
<br />
<p>On June 3rd, 2013, 2:17 p.m. UTC, Aleix Pol Gonzalez wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for KDE Frameworks.</div>
<div>By Aleix Pol Gonzalez.</div>
<p style="color: grey;"><i>Updated June 3, 2013, 2:17 p.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;">Just moves the file, nobody seems to be using it in kdelibs anyway.</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;">Builds and installs fine.</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>staging/kde4support/src/kdeui/ktoolbarspaceraction.h <span style="color: grey">(32e8fb2)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/110674/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<ul>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/06/03/k3b-nospaceraction.png">::addSeparator()</a></li>
<li><a href="http://git.reviewboard.kde.org/media/uploaded/files/2013/06/03/k3b-spaceraction.png">KToolBarSpacerAction</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>