Port to KGlobalAccel
AcceptedPublic

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

Details

Reviewers
davidedmundson
mlaurent
Group Reviewers
Plasma
Maniphest Tasks
T2050: sunsetting KHotKeys
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 18693
Build 18711: arc lint + arc unit
davidre created this revision.Thu, Oct 31, 10:46 AM
Restricted Application added a project: Plasma. · View Herald TranscriptThu, Oct 31, 10:46 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
davidre requested review of this revision.Thu, Oct 31, 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.Thu, Oct 31, 12:29 PM
This revision now requires changes to proceed.Thu, Oct 31, 12:29 PM
davidre updated this revision to Diff 69105.Thu, Oct 31, 12:54 PM
  • Don't delete disabled
  • Remove guards
davidre marked an inline comment as done.Thu, Oct 31, 12:54 PM
davidre added inline comments.
CMakeLists.txt
39

Accidentally

mlaurent added inline comments.Thu, Oct 31, 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.Thu, Oct 31, 1:13 PM
davidre marked 4 inline comments as done.
  • comments
apol added a subscriber: apol.Thu, Oct 31, 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.Thu, Oct 31, 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.Fri, Nov 1, 6:02 AM
basictab.cpp
37–38

still necessary to #ifndef ?

davidre updated this revision to Diff 69279.Mon, Nov 4, 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.Fri, Nov 8, 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.Fri, Nov 8, 8:22 PM
davidre edited the summary of this revision. (Show Details)
mlaurent requested changes to this revision.Sat, Nov 9, 9:03 AM

After that it seems ok for me

kconf_update/globalaccel.cpp
26

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

39

same here

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

QLatin1String

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

for me all is ok.

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