Port to KGlobalAccel
ClosedPublic

Authored by davidre on Oct 31 2019, 10:46 AM.

Details

Summary

Replace KHotkeys with KGlobalAccel

Diff Detail

Repository
R103 KMenu Editor
Branch
globalaccel (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18634
Build 18652: arc lint + arc unit
davidre created this revision.Oct 31 2019, 10:46 AM
Restricted Application added a project: Plasma. · View Herald TranscriptOct 31 2019, 10:46 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidre requested review of this revision.Oct 31 2019, 10:46 AM

Concept ++++

We don't need WITH_GLOBALACCEL guards. It's a dep of kxmlgui which we link against anyway.

mlaurent added inline comments.
CMakeLists.txt
39

Why removing it ?

mlaurent requested changes to this revision.Oct 31 2019, 12:29 PM
This revision now requires changes to proceed.Oct 31 2019, 12:29 PM
davidre updated this revision to Diff 69105.Oct 31 2019, 12:54 PM
  • Don't delete disabled
  • Remove guards
davidre marked an inline comment as done.Oct 31 2019, 12:54 PM
davidre added inline comments.
CMakeLists.txt
39

Accidentally

mlaurent added inline comments.Oct 31 2019, 1:08 PM
globalaccel.cpp
56

if (!shortcut.isEmpty()) {

menuinfo.cpp
179–180

you can remove this ifdef

davidre updated this revision to Diff 69107.Oct 31 2019, 1:13 PM
davidre marked 4 inline comments as done.
  • comments
apol added a subscriber: apol.Oct 31 2019, 1:45 PM
apol added inline comments.
globalaccel.cpp
51

Why just return one if it can be a list?

globalaccel.h
28

I'd call them menuEntryShortcut and setMenuEntryShortcut

davidre added inline comments.Oct 31 2019, 1:54 PM
globalaccel.cpp
51

I think the list that is returned can have up to 2 entries. The shortcut and the alternate shortcut. However the currently Gui shows/configures only one shortcut.

globalaccel.h
28

I just used the old names.

mlaurent added inline comments.Nov 1 2019, 6:02 AM
basictab.cpp
37–38

still necessary to #ifndef ?

davidre updated this revision to Diff 69279.Nov 4 2019, 3:10 PM
davidre marked an inline comment as done.

Thanks for spotting!

For me it seems ok

Did you finish it ? is it work ? or still in progress ?

For me it seems ok

Did you finish it ? is it work ? or still in progress ?

Shortcut setting and displaying works so far in my testing.
I still need to do the migration of existing shortcuts otherwise if user had set a shortcut though kmenuedit before it wouldn't be displayed in kmenuedit (although it would be functional).

davidre updated this revision to Diff 69483.Nov 8 2019, 8:21 PM

update application for migration based on the one I did for spectacle

davidre retitled this revision from [WIP] Port to KGlobalAccel to Port to KGlobalAccel.Nov 8 2019, 8:22 PM
davidre edited the summary of this revision. (Show Details)
mlaurent requested changes to this revision.Nov 9 2019, 9:03 AM

After that it seems ok for me

kconf_update/globalaccel.cpp
25

QLatin1String("...") it's a minor optimization

38

same here

This revision now requires changes to proceed.Nov 9 2019, 9:03 AM
davidre updated this revision to Diff 69577.Nov 11 2019, 10:13 AM
davidre marked 2 inline comments as done.

QLatin1String

mlaurent accepted this revision.Nov 11 2019, 11:40 AM

for me all is ok.

This revision is now accepted and ready to land.Nov 11 2019, 11:40 AM

Did you have some problem for commiting it ?
Regards

No, I wanted to let David take a second look. But I can commit later.

This revision was automatically updated to reflect the committed changes.