[RP] [GIT PATCH 02/02] clean up set_active_window_body() - refactor new helper functions

[J.R. Mauro]

>From d364b0cbb67d1160d73f750b5dcfd1df51223622 Mon Sep 17 00:00:00 2001
From: J.R. Mauro <address@hidden>
Date: Tue, 29 Jul 2008 21:24:51 -0400
Subject: [PATCH] Refined new set_active_window_body helper functions a little

---
 src/window.c |   60 ++++++++++++++++++++++++++++-----------------------------
 1 files changed, 29 insertions(+), 31 deletions(-)

diff --git a/src/window.c b/src/window.c
index d98b9f0..305a5e3 100644
--- a/src/window.c
+++ b/src/window.c
@@ -553,8 +553,6 @@ void set_active_window_force (rp_window *win)
   set_active_window_body(win, 1);
 }
 
-/* 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)
 {
@@ -599,51 +597,48 @@ set_active_window_body (rp_window *win, int force)
   hook_run (&rp_switch_win_hook);
 }
 
-/* Gets frame from window 
+/* Gets frame from window -- helper to set_active_window_body
  *
- * last_frame is modified by this... but since this is trying to fix ugly code
- * I guess it can be a little ugly.
+ * While this fct returns a value, it also modifies a param, last_frame,
+ * which is ugly... but since this is trying to fix ugly code I guess it
+ * can be a little ugly.
  */
 rp_frame*
 get_frame(rp_window *win, rp_frame *last_frame)
 {
        rp_frame* frame=NULL;
-       /* 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.
+
+       /* The old function had a big if/else, but only 2 statements differed,
+        * so we should wrap just those in an if/else block
         */
-       if (rp_have_xinerama)
+
+       /* 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)
-               {
+               /* 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)
                        frame = screen_get_frame (current_screen(), 
win->intended_frame_number);
-                       win->intended_frame_number = -1;
-                       if (frame != current_frame())
-                               last_frame = current_frame();
-               }
+               else
+                       frame = screen_get_frame (win->scr, 
win->intended_frame_number);
 
-               if (!frame)
-                       frame = screen_get_frame (current_screen(), 
current_screen()->current_frame);
+               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)
+       if (!frame) {
+               if (rp_have_xinerama)
+                       frame = screen_get_frame (current_screen(), 
current_screen()->current_frame);
+               else
                        frame = screen_get_frame (win->scr, 
win->scr->current_frame);
        }
 
        return frame;
 }
 
-/* Finds a non-dedicated frame */
+/* Finds a non-dedicated frame -- helper to set_active_window_body */
 void
 find_non_dedicated_frame(rp_window *win, rp_frame *frame, rp_frame *last_frame)
 {
@@ -657,7 +652,7 @@ find_non_dedicated_frame(rp_window *win, rp_frame *frame, 
rp_frame *last_frame)
 
        /* Try the only / current screen... */
        for (cur = list_next_entry (frame, &scr->frames, node);
-                       cur != frame && !done;
+                       cur != frame;
                        cur = list_next_entry (cur, &scr->frames, node))
        {
                if (!cur->dedicated)
@@ -665,11 +660,12 @@ find_non_dedicated_frame(rp_window *win, rp_frame *frame, 
rp_frame *last_frame)
                        set_active_frame (cur, 0);
                        last_frame = frame;
                        frame = cur;
-                       done = 1;
+                       break; /* we do not need the done variable */
                }
        }
 
        /* If we have Xinerama, we can check *all* screens... */
+       /* TODO: could this be put into yet another function? */
        if (rp_have_xinerama && !done)
        {
                int i;
@@ -684,6 +680,8 @@ find_non_dedicated_frame(rp_window *win, rp_frame *frame, 
rp_frame *last_frame)
                                        set_active_frame (cur, 0);
                                        last_frame = frame;
                                        frame = cur;
+
+                                       /* Good case for a goto here? */
                                        done = 1; /* Break outer loop. */
                                        break;    /* Break inner loop. */
                                }
-- 
1.5.4.3