D15599: Port the "Switch Desktop" containment action to libtaskmanager

2018-09-20 Thread Eike Hein 
hein updated this revision to Diff 41997.
hein added a comment.


  Fix wrong conditional.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15599?vs=41932=41997

BRANCH
  arcpatch-D13745

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

AFFECTED FILES
  containmentactions/switchdesktop/CMakeLists.txt
  containmentactions/switchdesktop/desktop.cpp
  containmentactions/switchdesktop/desktop.h

To: hein, mart, davidedmundson
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15599: Port the "Switch Desktop" containment action to libtaskmanager

2018-09-20 Thread Eike Hein 
hein added inline comments.

INLINE COMMENTS

> broulik wrote in desktop.cpp:65
> This codes doesn't take into account when the virtual desktop names change 
> etc. I think it should re-create the actions every time, and not try to be 
> "smart" by only changing the ones that are "superfluous" or "missing" as the 
> names and order inbetween could have changed

You misread the diff. This was indeed a bug the old code has, but the new code 
sets the text and data on all actions every time (below).

REPOSITORY
  R120 Plasma Workspace

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

To: hein, mart, davidedmundson
Cc: broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Nathaniel Graham 
ngraham added a comment.


  Darn, looks like arc didn't notice my rebase on 5.14. Oh well, I guess 
this'll go into 5.15.

REPOSITORY
  R99 KDE Gtk Configuration Tool

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Nathaniel Graham 
This revision was automatically updated to reflect the committed changes.
Closed by commit R99:b943d4ac6e2c: Sort icon and cursor themes 
case-insensitively (authored by ngraham).

REPOSITORY
  R99 KDE Gtk Configuration Tool

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15598?vs=41983=41995

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

AFFECTED FILES
  src/cursorthemesmodel.cpp
  src/gtkconfigkcmodule.cpp
  src/iconthemesmodel.cpp

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Nathaniel Graham 
ngraham added a comment.


  I'll keep `setSortRole(Qt::DisplayRole)` for explicitness if nobody objects.

REPOSITORY
  R99 KDE Gtk Configuration Tool

BRANCH
  sort-icon-and-cursor-themes-case-insensitively (branched from master)

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Nathaniel Graham 
ngraham added a comment.


  @wbauer, thanks for the hint. I've tested this with `xcursor-themes` and 
verified that it works for me too.

REPOSITORY
  R99 KDE Gtk Configuration Tool

BRANCH
  sort-icon-and-cursor-themes-case-insensitively (branched from master)

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15613: [Hotplug Engine] Update actions at runtime and listen for added/removed

2018-09-20 Thread Aleix Pol Gonzalez 
apol added a comment.


  +1

INLINE COMMENTS

> broulik wrote in hotplugengine.cpp:152
> I don't think so, the icon and emblem don't depend on the predicate but the 
> device's state.
> 
> Granted, we currently don't update them *at all* in this dataengine but 
> that's a separate issue. Device Notifier actually queries those from the 
> Solid Device data engine which does seem to update everything correctly.

Ok, makes sense to me.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: apol, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Wolfgang Bauer 
wbauer added a comment.


  Works (and looks) fine for me now.
  
  Btw, it wouldn't really be necessary to call `setSortRole(Qt::DisplayRole)`, 
as the default value is `Qt::DisplayRole` anyway. (but of course doing it is 
not wrong either...)

REPOSITORY
  R99 KDE Gtk Configuration Tool

BRANCH
  sort-icon-and-cursor-themes-case-insensitively (branched from master)

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Aleix Pol Gonzalez 
apol accepted this revision.
apol added a comment.
This revision is now accepted and ready to land.


  LGTM, not sure why sort() doesn't work though.

REPOSITORY
  R99 KDE Gtk Configuration Tool

BRANCH
  sort-icon-and-cursor-themes-case-insensitively (branched from master)

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Nathaniel Graham 
ngraham added a comment.


  In D15598#328774 , @wbauer wrote:
  
  > Maybe it would be better to derive CursorThemesModel and IconThemesModel 
