<table><tr><td style="">davidedmundson 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/D5504" rel="noreferrer">View Revision</a></tr></table><br /><div><div><p>appsMatch depends on AppId and LauncherURLWithoutIcon</p>
<p>You're handling launcherTasksModel changing, but can we rely on this data being static for a proxyIndex?</p>
<p>If not, we need to connect to TasksModel::dataChanged for those roles and potentially then emit that HasLauncher changed.</p>
<hr class="remarkup-hr" />
<p>Having read your other patch you could argue that you don't need to signal this role as you (currently) only use this role from method calls triggered by other events....but if you take that POV, there's no point doing the launcherListChanged connect.</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>R120 Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D5504" rel="noreferrer">https://phabricator.kde.org/D5504</a></div></div><br /><div><strong>To: </strong>hein, Plasma, davidedmundson<br /><strong>Cc: </strong>plasma-devel, spstarr, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol<br /></div>