<table><tr><td style="">hein marked 3 inline comments as done.<br />hein 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/D13073">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/D13073#inline-67622">View Inline</a><span style="color: #4b4d51; font-weight: bold;">broulik</span> wrote in <span style="color: #4b4d51; font-weight: bold;">tasktools.cpp:374</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;"><tt style="background: #ebebeb; font-size: 13px;">isEmpty</tt> (unless consistent with the rest but it seems to be mixed already)</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Will change.</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/D13073#inline-67618">View Inline</a><span style="color: #4b4d51; font-weight: bold;">broulik</span> wrote in <span style="color: #4b4d51; font-weight: bold;">tasktools.cpp:228</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">This comment makes it sound like this pretends to be a generic solution to a particular workaround. But you told me there's also Telegram and others being affected (fixed) by this, right?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">I started doing this for the Wine thing based on a user bug report. But for some reason I did wind up with two .desktop files for Telegram on my system, I think one from the Fedora package and one from ... a Flatpak? Their tarball? Who knows. In any case, this algo helps pick the right one there, too. The general idea of "if we find more than one service, the one with a menuId that's more closely related to the search term we found them with" seems to be better than "just use whatever KSTT returns first", considering KSTT does no sorting whatsoever.</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/D13073#inline-67620">View Inline</a><span style="color: #4b4d51; font-weight: bold;">broulik</span> wrote in <span style="color: #4b4d51; font-weight: bold;">tasktools.cpp:243</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">Is there some nicer algo in place of this loop?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">You tell me? :)</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/D13073#inline-67621">View Inline</a><span style="color: #4b4d51; font-weight: bold;">broulik</span> wrote in <span style="color: #4b4d51; font-weight: bold;">tasktools.cpp:245</span></div>
<div style="margin: 8px 0; padding: 0 12px; color: #74777D;"><p style="padding: 0; margin: 8px;">You're mutating the list (argument <tt style="background: #ebebeb; font-size: 13px;">services</tt> is a non-<tt style="background: #ebebeb; font-size: 13px;">const</tt> reference) and also returning it. Is this intentional?</p></div></div>
<div style="margin: 8px 0; padding: 0 12px;"><p style="padding: 0; margin: 8px;">Yes. It's just faster and less costly than actually sorting the list. This is a bit ugly, but it takes the stance of "this is a private anonymous function so it can't hurt any lib user, and if this sorting algo ever needs to be more sophisticated we'll fill it in then".</p></div></div></div></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/D13073">https://phabricator.kde.org/D13073</a></div></div><br /><div><strong>To: </strong>hein, broulik, davidedmundson<br /><strong>Cc: </strong>cfeck, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart<br /></div>