D26718: aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski added a comment.


  Moved to GitLab: 
https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/148

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: pzro, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski added a comment.


  In D27972#645441 , @ngraham wrote:
  
  > It almost sounds like we need a "full manual control" mode where it's 
expected that the system tray applet or keyboard shortcut will be used to turn 
it on and off on demand.
  
  
  Well, isn't this what current inhibition mode does anyway? Both in 
manual/always-on and in timed modes.
  
  And yes, I have a global shortcut set for this tray icon :) And it somewhat 
works.
  The only annoying issue is that the tray widget for "Night Color Control" 
gets shown whenever I press this shortcut.
  
  And btw "Night Color Control" widget is completely empty. And it is possible 
to run into this empty widget - either with the shortcut as mentioned above, or 
using the vertical bar with icons when the tray icon is hidden.
  This widget could probably contain (at least) the information from the 
tooltip as mocked above or in D26688 . Or 
maybe even some settings (color temperature slidebar?).

REVISION DETAIL
  https://phabricator.kde.org/D27972

To: pdabrowski, #plasma, zzag, ngraham, #vdg
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski added a comment.


  I totally missed the D26688  request.
  And it proposes some great ideas.
  So just to update my mockup accordingly:
  
  As for the tray icon visibility (for the states above):
  
  - Manual mode:
- disabled = F8230305: off22.png  
invisible ("Night Color is disabled. Click to enable.")
- active = F8230306: on22.png  
VISIBLE ("Night Color is active. Click to disable.")
  - Timed mode:
- disabled = F8230305: off22.png  
VISIBLE ("Night Color is disabled. Click to enable.")
- inactive = F8230305: off22.png  
invisible ("Night Color is inactive. Activation will begin at...")
- active = F8230306: on22.png  
VISIBLE ("Night Color is active. Colors will return to normal at...")
  
  And of course after setting the tray icon to "Always shown":
  
  - Manual mode:
- disabled = F8230305: off22.png  
VISIBLE ("Night Color is disabled. Click to enable.")
- active = F8230306: on22.png  
VISIBLE ("Night Color is active. Click to disable.")
  - Timed mode:
- disabled = F8230305: off22.png  
VISIBLE ("Night Color is disabled. Click to enable.")
- inactive = F8230305: off22.png  
VISIBLE ("Night Color is inactive. Activation will begin at...")
- active = F8230306: on22.png  
VISIBLE ("Night Color is active. Colors will return to normal at...")
  
  Plus "activating" and "deactivating" states.

REVISION DETAIL
  https://phabricator.kde.org/D27972

To: pdabrowski, #plasma, zzag, ngraham, #vdg
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski added a comment.


  Maybe I should explain my motives and probably that would let us (including 
me) understand the goal better.
  
  I use Night Color only occasionally and I want to turn it on *manually*.
  And I like my tray clean.
  So I only need this icon when I have it manually engaged (i.e. un-inhibited).
  
  But I perfectly understand the need for Timed modes, which are a great 
feature.
  So they should provide a good user experience as well.
  
  **CURRENT STATE:**
  
  Currently there is no Manual mode available. You can select "Always on" in 
configuration, which will engage it every time you log in. This is not good.
  
  And also the "[ ] Activate Night Color" checkbox together with the 
possibility to inhibit the feature is confusing.
  There seem to be two separate ways of controlling this feature.
  
  F8230238: night-color-0.png 
  
  **PROPOSAL:**
  
  Maybe we could go with something like this:
  
  - Night Color system is always enabled (no "[ ] Activate Night Color" 
checkbox in the configuration)
  - by default set to Manual mode (see screenshot below)
  - Manual mode has a configuration option for setting the state at logon: "[ ] 
Active at logon" (unchecked by default)
  - the only way to disable the entire feature is what currently is named 
inhibition , which should be renamed to Disable
  
  F8230240: night-color-2.png 
  
  Also we could also polish the naming... current wording used all over this 
feature are very confusing:
  
  - "activate" in the configuration
  - "active", "inhibited", "not running", "disabled" in the tooltip
  - "On" and "Off" in the toast OSD message
  
  My suggestion (consistent with the proposal above):
  
  - "active", "inactive" (used in tooltip) = Night Color in effect or not
  - "disabled" (used both in the tooltip and OSD) = what currently is named 
inhibition
  
  **CONCLUSION:**
  
  So with the Manual mode you have 2 states toggled from the tray icon: 
