<table><tr><td style="">dfaure added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D8773" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>I like the idea.</p>
<p>I'm just not sure about the exact wording of the action, but I'll let the VDG decide on that.</p></div></div><br /><div><strong>INLINE COMMENTS</strong><div><div style="margin: 6px 0 12px 0;"><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D8773#inline-38616" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">apol</span> wrote in <span style="color: #4b4d51; font-weight: bold;">ktoolbar.cpp:410</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Isn't it better to set a parent like for the other ona than use the QScopedPointer?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I'm also surprised by the inconsistency, and I bet future readers will be too.</p>
<p style="padding: 0; margin: 8px;">As to whether it should all be ported to QScopedPointer, I'm not convinced.</p>
<p style="padding: 0; margin: 8px;">Technically, I would bet that deletion by the parent is faster than deleting each children individually with e.g. QScopedPointer (because that will have to remove from the list of children in the parent, so it means N linear searches).</p></div></div><br /><div style="border: 1px solid #C7CCD9; border-radius: 3px;"><div style="padding: 0; background: #F7F7F7; border-color: #e3e4e8; border-style: solid; border-width: 0 0 1px 0; margin: 0;"><div style="color: #74777d; background: #eff2f4; padding: 6px 8px; overflow: hidden;"><a style="float: right; text-decoration: none;" href="https://phabricator.kde.org/D8773#inline-38669" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">ktoolbar.cpp:412</span></div>
<div style="font: 11px/15px "Menlo", "Consolas", "Monaco", monospace; white-space: pre-wrap; clear: both; padding: 4px 0; margin: 0;"><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">contextEditableAction</span><span style="color: #aa2211">-></span><span class="n">setChecked</span><span class="p">(</span><span class="n">q</span><span style="color: #aa2211">-></span><span class="n">toolBarsEditable</span><span class="p">());</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">connect</span><span class="p">(</span><span class="n">contextEditableAction</span><span class="p">.</span><span class="n">data</span><span class="p">(),</span> <span style="color: #aa2211">&</span><span class="n">QAction</span><span style="color: #aa2211">::</span><span class="n">toggled</span><span class="p">,</span> <span class="n">q</span><span class="p">,</span> <span class="p">[</span><span style="color: #aa4000">this</span><span class="p">](</span><span style="color: #aa4000">bool</span> <span class="n">toggled</span><span class="p">)</span> <span class="p">{</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: rgba(151, 234, 151, .6);"> <span class="n">q</span><span style="color: #aa2211">-></span><span class="n">setToolBarsEditable</span><span class="p">(</span><span class="n">toggled</span><span class="p">);</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Is this a case where &KToolBar::setToolBarsEditable would work, without the need for a lambda?</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R263 KXmlGui</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D8773" rel="noreferrer">https://phabricator.kde.org/D8773</a></div></div><br /><div><strong>To: </strong>elvisangelaccio, Frameworks, VDG, dfaure<br /><strong>Cc: </strong>apol<br /></div>