patch: minor refactor of long function

Jeff Abrahamson <[email protected]> Tue, 16 Sep 2014 21:50:43 +0200

Newsgroups	gmane.comp.window-managers.ratpoison.devel
Message-ID	<CAM4Y7zxM7Rb+7BpGZV8EdrpW+RnN0EbY3tefU8tcqzv98Do=Jw@mail.gmail.com>

I'm doing clean-up tonight. I may have mailed this previously, but since
it's not in HEAD, I'll assume I didn't.

This is just a minor refactor of cmd_select(), which is hard to understand
in part due to its length. This patch should not change functionality.

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-cmd_select-into-two-shorter-functions.patch (text/x-patch, 3.6 KB)

From 154b22fe570a7e60b2438ab71e099d03ce0f93aa Mon Sep 17 00:00:00 2001
From: Jeff Abrahamson <[email protected]>
Date: Mon, 28 Jul 2014 16:03:39 +0100
Subject: [PATCH] Refactor cmd_select() into two shorter functions.

The first does basic argument checking, then calls the second to do
the actual select if appropriate.
---
 src/actions.c | 91 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 49 insertions(+), 42 deletions(-)

diff --git a/src/actions.c b/src/actions.c
index 7579101..8705078 100644
--- a/src/actions.c
+++ b/src/actions.c
@@ -1363,13 +1363,56 @@ window_completions (char* str UNUSED)
   return list;
 }
 
+/* Helper function for cmd_select(). */
+cmdret *
+cmd_select_string (char *str)
+{
+  int n;
+
+  if (strlen (str) == 1 && str[0] == '-')
+    {
+      blank_frame (current_frame());
+      return cmdret_new (RET_SUCCESS, NULL);
+    }
+  /* try by number */
+  if ((n = string_to_positive_int (str)) >= 0)
+    {
+      rp_window_elem *elem = group_find_window_by_number (rp_current_group, n);
+
+      if (elem)
+        {
+          goto_window (elem->win);
+          return cmdret_new (RET_SUCCESS, NULL);
+        }
+      else
+        {
+          /* show the window list as feedback */
+          show_bar (current_screen (), defaults.window_fmt);
+          return cmdret_new (RET_FAILURE,
+                            "select: unknown window number '%d'", n);
+        }
+    }
+
+  /* try by name */
+  rp_window *win = find_window_name (str, 1);
+  
+  if (!win)
+    win = find_window_name (str, 0);
+  
+  if (win)
+    {
+      goto_window (win);
+      return cmdret_new (RET_SUCCESS, NULL);
+    }
+  return cmdret_new (RET_FAILURE, "select: unknown window '%s'", str);
+}
+
 /* switch to window number or name */
 cmdret *
 cmd_select (int interactive UNUSED, struct cmdarg **args)
 {
   cmdret *ret = NULL;
   char *str;
-  int n;
 
   /* FIXME: This is manually done because of the kinds of things
      select accepts. */
@@ -1386,49 +1429,13 @@ cmd_select (int interactive UNUSED, struct cmdarg **args)
   /* Only search if the string contains something to search for. */
   if (strlen (str) > 0)
     {
-      if (strlen (str) == 1 && str[0] == '-')
-        {
-          blank_frame (current_frame());
-          ret = cmdret_new (RET_SUCCESS, NULL);
-        }
-      /* try by number */
-      else if ((n = string_to_positive_int (str)) >= 0)
-        {
-          rp_window_elem *elem = group_find_window_by_number (rp_current_group, n);
-
-          if (elem)
-            {
-              goto_window (elem->win);
-              ret = cmdret_new (RET_SUCCESS, NULL);
-            }
-          else
-            {
-              /* show the window list as feedback */
-              show_bar (current_screen (), defaults.window_fmt);
-              ret = cmdret_new (RET_FAILURE,
-                                "select: unknown window number '%d'", n);
-            }
-        }
-      else
-        /* try by name */
-        {
-          rp_window *win = find_window_name (str, 1);
-
-          if (!win)
-            win = find_window_name (str, 0);
-
-          if (win)
-            {
-              goto_window (win);
-              ret = cmdret_new (RET_SUCCESS, NULL);
-            }
-          else
-            ret = cmdret_new (RET_FAILURE, "select: unknown window '%s'", str);
-        }
+      ret = cmd_select_string(str);
     }
   else
-    /* Silently fail, since the user didn't provide a window spec */
-    ret = cmdret_new (RET_SUCCESS, NULL);
+    {
+      /* User provided no window spec, so succeed but do nothing. */
+      ret = cmdret_new (RET_SUCCESS, NULL);
+    }
 
   free (str);
 
-- 
1.9.1