Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-15 Thread Commit Hook

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12477
---


This review has been submitted with commit 
7e2ef3c7f0a6584a040b4fe70e1012de9b1511d8 by Makis Marimpis to branch master.

- Commit Hook


On April 12, 2012, 8:23 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 12, 2012, 8:23 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-15 Thread Commit Hook

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12476
---


This review has been submitted with commit 
f0983fa9a3a1f4466b7d4fa19cfa55914c0cea8a by Makis Marimpis to branch KDE/4.8.

- Commit Hook


On April 12, 2012, 8:23 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 12, 2012, 8:23 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-14 Thread makis marimpis


> On April 12, 2012, 8:37 a.m., Ivan Čukić wrote:
> > Ship It!

I just realized that i closed it by accident.


- makis


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12348
---


On April 12, 2012, 8:23 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 12, 2012, 8:23 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-12 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated April 12, 2012, 8:23 a.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Replaced the "QTimer::singleShot" call, with "QMetaObject::invokeMethod(this, 
"realInit", Qt::QueuedConnection)" - works just fine :D


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-12 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated April 11, 2012, 9:15 p.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Added some description comments about the singleShort usage.


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-12 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12343
---



service/plugins/globalshortcuts/globalshortcuts.cpp


remove extra empty line.



service/plugins/globalshortcuts/globalshortcuts.cpp


You could add a comment here explaining why you are using 
QTimer::singleShot. Something like "this plugin is loaded from 
ActivityManager's constructor, but it needs ActivityManager up and running to 
list all activities. Adding a 1s delay here gives ActivityManager some time to 
load the activity list and get ready to be contacted through dbus in 
realInit()." After that the patch is good to go from my part.


- Lamarque Vieira Souza


On April 11, 2012, 8:27 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 11, 2012, 8:27 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-12 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated April 11, 2012, 8:27 p.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Fixed memory leak and some bogus indentation.


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-12 Thread Ivan Čukić

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12348
---

Ship it!


Ship It!

- Ivan Čukić


On April 12, 2012, 8:23 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 12, 2012, 8:23 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread Ivan Čukić

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12347
---



service/plugins/globalshortcuts/globalshortcuts.cpp


Can you try the following instead of using a timer:

QMetaObject::invokeMethod(this, "realInit", Qt::QueuedConnection);

If it works properly when done that way, you can ship the patch with it. If 
not, you can push the current one.


- Ivan Čukić


On April 11, 2012, 9:15 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 11, 2012, 9:15 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread makis marimpis


> On April 11, 2012, 6:18 p.m., Lamarque Vieira Souza wrote:
> > service/plugins/globalshortcuts/globalshortcuts.cpp, line 26
> > 
> >
> > Maybe setting this and all other variables related to dbus in 
> > ::realInit() solves the delay problem. You should try to figure out exactly 
> > where the delay really happens (each line in the source code). Without that 
> > information any suggestion will be just guessing.

I did what you suggested and nailed the problem :D


- makis


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12335
---


On April 11, 2012, 8:06 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 11, 2012, 8:06 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12342
---



service/plugins/globalshortcuts/globalshortcuts.cpp


There is a missing "watcher->deleteLater()" here.

Awesome, now we are moving forward :-)


- Lamarque Vieira Souza


On April 11, 2012, 8:06 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 11, 2012, 8:06 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated April 11, 2012, 8:06 p.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Fixed: loading speed issue. now the plugin does not delays kamd and works 
perfectly.


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12335
---



service/plugins/globalshortcuts/globalshortcuts.cpp


Maybe setting this and all other variables related to dbus in ::realInit() 
solves the delay problem. You should try to figure out exactly where the delay 
really happens (each line in the source code). Without that information any 
suggestion will be just guessing.



service/plugins/globalshortcuts/globalshortcuts.cpp


Well, it was not exact this what I suggested you to do. You should have 
kept the ::init() with this content:

{
QTimer::singleShot(0, this, SLOT(realInit()));
}

According to Plugin.h the "virtual init()" should be explictitly called and 
the way you did you are doing it automatically.


- Lamarque Vieira Souza


On April 11, 2012, 5:41 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 11, 2012, 5:41 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread makis marimpis


> On April 10, 2012, 9:48 a.m., Lamarque Vieira Souza wrote:
> > service/plugins/globalshortcuts/globalshortcuts.cpp, line 41
> > 
> >
> > This plugin is loaded in EventProcessor's constructor, which is called 
> > from ActivityManager's contructor, so we have a chicken and egg problem 
> > here. The plugin needs ActivityManager to list activities but the 
> > ActivityManager's constructor has not finished to run when the plugin tries 
> > to use it.

Hm, i would like the chicken.
I used the singleShot trick, but the speed issues remains. I even tried longer 
time intervals.

