Re: Review Request: Fix Add to desktop from Kickoff when you have several virtual desktops and you enable Different widgets for each desktop

2012-01-11 Thread Aaron J. Seigo

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

Ship it!


tiny ws issue, but otherwise ok.

and why containment starts from 0 - because that's where counting starts. that 
virtual desktops in KWindowSystem start at '1' is pretty silly, really. 
otherwise we end up with everything else being zeroth counted as per usual, 
except desktops which are counted from 1. i chose not to extend that oddity 
into plasma code.


plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp
http://git.reviewboard.kde.org/r/103645/#comment8045

spaces around the '-' please :)


- Aaron J. Seigo


On Jan. 6, 2012, 9:11 p.m., Anne-Marie Mahfouf wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/103645/
 ---
 
 (Updated Jan. 6, 2012, 9:11 p.m.)
 
 
 Review request for Plasma and Aaron J. Seigo.
 
 
 Description
 ---
 
 From Kickoff using Add to desktop when you have several virtual desktops 
 and you enable Different widgets for each desktop in the pager settings. 
 KWindowSystem starts counting from 1 and Plasma from 0
 
 Without this fix Add to desktop adds to the next desktop or does not add if 
 you're on the last desktop. 
 
 
 This addresses bug https://bugs.kde.org/show_bug.cgi?id=290368.
 
 http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290368
 
 
 Diffs
 -
 
   plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp cf12903 
 
 Diff: http://git.reviewboard.kde.org/r/103645/diff/diff
 
 
 Testing
 ---
 
 Local tests as thorough as I could do.
 
 
 Thanks,
 
 Anne-Marie Mahfouf
 


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


Re: Review Request: Fix Add to desktop from Kickoff when you have several virtual desktops and you enable Different widgets for each desktop

2012-01-11 Thread Commit Hook

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


This review has been submitted with commit 
105b03bec8be119380c8e7ff2f1a53ea38177cf0 by Anne-Marie Mahfouf to branch master.

- Commit Hook


On Jan. 6, 2012, 9:11 p.m., Anne-Marie Mahfouf wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/103645/
 ---
 
 (Updated Jan. 6, 2012, 9:11 p.m.)
 
 
 Review request for Plasma and Aaron J. Seigo.
 
 
 Description
 ---
 
 From Kickoff using Add to desktop when you have several virtual desktops 
 and you enable Different widgets for each desktop in the pager settings. 
 KWindowSystem starts counting from 1 and Plasma from 0
 
 Without this fix Add to desktop adds to the next desktop or does not add if 
 you're on the last desktop. 
 
 
 This addresses bug https://bugs.kde.org/show_bug.cgi?id=290368.
 
 http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290368
 
 
 Diffs
 -
 
   plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp cf12903 
 
 Diff: http://git.reviewboard.kde.org/r/103645/diff/diff
 
 
 Testing
 ---
 
 Local tests as thorough as I could do.
 
 
 Thanks,
 
 Anne-Marie Mahfouf
 


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


Re: Review Request: Fix Add to desktop from Kickoff when you have several virtual desktops and you enable Different widgets for each desktop

2012-01-11 Thread Martin Gräßlin


 On Jan. 11, 2012, 1:29 p.m., Aaron J. Seigo wrote:
  tiny ws issue, but otherwise ok.
  
  and why containment starts from 0 - because that's where counting starts. 
  that virtual desktops in KWindowSystem start at '1' is pretty silly, 
  really. otherwise we end up with everything else being zeroth counted as 
  per usual, except desktops which are counted from 1. i chose not to extend 
  that oddity into plasma code.

thanks for the info. Maybe we should put that on a TODO list for frameworks 5 
to change it to start at 0 like it is with NETWM anyway. I guess that it 
started with 1 in KDE before NETWM had been introduced?


- Martin


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


On Jan. 6, 2012, 9:11 p.m., Anne-Marie Mahfouf wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/103645/
 ---
 
 (Updated Jan. 6, 2012, 9:11 p.m.)
 
 
 Review request for Plasma and Aaron J. Seigo.
 
 
 Description
 ---
 
 From Kickoff using Add to desktop when you have several virtual desktops 
 and you enable Different widgets for each desktop in the pager settings. 
 KWindowSystem starts counting from 1 and Plasma from 0
 
 Without this fix Add to desktop adds to the next desktop or does not add if 
 you're on the last desktop. 
 
 
 This addresses bug https://bugs.kde.org/show_bug.cgi?id=290368.
 
 http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290368
 
 
 Diffs
 -
 
   plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp cf12903 
 
 Diff: http://git.reviewboard.kde.org/r/103645/diff/diff
 
 
 Testing
 ---
 
 Local tests as thorough as I could do.
 
 
 Thanks,
 
 Anne-Marie Mahfouf
 


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


