<table><tr><td style="">davidedmundson added inline comments.
</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/D3950" rel="noreferrer">View Revision</a></tr></table><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/D3950#inline-15780" rel="noreferrer">View Inline</a><span style="color: #4b4d51; font-weight: bold;">tasksmodel.cpp:388</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: #d0ffd0;">    <span class="n">QObject</span><span style="color: #aa2211">::</span><span class="n">connect</span><span class="p">(</span><span class="n">filterProxyModel</span><span class="p">,</span> <span style="color: #aa2211">&</span><span class="n">QAbstractItemModel</span><span style="color: #aa2211">::</span><span class="n">rowsRemoved</span><span class="p">,</span> <span class="n">q</span><span class="p">,</span>
</div><div style="padding: 0 8px; margin: 0 4px; background: #d0ffd0;">        <span class="p">[</span><span style="color: #aa4000">this</span><span class="p">](</span><span style="color: #aa4000">const</span> <span class="n">QModelIndex</span> <span style="color: #aa2211">&</span><span class="n">parent</span><span class="p">,</span> <span style="color: #aa4000">int</span> <span class="n">first</span><span class="p">,</span> <span style="color: #aa4000">int</span> <span class="n">last</span><span class="p">)</span> <span class="p">{</span>
</div></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I think it's a bit weird that we have a pair:</p>

<p style="padding: 0; margin: 8px;">groupingProxyModel::aboutToRemove<br />
filterProxyModel:rowsRemoved</p>

<p style="padding: 0; margin: 8px;">As it doesn't match up evenly.</p>

<p style="padding: 0; margin: 8px;">It'll work, but having them both the same will be less confusing long term.</p>

<p style="padding: 0; margin: 8px;">I think changing the first connect to filterProxyModel might result in logic being overall simpler. You'd be processing the launcher every time any window got removed, but that might fix what's in the bug reports.</p>

<p style="padding: 0; margin: 8px;">Other than that, the change looks good.</p></div></div></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R120 Plasma Workspace</div></div></div><br /><div><strong>BRANCH</strong><div><div>master</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3950" rel="noreferrer">https://phabricator.kde.org/D3950</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>hein, davidedmundson, mart<br /><strong>Cc: </strong>plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, andreaska, sebas<br /></div>