Thanks for your continuous feedback! :D


- makis


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12288
---


On April 11, 2012, 5:41 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 11, 2012, 5:41 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated April 11, 2012, 5:41 p.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Fixed issues indicated by Lamarque Vieira Souza.


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread Lamarque Vieira Souza


> On April 4, 2012, 5:13 p.m., Lamarque Vieira Souza wrote:
> > service/plugins/globalshortcuts/globalshortcuts.cpp, line 41
> > 
> >
> > Maybe the slowness has nothing to do with your patch. Last weekend I 
> > was debugging a crash in Bluedevil (when I turn off my bluetooth 
> > controller). I had to restart kded4 to do some tests and I noticed that 
> > while kded is loading its modules the whole desktop freezes.
> > 
> > Usually the splash screen hides this problem, in my case there were no 
> > splash screen since I restarted only kded4.
> 
> makis marimpis wrote:
> Hm, in every case i tried the plugin by restarting my kde session.
> Without the plugin, all icons in the loading screen, load (doh) almost 
> simultaneously.
> When the plugin kicks in all icons load as before (fast) - but it stops 
> (for a long time) when loading (what i assume it is) the plasma desktop (the 
> icon before the enormous K)...
> 
> Is there some Qt way to do some kind of profiling in KDE?
> 
> makis marimpis wrote:
> If the DBus slowness thing, is related to Plasma, should i wait until it 
> is fixed somehow to the next point release?
> Or, the "modifying SharedInfo" answer could also be accepted (as long as 
> it is compatible with the master branch)?

This plugin is loaded in EventProcessor's constructor, which is called from 
ActivityManager's constructor, so we have a chicken and egg problem here that 
may be the cause of the delay. This plugin tries to contact ActivityManager 
through dbus to list the current list of activities but ActivityManager loads 
the list from disk after allocating the EventProcessor object. You can try 
adding a slot GlobalShortcutsPlugin::realInit() containg everything that is in 
GlobalShortcutsPlugin::init() today and call it from 
GlobalShortcutsPlugin::init() using "QTimer::singleShot(1000, this, 
SLOT(realInit()));". That will make sure ActivityManager is already allocated 
when the plugin is initialized.


- Lamarque Vieira


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12169
---


On April 4, 2012, 6:56 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 4, 2012, 6:56 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12288
---



service/plugins/globalshortcuts/globalshortcuts.cpp


This plugin is loaded in EventProcessor's constructor, which is called from 
ActivityManager's contructor, so we have a chicken and egg problem here. The 
plugin needs ActivityManager to list activities but the ActivityManager's 
constructor has not finished to run when the plugin tries to use it.



service/plugins/globalshortcuts/globalshortcuts.cpp


According to Qt's documentation you should use asyncCall here instead of 
asyncCallWithArgumentList.



service/plugins/globalshortcuts/globalshortcuts.cpp


same here. Besides, callWithArgumentList is synchronous and since you do 
not care about the returned valued then it's better change this to asynchronous 
call.


- Lamarque Vieira Souza


On April 4, 2012, 6:56 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 4, 2012, 6:56 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-11 Thread makis marimpis


> On April 4, 2012, 5:13 p.m., Lamarque Vieira Souza wrote:
> > service/plugins/globalshortcuts/globalshortcuts.cpp, line 41
> > 
> >
> > Maybe the slowness has nothing to do with your patch. Last weekend I 
> > was debugging a crash in Bluedevil (when I turn off my bluetooth 
> > controller). I had to restart kded4 to do some tests and I noticed that 
> > while kded is loading its modules the whole desktop freezes.
> > 
> > Usually the splash screen hides this problem, in my case there were no 
> > splash screen since I restarted only kded4.
> 
> makis marimpis wrote:
> Hm, in every case i tried the plugin by restarting my kde session.
> Without the plugin, all icons in the loading screen, load (doh) almost 
> simultaneously.
> When the plugin kicks in all icons load as before (fast) - but it stops 
> (for a long time) when loading (what i assume it is) the plasma desktop (the 
> icon before the enormous K)...
> 
> Is there some Qt way to do some kind of profiling in KDE?

If the DBus slowness thing, is related to Plasma, should i wait until it is 
fixed somehow to the next point release?
Or, the "modifying SharedInfo" answer could also be accepted (as long as it is 
compatible with the master branch)?


- makis


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12169
---


On April 4, 2012, 6:56 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 4, 2012, 6:56 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-04 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12169
---



service/plugins/globalshortcuts/globalshortcuts.cpp


Maybe the slowness has nothing to do with your patch. Last weekend I was 
debugging a crash in Bluedevil (when I turn off my bluetooth controller). I had 
to restart kded4 to do some tests and I noticed that while kded is loading its 
modules the whole desktop freezes.