Review Request: Fix Add to desktop from Kickoff when you have several virtual desktops and you enable Different widgets for each desktop

2012-01-06 Thread Anne-Marie Mahfouf

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

Review request for Plasma and Aaron J. Seigo.


Description
---

From Kickoff using Add to desktop when you have several virtual desktops and 
you enable Different widgets for each desktop in the pager settings. 
KWindowSystem starts counting from 1 and Plasma from 0

Without this fix Add to desktop adds to the next desktop or does not add if 
you're on the last desktop. 


This addresses bug https://bugs.kde.org/show_bug.cgi?id=290368.

http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290368


Diffs
-

  plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp cf12903 

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


Testing
---

Local tests as thorough as I could do.


Thanks,

Anne-Marie Mahfouf

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


Re: Review Request: Fix Add to desktop from Kickoff when you have several virtual desktops and you enable Different widgets for each desktop

2012-01-06 Thread Anne-Marie Mahfouf

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

(Updated Jan. 6, 2012, 9:11 p.m.)


Review request for Plasma and Aaron J. Seigo.


Description
---

From Kickoff using Add to desktop when you have several virtual desktops and 
you enable Different widgets for each desktop in the pager settings. 
KWindowSystem starts counting from 1 and Plasma from 0

Without this fix Add to desktop adds to the next desktop or does not add if 
you're on the last desktop. 


This addresses bug https://bugs.kde.org/show_bug.cgi?id=290368.

http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290368


Diffs
-

  plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp cf12903 

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


Testing
---

Local tests as thorough as I could do.


Thanks,

Anne-Marie Mahfouf

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


Re: Review Request: Fix Add to desktop from Kickoff when you have several virtual desktops and you enable Different widgets for each desktop

2012-01-06 Thread Martin Gräßlin

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


I just consulted the documentation and checked the properties on my system: 
that looks like KWindowSystem is wrong. The first desktop is 0. And further 
studies show that the bug might be in KWin. Will investigate further...

- Martin Gräßlin


On Jan. 6, 2012, 9:11 p.m., Anne-Marie Mahfouf wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/103645/
 ---
 
 (Updated Jan. 6, 2012, 9:11 p.m.)
 
 
 Review request for Plasma and Aaron J. Seigo.
 
 
 Description
 ---
 
 From Kickoff using Add to desktop when you have several virtual desktops 
 and you enable Different widgets for each desktop in the pager settings. 
 KWindowSystem starts counting from 1 and Plasma from 0
 
 Without this fix Add to desktop adds to the next desktop or does not add if 
 you're on the last desktop. 
 
 
 This addresses bug https://bugs.kde.org/show_bug.cgi?id=290368.
 
 http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290368
 
 
 Diffs
 -
 
   plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp cf12903 
 
 Diff: http://git.reviewboard.kde.org/r/103645/diff/diff
 
 
 Testing
 ---
 
 Local tests as thorough as I could do.
 
 
 Thanks,
 
 Anne-Marie Mahfouf
 


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


Re: Review Request: Fix Add to desktop from Kickoff when you have several virtual desktops and you enable Different widgets for each desktop

2012-01-06 Thread Martin Gräßlin


 On Jan. 6, 2012, 9:33 p.m., Martin Gräßlin wrote:
  I just consulted the documentation and checked the properties on my system: 
  that looks like KWindowSystem is wrong. The first desktop is 0. And further 
  studies show that the bug might be in KWin. Will investigate further...

so just checked the relevant code in KWin and KWindowSystem: we start counting 
at 1 and just decrement/increment 1 when writing/reading the X property. 
Everything is fine with KWindowSystem reporting 1, the question is why 
Containment uses 0?


- Martin


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


On Jan. 6, 2012, 9:11 p.m., Anne-Marie Mahfouf wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/103645/
 ---
 
 (Updated Jan. 6, 2012, 9:11 p.m.)
 
 
 Review request for Plasma and Aaron J. Seigo.
 
 
 Description
 ---
 
 From Kickoff using Add to desktop when you have several virtual desktops 
 and you enable Different widgets for each desktop in the pager settings. 
 KWindowSystem starts counting from 1 and Plasma from 0
 
 Without this fix Add to desktop adds to the next desktop or does not add if 
 you're on the last desktop. 
 
 
 This addresses bug https://bugs.kde.org/show_bug.cgi?id=290368.
 
 http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=290368
 
 
 Diffs
 -
 
   plasma/desktop/applets/kickoff/ui/contextmenufactory.cpp cf12903 
 
 Diff: http://git.reviewboard.kde.org/r/103645/diff/diff
 
 
 Testing
 ---
 
 Local tests as thorough as I could do.
 
 
 Thanks,
 
 Anne-Marie Mahfouf
 


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