[kdiroperator] Add method for accessing actions without KActionCollection
Needs ReviewPublic

Authored by nicolasfella on Jan 4 2020, 11:28 PM.

Details

Summary

To replace the actionCollection() method. This will allow us to get rid of the KActionCollection and thus KXmlGui dependency

Test Plan

Diff Detail

Repository
R241 KIO
Branch
kac
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 20633
Build 20651: arc lint + arc unit
nicolasfella created this revision.Jan 4 2020, 11:28 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptJan 4 2020, 11:28 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
nicolasfella requested review of this revision.Jan 4 2020, 11:28 PM
  • Add since
nicolasfella edited the test plan for this revision. (Show Details)Jan 4 2020, 11:45 PM
aacid added a subscriber: aacid.Jan 5 2020, 12:48 AM

What's the point of removing the KXMLGui dependency?

I mean it's not like you will be able to do Kate or KDevelop without kxmlgui, no?

The goal is to remove the overall dependencies of KIO. There are multiple apps/other frameworks that use KIO without using KXmlGui.

dhaumann added inline comments.
src/filewidgets/kdiroperator.h
428

Imho the API documentation should be improved (I am aware it was copied).

Returns a map of all available actions. The action map remains unchanged throughout the entire lifetime of the diroperator instance.

The key of the map can be used to obtain the respective action. The full list of available actions is as follows:

- popupMenu: ...
- up: ...

+1, API and code look good

aacid added a comment.Jan 5 2020, 11:51 AM

Why are we using a string instead of an enum?

It's not like this is KXMLGui where people can define their own actions, is it?

dfaure added a comment.Jan 5 2020, 5:58 PM

Interesting suggestion. We're used to using names for actions everywhere, but in this specific case an enum would do indeed.

When this is ported away from KActionCollection in the future, one should remember to do action->setObjectName("...") for code that actually wants strings (like KActionCollection).

meven added a subscriber: meven.Jan 6 2020, 9:00 PM
meven added inline comments.
src/filewidgets/kdiroperator.h
428

Personally I would add an enum of static strings, so that user code can discover easily the features and that the documentation follows the code more easily.

dfaure added inline comments.Jan 6 2020, 10:51 PM
src/filewidgets/kdiroperator.h
428

What's an enum of strings? You mean just an enum, right?
That was aacid's suggestion too.