Usually the splash screen hides this problem, in my case there were no 
splash screen since I restarted only kded4.


- Lamarque Vieira Souza


On April 4, 2012, 6:56 a.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated April 4, 2012, 6:56 a.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-04 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated April 4, 2012, 6:56 a.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Converted the rest of the dbus calls to async using the code provided by 
Lamarque Vieira Souza.
Still logging in to KDE is slow.

In the weekend, i tried some threading code (by moving most of the dbus calls 
to a qthread object) but still no progress.

Maybe modifying SharedInfo class would be the best option.


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-04-04 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12132
---



service/plugins/globalshortcuts/globalshortcuts.cpp


You can replace the code from this line to line 81 using something like 
this:

// m_activities is class member variable, not local.
m_activities = activities;
updateNextActivityName(0);
// end of GlobalShortcutsPlugin::listActivitiesFinished


void GlobalShortcutsPlugin::updateNextActivityName(QDBusPendingCallWatcher 
* watcher)
{
if (watcher) {
QDBusPendingReply reply = *watcher;
if (reply.isValid()) {
KAction * action = 
m_actionCollection->addAction(QString("switch-to-activity-%1").arg(watcher->property("activity").toString());
action->setText(i18nc("@action", "Switch to activity \"%1\"", 
reply.value()));
action->setGlobalShortcut(KShortcut());
connect(action, SIGNAL(triggered()), m_signalMapper, 
SLOT(map()));
m_signalMapper->setMapping(action, activity);   
}
}

if (m_activities.isEmpty()) {
connect(m_signalMapper, SIGNAL(mapped(QString)), this, 
SLOT(setCurrentActivity(QString)));
m_actionCollection->readSettings();
return;
}

QString activity = m_activities.takeFirst();
QDBusPendingReply reply = 
m_dbusInterface->asyncCallWithArgumentList(
QDBus::AutoDetect,
"ActivityName",
QList() << activity
);
  
QDBusPendingCallWatcher* watcher = new QDBusPendingCallWatcher(reply, 
this);
watcher->setProperty("activity", activity)
connect(watcher, SIGNAL(finished(QDBusPendingCallWatcher*)),
this, SLOT(updateNextActivityName(QDBusPendingCallWatcher*)));  

}


- Lamarque Vieira Souza


On March 31, 2012, 7:48 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated March 31, 2012, 7:48 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-31 Thread makis marimpis


> On March 31, 2012, 5:04 p.m., Lamarque Vieira Souza wrote:
> > service/plugins/globalshortcuts/globalshortcuts.cpp, line 66
> > 
> >
> > please stick to the code style, use "QString & activity" here.

It will not compile without the "const"


- makis


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12067
---


On March 31, 2012, 7:48 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated March 31, 2012, 7:48 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-31 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12071
---



service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop


You are the author of this plugin, right? You should write your name here 
and your e-mail below.



service/plugins/globalshortcuts/globalshortcuts.h


Is this copyright line correct? Who is Nick Shaforostoff? Is him aware you 
are submitting a patch using his name?

In the globalshortcuts.cpp the copyright line states Ivan Cukic as 
copyright holder. Who is the correct copyright holder?



service/plugins/globalshortcuts/globalshortcuts.h


guards are always upper case.



service/plugins/globalshortcuts/globalshortcuts.cpp


Please move this lines to before "#include " line.



service/plugins/globalshortcuts/globalshortcuts.cpp


In my old review I asked to do not use QDBusReply or waitForFinished, they 
can cause freezes in the whole Plasma Desktop.



service/plugins/globalshortcuts/globalshortcuts.cpp


In my old review I wrote that you can replace this line and the next two 
lines with only one call. Read my old review to see how to do that.



service/plugins/globalshortcuts/globalshortcuts.cpp


In my old review I asked to normalize this connect statement. Read my old 
review to know how to do that.


- Lamarque Vieira Souza


On March 31, 2012, 6:15 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated March 31, 2012, 6:15 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-31 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated March 31, 2012, 7:48 p.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Fixed all issues except from the usage of "QDBusReply", i not quite sure how to 
fix it.


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-31 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated March 31, 2012, 6:15 p.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Please forgive me, but because of work i forgot to post this update.
I converted the "ListActivities" dbus call to async - but the speed issue still 
remains.

So, some of the issues are invalid or fixed.
I'll investigate further to solve the speed issue - possibly by inheriting 
QThread and i`ll keep in mind the coding styles :D


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs (updated)
-

  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-31 Thread Lamarque Vieira Souza

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12067
---



service/plugins/globalshortcuts/globalshortcuts.h


Use "fetchAllActivities(const QDBusMessage & message)" here if possible.



service/plugins/globalshortcuts/globalshortcuts.cpp


remove extra line break.



service/plugins/globalshortcuts/globalshortcuts.cpp


please stick to the code style, use "QString & activity" here.



service/plugins/globalshortcuts/globalshortcuts.cpp


Please avoid using QDBusReply or waitForFinished. Since kactivitymanager is 
a key part of Plasma Desktop any delay here can cause the whole desktop to 
freeze for some seconds.



service/plugins/globalshortcuts/globalshortcuts.cpp


Code style, please use "KAction * action"



service/plugins/globalshortcuts/globalshortcuts.cpp


You can replace this and the next two lines with:

action->setText(i18nc("@action", "Switch to activity \"%1\"", 
reply.value()));



service/plugins/globalshortcuts/globalshortcuts.cpp


Normalize this connect statement (just remove "const" word, "&" char and 
any space inside the parentesis in SIGNAL() and SLOT(). They are not needed and 
cause QObject::connect to be a bit slower.


- Lamarque Vieira Souza


On March 31, 2012, 4:19 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated March 31, 2012, 4:19 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
>   service/plugins/globalshortcuts/keyboardbindings.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-31 Thread Ivan Čukić

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review12066
---


You're awesome mate :)


service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop


You need to remove the old stuff from the .desktop file, and fix author and 
stuff...



service/plugins/globalshortcuts/globalshortcuts.cpp


Instead of accessing this via d-bus, you can add (or I can) property with a 
list of activities to SharedInfo.

This way, there will be no problems with slowness.

On the other hand, key-stuff registration can be done in a thread or 
something.



service/plugins/globalshortcuts/globalshortcuts.cpp


The default shortcuts is something I'd like discussed on the ML.

Personally, I'd have them empty by default.


- Ivan Čukić


On March 31, 2012, 4:19 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated March 31, 2012, 4:19 p.m.)
> 
> 
> Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
>   service/plugins/globalshortcuts/keyboardbindings.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-31 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/
---

(Updated March 31, 2012, 4:19 p.m.)


Review request for KDE Base Apps, KDE Runtime, Plasma, and Ivan Čukić.


Changes
---

Added groups kde-baseapps and kde-runtime


Description
---

(follows discarded review: 104391)

The plugin adds global keyboard shortcuts for changing the activity.
Defaults start from Qt::MetaModifier + Qt::Key_F1.

Initially, this functionality was implemented (without correct results) using 
KActitivities::Controller.
Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
delay - so it really needs modification/and i would like more guidance related 
to dbus :D.
Other than that, feedback is needed. 


This addresses bugs 265069 and 273467.
http://bugs.kde.org/show_bug.cgi?id=265069
http://bugs.kde.org/show_bug.cgi?id=273467


Diffs
-

  service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
  service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
  
service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop 
PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
  service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
  service/plugins/globalshortcuts/keyboardbindings.cpp PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/104428/diff/


Testing
---

Logged in.
Used the default shortcuts to switch activities.
It works!


Thanks,

makis marimpis

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request: GlobalShortcuts Plugin for ActivityManager (kamd)

2012-03-27 Thread makis marimpis

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104428/#review11918
---


- makis marimpis


On March 27, 2012, 6:36 p.m., makis marimpis wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104428/
> ---
> 
> (Updated March 27, 2012, 6:36 p.m.)
> 
> 
> Review request for Plasma and Ivan Čukić.
> 
> 
> Description
> ---
> 
> (follows discarded review: 104391)
> 
> The plugin adds global keyboard shortcuts for changing the activity.
> Defaults start from Qt::MetaModifier + Qt::Key_F1.
> 
> Initially, this functionality was implemented (without correct results) using 
> KActitivities::Controller.
> Using DBus seems fine, just that... it forces kamd to start with 3-4 seconds 
> delay - so it really needs modification/and i would like more guidance 
> related to dbus :D.
> Other than that, feedback is needed. 
> 
> 
> This addresses bugs 265069 and 273467.
> http://bugs.kde.org/show_bug.cgi?id=265069
> http://bugs.kde.org/show_bug.cgi?id=273467
> 
> 
> Diffs
> -
> 
>   service/plugins/CMakeLists.txt 3e965c0a201a7eee2c9868fcf163cf55af636858 
>   service/plugins/globalshortcuts/CMakeLists.txt PRE-CREATION 
>   
> service/plugins/globalshortcuts/activitymanager-plugin-globalshortcuts.desktop
>  PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.h PRE-CREATION 
>   service/plugins/globalshortcuts/globalshortcuts.cpp PRE-CREATION 
>   service/plugins/globalshortcuts/keyboardbindings.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/104428/diff/
> 
> 
> Testing
> ---
> 
> Logged in.
> Used the default shortcuts to switch activities.
> It works!
> 
> 
> Thanks,
> 
> makis marimpis
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel