Re: fixing FvwmProxyMove.

2010-09-16 Thread Sergey Vlasov
On Tue, 31 Aug 2010 08:16:03 +0200 Gerard Vermeulen wrote:

 playing around with FvwmProxy, I discovered that the proxied window only
 moved right- and down-wards when dragging the proxy window.
 The move commands sent by FvwmProxy for dragging left- and upwards are
 of the form (from a log file of my login manager 'slim'):
 07:14:51  1 [fvwm][LOG]: M: Move w+-5p w+-2p
 and rejected by the code in fvwm/move_resize.c.
 
 The attached patch provides a fix (one can infer from the manual that
 'Move w+-5p w+-2p' should be valid).
[...]
 --- fvwm/move_resize.c28 Feb 2010 01:11:37 -
 1.307 +++ fvwm/move_resize.c  31 Aug 2010 05:56:41 -
 @@ -401,7 +401,19 @@
   float f;
  
   /* parse value */
 - if (sscanf(s1, %d%n, val, n)  1)
 + if (sscanf(s1, -%d%n, val, n) = 1)
 + {
 + /* i.e. -1, -+1 or --1 */
 + final_pos += (screen_size - window_size);
 + val = -val;
 + }
 + else if (
 + sscanf(s1, +%d%n, val, n) = 1 ||
 + sscanf(s1, %d%n, val, n) = 1)
 + {
 + /* i.e. 1, +1, ++1 or +-1 */
 + }
 + else
   {
   /* syntax error, ignore rest of string */
   break;

This patch breaks other documented forms of the Move command - e.g.,
Move 50-50w 50-50w, which should move window to the center of
screen.  The final_pos adjustment for positions with '-' here is not
needed - it is useful only for the initial position (to indicate
distance from the right/bottom edge of the screen to the corresponding
edge of the window), not for the subsequent relative additions which
are handled by this part of code.

Removing the offending line fixes the problem.

===

2010-09-16  Sergey Vlasov v...@altlinux.ru
* fvwm/move_resize.c (GetOnePositionArgument):
Fix parsing of commands like 'Move 50-50w 50-50w'.

Index: fvwm/move_resize.c
===
RCS file: /home/cvs/fvwm/fvwm/fvwm/move_resize.c,v
retrieving revision 1.308
diff -u -p -r1.308 move_resize.c
--- fvwm/move_resize.c  31 Aug 2010 07:05:06 -  1.308
+++ fvwm/move_resize.c  16 Sep 2010 11:35:28 -
@@ -404,7 +404,6 @@ static int GetOnePositionArgument(
if (sscanf(s1, -%d%n, val, n) = 1)
{
/* i.e. -1, -+1 or --1 */
-   final_pos += (screen_size - window_size);
val = -val;
}
else if (




Re: fixing FvwmProxyMove.

2010-09-16 Thread Thomas Adam
On 16 September 2010 13:50, Sergey Vlasov v...@altlinux.ru wrote:
 Removing the offending line fixes the problem.

Thanks.  I had to copy and paste what looked like your Changelog
entry.  Annoyingly.   Please read up on how we accept patches here.

If there's another bug that crops up with this again, be warned I'll
revert both this, and Vermeulen's patch.

-- Thomas Adam



Re: fixing FvwmProxyMove.

2010-09-16 Thread Sergey Vlasov
On Thu, Sep 16, 2010 at 02:14:57PM +0100, Thomas Adam wrote:
 On 16 September 2010 13:50, Sergey Vlasov v...@altlinux.ru wrote:
  Removing the offending line fixes the problem.
 
 Thanks.  I had to copy and paste what looked like your Changelog
 entry.  Annoyingly.   Please read up on how we accept patches here.

Oops.  (Some other projects prefer Changelog separate from the patch -
most likely to avoid rejects when someone else had also added a
Changelog entry.)

 If there's another bug that crops up with this again, be warned I'll
 revert both this, and Vermeulen's patch.

Yes, there is a bug - the patch was applied at the wrong place (I
wonder how that could happen, there are different number of leading
tabs in those places).

Here is the fixup patch, if you will not decide to revert the whole
thing (with -U10, so that at least one different context line would be
visible - yes, that much code is duplicated there):

Index: ChangeLog
===
RCS file: /home/cvs/fvwm/fvwm/ChangeLog,v
retrieving revision 1.3131
diff -u -p -U 10 -r1.3131 ChangeLog
--- ChangeLog   16 Sep 2010 13:15:51 -  1.3131
+++ ChangeLog   16 Sep 2010 13:53:26 -
@@ -1,13 +1,13 @@
 2010-09-16  Sergey Vlasov v...@altlinux.ru
* fvwm/move_resize.c (GetOnePositionArgument):
-   Fix parsing of commands like 'Move 50-50w 50-50w'
+   Fix parsing of commands like 'Move 50-50w 50-50w'.
 
 2010-08-31  Gerard Vermeulen gav...@gmail.org
* fvwm/move_resize.c (GetOnePositionArgument):
Parse commands like 'Move w+-5p w+-2p'.
 
 2010-08-09  Thomas Adam tho...@xteddy.org
* NEWS:
* configure.ac:
Updated for the FVWM 2.5.31 release.
 
Index: fvwm/move_resize.c
===
RCS file: /home/cvs/fvwm/fvwm/fvwm/move_resize.c,v
retrieving revision 1.309
diff -u -p -U 10 -r1.309 move_resize.c
--- fvwm/move_resize.c  16 Sep 2010 13:15:51 -  1.309
+++ fvwm/move_resize.c  16 Sep 2010 13:53:26 -
@@ -363,20 +363,21 @@ static int GetOnePositionArgument(
if (*s1 != 0)
{
int val;
int n;
float f;
 
/* parse value */
if (sscanf(s1, -%d%n, val, n) = 1)
{
/* i.e. -1, -+1 or --1 */
+   final_pos += (screen_size - window_size);
val = -val;
}
else if (
sscanf(s1, +%d%n, val, n) = 1 ||
sscanf(s1, %d%n, val, n) = 1)
{
/* i.e. 1, +1, ++1 or +-1 */
}
else
{
@@ -396,21 +397,20 @@ static int GetOnePositionArgument(
while (*s1 != 0)
{
int val;
int n;
float f;
 
/* parse value */
if (sscanf(s1, -%d%n, val, n) = 1)
{
/* i.e. -1, -+1 or --1 */
-   final_pos += (screen_size - window_size);
val = -val;
}
else if (
sscanf(s1, +%d%n, val, n) = 1 ||
sscanf(s1, %d%n, val, n) = 1)
{
/* i.e. 1, +1, ++1 or +-1 */
}
else
{




Re: fixing FvwmProxyMove.

2010-09-16 Thread Thomas Adam
On Thu, Sep 16, 2010 at 05:55:38PM +0400, Sergey Vlasov wrote:
 On Thu, Sep 16, 2010 at 02:14:57PM +0100, Thomas Adam wrote:
  On 16 September 2010 13:50, Sergey Vlasov v...@altlinux.ru wrote:
   Removing the offending line fixes the problem.
  
  Thanks.  I had to copy and paste what looked like your Changelog
  entry.  Annoyingly.   Please read up on how we accept patches here.
 
 Oops.  (Some other projects prefer Changelog separate from the patch -
 most likely to avoid rejects when someone else had also added a
 Changelog entry.)

Not here.  We prefer them together.

 Yes, there is a bug - the patch was applied at the wrong place (I
 wonder how that could happen, there are different number of leading
 tabs in those places).
 
 Here is the fixup patch, if you will not decide to revert the whole
 thing (with -U10, so that at least one different context line would be
 visible - yes, that much code is duplicated there):

For crying out loud.

I've pushed this for now -- when I get home, this entire function gets my
own special treatment of refactoring.  This shouldn't take two bloody
patches to get right.

Watch this space.

-- Thomas Adam