D22594: [Dolphin] Open Preferred Search Tool action

2020-10-02 Thread F. Fox
firef added a comment.


  In D22594#498890 , @ngraham wrote:
  
  > 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.
  
  
  I am unsure whether //"basis that people should just use the built-in 
Baloo-based search instead.//" is a good enough justification for many people.
  
  Note that the KFind-toolbar-button - in 1 click distance - is hugely superior 
from a usability perspective to a menu entry in **2 click distance**!
  
  Having to set up KFind if, say, catfish-search was hypothetically the preset, 
with KFind on offer only in a secondary settings menu in 3+ click distance, 
would be a chore escaping 90+ % of users in my estimation.
  
  That fact that Dolphin now has two **separate** configuration menus (one of 
the two made for search tools only) may not be 100% ideal from a usability 
perspective.
  
  But the KFind button is a huge improvement, and I don't mean "huge" in the 
Trumpian sense.  see also : KFind on right-click 


REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: firef, broulik, pkloc, kfm-devel, kde-doc-english, waitquietly, azyx, 
nikolaik, pberestov, iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, 
Codezela, feverfew, meven, spoorun, navarromorales, ngraham, andrebarros, 
skadinna, emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-09 Thread Piotr Dabrowski
pdabrowski added inline comments.

INLINE COMMENTS

> broulik wrote in dolphinmainwindow.cpp:2289
> > It is still impossible to detect (un)installation of such tool.
> 
> There's signals when the KSycoca service database changes which we use in the 
> application menu to update when apps are installed/uninstalled.
> Might not work for non-gui tools without a desktop file, but I don't think 
> KMoreTools offers to open command line tools.
> Still better than updating the menu all the time :)

D29568 : use KSycoca for updating 
OpenPreferredSearchTool action

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, waitquietly, azyx, nikolaik, 
pberestov, iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, 
feverfew, meven, spoorun, navarromorales, firef, ngraham, andrebarros, 
skadinna, emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-05 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> broulik wrote in dolphinmainwindow.cpp:2289
> > It is still impossible to detect (un)installation of such tool.
> 
> There's signals when the KSycoca service database changes which we use in the 
> application menu to update when apps are installed/uninstalled.
> Might not work for non-gui tools without a desktop file, but I don't think 
> KMoreTools offers to open command line tools.
> Still better than updating the menu all the time :)

I agree with @broulik. This is a niche use case anyway, so if we really need to 
support it, we should do it right without nasty side effects.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-05 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> pdabrowski wrote in dolphinmainwindow.cpp:2289
> It is still impossible to detect (un)installation of such tool.
> "More Search Tools" menu currently recreates the menu from 
> KMoreToolsMenuFactory when it is shown.

> It is still impossible to detect (un)installation of such tool.

There's signals when the KSycoca service database changes which we use in the 
application menu to update when apps are installed/uninstalled.
Might not work for non-gui tools without a desktop file, but I don't think 
KMoreTools offers to open command line tools.
Still better than updating the menu all the time :)

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-05 Thread Piotr Dabrowski
pdabrowski added inline comments.

INLINE COMMENTS

> broulik wrote in dolphinmainwindow.cpp:2289
> > (can be modified with More Search Tools -> More -> Configure)
> 
> Then there should be a signal in `KMoreTools` a user of this class can 
> connect to to get notified when user preference changes. Or, the actions were 
> actually governed by `KMoreTools` and automatically updated.

It is still impossible to detect (un)installation of such tool.
"More Search Tools" menu currently recreates the menu from 
KMoreToolsMenuFactory when it is shown.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-05 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> pdabrowski wrote in dolphinmainwindow.cpp:2289
> > So, the URL does not have any impact on the search tool chosen? I thought 
> > that was the main reason for using the URL and updating it all the time.
> 
> This depends on KMoreToolsMenuFactory. But it doesn't seem to choose 
> different tools for different URLs.
> 
> The main reason for the updates was so that all the Open Preferred Search 
> Tool actions reflect the user choice of preferred tool (can be modified with 
> More Search Tools -> More -> Configure)
> See: https://phabricator.kde.org/D22594#499652

> (can be modified with More Search Tools -> More -> Configure)

Then there should be a signal in `KMoreTools` a user of this class can connect 
to to get notified when user preference changes. Or, the actions were actually 
governed by `KMoreTools` and automatically updated.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-05 Thread Piotr Dabrowski
pdabrowski added inline comments.