"disabled" or "active".
  And in Timed modes you can have it "disabled" from the tray icon or 
"inactive" (daytime) and "active" (at night) otherwise.
  
  **TRAY ICON:**
  
  As for the tray icon visibility (for the states above):
  
  - Manual mode: disabled=invisible, active=visible
  - Timed mode: disabled=invisible, inactive=invisible, active=visible
  
  And of course after setting the tray icon to "Always shown":
  
  - Manual mode: disabled=visible, active=visible
  - Timed mode: disabled=visible, inactive=visible, active=visible

REVISION DETAIL
  https://phabricator.kde.org/D27972

To: pdabrowski, #plasma, zzag, ngraham, #vdg
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 78317.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27972?vs=78315=78317

REVISION DETAIL
  https://phabricator.kde.org/D27972

AFFECTED FILES
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h

To: pdabrowski, #plasma, zzag, ngraham, #vdg
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski added a comment.


  In D27972#627281 , @ngraham wrote:
  
  > In D27972#626302 , @broulik 
wrote:
  >
  > >
  >
  >
  > Could you update the description to be a bit more clear about what exactly 
has been changed for which cases?
  
  
  Added a detailed comparison of the current and new behavior.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D27972

To: pdabrowski, #plasma, zzag, ngraham, #vdg
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 78315.
pdabrowski edited the summary of this revision.
pdabrowski edited the test plan for this revision.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27972?vs=77473=78315

REVISION DETAIL
  https://phabricator.kde.org/D27972

AFFECTED FILES
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h

To: pdabrowski, #plasma, zzag, ngraham, #vdg
Cc: broulik, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, 
jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 77473.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27972?vs=77472=77473

REVISION DETAIL
  https://phabricator.kde.org/D27972

AFFECTED FILES
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h

To: pdabrowski, #plasma, zzag, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski marked an inline comment as done.
pdabrowski added a comment.


  In D27972#625596 , @zzag wrote:
  
  > > Fix for NightColor plasmoid active/passive status
  >
  > Could you please describe what's wrong with it right now?
  
  
  I wanted the icon to hide when Night Color is not affecting color correction 
(both in timed modes and manual "Always on" mode). Currently the icon is almost 
always visible.
  See updated description.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D27972

To: pdabrowski, #plasma, zzag, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski marked an inline comment as done.
pdabrowski added inline comments.

INLINE COMMENTS

> zzag wrote in main.qml:36-38
> We want to keep the applet visible even if Night Color is inhibited so user 
> can quickly uninhibit it. With this patch, the user would have to click the 
> system tray to show hidden icons and after that click the applet icon to 
> uninhibit Night Color.

> We want to keep the applet visible even if Night Color is inhibited so user 
> can quickly uninhibit it.

You are right. I have modified the patch to show the icon when inhibited in 
timed modes.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D27972

To: pdabrowski, #plasma, zzag, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 77472.
pdabrowski edited the summary of this revision.
pdabrowski added a comment.


  new patch

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27972?vs=77379=77472

REVISION DETAIL
  https://phabricator.kde.org/D27972

AFFECTED FILES
  applets/nightcolor/package/contents/ui/main.qml
  applets/nightcolor/plugin/monitor.cpp
  applets/nightcolor/plugin/monitor.h
  applets/nightcolor/plugin/monitor_p.h

To: pdabrowski, #plasma, zzag, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 77379.
pdabrowski added a comment.


  more context lines in diff

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27972?vs=77378=77379

REVISION DETAIL
  https://phabricator.kde.org/D27972

AFFECTED FILES
  applets/nightcolor/package/contents/ui/main.qml

To: pdabrowski, #plasma, zzag, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27972: [NightColor] fix for plasmoid status and tooltip subtext

[Piotr Dabrowski]

pdabrowski created this revision.
pdabrowski added reviewers: Plasma, zzag, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
pdabrowski requested review of this revision.

REVISION SUMMARY
  Fix for NightColor plasmoid active/passive status and its tooltip subtext.
  Especially noticeable when the tray icon is set to "Show when relevant".

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D27972

AFFECTED FILES
  applets/nightcolor/package/contents/ui/main.qml

To: pdabrowski, #plasma, zzag, ngraham
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski added a comment.


  In D26718#622311 , @ngraham wrote:
  
  > Are you planning to work on that other part in this patch, or in a separate 
one?
  
  
  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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 76799.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26718?vs=76798=76799

