Subject: One last patch honest

From: Tim Phipps <[email-masked]>
Date: Tue, 24 Nov 1998 15:54:46 +0000

OK last one from me for a while, Just a quick one that makes WindowList
put the current window at the top of the list rather than starting at
Scr.FvwmRoot.next

This makes Alt-TAB bound to Windowlist behave much more like that other
GUI and it doesn't break Next & Prev.  I had thought about changing the
Focus command to make sure that Scr.FvwmRoot.next was always the same as
Scr.Focus but got lost in the linked list stuff.

Cheers,
Tim.

PS there is only one reference to Scr.FvwmRoot.prev (in line 3758 of
builtins.c)
It should probably be replaced with NULL as that is what
Scr.FvwmRoot.prev is always set to and it might prevent future
confusion.

  if(Scr.Focus != NULL)
  {
    if(Direction == 1)
      fw = Scr.Focus->prev;
    else if(Direction == -1)
      fw = Scr.Focus->next;
    else
      fw = Scr.Focus;
  }
  else
    fw = Scr.FvwmRoot.prev; <<<<------!!!!!

  while((pass < 3)&&(found == NULL))
  {
    while((fw != NULL)&&(found==NULL)&&(fw != &Scr.FvwmRoot))
    {

Index: windows.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/windows.c,v
retrieving revision 1.7
diff -u -B -b -r1.7 windows.c
--- windows.c	1998/11/23 10:40:08	1.7
+++ windows.c	1998/11/24 15:40:17
@@ -184,11 +184,15 @@
   {
     return;
   }
-  ii = 0;
-  for (t = Scr.FvwmRoot.next; t != NULL; t = t->next)
+  /* get the windowlist starting from the current window (if any)*/
+  if ((t = Scr.Focus) == NULL) t = Scr.FvwmRoot.next;
+  for (ii = 0; ii < numWindows; ii++)
   {
     windowList[ii] = t;
-    ii++;
+    if (t->next)
+      t = t->next;
+    else
+      t = Scr.FvwmRoot.next;
   }
 
   /* Do alphabetic sort */

---

From: Tim Phipps <[email-masked]>
Date: Wed, 25 Nov 1998 11:58:25 +0000

> put the current window at the top of the list rather than starting at

I know, but I found a genuine bug in WindowList so here's another patch:

*) WindowList Unsort loops forever. I fixed this and renamed Unsort
to    NoDeskSort to make its meaning clear.

*) Became very brave and fixed SetFocus so that calling Focus from a
function moves the windowlist around to make the target window appears
at the top of the windowlist.

*) Cleaned up FlipFocus and fixed the man page.

Cheers,
Tim.

PS I think with this we have at last got a way to replicate the
Windows95 Alt-tab by using the windowlist menu.  The currently focused
window is always at the top if you use "WindowList NoDeskSort" or
"WindowList CurrentDesk" and if you replace the builtin windowlist
function with:

DestroyFunc WindowListFunc
AddToFunc WindowListFunc
+ "I" WindowId $0 Iconify -1
+ "I" WindowId $0 FlipFocus
+ "I" WindowId $0 Raise
+ "I" WindowId $0 WarpToWindow 5p 5p 

you get the most recently used ordering.

Index: focus.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/focus.c,v
retrieving revision 1.2
diff -u -B -b -u -r1.2 focus.c
--- focus.c	1998/10/31 11:52:48	1.2
+++ focus.c	1998/11/25 11:49:11
@@ -40,19 +40,35 @@
 
   /* ClickToFocus focus queue manipulation - only performed for
    * Focus-by-mouse type focus events */
-  if ((FocusByMouse)&&(Fw && Fw != Scr.Focus && Fw != &Scr.FvwmRoot))
+  if (Fw && Fw != Scr.Focus && Fw != &Scr.FvwmRoot)
+    if (FocusByMouse) /* pluck window from list and deposit at top */
     {
-      FvwmWindow *tmp_win1, *tmp_win2;
+      /* remove Fw from list */
+      Fw->prev->next = Fw->next;
+      if (Fw->next) Fw->next->prev = Fw->prev;
       
-      tmp_win1 = Fw->prev;
-      tmp_win2 = Fw->next;
+      /* insert Fw at start */
+      Fw->next = Scr.FvwmRoot.next;
+      Scr.FvwmRoot.next->prev = Fw;
+      Scr.FvwmRoot.next = Fw;
+      Fw->prev = &Scr.FvwmRoot;
+    }
+    else /* move the windowlist around so that Fw is at the top */
+    {
+      FvwmWindow *tmp_win;
       
-      if (tmp_win1) tmp_win1->next = tmp_win2;
-      if (tmp_win2) tmp_win2->prev = tmp_win1;
+      /* find the end of the windowlist */
+      tmp_win = Scr.FvwmRoot.next; 
+      while (tmp_win->next != NULL)
+        tmp_win = tmp_win->next;
       
-      Fw->next = Scr.FvwmRoot.next;
-      if(Scr.FvwmRoot.next)Scr.FvwmRoot.next->prev = Fw;
+      /* close the ends of the windowlist */
+      tmp_win->next = Scr.FvwmRoot.next;
+      Scr.FvwmRoot.next->prev = tmp_win;
+      
+      /* make Fw the new start of the list */
       Scr.FvwmRoot.next = Fw;
+      Fw->prev->next = NULL;
       Fw->prev = &Scr.FvwmRoot;
     }                                 
 
