Re: Review Request: Drop Xinerama related options in KWin

2012-01-25 Thread Martin Gräßlin 

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

(Updated Jan. 26, 2012, 6:47 a.m.)


Review request for kwin and Plasma.


Changes
---

* dropped one more option
* simplified the clientArea() method by merging the parts returning the same 
values.


Description
---

As discussed on the mailinglist: drop of the xinerama related options and the 
kcm. Given that the show unmanaged windows on option is in fact not used, I 
dropped the complete KCM.


Diffs (updated)
-

  kcontrol/CMakeLists.txt 8cd9a46 
  kcontrol/xinerama/CMakeLists.txt fe332e5 
  kcontrol/xinerama/Messages.sh f4aa134 
  kcontrol/xinerama/interface_util.h 8a4fcd1 
  kcontrol/xinerama/kcmxinerama.h 18b9241 
  kcontrol/xinerama/kcmxinerama.cpp a456b2c 
  kcontrol/xinerama/test_kcm_xinerama.cpp a358a2c 
  kcontrol/xinerama/xinerama.desktop e8ce525 
  kcontrol/xinerama/xineramawidget.h 2c446a2 
  kcontrol/xinerama/xineramawidget.cpp df9cb2f 
  kcontrol/xinerama/xineramawidget.ui 90ec4d4 
  kwin/geometry.cpp a414e26 
  kwin/manage.cpp c551eac 
  kwin/options.h 9dc29cf 
  kwin/options.cpp d496569 
  kwin/tabbox/tabbox.cpp 3051316 
  kwin/toplevel.cpp ffe7f0c 
  kwin/workspace.cpp 69b4ecb 

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


Testing
---

Fullscreen: works
Maximize: works
Movment: works
Placement: works


Thanks,

Martin Gräßlin

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

Re: Review Request: Drop Xinerama related options in KWin

2012-01-25 Thread Martin Gräßlin 


> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/geometry.cpp, line 250
> > 
> >
> > should possibly be
> > 
> > if (is_multihead)
> >screen = screen_number;
> > else if (screen == -1)
> >  ??
> > 
> > is screen_number part of the "make kwin work on real multihead" thing? 
> > looks like it's statically assigned to DefaultScreen(dpy) in main.cpp what 
> > does not cover the case when one head has multiple (randr) screens?!

I'm not sure why it is that way. Last commit which changed it: "Replace -1 with 
active screen in clientArea() if needed."
So no idea whether this is related to multi head at all. -1 means active 
screen, I think.
> does not cover the case when one head has multiple (randr) screens?!
I think it is a save assumption to say that someone using multi head won't use 
xrandr as well. Even if not I think we can make that a limitation. If someone 
wants multi head he has to live with it.


> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/geometry.cpp, line 255
> > 
> >
> > not your change, but looks like senseless code duplication.
> > esp: why does the is_multihead block test "screen < 
> > screenarea[desktop].size()" but uses screenarea[desktop][screen_number]?
> > 
> > Otherwise the blocks are equal but for screen/screen_number

will keep that for now as well and probably have a more close look on why we do 
all these calculations in the first place. Seems all a bit redundant.


> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/geometry.cpp, line 283
> > 
> >
> > since we acquire sarea/warea above (including some sanity check), can't 
> > we just return it here?
> > 
> > Otherwise it maybe shouldn't be calculated / fetched against 
> > screenarea[desktop] unconditionally but just for MaximizeArea & resp. 
> > WorkArea

will keep it for now as well.


> On Jan. 22, 2012, 7:54 p.m., Thomas Lübking wrote:
> > kwin/manage.cpp, line 229
> > 
> >
> > afaics this option is dead.
> > I've that key nowhere in no config and it wasn't provided by the 
> > xinerama kcm either

yes, you are right, it's nowhere set and I dropped it.


- Martin


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


On Jan. 22, 2012, 10:21 a.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103756/
> ---
> 
> (Updated Jan. 22, 2012, 10:21 a.m.)
> 
> 
> Review request for kwin and Plasma.
> 
> 
> Description
> ---
> 
> As discussed on the mailinglist: drop of the xinerama related options and the 
> kcm. Given that the show unmanaged windows on option is in fact not used, I 
> dropped the complete KCM.
> 
> 
> Diffs
> -
> 
>   kcontrol/CMakeLists.txt 8cd9a46 
>   kcontrol/xinerama/CMakeLists.txt fe332e5 
>   kcontrol/xinerama/Messages.sh f4aa134 
>   kcontrol/xinerama/interface_util.h 8a4fcd1 
>   kcontrol/xinerama/kcmxinerama.h 18b9241 
>   kcontrol/xinerama/kcmxinerama.cpp a456b2c 
>   kcontrol/xinerama/test_kcm_xinerama.cpp a358a2c 
>   kcontrol/xinerama/xinerama.desktop e8ce525 
>   kcontrol/xinerama/xineramawidget.h 2c446a2 
>   kcontrol/xinerama/xineramawidget.cpp df9cb2f 
>   kcontrol/xinerama/xineramawidget.ui 90ec4d4 
>   kwin/geometry.cpp a414e26 
>   kwin/manage.cpp c551eac 
>   kwin/options.h 9dc29cf 
>   kwin/options.cpp d496569 
>   kwin/tabbox/tabbox.cpp 3051316 
>   kwin/toplevel.cpp ffe7f0c 
>   kwin/workspace.cpp 69b4ecb 
> 
> Diff: http://git.reviewboard.kde.org/r/103756/diff/diff
> 
> 
> Testing
> ---
> 
> Fullscreen: works
> Maximize: works
> Movment: works
> Placement: works
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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

Re: Review Request: Make text color apply even when you go back to the first char of the notes, see bug 291791

2012-01-25 Thread Commit Hook 

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


This review has been submitted with commit 
5e772a8c2aea8d24f0a59e09e78c096e9b25aa51 by Anne-Marie Mahfouf to branch 
KDE/4.8.

- Commit Hook


On Jan. 22, 2012, 10:56 a.m., Anne-Marie Mahfouf wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103758/
> ---
> 
> (Updated Jan. 22, 2012, 10:56 a.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> ---
> 
> Fixes 291791 
> 
> 
> This addresses bug 291791.
> http://bugs.kde.org/show_bug.cgi?id=291791
> 
> 
> Diffs
> -
> 
>   applets/notes/notes.cpp e8ef779 
> 
> Diff: http://git.reviewboard.kde.org/r/103758/diff/diff
> 
> 
> Testing
> ---
> 
> Local tests following bug 291791 steps 
> 
> 
> Thanks,
> 
> Anne-Marie Mahfouf
> 
>

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