Re: fixing FvwmProxyMove.
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 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
Re: fixing FvwmProxyMove.
On Thu, Sep 16, 2010 at 02:14:57PM +0100, Thomas Adam wrote: > On 16 September 2010 13:50, Sergey Vlasov 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 * 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 * fvwm/move_resize.c (GetOnePositionArgument): Parse commands like 'Move w+-5p w+-2p'. 2010-08-09 Thomas Adam * 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.
On 16 September 2010 13:50, Sergey Vlasov 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.
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 * 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 (