Index: fvwm2.1
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/fvwm2.1,v
retrieving revision 1.14
diff -u -B -b -u -r1.14 fvwm2.1
--- fvwm2.1	1998/11/24 10:48:20	1.14
+++ fvwm2.1	1998/11/25 11:49:50
@@ -1201,8 +1201,10 @@

 .IP "FlipFocus"
-Toggles focus between the last two focused windows. Note that
-FlipFocus works only as the command argument of \fIPrev\fP.
+Executes a \fIFocus\fP command as if the user had used the pointer to select
+the window.  This command alters the order of the windowlist in the same way
+as clicking in a window to focus, i.e. the target window is removed from the
+windowlist and placed at the start.

 .IP "Focus"
@@ -1210,6 +1212,8 @@
 visible.  Sets the keyboard focus to the selected window.  Raises the
 window if needed to make it visible.  Does not warp the pointer into
 the selected window (see WarpToWindow function).  Does not de-iconify.
+This command does not alter the order of the windowlist, it rotates the
+windowlist around so that the target window is at the start.

 .IP "Function \fI\FunctionName\\fP"
@@ -2382,7 +2386,7 @@
 The \fIoptions\fP passed to WindowList can be "NoGeometry", "Function
 <funcname>", "Desk <desknum>", "CurrentDesk", "NoIcons", "Icons",
 "OnlyIcons", "NoNormal", "Normal", "OnlyNormal", "NoSticky", "Sticky",
-"OnlySticky", "NoOnTop", "OnTop", "OnlyOnTop", "Unsorted", "UseIconName",
+"OnlySticky", "NoOnTop", "OnTop", "OnlyOnTop", "NoDeskSort", "UseIconName",
 "Alphabetic", "NotAlphabetic".
 
 (Note - normal means not iconic, sticky, or ontop)
Index: windows.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/windows.c,v
retrieving revision 1.8
diff -u -B -b -u -r1.8 windows.c
--- windows.c	1998/11/25 10:04:00	1.8
+++ windows.c	1998/11/25 11:50:04
@@ -32,7 +32,7 @@
 #define SHOW_ICONIC   (1<<3)
 #define SHOW_STICKY   (1<<4)
 #define SHOW_ONTOP    (1<<5)
-#define DONT_SORT     (1<<6)
+#define NO_DESK_SORT  (1<<6)
 #define SHOW_ICONNAME (1<<7)
 #define SHOW_ALPHABETIC (1<<8)
 #define SHOW_EVERYTHING (SHOW_GEOMETRY | SHOW_ALLDESKS | SHOW_NORMAL | SHOW_ICONIC | SHOW_STICKY | SHOW_ONTOP)
@@ -68,7 +68,7 @@
   int dwidth,dheight;
   char tlabel[50]="";
   int last_desk_done = INT_MIN;
-  int next_desk;
+  int next_desk = 0;
   char *t_hot=NULL;		/* Menu label with hotkey added */
   char scut = '0';		/* Current short cut key */
   char *line=NULL,*tok=NULL;
@@ -117,8 +117,8 @@
         flags &= ~SHOW_ALPHABETIC;
       else if (StrEquals(tok,"Alphabetic"))
         flags |= SHOW_ALPHABETIC;
-      else if (StrEquals(tok,"Unsorted"))
-        flags |= DONT_SORT;
+      else if (StrEquals(tok,"NoDeskSort"))
+        flags |= NO_DESK_SORT;
       else if (StrEquals(tok,"UseIconName"))
         flags |= SHOW_ICONNAME;
       else if (StrEquals(tok,"NoGeometry"))
@@ -172,8 +172,6 @@
   mr=NewMenuRoot(tlabel, False);
   AddToMenu(mr, tlabel, "TITLE", FALSE, FALSE);
 