from QSortFilterProxyModel instead of QStandardItemModel, and keep the previous 
fix?
  
  
  I had that idea too and gave it a shot, but got into the weeds quickly and 
abandoned the effort because it seemed too challenging. If this seems like the 
correct approach, I can give it another go.

REPOSITORY
  R99 KDE Gtk Configuration Tool

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Nathaniel Graham 
ngraham updated this revision to Diff 41983.
ngraham added a comment.


  Address review comments

REPOSITORY
  R99 KDE Gtk Configuration Tool

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15598?vs=41956=41983

BRANCH
  sort-icon-and-cursor-themes-case-insensitively (branched from master)

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

AFFECTED FILES
  src/cursorthemesmodel.cpp
  src/gtkconfigkcmodule.cpp
  src/iconthemesmodel.cpp

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Wolfgang Bauer 
wbauer added a comment.


  In D15598#328791 , @broulik wrote:
  
  > > Although, I think that would need to be done every time the data changes.
  >
  > `QSortFilterProxyModel` does that automatically
  
  
  You mean it will keep the data sorted after changes if you call sort() once?
  It obviously doesn't sort it automatically by default.

REPOSITORY
  R99 KDE Gtk Configuration Tool

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

2018-09-20 Thread Anthony Fieroni 
anthonyfieroni updated this revision to Diff 41977.

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15616?vs=41976=41977

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15616: [Comic] Handle error state correctly

2018-09-20 Thread Anthony Fieroni 
anthonyfieroni created this revision.
anthonyfieroni added reviewers: davidedmundson, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
anthonyfieroni requested review of this revision.

REVISION SUMMARY
  This happen really rare, i've used comic applet on my desktop about years, 
but in such cases dataUpdated is not called and busy indicator *burns* cpu. So 
i provide possible fix in comic provider, data engine, when redirection is 
needed error timer is not restarted in such case error is never handle 
properly. But *every* case i made a more aggressive solution, a timer in comic 
applet, if in 3 minutes dataUpdated isn't called it just force disconnect 
engine from source and stops indicator.
  
  CCBUG: 363292

REPOSITORY
  R114 Plasma Addons

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h
  dataengines/comic/comicprovider.cpp

To: anthonyfieroni, davidedmundson, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Kai Uwe Broulik 
broulik added a comment.


  > Although, I think that would need to be done every time the data changes.
  
  `QSortFilterProxyModel` does that automatically

REPOSITORY
  R99 KDE Gtk Configuration Tool

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D12040: Add wallpaperplugin.knsrc + QML function to open GHNS dialog

2018-09-20 Thread David Edmundson 
davidedmundson added a comment.


  In principle, sure.
  
  My previous review comments remain to be done.

REPOSITORY
  R120 Plasma Workspace

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

To: Zren, #plasma
Cc: ngraham, cfeck, mart, davidedmundson, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Wolfgang Bauer 
wbauer added a comment.


  In D15598#328747 , @wbauer wrote:
  
  > I suppose that's because, according to the QSortFilterProxyModel docs, `At 
