Differential D2657

Add plugin actions to popup menu
ClosedPublic
Actions

Authored by martinkostolny on Sep 4 2016, 8:55 PM.

Details

Reviewers
abika
Group Reviewers
Krusader
Commits
R167:efb3f500e93f: Add plugin actions to popup menu
Summary

Since recently (I cannot say exactly but probably since KF 5.25) previously added (D2361) service menus stopped supplying 'Extract' and 'Compress' submenus. Instead, these submenus are now provided as standalone plugins. So I'm proposing to start adding plugin actions as well. I've basically copied the required code from dolphin/src/dolphincontextmenu.cpp.

Is this the way we want to go? The downside of this approach is that we don't supply a configuration interface for visibility of these plugins. It is now configurable by dolphin Settings only. Personally I find it quite ok but I'd like to hear your opinions.

Test Plan
  • Extract or Compress is showed in popup menu when right clicking selected file(s)/folder(s)
  • Extract and Compress action is successfully performed

Diff Detail

Repository
R167 Krusader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
martinkostolny updated this revision to Diff 6431.Sep 4 2016, 8:55 PM
martinkostolny retitled this revision from to Add plugin actions to popup menu.
martinkostolny updated this object.
martinkostolny edited the test plan for this revision. (Show Details)
martinkostolny added a reviewer: Krusader.
martinkostolny set the repository for this revision to R167 Krusader.
martinkostolny added a project: Krusader.
abika added a subscriber: abika.Sep 6 2016, 5:18 PM

ok, then it has to be 'plugins' and not 'service actions', whatever.

But its a bad approach if settings in Dolphin have impact on Krusaders view. I recommend to ignore the Dolphin settings and make all available plugins visible. If enabling/disabling is needed those options have to be added to Krusaders settings.

And we shouldn't forget to remove the old code once it is not needed anymore (if everybody is using KF5 >= 5.25)

martinkostolny updated this revision to Diff 6626.Sep 11 2016, 12:31 PM

Thanks for Your thoughts, I agree with You, Alex. Here is updated diff to allow all plugins. I've tested it in Kubuntu 16.04 and Arch.

However there is a question about removing the old code. Currently with updated KF5 the "old code" (fileItemActions.addServiceActionsTo) is adding "Actions" submenu. Do we want to get rid of this when all distros have KF 5.25+? I mean do we want:
a) *only* "Compress" and "Extract" eventually? (I've this kind of patch locally prepared as well)
b) Or do we want all the plugins along with "Actions" submenu (like we have here now)?

asensi added a subscriber: asensi.Sep 12 2016, 6:48 AM

Hi! Unfortunately nowadays I'm not able to help, my former job ended and the new one is eating all my time. However, I hope :-) that things will get better and I'll have more time. Greetings!

martinkostolny added a comment.Sep 14 2016, 10:13 PM

Toni, that's certainly fine, I also have hardly time for this. Best of luck in Your new work!

abika added a comment.Sep 15 2016, 7:00 PM

@asensi
Thanks for all the work you have done so far! Hope, you have more time in the future.

@martinkostolny
I personally would go for (a). "Extract" and "compress" are the only useful options for me.

The actions ("Open a terminal here" and "Start a slideshow") and specially the "Activities" submenu doesn't make much sense.

martinkostolny updated this revision to Diff 6784.Sep 16 2016, 5:43 PM

I'd also like a) better. Here is the code. Tested on Kubuntu with KF 5.23 as well as Arch with KF 5.25.

abika accepted this revision.Oct 7 2016, 3:02 PM
abika added a reviewer: abika.

Sorry for the long delay Martin. Totally forgot about this. I have a lot of other stuff going on right now.

But it works great, thanks! Formatting could be improved (max. line length).

(And QStringLiteral() seems odd here. We should use it everywhere or not at all.)

This revision is now accepted and ready to land.Oct 7 2016, 3:02 PM
martinkostolny added a comment.Oct 7 2016, 5:18 PM

Thanks for Your suggestions, I'll repair the long lines and remove the QStringLiteral. I used it because it was in the copied code from Dolphin.

And I totally understand long responses, You don't have to worry. Thanks a lot for your great work in Krusader and Your guidance :).

Closed by commit R167:efb3f500e93f: Add plugin actions to popup menu (authored by martinkostolny). · Explain WhyOct 8 2016, 8:40 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 7210

View Options

krusader/Panel/krpopupmenu.h

Loading...
View Options

krusader/Panel/krpopupmenu.cpp

Loading...