Aggregate text for KRunner in DesktopView to prevent eating fast-typed characters.
CCBUG: 416145
partial solution: resolved for DesktopView, not resolved for running KRunner from shortcut
davidedmundson | |
ngraham | |
broulik |
Plasma | |
Plasma: Workspaces |
Aggregate text for KRunner in DesktopView to prevent eating fast-typed characters.
CCBUG: 416145
partial solution: resolved for DesktopView, not resolved for running KRunner from shortcut
When DesktopView is focused, typed characters show KRunner query.
All fast-typed characters should now appear correctly in KRunner window,
instead of only first typed character.
Lint Skipped |
Unit Tests Skipped |
IIRC we used to do this, but changed it to run-on-demand to save some memory on startup.
Might be worth revisiting that though, since I can confirm the latency that leads to the first few characters getting typed into the open app instead of KRunner, which is really annoying. We have a recent bug report about this: https://bugs.kde.org/show_bug.cgi?id=416145
Hmm, I'd appreciate some startup time optimizations for KRunner first.
Maybe if we autostarted KRunner based on whether it was used previously would be a nice trade-off between resources for systems that don't use it and power users that always use it and get annoyed by the lag.
shell/main.cpp | ||
---|---|---|
231 ↗ | (On Diff #73737) | This is not how we want to autostart something :) Instead, you want to place the desktop file in autostart folder again |
"Used previously" as in opened at least once?
I guess this is always true for every user, because KRunner probably was opened even accidentally (DesktopView typing, Alt+Space) by everyone.
Used to actually run something at least once?
That could work. Still if someone cares for KRunner *not* starting up with Plasma, they may want better control over it.
So maybe a simple checkbox somewhere in System Settings?
shell/main.cpp | ||
---|---|---|
231 ↗ | (On Diff #73737) | Right :) |
The continual forwarding of keys makes sense. I'm happy to accept that.
I don't want us to make krunner autostart. It's a hack, not a fix.
shell/desktopview.cpp | ||
---|---|---|
262 | TODO: allow spaces as non-first character |
I'm gonna play devil's advocate here :)
If having an already running KRunner service is a hack, then why not terminate it each time after its job is done - and restart on request each time it is needed again?
Bug #416145 says exactly this: "When krunner is invoked again during the same session it will work correctly."
So you oppose preloading of KRunner service, but accept it continuing to run forever awaiting its next use. I don't find this consistent.
IMHO (correct me if I'm wrong!):
Something that is supposed to act immediately should be preloaded (sure: it may be a lighter version).
Currently to show the KRunner window for the first time:
This can't work fast. Meanwhile all user's keystrokes go to previously active window.
So you oppose preloading of KRunner service, but accept it continuing to run forever awaiting its next use. I don't find this consistent.
It's a common lazy-load pattern.
Though that extension of closing down would also be a direction I would happily support.
This can't work fast.
Any profiling to support that? I spent a few seconds and can see that we're still using PC2, we're loading not two results views which could happen later. One of the plasma framesvgs is emitted it's changed on startup, there's a blocking gap where nothing is happening...
If this approach was suggested after extensive hours of profiling and optimisations, I'd have a different view, but it shouldn't be our first step.
Dropped autostart feature leaving only text aggregation in DesktopView (modified to allow spaces and national characters).
shell/desktopview.cpp | ||
---|---|---|
246 ↗ | (On Diff #73737) | TODO: code style: { |
Can confirm the fix when running from the desktop, but not when using the shortcut. So I wouldn't call 416145 fully fixed yet. Are you planning to work on that other part in this patch, or in a separate one?
Are you planning to work on that other part in this patch, or in a separate one?
To do what exactly?
Find another way to fix the problem of keystrokes getting eaten when activating KRunner using a global shortcut.
I do not plan to change this patch.
As for solving the issue with KRunner invoked from shortcut, I still do not have a good solution for it.
Hey guys, this is what I think
I say solution 1 is hackier than 2 and 1 doesn't even solve all the problems (like when invoking with alt+space). I think 2 is better (at least until things change and lazy loading KRunner is no problem). Yes, then there will more memory usage from the start (I see about 36M just checking in KSysGuard) but a worthwhile trade off for a better experience. KRunner is one of the best things in KDE.