<table><tr><td style="">ngraham requested changes to this revision.<br />ngraham added a reviewer: elvisangelaccio.<br />ngraham added a comment.<br />This revision now requires changes to proceed.
</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/D22594">View Revision</a></tr></table><br /><div><div><p>This is a very well-formed patch, with good code (including correct usage of KUIT markup!), a good commit message, and a good screenshot. Well done! The only thing that could be improved to to use <tt style="background: #ebebeb; font-size: 13px;">arc</tt> for the next one. :) <a href="https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist" class="remarkup-link" target="_blank" rel="noreferrer">https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist</a></p>

<p>As for the feature itself, for the past few years I'd been opposed to this, on the basis that people should just use the built-in Baloo-based search instead. But unfortunately the requests keep coming in, and there are use cases that Baloo doesn't work for (e.g. on external disks), so there does seem to be a need, or at least a desire. And we do already have this accessible from the search panel, so this is basically just a shortcut to that. I'll give this patch a fair shake.</p>

<p>In general +1. Here are a few suggestions:</p>

<ol class="remarkup-list">
<li class="remarkup-list-item">Instead of showing a generic text, how about making it actually say "Search with <name of preferred search tool"?</li>
<li class="remarkup-list-item">For the keyboard shortcut let's use Alt instead of Shift. It is an alternate search, after all.</li>
<li class="remarkup-list-item">Whenever you change anything in a <tt style="background: #ebebeb; font-size: 13px;">.rc</tt> file, you need to bump the version number that appears at the top of the file.</li>
</ol></div></div><br /><div><strong>REPOSITORY</strong><div><div>R318 Dolphin</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D22594">https://phabricator.kde.org/D22594</a></div></div><br /><div><strong>To: </strong>pdabrowski, Dolphin, ngraham, elvisangelaccio<br /><strong>Cc: </strong>kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, emmanuelp, mikesomov<br /></div>