patch: trying to make sense of set_active_window_body()

Jeff Abrahamson <[email protected]> Tue, 16 Sep 2014 21:57:40 +0200
Newsgroups gmane.comp.window-managers.ratpoison.devel
Message-ID <CAM4Y7zyxHL1EF4crPCc-+aRUpLRqM-XD42U+fW_7K+4p50EGVQ@mail.gmail.com>
A minor refactor of set_active_window_body() in an attempt to make it a bit
easier to follow. This patch should not change behavior.

(This, too, I may have sent once upon a time. My apologies if it is already
sitting on a queue waiting patiently.)

Jeff Abrahamson
+33 6 24 40 01 57
+44 7920 594 255    <-- only if I'm in the UK

http://jeff.purple.com/
http://blog.purple.com/jeff/

_______________________________________________
Ratpoison-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/ratpoison-devel
0001-Refactor-duplicate-branches-of-if-into-a-single-bloc.patch (text/x-patch, 2 KB)
From 06a3f96fe162e5aadb032c8b00b37815abbfec1d Mon Sep 17 00:00:00 2001
From: Jeff Abrahamson <[email protected]>
Date: Mon, 7 Jul 2014 19:59:05 +0200
Subject: [PATCH 1/2] Refactor duplicate branches of if() into a single block
 with leading ?:.

---
 src/window.c | 34 +++++++++-------------------------
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/src/window.c b/src/window.c
index de9032a..9d784ea 100644
--- a/src/window.c
+++ b/src/window.c
@@ -467,34 +467,18 @@ set_active_window_body (rp_window *win, int force)
   /* With Xinerama, we can move a window over to the current screen; otherwise
    * we have to switch to the screen that the window belongs to.
    */
-  if (rp_have_xinerama)
+  rp_screen *screen = (rp_have_xinerama ? current_screen() : win->scr);
+  /* use the intended frame if we can. */
+  if (win->intended_frame_number >= 0)
     {
-      /* use the intended frame if we can. */
-      if (win->intended_frame_number >= 0)
-        {
-          frame = screen_get_frame (current_screen(), win->intended_frame_number);
-          win->intended_frame_number = -1;
-          if (frame != current_frame())
-            last_frame = current_frame();
-        }
-
-      if (!frame)
-        frame = screen_get_frame (current_screen(), current_screen()->current_frame);
+      frame = screen_get_frame (screen, win->intended_frame_number);
+      win->intended_frame_number = -1;
+      if (frame != current_frame())
+        last_frame = current_frame();
     }
-  else
-    {
-      /* use the intended frame if we can. */
-      if (win->intended_frame_number >= 0)
-        {
-          frame = screen_get_frame (win->scr, win->intended_frame_number);
-          win->intended_frame_number = -1;
-          if (frame != current_frame())
-            last_frame = current_frame();
-        }
 
-      if (!frame)
-        frame = screen_get_frame (win->scr, win->scr->current_frame);
-    }
+  if (!frame)
+    frame = screen_get_frame (screen, screen->current_frame);
 
   if (frame->dedicated && !force)
     {
-- 
1.9.1
0002-Refactor-a-bit-of-set_active_window_body-into-a-help.patch (text/x-patch, 3.9 KB)
From ca95ed9c335d31dc774738509a162c9eb710e222 Mon Sep 17 00:00:00 2001
From: Jeff Abrahamson <[email protected]>
Date: Tue, 8 Jul 2014 00:25:02 +0200
Subject: [PATCH 2/2] Refactor a bit of set_active_window_body() into a helper
 function.

---
 src/window.c | 99 ++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 53 insertions(+), 46 deletions(-)

diff --git a/src/window.c b/src/window.c
index 9d784ea..ad8c9fc 100644
--- a/src/window.c
+++ b/src/window.c
@@ -452,13 +452,64 @@ void set_active_window_force (rp_window *win)
   set_active_window_body(win, 1);
 }
 
+/* Helper function for set_active_window_body(). */
+static void set_active_window_body_update_frame(rp_window *win,
+                                                rp_frame **frame,
+                                                rp_frame **last_frame)
+{
+  /* Try to find a non-dedicated frame.  */
+  rp_frame *cur;
+  rp_screen *scr;
+  int done;
+
+  scr = (rp_have_xinerama ? &screens[rp_current_screen] : win->scr);
+  done = 0;
+
+  /* Try the only / current screen... */
+  for (cur = list_next_entry ((*frame), &scr->frames, node);
+       cur != *frame && !done;
+       cur = list_next_entry (cur, &scr->frames, node))
+    {
+      if (!cur->dedicated)
+        {
+          set_active_frame (cur, 0);
+          *last_frame = *frame;
+          *frame = cur;
+          done = 1;
+        }
+    }
+
+  /* If we have Xinerama, we can check *all* screens... */
+  if (rp_have_xinerama && !done)
+    {
+      int i;
+
+      for (i=0; i<num_screens && !done; i++)
+        {
+          if (scr == &screens[i]) continue;
+          list_for_each_entry (cur,&screens[i].frames,node)
+            {
+              if (!cur->dedicated)
+                {
+                  set_active_frame (cur, 0);
+                  *last_frame = *frame;
+                  *frame = cur;
+                  done = 1; /* Break outer loop. */
+                  break;    /* Break inner loop. */
+                }
+            }
+        }
+    }
+}
+
 /* FIXME: This function is probably a mess. I can't remember a time
    when I didn't think this. It probably needs to be fixed up. */
 void
 set_active_window_body (rp_window *win, int force)
 {
   rp_window *last_win;
-  rp_frame *frame = NULL, *last_frame = NULL;
+  rp_frame *frame = NULL;
+  rp_frame *last_frame = NULL;
 
   if (win == NULL) return;
 
@@ -481,51 +532,7 @@ set_active_window_body (rp_window *win, int force)
     frame = screen_get_frame (screen, screen->current_frame);
 
   if (frame->dedicated && !force)
-    {
-      /* Try to find a non-dedicated frame.  */
-      rp_frame *cur;
-      rp_screen *scr;
-      int done;
-
-      scr = (rp_have_xinerama)?&screens[rp_current_screen]:win->scr;
-      done = 0;
-
-      /* Try the only / current screen... */
-      for (cur = list_next_entry (frame, &scr->frames, node);
-           cur != frame && !done;
-           cur = list_next_entry (cur, &scr->frames, node))
-        {
-          if (!cur->dedicated)
-            {
-              set_active_frame (cur, 0);
-              last_frame = frame;
-              frame = cur;
-              done = 1;
-            }
-        }
-
-      /* If we have Xinerama, we can check *all* screens... */
-      if (rp_have_xinerama && !done)
-        {
-          int i;
-
-          for (i=0; i<num_screens && !done; i++)
-            {
-              if (scr == &screens[i]) continue;
-              list_for_each_entry (cur,&screens[i].frames,node)
-                {
-                  if (!cur->dedicated)
-                    {
-                      set_active_frame (cur, 0);
-                      last_frame = frame;
-                      frame = cur;
-                      done = 1; /* Break outer loop. */
-                      break;    /* Break inner loop. */
-                    }
-                }
-            }
-        }
-    }
+    set_active_window_body_update_frame(win, &frame, &last_frame);
 
   last_win = set_frames_window (frame, win);
 
-- 
1.9.1