Added "Open Preferred Search Tool" action to Tools menu.
It runs preferred (topmost) external search tool as configured in the "More Search Tools" menu.
By default Ctrl+Shift+F shortcut is assigned to this action.
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 arc for the next one. :) https://community.kde.org/Infrastructure/Phabricator#Using_Arcanist
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.
In general +1. Here are a few suggestions:
- For the keyboard shortcut let's use Alt instead of Shift. It is an alternate search, after all.
I found Ctrl+Shift+F to be much more popular for "search", "advanced search", "search in all ...", "find in ...", "advanced find", etc.:
https://defkey.com/what-means/ctrl-shift-f (40 hits) vs
https://defkey.com/what-means/ctrl-alt-f (7 hits)
And Ctrl+Shift+F seems to be widely used by virtually all of the most professional applications:
Mozilla Thunderbird, Opera, Sublime, Android Studio, Atom, IntelliJ IDEA, Visual Studio, NetBeans, Notepad++, Qt Creator, Visual Studio Code, Outlook
That's why I have used this shortcut.
My personal opinion is that it also matches Shift+F4 shortcut for "Open Terminal" better (see screenshot).
Both of which open an external tool, which I think goes well with the Shift modifier.
All right, you've convinced me on the shortcut.
The way you've implemented the menu item naming seems a bit fragile though (and in line 1410 of src/dolphinmainwindow.cpp, it's not used at all). In my testing, the name doesn't show up and it falls back to the static name. If you can't find a way to implement the feature reliably, it may be best to just always show static text. But it would be nice to have the name of the app reliably show up there, though.
The way you've implemented the menu item naming seems a bit fragile though
True. It's really hard to update it reliably.
KMoreTools* do not provide an easy way to get notified that the tools changed. Even current solution in "More Search Tools" menu recreates the menu every time it is displayed.
So I tried updating the action every time menu containing it is shown. Connect()ions for this aren't pretty, but this should actually work...
In my testing, the name doesn't show up and it falls back to the static name.
This is strange, because this action should be updated on many occasions, including Dolphin startup.
Or is your testcase installing KFind while Dolphin is open, or doing something similar? (Either way, that should work too).
Please let me know a test scenario that fails.
I did some more testing, and everything seems to work fine:
$ sudo su # cd /usr/share/applications
# sleep 2 && mv org.kde.kfind.desktop org.kde.kfind.desktop_ # no KFind now
# sleep 2 && mv org.kde.kfind.desktop_ org.kde.kfind.desktop # KFind installed
Actions to test (KFind installed/not installed):
Where? Do we have an open wish on bugs.kde.org ?
I'd like to remind that the menubar is hidden by default and the average user won't know about the CTRL+Shift+F shortcut.
So is this feature only aimed at advanced users who expect CTRL+Shift+F to do something?
This feature is also available from the Control toolbutton's menu:
[Control] toolbutton (menubar hidden) -> [Tools] -> [Open KFind]
and you can add that entry directly into the Dolphin's toolbar.
I guess @ngraham will be able to tell the full story on this, but here are some links:
its duplicates related issues:
Probably some more I didn't find.
And the "More Search Tools" menu was implemented for this exact reason in the first place.
Of course this Revision is also my own Wish in a form of a patch :)
As a person who opened https://bugs.kde.org/show_bug.cgi?id=391677 I would like to note that it is not in fact a duplicate of https://bugs.kde.org/show_bug.cgi?id=384798
My reasoning for opening that bug was because baloo is sometimes very unreliable at finding anything on my machine. And thanks to solid state drives there is no issue of waiting for spinning rust so lack of indexing is not an issue, also you could argue that indexing is a privacy risk.
This might detach the QList container. Either use a temp 'const QList` variable or use qAsConst
I don't get this connection. Can you explain what's needed for?
I know this is the old comment, but it's not clear. How about something like this?
Returns the KIO::StatJob::mostLocalUrl() for the active container URL if it's a local file. Otherwise returns the user's home path.
I'd call this method activeContainerLocalPath() to make it more clear what it is about.
Hmm, what about KParts::ReadOnlyPart::localFilePath() ? Could we use that?
There is no other way to update the open_preferred_search_tool action *before* the Configure Shortcuts window is shown.