INLINE COMMENTS

> broulik wrote in dolphinmainwindow.cpp:2289
> > It is a rare occasion that user changes preferred search tools,
> 
> So, the URL does not have any impact on the search tool chosen? I thought 
> that was the main reason for using the URL and updating it all the time.

> So, the URL does not have any impact on the search tool chosen? I thought 
> that was the main reason for using the URL and updating it all the time.

This depends on KMoreToolsMenuFactory. But it doesn't seem to choose different 
tools for different URLs.

The main reason for the updates was so that all the Open Preferred Search Tool 
actions reflect the user choice of preferred tool (can be modified with More 
Search Tools -> More -> Configure)
See: https://phabricator.kde.org/D22594#499652

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-05 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> pdabrowski wrote in dolphinmainwindow.cpp:2289
> It was added to handle this action as a button in toolbar (can be added by 
> user, and this was requested in #384798).
> 
> I didn't notice this causes such a problem with remote connections, sorry.
> 
> It is a rare occasion that user changes preferred search tools, but I guess 
> they would like to have it updated all over Dolphin when they do.

> It is a rare occasion that user changes preferred search tools,

So, the URL does not have any impact on the search tool chosen? I thought that 
was the main reason for using the URL and updating it all the time.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-05 Thread Piotr Dabrowski
pdabrowski added inline comments.

INLINE COMMENTS

> broulik wrote in dolphinmainwindow.cpp:943
> `exec()` on a job is generally bad, but looks like it was like this already?

Yes, it was like that in DolphinMainWindow::openTerminal() before.
It was needed as explained in this comment:

  // If the given directory is not local, it can still be the URL of an
  // ioslave using UDS_LOCAL_PATH which to be converted first.

> broulik wrote in dolphinmainwindow.cpp:2289
> This causes quite severe bug https://bugs.kde.org/show_bug.cgi?id=420911
> Why is this needed anyway, we already monitor `aboutToShow` for all the 
> relevant menus, no?

It was added to handle this action as a button in toolbar (can be added by 
user, and this was requested in #384798).

I didn't notice this causes such a problem with remote connections, sorry.

It is a rare occasion that user changes preferred search tools, but I guess 
they would like to have it updated all over Dolphin when they do.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2020-05-04 Thread Kai Uwe Broulik
broulik added inline comments.

INLINE COMMENTS

> dolphinmainwindow.cpp:943
>  KJobWidgets::setWindow(statJob, this);
>  statJob->exec();
>  QUrl url = statJob->mostLocalUrl();

`exec()` on a job is generally bad, but looks like it was like this already?

> dolphinmainwindow.cpp:2289
>  return true;
> +} else if (event->type() == QEvent::WindowActivate) {
> +updateOpenPreferredSearchToolAction();

This causes quite severe bug https://bugs.kde.org/show_bug.cgi?id=420911
Why is this needed anyway, we already monitor `aboutToShow` for all the 
relevant menus, no?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: broulik, pkloc, kfm-devel, kde-doc-english, azyx, nikolaik, pberestov, 
iasensio, aprcela, fprice, gennad, fbampaloukas, alexde, Codezela, feverfew, 
meven, spoorun, navarromorales, firef, ngraham, andrebarros, skadinna, 
emmanuelp, rdieter, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-17 Thread Elvis Angelaccio
This revision was automatically updated to reflect the committed changes.
Closed by commit R318:537dc7864ae3: [Dolphin] Open Preferred Search Tool action 
(authored by pdabrowski, committed by elvisangelaccio).

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22594?vs=69855=69884

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-17 Thread Elvis Angelaccio
elvisangelaccio accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-17 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.


  Okay, that makes sense.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-17 Thread Nathaniel Graham
ngraham added a comment.


  @elvisangelaccio, all good now?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-17 Thread Piotr Dabrowski
pdabrowski added inline comments.

INLINE COMMENTS

> ngraham wrote in dolphinmainwindow.cpp:1162
> Since this patch was made, we've re-organized the hamburger menu a bit to 
> simplify it and hide not-commonly-used functionality. I think this feature 
> counts, since it's an alternative to the built-in search, and as such, it 
> won't be useful for the majority of Dolphin's users who use the built-in 
> search instead. Could you remove it from the hamburger menu?

One more thing to consider: no alternative tool (including KFind) is installed 
by default (at least in Kubuntu and KDE Neon). So by default this action will 
not appear at all.
And if user chooses to manually install KFind or another alternative, then they 
may expect to have it easily accessible from this menu.
What do you think?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-17 Thread Piotr Dabrowski
pdabrowski added inline comments.

INLINE COMMENTS

> ngraham wrote in dolphinmainwindow.cpp:1162
> Since this patch was made, we've re-organized the hamburger menu a bit to 
> simplify it and hide not-commonly-used functionality. I think this feature 
> counts, since it's an alternative to the built-in search, and as such, it 
> won't be useful for the majority of Dolphin's users who use the built-in 
> search instead. Could you remove it from the hamburger menu?

I've added this action everywhere where "Open Terminal" appeared. If you say 
the hamburger menu should not contain it though, I will remove it from there.
My only concern is that it will be hard for users to notice that it become 
available as optional toolbar button or direct shortcut. Even if someone 
anticipated this feature, they may easily miss it and continue not to use it.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-16 Thread Nathaniel Graham
ngraham added inline comments.

INLINE COMMENTS

> dolphinmainwindow.cpp:1162
>  addActionToMenu(ac->action(QStringLiteral("show_filter_bar")), menu);
> +
> addActionToMenu(ac->action(QStringLiteral("open_preferred_search_tool")), 
> menu);
>  addActionToMenu(ac->action(QStringLiteral("open_terminal")), menu);

Since this patch was made, we've re-organized the hamburger menu a bit to 
simplify it and hide not-commonly-used functionality. I think this feature 
counts, since it's an alternative to the built-in search, and as such, it won't 
be useful for the majority of Dolphin's users who use the built-in search 
instead. Could you remove it from the hamburger menu?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-11-16 Thread Piotr Dabrowski
pdabrowski updated this revision to Diff 69855.
pdabrowski marked 5 inline comments as done.

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22594?vs=67500=69855

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, pberestov, iasensio, fprice, gennad, 
MrPepe, fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, 
navarromorales, firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-10-13 Thread Elvis Angelaccio
elvisangelaccio requested changes to this revision.
elvisangelaccio added a comment.
This revision now requires changes to proceed.


  Almost there!

INLINE COMMENTS

> pdabrowski wrote in dolphinmainwindow.cpp:916-919
> There is no other way to update the open_preferred_search_tool action 
> *before* the Configure Shortcuts window is shown.
> This action is then listed in that window, so it should be up-to-date when it 
> is displayed.
> This update is instantaneous if user made no changes to the search tools in 
> the meantime.

Please write this information in a comment above that `connect()`.

> dolphinmainwindow.cpp:968
> +QAction* openPreferredSearchTool = 
> actionCollection()->action(QStringLiteral("open_preferred_search_tool"));
> +QList widgets = openPreferredSearchTool->associatedWidgets();
> +for (QWidget* widget : widgets) {

Missing `const`

> dolphinpart.cpp:536
>  {
> -QString dir(QDir::homePath());
> -
> -QUrl u(url());
> -
> -// If the given directory is not local, it can still be the URL of an
> -// ioslave using UDS_LOCAL_PATH which to be converted first.
> -KIO::StatJob* statJob = KIO::mostLocalUrl(u);
> -KJobWidgets::setWindow(statJob, widget());
> -statJob->exec();
> -u = statJob->mostLocalUrl();
> -
> -//If the URL is local after the above conversion, set the directory.
> -if (u.isLocalFile()) {
> -dir = u.toLocalFile();
> -}
> -
> -KToolInvocation::invokeTerminal(QString(), dir);
> +KToolInvocation::invokeTerminal(QString(), 
> KParts::ReadOnlyPart::localFilePath());
>  }

This is an unrelated change actually, isn't it? It makes sense, but it should 
go in another commit.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-10-08 Thread Piotr Dabrowski
pdabrowski marked 2 inline comments as done.
pdabrowski added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in dolphinmainwindow.cpp:916-919
> I don't get this connection. Can you explain what's needed for?

There is no other way to update the open_preferred_search_tool action *before* 
the Configure Shortcuts window is shown.
This action is then listed in that window, so it should be up-to-date when it 
is displayed.
This update is instantaneous if user made no changes to the search tools in the 
meantime.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-10-08 Thread Piotr Dabrowski
pdabrowski updated this revision to Diff 67500.
pdabrowski marked 4 inline comments as done.

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22594?vs=66163=67500

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, ngraham, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-09-22 Thread Elvis Angelaccio
elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> dolphinmainwindow.cpp:894
> +KMoreToolsMenuFactory("dolphin/search-tools").fillMenuFromGroupingNames(
> +_searchTools, { "files-find" }, QUrl(localPath())
> +);

`QUrl::fromLocalFile()`

> dolphinmainwindow.cpp:910
> +QAction* openPreferredSearchTool = 
> actionCollection()->action(QStringLiteral("open_preferred_search_tool"));
> +for (QWidget* widget : openPreferredSearchTool->associatedWidgets()) {
> +QMenu* menu = dynamic_cast(widget);

This might detach the QList container. Either use a temp  'const QList` 
variable or use `qAsConst`

> dolphinmainwindow.cpp:911
> +for (QWidget* widget : openPreferredSearchTool->associatedWidgets()) {
> +QMenu* menu = dynamic_cast(widget);
> +if (menu) {

`qobject_cast`

> dolphinmainwindow.cpp:916-919
> +connect(
> +
> actionCollection()->action(KStandardAction::name(KStandardAction::KeyBindings)),
>  ::hovered,
> +this, ::updateOpenPreferredSearchToolAction
> +);

I don't get this connection. Can you explain what's needed for?

> dolphinmainwindow.h:579-584
> +/**
> + * Returns local path for the active container URL.
> + * If the given directory is not local, it can still be the URL of an
> + * ioslave using UDS_LOCAL_PATH which to be converted first.
> + * Return user's home path if unsuccessful.
> + */

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.

> dolphinmainwindow.h:585
> + */
> +QString localPath();
> +

I'd call this method  `activeContainerLocalPath()` to make it more clear what 
it is about.

> dolphinpart.h:237
>  
> +QString localPath();
> +

Hmm, what about `KParts::ReadOnlyPart::localFilePath()` ? Could we use that?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-09-15 Thread Piotr Dabrowski
pdabrowski updated this revision to Diff 66163.
pdabrowski added a comment.


  Rebased.

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22594?vs=66098=66163

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-09-15 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  Needs one more rebase, sorry

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-09-14 Thread Piotr Dabrowski
pdabrowski added a comment.


  Done.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-09-14 Thread Piotr Dabrowski
pdabrowski updated this revision to Diff 66098.
pdabrowski marked 2 inline comments as done.

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22594?vs=62910=66098

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-09-14 Thread Elvis Angelaccio
elvisangelaccio added inline comments.

INLINE COMMENTS

> dolphinui.rc:2
>  
> -
> +
>  

Please rebase, thanks!

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, iasensio, fprice, gennad, MrPepe, 
fbampaloukas, alexde, Codezela, feverfew, meven, spoorun, navarromorales, 
firef, andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-08-14 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  I'll be AFK for a week for vacation. Will do a proper review once I'm back ;)

INLINE COMMENTS

> dolphinmainwindow.cpp:874
> +
> +void DolphinMainWindow::setupUpdateOpenPreferredSearchToolAction() {
> +QAction* openPreferredSearchTool = 
> actionCollection()->action(QStringLiteral("open_preferred_search_tool"));

Coding style: opening brace should go to next line

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
MrPepe, fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-08-13 Thread Nathaniel Graham
ngraham added a comment.


  @elvisangaleccio?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-08-01 Thread Nathaniel Graham
ngraham accepted this revision.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-08-01 Thread Piotr Dabrowski
pdabrowski edited the summary of this revision.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-08-01 Thread Piotr Dabrowski
pdabrowski updated this revision to Diff 62910.
pdabrowski added a comment.


  Small fix for the "Open Preferred Search Tool" as a toolbar button.

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22594?vs=62212=62910

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-08-01 Thread Piotr Dabrowski
pdabrowski edited the summary of this revision.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-29 Thread Petr K
pkloc added a comment.


  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=391677
  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.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: pkloc, kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-29 Thread Piotr Dabrowski
pdabrowski added a comment.


  In D22594#503270 , 
@elvisangelaccio wrote:
  
  > In D22594#498890 , @ngraham 
wrote:
  >
  > > 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
  >
  >
  > Where? Do we have an open wish on bugs.kde.org ?
  
  
  I guess @ngraham will be able to tell the full story on this, but here are 
some links:
  https://bugs.kde.org/show_bug.cgi?id=384798
  and its duplicates:
  https://bugs.kde.org/show_bug.cgi?id=391677
  https://bugs.kde.org/show_bug.cgi?id=401317
  https://bugs.kde.org/show_bug.cgi?id=400949
  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 :)

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-29 Thread Piotr Dabrowski
pdabrowski added a comment.


  In D22594#503270 , 
@elvisangelaccio wrote:
  
  > 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]

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-28 Thread Elvis Angelaccio
elvisangelaccio added a comment.


  In D22594#498890 , @ngraham wrote:
  
  > 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
  
  
  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?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, vmarinescu, fprice, gennad, 
fbampaloukas, alexde, feverfew, meven, spoorun, navarromorales, firef, 
andrebarros, skadinna, emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Piotr Dabrowski
pdabrowski added a comment.


  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):
  
  - menubar -> `Tools` -> `Open KFind`/none
  - `Control` toolbutton (menubar hidden) -> `Tools` -> `Open KFind`/none
  - [`Settings`] -> `Configure Shortcuts...` -> shortcut for `Open KFind`/`Open 
Preferred Search Tool` in the list

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Piotr Dabrowski
pdabrowski added a comment.


  No problem. I'm glad it works :)

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Well now I'm embarrassed to admit that I probably didn't. I think I updated 
it but forgot to compile lol. Sorry for wasting your time!
  
  This looks great to me now. @elvisangelaccio do you agree?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Piotr Dabrowski
pdabrowski added a comment.


  Hmm... Are you using the latest diff 
(https://phabricator.kde.org/D22594?id=62212)?

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Nathaniel Graham
ngraham added a comment.


  No, I already had KFind installed. I just opened Dolphin and looked at the 
menu item, nothing fancy. :/
  
  F7070963: repro-2019-07-21_16.08.02.webm 


REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Piotr Dabrowski
pdabrowski added a comment.


  > 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.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Nathaniel Graham
ngraham added a comment.


  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.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Piotr Dabrowski
pdabrowski added a comment.


  > 2. 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.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Piotr Dabrowski
pdabrowski updated this revision to Diff 62212.
pdabrowski edited the summary of this revision.
pdabrowski added a comment.


  > 1. Instead of showing a generic text, how about making it actually say 
"Search with https://phabricator.kde.org/D22594?vs=62211=62212

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-21 Thread Piotr Dabrowski
pdabrowski updated this revision to Diff 62211.
pdabrowski added a comment.


  > 3. Whenever you change anything in a .rc file, you need to bump the version 
number that appears at the top of the file.
  
  Done.

REPOSITORY
  R318 Dolphin

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22594?vs=62143=62211

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-20 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a reviewer: elvisangelaccio.
ngraham added a comment.
This revision now requires changes to proceed.


  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:
  
  1. Instead of showing a generic text, how about making it actually say 
"Search with https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham, elvisangelaccio
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-20 Thread Piotr Dabrowski
pdabrowski edited the summary of this revision.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

To: pdabrowski, #dolphin, ngraham
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov


D22594: [Dolphin] Open Preferred Search Tool action

2019-07-20 Thread Piotr Dabrowski
pdabrowski created this revision.
pdabrowski added reviewers: Dolphin, ngraham.
Herald added projects: Dolphin, Documentation.
Herald added subscribers: kde-doc-english, kfm-devel.
pdabrowski requested review of this revision.

REVISION SUMMARY
  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.
  
  F7060306: dolphin--open-preferred-search-tool-action-A-1.png 


REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D22594

AFFECTED FILES
  doc/index.docbook
  src/dolphinmainwindow.cpp
  src/dolphinmainwindow.h
  src/dolphinpart.cpp
  src/dolphinpart.h
  src/dolphinui.rc

To: pdabrowski, #dolphin, ngraham
Cc: kfm-devel, kde-doc-english, aprcela, fprice, gennad, fbampaloukas, alexde, 
feverfew, meven, spoorun, navarromorales, firef, andrebarros, skadinna, 
emmanuelp, mikesomov