use desktop file actions instead of global shortcuts
ClosedPublic

Authored by broulik on Nov 28 2017, 4:38 PM.

Details

Summary
  • Don't autostart KRunner, use actions in the desktop file to

define global shortcuts and actions

  • install its desktop file in share/kglobalaccel to make shortcuts default
  • expand commandline options to permit more control via commandline
Test Plan

works, as expected except

Diff Detail

Repository
R120 Plasma Workspace
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mart created this revision.Nov 28 2017, 4:38 PM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 28 2017, 4:38 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
mart requested review of this revision.Nov 28 2017, 4:38 PM

Needs the things you pointed out in the comment, but otherwise looks good.

Can you confirm you can set shortcuts in systemsettings correctly?

apol added a subscriber: apol.Nov 28 2017, 5:09 PM

Looks cool!

krunner/main.cpp
110–111

You can do const QString query = parser.positionalArguments().value(0);

115

^ value(0).

Overall fine with me.

What hapens to places like Plasma's context menu that show "Run command (Alt+F2)"? What shortcut will they show and do they still work? Same for typing on the desktop.

krunner/main.cpp
72

Use initializer list {QStringLiteral("c"), QStringLiteral("clipboard")}

78

Why no i18n() here?

Also, QStringLiteral("query")

117

This stuffin the lambda is somewhat duplicated, isn't it?

mart added a comment.Nov 29 2017, 10:05 AM

Overall fine with me.

What hapens to places like Plasma's context menu that show "Run command (Alt+F2)"? What shortcut will they show and do they still work? Same for typing on the desktop.

they still work, tough typing on desktop may lose some letters.. i wonder if would make sense to catch them plasmashell-side then launch krunner with the query as parameter...

mart updated this revision to Diff 23123.Nov 29 2017, 10:55 AM
  • delay first show
mart marked 5 inline comments as done.Nov 29 2017, 10:56 AM

delaying the first show seems to fix the focus issue.
a fixed timer of 100ms is of course not a solution but may give some insight in what the problem actually is (a 0 delay timer doesn't work either)

Did you find the cause for the input lag?

mart updated this revision to Diff 32982.Apr 24 2018, 3:06 PM
  • Merge branch 'master' into arcpatch-D9037
  • add a config update
davidedmundson requested changes to this revision.Apr 24 2018, 3:12 PM
davidedmundson added inline comments.
krunner/update/krunnerglobalshortcuts.cpp
34
  1. you don't need an exe to do that, this whole thing could have been

File=kglobalshortcutsrc
RemoveGroup=krunner

in the .upd. The syntax allows for quite a lot of manipulation.

  1. I did something similar for a powerdevil shortcut migration, it has a huge problem.

kconf_update runs, the config updates, kglobalacceld is still running so later syncs undoing your script.

Sorry :/

This revision now requires changes to proceed.Apr 24 2018, 3:12 PM

I have a plan for a kconf_update v2 which would resolve the problem above nicely
Probably means this would be 5.14.

Otherwise you can copy my 4ae36ddddaee91a23dcb0736418295269da14152 in powerdevil

broulik commandeered this revision.May 29 2019, 9:42 AM
broulik added a reviewer: mart.
broulik updated this revision to Diff 58828.May 29 2019, 9:46 AM
broulik edited the test plan for this revision. (Show Details)
  • Write migration script that uses kglobalaccel at runtime
  • Adjust desktop context menu to read the new shortcut
  • Remove activation timer as here the window gets focus mostly fine
broulik updated this revision to Diff 58829.May 29 2019, 9:58 AM
  • Remove accidental file rename
davidedmundson accepted this revision.Jun 6 2019, 7:34 AM
This revision is now accepted and ready to land.Jun 6 2019, 7:34 AM
This revision was automatically updated to reflect the committed changes.