REVISION DETAIL
  https://phabricator.kde.org/D26718

AFFECTED FILES
  shell/desktopview.cpp
  shell/desktopview.h

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski added inline comments.

INLINE COMMENTS

> desktopview.cpp:246
>  
> +bool DesktopView::handleKRunnerTextInput(QKeyEvent *e) {
> +// allow only Shift and GroupSwitch modifiers

TODO: code style: {

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 76798.
pdabrowski retitled this revision from "autostart KRunner with Plasma, 
aggregate text for KRunner in DesktopView" to "aggregate text for KRunner in 
DesktopView".
pdabrowski edited the summary of this revision.
pdabrowski added a comment.


  Dropped autostart feature leaving only text aggregation in DesktopView.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26718?vs=73740=76798

REVISION DETAIL
  https://phabricator.kde.org/D26718

AFFECTED FILES
  shell/desktopview.cpp
  shell/desktopview.h

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: autostart KRunner with Plasma, aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski added a comment.


  In D26718#595756 , @davidedmundson 
wrote:
  
  > I don't want us to make krunner autostart. It's a hack, not a fix.
  
  
  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:
  
  - a call is made to D-Bus
  - which detects there is no KRunner service running
  - so it executes the KRunner binary
  - which then builds its UI
  - shows itself
  - and only then accepts user input
  
  This can't work fast. Meanwhile all user's keystrokes go to previously active 
window.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: autostart KRunner with Plasma, aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski planned changes to this revision.
pdabrowski added inline comments.

INLINE COMMENTS

> desktopview.cpp:262
>  if (!e->modifiers() || e->modifiers() == Qt::ShiftModifier) {
>  const QString text = e->text().trimmed();
>  if (!text.isEmpty() && text[0].isPrint()) {

TODO: allow spaces as non-first character

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: autostart KRunner with Plasma, aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 73740.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26718?vs=73737=73740

REVISION DETAIL
  https://phabricator.kde.org/D26718

AFFECTED FILES
  krunner/CMakeLists.txt
  krunner/dbus/org.kde.krunner.service.in
  krunner/krunner-daemon.desktop.cmake
  shell/desktopview.cpp
  shell/desktopview.h

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: autostart KRunner with Plasma, aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski added inline comments.

INLINE COMMENTS

> broulik wrote in main.cpp:231
> This is not how we want to autostart something :) Instead, you want to place 
> the desktop file in autostart folder again

Right :)
I thought of this so that everytime Plasma starts it ensures KRunner is running 
too.
But I guess this is not really needed - both KRunner and Plasma would have to 
crash for this to be useful.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: autostart KRunner with Plasma, aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski added a comment.


  In D26718#595688 , @broulik wrote:
  
  > Maybe if we autostarted KRunner based on whether it was used previously
  
  
  "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?

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham, broulik
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26718: autostart KRunner with Plasma, aggregate text for KRunner in DesktopView

[Piotr Dabrowski]

pdabrowski created this revision.
pdabrowski added reviewers: Plasma, Plasma: Workspaces, davidedmundson, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
pdabrowski requested review of this revision.

REVISION SUMMARY
  autostart KRunner with Plasma as daemon to minimize query latency
  
  aggregate text for KRunner in DesktopView to prevent eating fast-typed 
characters

TEST PLAN
  KRunner should be now running when Plasma starts.
  (when testing from your own KDE install location, watch out for conflicts with
  "/usr/share/dbus-1/services/org.kde.krunner.service" KRunner DBUS service)
  
  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.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D26718

AFFECTED FILES
  krunner/dbus/org.kde.krunner.service.in
  shell/desktopview.cpp
  shell/desktopview.h
  shell/main.cpp

To: pdabrowski, #plasma, #plasma_workspaces, davidedmundson, ngraham
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski added a comment.


  Is this OK now?
  
  I think the privacy issue caused by that reported regression (#409366) should 
really be fixed, one way or another. Most people are probably totally unaware 
their clipboard still contains the last content after they command it to be 
cleared...

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D22684

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: davidre, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski added inline comments.

INLINE COMMENTS

> davidedmundson wrote in historyimageitem.cpp:45
> what's this square about?
> 
> Also if "bpp" is bits per pixel, we need to put it in i18n() for translation

The square indicates that the clipboard contains an image, not a literal string 
like "1920x1080 32bpp".

> i18n()

Done.

> ngraham wrote in historyimageitem.cpp:65
> Does this have to be a pixmap? Can it just be a QIcon instead?

This is overridden from HistoryItem parent class.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D22684

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: davidre, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 69856.
pdabrowski marked 3 inline comments as done.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22684?vs=69775=69856

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  applets/clipboard/contents/ui/ClipboardPage.qml
  applets/clipboard/contents/ui/ImageItemDelegate.qml
  klipper/autotests/CMakeLists.txt
  klipper/historyimageitem.cpp
  klipper/historyimageitem.h
  klipper/historyitem.cpp
  klipper/historyitem.h
  klipper/historymodel.cpp
  klipper/historymodel.h
  klipper/klipper.cpp
  klipper/klipper.h

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: davidre, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski added a comment.


  Rebased.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D22684

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: davidre, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 69775.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22684?vs=63147=69775

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  applets/clipboard/contents/ui/ClipboardPage.qml
  applets/clipboard/contents/ui/ImageItemDelegate.qml
  klipper/historyimageitem.cpp
  klipper/historyimageitem.h
  klipper/historyitem.cpp
  klipper/historyitem.h
  klipper/historymodel.h
  klipper/klipper.cpp
  klipper/klipper.h

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: davidre, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski added a comment.


  Bump.
  This privacy issue (#409366) is still unresolved in latest Plasma.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D22684

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: davidre, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski added a comment.


  New, much better solution.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D22684

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 63147.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22684?vs=63145=63147

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  applets/clipboard/contents/ui/ClipboardPage.qml
  applets/clipboard/contents/ui/ImageItemDelegate.qml
  klipper/historyimageitem.cpp
  klipper/historyimageitem.h
  klipper/historyitem.cpp
  klipper/historyitem.h
  klipper/historymodel.h
  klipper/klipper.cpp
  klipper/klipper.h

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 63145.
pdabrowski edited the summary of this revision.
pdabrowski edited the test plan for this revision.
pdabrowski added a comment.


  New, much better solution.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22684?vs=62444=63145

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  applets/clipboard/contents/ui/ImageItemDelegate.qml
  klipper/historyimageitem.cpp
  klipper/historyimageitem.h
  klipper/historyitem.cpp
  klipper/historyitem.h
  klipper/historymodel.h
  klipper/klipper.cpp
  klipper/klipper.h

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22684: [Klipper] Fix clipboard history management

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 62444.
pdabrowski retitled this revision from "[Klipper] Fix clipboard history 
clearing" to "[Klipper] Fix clipboard history management".
pdabrowski edited the summary of this revision.
pdabrowski added a comment.
This revision is now accepted and ready to land.


  new diff: proper fix for clipboard history management

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22684?vs=62410=62444

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  klipper/klipper.cpp
  klipper/klipper.h
  klipper/tray.cpp
  klipper/tray.h

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22684: [Klipper] Fix clipboard history clearing

[Piotr Dabrowski]

pdabrowski added a comment.


  Weak pointer might not work well for this after all.
  I need to have a second look at this patch.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D22684

To: pdabrowski, #plasma, #plasma_workspaces, ngraham, davidedmundson
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22684: [Plasma Workspace][Klipper] Fixed clipboard history clearing (#409366)

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 62410.
pdabrowski added a comment.


  new diff: much better patch

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22684?vs=62398=62410

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  klipper/klipper.cpp
  klipper/klipper.h

To: pdabrowski, #plasma, #plasma_workspaces, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22684: [Plasma Workspace][Klipper] Fixed clipboard history clearing (#409366)

[Piotr Dabrowski]

pdabrowski updated this revision to Diff 62398.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22684?vs=62395=62398

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  klipper/klipper.cpp

To: pdabrowski, #plasma, #plasma_workspaces, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart

D22684: [Plasma Workspace][Klipper] Fixed clipboard history clearing (#409366)

[Piotr Dabrowski]

pdabrowski created this revision.
pdabrowski added reviewers: Plasma, Plasma: Workspaces, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
pdabrowski requested review of this revision.

REVISION SUMMARY
  Fixed clipboard history clearing in Klipper:
  https://bugs.kde.org/show_bug.cgi?id=409366

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D22684

AFFECTED FILES
  klipper/klipper.cpp

To: pdabrowski, #plasma, #plasma_workspaces, ngraham
Cc: plasma-devel, LeGast00n, jraleigh, fbampaloukas, GB_2, ragreen, Pitel, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, mart