Differential D29459

Scripting: Make client.activities writable
AbandonedPublic
Actions

Authored by tcorreabr on May 5 2020, 8:53 PM.

Details

Reviewers
zzag
Group Reviewers
KWin
Summary

Allows KWin scripts to change the value of client.activities property.

Diff Detail

Repository
R108 KWin
Lint
Lint Skipped
Unit
Unit Tests Skipped
tcorreabr created this revision.May 5 2020, 8:53 PM
Restricted Application added a project: KWin. · View Herald TranscriptMay 5 2020, 8:53 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
tcorreabr requested review of this revision.May 5 2020, 8:53 PM
zzag requested changes to this revision.May 6 2020, 6:51 AM
zzag added a subscriber: zzag.

The Toplevel class contains mostly read-only properties (I wonder whether it's actually worth to put properties in the Toplevel class since Deleted windows are not exposed to scripting). Could you please put Q_PROPERTY(QStringList activities READ activities WRITE setOnActivities NOTIFY activitiesChanged) in the AbstractClient class instead?

This revision now requires changes to proceed.May 6 2020, 6:51 AM
tcorreabr updated this revision to Diff 82115.May 6 2020, 3:58 PM

activities property moved from Toplevel to AbstractClient class.

tcorreabr retitled this revision from client.activities write access to scripting: Make client.activities writable.May 6 2020, 9:05 PM
zzag added inline comments.May 7 2020, 5:45 AM
toplevel.h
244 ↗(On Diff #82115)

Keep it here please. We'll basically override the definition of the activities property in AbstractClient. I know it's a bit weird, but we already do the same thing for the desktop property.

610 ↗(On Diff #82115)

Keep it here please.

tcorreabr updated this revision to Diff 82310.May 8 2020, 7:57 PM

toplevel.h changes removed.

zzag added inline comments.May 9 2020, 7:44 AM
abstract_client.h
888

It's already declared in the Toplevel class, remove it?

tcorreabr added inline comments.May 10 2020, 4:29 PM
abstract_client.h
888

Initially I had tried without it, but I got this compilation error:

[ 48%] Building CXX object CMakeFiles/kwin.dir/kwin_autogen/mocs_compilation.cpp.o
In file included from /home/tiago/kde/build/kwin/kwin_autogen/mocs_compilation.cpp:2:
/home/tiago/kde/build/kwin/kwin_autogen/EWIEGA46WW/moc_abstract_client.cpp: In function ‘void checkNotifySignalValidity_KWin__AbstractClient(KWin::AbstractClient*)’:
/home/tiago/kde/build/kwin/kwin_autogen/EWIEGA46WW/moc_abstract_client.cpp:1155:26: error: no matching function for call to ‘KWin::AbstractClient::activitiesChanged()’
 1155 |     t->activitiesChanged();
      |                          ^
In file included from /home/tiago/kde/src/kwin/abstract_client.h:24,
                 from /home/tiago/kde/build/kwin/kwin_autogen/EWIEGA46WW/moc_abstract_client.cpp:10,
                 from /home/tiago/kde/build/kwin/kwin_autogen/mocs_compilation.cpp:2:
/home/tiago/kde/src/kwin/toplevel.h:610:10: note: candidate: ‘void KWin::Toplevel::activitiesChanged(KWin::Toplevel*)’
  610 |     void activitiesChanged(KWin::Toplevel* toplevel);
      |          ^~~~~~~~~~~~~~~~~
/home/tiago/kde/src/kwin/toplevel.h:610:10: note:   candidate expects 1 argument, 0 provided
[ 48%] Linking CXX executable ../bin/testScriptedEffectLoader
zzag added inline comments.May 11 2020, 11:42 AM
abstract_client.h
888

Hmm, that's interesting... I'll need some time to think how to resolve this issue.

tcorreabr retitled this revision from scripting: Make client.activities writable to Scripting: Make client.activities writable.May 15 2020, 7:08 PM
abetts added a subscriber: abetts.May 22 2020, 6:41 PM

Hey man, can you join our vdg team on Telegram? Here is the link: https://t.me/vdgmainroom

We would like to talk about Parachute

meven added a subscriber: meven.May 30 2020, 6:22 AM
meven added inline comments.
abstract_client.h
888

Shouldn't activitiesChanged be activitiesChanged() instead of activitiesChanged(KWin::AbstractClient* client); or activitiesChanged(KWin::Toplevel* topLevel);

tcorreabr added inline comments.Sep 4 2020, 2:35 PM
abstract_client.h
888

I don't see the necessity of the parameter either, but I'm not an KWin expert. Can I submit the change or does anyone think differently?

zzag added inline comments.Sep 4 2020, 3:19 PM
abstract_client.h
888

I suggest to just remove activitiesChanged() signal in the Toplevel class. Is there any chance that you open a MR over here https://invent.kde.org/plasma/kwin/ ?

888

that you could open*

tcorreabr added inline comments.Sep 4 2020, 7:15 PM
abstract_client.h
888

Sure. I'll do it there.

tcorreabr abandoned this revision.Sep 4 2020, 8:15 PM

Revision Contents
Changeset List

PathPackages
Mabstract_client.h (6 lines)

Diff 82310

View Options

abstract_client.h

Loading...