-  next_desk = 0;
-
   numWindows = 0;
   for (t = Scr.FvwmRoot.next; t != NULL; t = t->next)
   {
@@ -203,8 +201,9 @@
   while(next_desk != INT_MAX)
   {
     /* Sort window list by desktop number */
-    if((flags & SHOW_ALLDESKS) && !(flags & DONT_SORT))
+    if((flags & SHOW_ALLDESKS) && !(flags & NO_DESK_SORT))
     {
+      /* run through the windowlist finding the first desk not already processed */
       next_desk = INT_MAX;
       for (ii = 0; ii < numWindows; ii++)
       {
@@ -215,16 +214,20 @@
     }
     if(!(flags & SHOW_ALLDESKS))
     {
+      /* if only doing one desk and it hasn't been done */
       if(last_desk_done  == INT_MIN)
-        next_desk = desk;
+        next_desk = desk; /* select the desk */
       else
-        next_desk = INT_MAX;
+        next_desk = INT_MAX; /* flag completion */
     }
+    if(flags & NO_DESK_SORT)
+      next_desk = INT_MAX; /* only go through loop once */
+      
     last_desk_done = next_desk;
     for (ii = 0; ii < numWindows; ii++)
     {
       t = windowList[ii];
-      if((t->Desk == next_desk)&&
+      if(((t->Desk == next_desk) || (flags & NO_DESK_SORT)) &&
          (!(t->flags & WINDOWLISTSKIP)))
       {
         if (!(flags & SHOW_ICONIC) && (t->flags & ICONIFIED))
@@ -239,13 +242,13 @@
               (t->flags & ONTOP)))
           continue; /* don't want "normal" ones - skip */
 
-        if (++scut == ('9' + 1)) scut = 'A';	/* Next shortcut key */
         if(flags & SHOW_ICONNAME)
           name = t->icon_name;
         else
           name = t->name;
         t_hot = safemalloc(strlen(name) + strlen(tname) + 48);
         sprintf(t_hot, "&%c.  %s", scut, name); /* Generate label */
+        if (scut++ == '9') scut = 'A';	/* Next shortcut key */
 
         if (flags & SHOW_GEOMETRY)
         {
Index: builtins.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/builtins.c,v
retrieving revision 1.27
diff -u -B -b -u -r1.27 builtins.c
--- builtins.c	1998/11/24 18:31:11	1.27
+++ builtins.c	1998/11/25 11:50:43
@@ -991,17 +991,6 @@
 #endif
 }
 
-
-void flip_focus_reorder(FvwmWindow *w)
-{
-  if( w->next ) w->next->prev = w->prev;
-  if( w->prev ) w->prev->next = w->next;
-  w->next = Scr.FvwmRoot.next;
-  w->prev = &Scr.FvwmRoot;
-  if(Scr.FvwmRoot.next)Scr.FvwmRoot.next->prev = w;
-  Scr.FvwmRoot.next = w;
-}
-
 void flip_focus_func(XEvent *eventp,Window w,FvwmWindow *tmp_win,
 		unsigned long context, char *action, int *Module)
 {
@@ -1009,14 +998,7 @@
     return;
 
   /* Reorder the window list */
-  if( Scr.Focus ){
-    flip_focus_reorder(Scr.Focus);
-  }
-  if( tmp_win != Scr.Focus ){
-    flip_focus_reorder(tmp_win);
-  }
-
-  FocusOn(tmp_win,0);
+  FocusOn(tmp_win,TRUE);
 }

@@ -1026,7 +1008,7 @@
   if (DeferExecution(eventp,&w,&tmp_win,&context,SELECT,ButtonRelease))
     return;
 
-  FocusOn(tmp_win,0);
+  FocusOn(tmp_win,FALSE);
 }

@@ -3755,7 +3737,7 @@
       fw = Scr.Focus;
   }
   else
-    fw = Scr.FvwmRoot.prev;
+    fw = NULL;

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 26 Nov 1998 11:09:20 +0100

> *) Cleaned up FlipFocus and fixed the man page.

It coredumps. Stack trace:

Core was generated by `fvwm2 -cmd FvwmCpp -Cppprog cpp -P -C /home/avatar/.fvwm2rc'.
Program terminated with signal 11, Segmentation fault.
#0  0x805af54 in SetFocus (w=29360170, Fw=0x809b7f0, FocusByMouse=1)
    at focus.c:52
52            Scr.FvwmRoot.next->prev = Fw;
(gdb) where
#0  0x805af54 in SetFocus (w=29360170, Fw=0x809b7f0, FocusByMouse=1)
    at focus.c:52
#1  0x805a420 in HandleButtonPress () at events.c:1044
#2  0x805916a in DispatchEvent () at events.c:155
#3  0x80591aa in HandleEvents () at events.c:176
#4  0x805bf2d in main (argc=3, argv=0xbffff7c8) at fvwm.c:469
#5  0x804aeae in ___crt_dummy__ ()

and another one:

#0  0x805af54 in SetFocus (w=16777218, Fw=0x8096e60, FocusByMouse=1)
    at focus.c:52
52            Scr.FvwmRoot.next->prev = Fw;
(gdb) where
#0  0x805af54 in SetFocus (w=16777218, Fw=0x8096e60, FocusByMouse=1)
    at focus.c:52
#1  0x805a7fe in HandleEnterNotify () at events.c:1200
#2  0x805916a in DispatchEvent () at events.c:155
#3  0x80591aa in HandleEvents () at events.c:176
#4  0x805bf2d in main (argc=3, argv=0xbffff7c8) at fvwm.c:469
#5  0x804aeae in ___crt_dummy__ ()

Scr.FvwmRoot. prev may not have been useless after all.
There seems to be a ring list.

Bye

Dominik
-- 
Dominik Vogt, Hewlett-Packard GmbH, Dept. BVS
Herrenberger Str.130, 71034 Boeblingen, Germany
phone: 07031/14-4596, fax: 07031/14-3883, [email-masked]
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked].

---

From: Tim Phipps <[email-masked]>
Date: Thu, 26 Nov 1998 12:44:27 +0000

> It coredumps. Stack trace:

Looks like I was bit over zealous in removing some tests that I thought
were always going to return true.  I'm looking at fixing this and in
hunting around the code I found something a bit funny looking.  In
events.c around line 880 is the following:

  if((Tmp_win->flags & ClickToFocus)&&(Scr.Focus)&&
     ((!Scr.Focus)||(Scr.Focus->flags & ClickToFocus)))
    {
      if (OnThisPage)
        {
          SetFocus(Tmp_win->w,Tmp_win,1);
        }
    }

Surely (Scr.Focus)&&(!Scr.Focus) is always false and the test should be

  if((Tmp_win->flags & ClickToFocus)&&
     ((!Scr.Focus)||(Scr.Focus->flags & ClickToFocus)))

i.e. focus on ClickToFocus windows if there is no focussed window or the
focussed window is also ClickToFocus.

Can we rely on the order of evaluation to make sure that if Scr.Focus is
NULL the Scr.Focus->flags reference never happens? I thought that C
specifies left to right evaluation for tests. If we can't rely on it
then this statement (and probably more) will have to be expanded into a
nested if construct.

Cheers,
Tim.

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 26 Nov 1998 14:03:29 +0100

> focussed window is also ClickToFocus.

Be careful what you change there. I don't know if you suggest the right
thing to do, but it is definitely not the same as the original code.
This would be:

  if((Tmp_win->flags & ClickToFocus)&&(Scr.Focus)&&
     (Scr.Focus->flags & ClickToFocus))

Reason: the condition can be written as:

   A and B and (not B or C)
-> (A and B and not B) or (A and B and C)
-> FALSE or (A and B and C)
-> A and B and C

> then this statement (and probably more) will have to be expanded into a

You can rely on that. If it doesn't work, the compiler is broken
and won't be of much use. As soon as the result of the test cannot
be changed by the expressions to the right, no further processing
is done. Otherwise simple things like the following wouldn't work:

  if (token && *token == 'a')

Bye

Dominik

---

From: Tim Phipps <[email-masked]>
Date: Thu, 26 Nov 1998 15:14:51 +0000

> It coredumps. Stack trace:

I had missed the case of focussing on a window that is not on the
windowlist.

Here it is again, diff's against CVS from today.  Hope it works but
don't bust a gut trying to commit it before the release.

Cheers,
Tim.

Index: add_window.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/add_window.c,v
retrieving revision 1.12
diff -u -B -b -u -r1.12 add_window.c
--- add_window.c	1998/11/24 10:25:28	1.12
+++ add_window.c	1998/11/26 15:00:22
@@ -378,12 +378,17 @@
   }

-  /* add the window into the fvwm list */
+  /* add the window to the end of the fvwm list */
   tmp_win->next = Scr.FvwmRoot.next;
-  if (Scr.FvwmRoot.next != NULL)
-    Scr.FvwmRoot.next->prev = tmp_win;
   tmp_win->prev = &Scr.FvwmRoot;
-  Scr.FvwmRoot.next = tmp_win;
+  while (tmp_win->next != NULL)
+  {
+    tmp_win->prev = tmp_win->next;
+    tmp_win->next = tmp_win->next->next;
+  }
+  /* tmp_win->prev points to the last window in the list, tmp_win->next is NULL.
+     Now fix the last window to point to tmp_win */
+  tmp_win->prev->next = tmp_win;
 
   /*
       RBW - 11/13/1998 - add it into the stacking order chain also.
Index: builtins.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/builtins.c,v
retrieving revision 1.28
diff -u -B -b -u -r1.28 builtins.c
--- builtins.c	1998/11/26 11:44:14	1.28
+++ builtins.c	1998/11/26 15:00:54
@@ -991,17 +991,6 @@
 #endif
 }
 
-

@@ -1026,7 +1008,7 @@
 
-  FocusOn(tmp_win,0);

@@ -3755,7 +3737,7 @@
       fw = Scr.Focus;
   }
   else
 
+++ focus.c	1998/11/26 15:01:08
@@ -40,21 +40,48 @@
 
+  /* Watch out: Fw may not be on the windowlist and the windowlist may be empty */
+      if (Fw->prev) Fw->prev->next = Fw->next;
       
-      tmp_win1 = Fw->prev;
-      tmp_win2 = Fw->next;
+      /* insert Fw at start */
+      Fw->next = Scr.FvwmRoot.next;
+      if (Scr.FvwmRoot.next) Scr.FvwmRoot.next->prev = Fw;
+      Scr.FvwmRoot.next = Fw;
+      Fw->prev = &Scr.FvwmRoot;
+    }
+    else
+    {
+      /* move the windowlist around so that Fw is at the top */

-      Fw->next = Scr.FvwmRoot.next;
-      if(Scr.FvwmRoot.next)Scr.FvwmRoot.next->prev = Fw;
+      /* find the window on the windowlist */
+      tmp_win = &Scr.FvwmRoot; 
+      while (tmp_win && tmp_win != Fw)
+        tmp_win = tmp_win->next;
+        
+      if (!tmp_win) /* the window is on the (non-zero length) windowlist */
+      {
+        /* make tmp_win point to the last window on the list */
+        while (tmp_win->next)
+          tmp_win = tmp_win->next;
+
+        /* close the ends of the windowlist */
+        tmp_win->next = Scr.FvwmRoot.next;
+        Scr.FvwmRoot.next->prev = tmp_win;
+
+        /* make Fw the new start of the list */
       Scr.FvwmRoot.next = Fw;
+        /* open the closed loop windowlist */
+        Fw->prev->next = NULL;
       Fw->prev = &Scr.FvwmRoot;
     }                                 
+    }
 
   if(Scr.NumberOfScreens > 1)
     {
Index: fvwm2.1
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/fvwm2.1,v
retrieving revision 1.14
diff -u -B -b -u -r1.14 fvwm2.1
--- fvwm2.1	1998/11/24 10:48:20	1.14
+++ fvwm2.1	1998/11/26 15:01:39
@@ -1201,8 +1201,10 @@

 .IP "FlipFocus"

 .IP "Focus"
@@ -1210,6 +1212,8 @@

+++ windows.c	1998/11/26 15:01:55
 
-  next_desk = 0;
-
   numWindows = 0;

---

From: Tim Phipps <[email-masked]>
Date: Thu, 26 Nov 1998 15:25:15 +0000

> thing to do, but it is definitely not the same as the original code.

Don't worry, I'm not going to touch it.  What I suggest is definitely
not the same as the existing code.  I think the existing code is not
what ClickToFocus type people want.  I'm not one of them so I will leave
it to one of them to fix and test.  The existing code won't give focus
to a new window if there is no currently focussed window.  I think that
ClickToFocus People want any new window to get the focus.  I think the
intent of this section is to give focus to new clickToFocus windows
unless the focus is on a Mouse/SloppyFocus window.  The current logic is
to give the focus to new ClickToFocus windows if the focus is on another
ClickToFocus window.

It's all a bit weird because Fvwm lets you mix ClickToFocus with
MouseFocus and it is very hard to come up with a strategy that everyone
is happy with.  e.g. what happens if the focus is on a SloppyFocus
window and a ClickToFocus window pops up under the pointer?

Cheers,
Tim.

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 26 Nov 1998 17:04:15 +0100

> window and a ClickToFocus window pops up under the pointer?

I couldn't care less. 

Where did you find that code fragment?

Dominik

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 26 Nov 1998 17:07:03 +0100

> Where did you find that code fragment?

Sorry, forget it. I didn't see it the first time I read
your mail.

Dominik