Review Request: KMainWindow::parseGeometry() fails to position with positive coordinates

[Dawit Alemayehu]

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


Looks good, however should the new if statements have an else that defaulted to previous behavior ? That is should not

  if ( !(m & XValue) )
      x = this->x();
  if ( !(m & YValue) )
      y = this->y();

be 

  x = (!(m & XValue)) ? this->x() : geometry.x();
  y = (!(m & YValue)) ? this->y() : geometry.y();

instead or using the else statement if you prefer ?


- Dawit


On June 3, 2011, 6:58 p.m., Urban Widmark wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101492/
> -----------------------------------------------------------
> 
> (Updated June 3, 2011, 6:58 p.m.)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> When an X geometry is given on the command line, parseGeometry() will, for positive positions, use geometry().x()/.y() instead of the x/y value parsed from the string. This causes positive positions to not work. For negative values the string values are used.
> 
> No direct bugs reported on this that I can find, but the odd position behavior is noted in some --geometry related bugs:
> Comment #6, http://bugs.kde.org/show_bug.cgi?id=165355
> http://bugs.kde.org/show_bug.cgi?id=230663
> 
> 
> For the KMainWindow --geometry parsing to work for both size and position, the client application will have to call applyMainWindowSettings() or restoreWindowSize(). The parsing done by KMainWindowPrivate::init() will only set position. Not sure if that is good, as a user of the window I would expect it to use all of the --geometry data at the same time (either on creation or some later call).
> 
> 
> Diffs
> -----
> 
>   kdeui/widgets/kmainwindow.cpp 1d27722 
> 
> Diff: http://git.reviewboard.kde.org/r/101492/diff
> 
> 
> Testing
> -------
> 
> Using a KApplication program with a KMainWindow that also calls applyMainWindowSettings (keditbookmarks), positions verified using xwininfo:
> keditbookmarks --geometry 400x300+100+200
> keditbookmarks --geometry 400x300-100+200
> keditbookmarks --geometry 400x300+100-200
> keditbookmarks --geometry 400x300-100+200
> keditbookmarks --geometry 400x300
> keditbookmarks --geometry +100+200
> 
> Without patch all +coords are replaced by 0.
> Negative positions do not account for window decorations as the size of those are not known. I suspect the user will have to adjust for that in their input.
> 
> 
> Thanks,
> 
> Urban
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20110603/cbcc2fc8/attachment.htm>