this point, neither sorting nor filtering is enabled; the original data is 
displayed in the view.`
  >  Apparently you need to explicitly call sort() on the proxy model...
  
  
  Yes, adding `cursorsProxyModel->sort(0);` and `iconsProxyModel->sort(0);` 
here works.
  Although, I think that would need to be done every time the data changes. 
(can that happen later on? I haven't looked at all the code...)
  
  Maybe it would be better to derive CursorThemesModel and IconThemesModel from 
QSortFilterProxyModel instead of QStandardItemModel, and keep the previous fix?
  Or maybe reimplement sort() in these classes, the base implementation of 
QStandardItemModel just does nothing if I read the docs correctly (that also 
means that the previous fix actually did nothing either).
  
  Just some thoughts, though.

REPOSITORY
  R99 KDE Gtk Configuration Tool

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15613: [Hotplug Engine] Update actions at runtime and listen for added/removed

2018-09-20 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> apol wrote in hotplugengine.cpp:152
> Don't we need to pass the icon and emblems here too?

I don't think so, the icon and emblem don't depend on the predicate but the 
device's state.

Granted, we currently don't update them *at all* in this dataengine but that's 
a separate issue. Device Notifier actually queries those from the Solid Device 
data engine which does seem to update everything correctly.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: apol, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

D15613: [Hotplug Engine] Update actions at runtime and listen for added/removed

2018-09-20 Thread Aleix Pol Gonzalez 
apol added a comment.


  Sorry, very naif review.

INLINE COMMENTS

> hotplugengine.cpp:152
>  Plasma::DataEngine::Data data;
>  data.insert(QStringLiteral("predicateFiles"), predicates);
> +data.insert(QStringLiteral("actions"), 
> actionsForPredicates(predicates));

Don't we need to pass the icon and emblems here too?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: apol, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, mart

D15598: Sort icon and cursor themes case-insensitively

2018-09-20 Thread Wolfgang Bauer 
wbauer added a comment.


  Well, I tried this patch (applied upon 4.13.90a) and it doesn't seem to 
change anything.
  I still get icon and cursor themes starting with a lower-case character 
sorted after all those with an upper-case character.
  
  I suppose that's because, according to the QSortFilterProxyModel docs, `At 
this point, neither sorting nor filtering is enabled; the original data is 
displayed in the view.`
  Apparently you need to explicitly call sort() on the proxy model...
  See http://doc.qt.io/qt-5/qsortfilterproxymodel.html#details
  
  Regardless of that, using a QSortFilterProxyModel would render the previous 
fix (https://phabricator.kde.org/R99:7b56c23870798adbeb3f110eacae7f1020dcad6f) 
useless (as I see it there's no need to sort the model if it goes through QSFPM 
anyway), so it should be reverted here IMHO.
  
  Btw, the lower-case cursor themes I have installed ("handhelds", "redglass", 
"whiteglass") come from the (openSUSE) package xcursor-themes.
  From the package's README:
  
  > This is a default set of cursor themes for use with libXcursor,
  >  originally created for the XFree86 Project, and now shipped as part
  >  of the X.Org software distribution.
  >  ...
  >  The master development code repository can be found at:
  > 
  >   git://anongit.freedesktop.org/git/xorg/data/cursors
  >
  >   http://cgit.freedesktop.org/xorg/data/cursors
  >
  
  Although, just renaming or copying an existing cursor theme folder (they are 
in /usr/share/icons/) accordingly should suffice for testing as well.

REPOSITORY
  R99 KDE Gtk Configuration Tool

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

To: ngraham, apol, #plasma
Cc: wbauer, cfeck, broulik, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D15613: [Hotplug Engine] Update actions at runtime and listen for added/removed

2018-09-20 Thread Kai Uwe Broulik 
broulik created this revision.
broulik added a reviewer: Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  It would only detect changes to existing device actions but not adding or 
removing them.
  Moreover, the list of actions was never updated at runtime, only the list of 
predicates for the device.
  This causes Device Notifier to pick up added, removed, or modified device 
actions only on startup.

TEST PLAN
  - Added a device action, would show up in device notifier right away
  - Renamed a device action, change would be reflected in device notifier right 
away
  - Removed a device action, would disappear in device notifier right away
  - Changed the predicate of an action to no longer match storage devices, it 
disappeared right away
  - Invoking actions like "Open in file manager" still works

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  dataengines/hotplug/hotplugengine.cpp
  dataengines/hotplug/hotplugengine.h

To: broulik, #plasma
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D15233: Add a tooltip for the appentry in the kicker

2018-09-20 Thread Alexey Min 
alexeymin added a comment.


  Why isn't repository for this review set? Which repo it should be?

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

To: underwit, #plasma
Cc: alexeymin, abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D15233: Add a tooltip for the appentry in the kicker

2018-09-20 Thread Ivan Razzhivin 
underwit added a comment.


  Hello! There is a chance that my patch will be apply?

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

To: underwit, #plasma
Cc: abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart