Re: select patch

Johannes Altmanninger <[email protected]> Tue, 24 Jun 2014 15:22:51 +0200
Newsgroups gmane.comp.window-managers.ratpoison.devel
Message-ID <[email protected]>
Thanks for your help! There is a whole lot to learn for me..
I corrected most issues you pointed out, this keeps getting better ;)
I changed the default behavior to prefer unselected windows, the option 
to change it is still there. Most users would probably not care too much 
about this I guess.
Now there is a function called get_compare_string_function() which is 
used in select_name(). It takes the match_type bitmask and the address 
of compare_length.
I am not sure how I would implement a function that does not take the 
match type as argument.
I noticed that there are 8 occurrences of str_comp(), is there a reason 
why this is not replaced by `!strncasecmp()`?

On 06/23/2014 04:30 PM, Jeff Abrahamson wrote:
 > Funny how documentation is always the trailing bit. ;-)
 > This is looking very nice.  Thanks for all your work on this!
 > A couple (easy) comments:
 > Just delete the stuff you're deleting, don't comment it out. Git 
remembers what was there before, and it just makes your diffs longer.  I 
know it's useful while you're developing.
 > I'm acutely aware that I'm the guy who called for the optional 
behavior -- but the more I think about it, the more I think the default 
should be the behavior you propose.  It is a very good idea.
 > On that subject, the name "SELECT_SIMPLE" seems prejudiced by the old 
behavior and otherwise devoid of meaning.  What do you think of 
SELECT_OK_SELECTED ?
 > When you define MAX_WINDOW_NAME_LENGTH, I think the value is 
arbitrary, just long enough to distinguish between valid and invalid 
strings in reasonable time.  Perhaps include a comment to that effect so 
that future readers don't pause to wonder on the significance of that 
number.
 > Personal tick:  Functions are most readable below ten or so lines.  
By the time I can't see the whole function in my editor, I have more 
trouble understanding as I read.  In that light, and without remarking 
on other examples of such, perhaps find_window_name() could easily be 
broken into a helper function or two (with local linkage), such as a
 >
 >     compare_window_name_function window_name_matcher(size_t& 
compare_length).
 >
 > Jeff Abrahamson
 > +33 6 24 40 01 57   <-- brièvement indisponible le 4 juillet
 > +44 7920 594 255    <-- will change 18 July
 >
 > http://jeff.purple.com/
 > http://blog.purple.com/jeff/

_______________________________________________
Ratpoison-devel mailing list
[email protected]
https://lists.nongnu.org/mailman/listinfo/ratpoison-devel
select.patch (text/x-patch, 9.5 KB)
diff --git a/src/actions.c b/src/actions.c
index 7579101..97e4fd8 100644
--- a/src/actions.c
+++ b/src/actions.c
@@ -81,6 +81,7 @@ static cmdret * set_topkmap (struct cmdarg **args);
 static cmdret * set_historysize (struct cmdarg **args);
 static cmdret * set_historycompaction (struct cmdarg **args);
 static cmdret * set_historyexpansion (struct cmdarg **args);
+static cmdret * set_selectstyle (struct cmdarg **args);
 
 LIST_HEAD(set_vars);
 
@@ -150,6 +151,7 @@ init_set_vars(void)
   add_set_var ("historysize", set_historysize, 1, "", arg_NUMBER);
   add_set_var ("historycompaction", set_historycompaction, 1, "", arg_NUMBER);
   add_set_var ("historyexpansion", set_historyexpansion, 1, "", arg_NUMBER);
+  add_set_var ("selectstyle", set_selectstyle, 1, "", arg_STRING);
 }
 
 /* rp_keymaps is ratpoison's list of keymaps. */
@@ -1412,10 +1414,13 @@ cmd_select (int interactive UNUSED, struct cmdarg **args)
       else
         /* try by name */
         {
-          rp_window *win = find_window_name (str, 1);
-
+          rp_window *win = find_window_name (str, MATCH_EXACT);
+          if (!win)
+            win = find_window_name (str, MATCH_IGNORECASE);
+          if (!win)
+            win = find_window_name (str, MATCH_PREFIX);
           if (!win)
-            win = find_window_name (str, 0);
+            win = find_window_name (str, MATCH_IGNORECASE | MATCH_PREFIX);
 
           if (win)
             {
@@ -1885,56 +1890,6 @@ read_frame (struct sbuf *s,  struct cmdarg **arg)
     return cmdret_new (RET_FAILURE, "frame not found");
 }
 
-static cmdret *
-read_window (struct argspec *spec, struct sbuf *s, struct cmdarg **arg)
-{
-  rp_window *win = NULL;
-  char *name;
-  int n;
-
-  if (s)
-    name = xstrdup (sbuf_get (s));
-  else
-    name = get_input (spec->prompt, hist_WINDOW, window_completions);
-
-  if (name)
-    {
-      /* try by number */
-      if ((n = string_to_positive_int (name)) >= 0)
-        {
-          rp_window_elem *elem = group_find_window_by_number (rp_current_group, n);
-          if (elem)
-            win = elem->win;
-        }
-      else
-        /* try by name */
-        {
-          win = find_window_name (name, 1);
-          if (win == NULL)
-            win = find_window_name (name, 0);
-        }
-
-      if (win)
-        {
-          *arg = xmalloc (sizeof(struct cmdarg));
-          (*arg)->type = arg_WINDOW;
-          (*arg)->arg.win = win;
-          (*arg)->string = name;
-          return NULL;
-        }
-      else
-        {
-          free (name);
-          *arg = NULL;
-          return cmdret_new (RET_SUCCESS, NULL);
-        }
-    }
-
-  /* user abort. */
-  *arg = NULL;
-  return cmdret_new (RET_SUCCESS, NULL);
-}
-
 static int
 parse_wingravity (char *data)
 {
@@ -2237,9 +2192,6 @@ read_arg (struct argspec *spec, struct sbuf *s, struct cmdarg **arg, const char
     case arg_SHELLCMD:
       ret = read_shellcmd (spec, s, arg, command_name);
       break;
-    case arg_WINDOW:
-      ret = read_window (spec, s, arg);
-      break;
     case arg_FRAME:
       ret = read_frame (s, arg);
       break;
@@ -2460,7 +2412,6 @@ arg_free (struct cmdarg *arg)
         case arg_REST:
         case arg_STRING:
         case arg_NUMBER:
-        case arg_WINDOW:
         case arg_FRAME:
         case arg_COMMAND:
         case arg_SHELLCMD:
@@ -5684,6 +5635,25 @@ set_maxundos (struct cmdarg **args)
   return cmdret_new (RET_SUCCESS, NULL);
 }
 
+static cmdret *
+set_selectstyle (struct cmdarg **args)
+{
+  if (args[0] == NULL)
+    return cmdret_new (RET_SUCCESS, "%s",
+            defaults.select_style == SELECT_OK_SELECTED
+                ? "ok_selected"
+                : "prefer_unselected"
+            );
+
+  if (!strncmp ("ok_selected", ARG_STRING(0), 5))
+      defaults.select_style = SELECT_OK_SELECTED;
+  else if (!strncmp ("prefer_unselected", ARG_STRING(0), 5))
+      defaults.select_style = SELECT_PREFER_NOTSELECTED;
+  else
+      return cmdret_new (RET_FAILURE, "set selectstyle: invalid argument");
+  return cmdret_new (RET_SUCCESS, NULL);
+}
+
 cmdret *
 cmd_cnext (int interactive, struct cmdarg **args)
 {
diff --git a/src/actions.h b/src/actions.h
index 519c1a7..d813bc9 100644
--- a/src/actions.h
+++ b/src/actions.h
@@ -34,7 +34,6 @@ enum argtype { arg_REST,
                arg_NUMBER,
                arg_STRING,
                arg_FRAME,
-               arg_WINDOW,
                arg_COMMAND,
                arg_SHELLCMD,
                arg_KEYMAP,
diff --git a/src/conf.h b/src/conf.h
index 708b688..014e938 100644
--- a/src/conf.h
+++ b/src/conf.h
@@ -131,4 +131,7 @@
  * the double of this value is tried before giving up */
 #define MAX_LEGACY_SNPRINTF_SIZE 102400
 
+/* Window names will only be compared by ther first 1024 chars */
+#define MAX_WINDOW_NAME_LENGTH   1024
+
 #endif /* !_ _RATPOISON_CONF_H */
diff --git a/src/data.h b/src/data.h
index f4bd185..01a8f1a 100644
--- a/src/data.h
+++ b/src/data.h
@@ -275,6 +275,9 @@ struct rp_defaults
 
   /* Frame indicator format */
   char *frame_fmt;
+
+  /* How the select command behaves */
+  int select_style;
 };
 
 /* Information about a child process. */
diff --git a/src/globals.h b/src/globals.h
index 1fb7e0f..2100a79 100644
--- a/src/globals.h
+++ b/src/globals.h
@@ -76,6 +76,10 @@
 #define UNUSED
 #endif
 
+/* toggles select behavior */
+#define SELECT_OK_SELECTED           0
+#define SELECT_PREFER_NOTSELECTED    1
+
 /* The list of groups. */
 extern struct list_head rp_groups;
 
diff --git a/src/main.c b/src/main.c
index 52dbcce..8f0146e 100644
--- a/src/main.c
+++ b/src/main.c
@@ -599,6 +599,7 @@ init_defaults (void)
   defaults.history_expansion = False;
   defaults.frame_selectors = xstrdup ("");
   defaults.maxundos = 20;
+  defaults.select_style = SELECT_PREFER_NOTSELECTED ;
 }
 
 int
diff --git a/src/sbuf.c b/src/sbuf.c
index 661ec83..8c38ff9 100644
--- a/src/sbuf.c
+++ b/src/sbuf.c
@@ -153,3 +153,22 @@ sbuf_chop (struct sbuf *b)
       b->data[--(b->len)] = '\0';
     }
 }
+compare_string_function
+get_compare_string_function (int *match_type, size_t *compare_length)
+{
+  switch(*match_type)
+    {
+    case MATCH_EXACT:
+      *compare_length = MAX_WINDOW_NAME_LENGTH;
+      return strncmp;
+    case MATCH_IGNORECASE:
+      *compare_length = MAX_WINDOW_NAME_LENGTH;
+      return strncasecmp;
+    case MATCH_PREFIX:
+      return strncmp;
+    case MATCH_IGNORECASE | MATCH_PREFIX:
+      return strncasecmp;
+    default:
+      return strncmp;
+    }
+}
diff --git a/src/sbuf.h b/src/sbuf.h
index e565efc..02a0dab 100644
--- a/src/sbuf.h
+++ b/src/sbuf.h
@@ -46,4 +46,15 @@ char *sbuf_printf (struct sbuf *b, char *fmt, ...);
 char *sbuf_printf_concat (struct sbuf *b, char *fmt, ...);
 void  sbuf_chop (struct sbuf *b);
 
+typedef int (*compare_string_function)
+              (const char *name1, const char *name2, size_t length);
+
+/* returns a strncmp-like function and modifies length
+ * depending on match_type */
+compare_string_function get_compare_string_function(int *match_type, size_t *length);
+/* match_types: */
+#define MATCH_EXACT              0x0000
+#define MATCH_PREFIX             0x0001
+#define MATCH_IGNORECASE         0x0002
+
 #endif /* ! _RATPOISON_SBUF_H */
diff --git a/src/window.c b/src/window.c
index de9032a..594127a 100644
--- a/src/window.c
+++ b/src/window.c
@@ -22,6 +22,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <ctype.h>
 
 #include "ratpoison.h"
 
@@ -283,31 +284,43 @@ find_window_number (int n)
 }
 
 rp_window *
-find_window_name (char *name, int exact_match)
+find_window_name (char *name, int match_type)
 {
+  size_t compare_length = strlen(name);
+  compare_string_function
+    compare_names = get_compare_string_function (&match_type, &compare_length);
+
+  /* number of the currently selected window, for comparison */
+  int current_win_number = current_window() ? current_window()->number : 0;
   rp_window_elem *cur;
 
-  if (!exact_match)
-    {
-      list_for_each_entry (cur, &rp_current_group->mapped_windows, node)
-        {
-          if (str_comp (name, window_name (cur->win), strlen (name)))
-            return cur->win;
-        }
-    }
-  else
+  int may_return = 1;
+  if (defaults.select_style == SELECT_PREFER_NOTSELECTED)
+      may_return = 0;
+
+  rp_window *first_match = NULL;
+
+  list_for_each_entry (cur, &rp_current_group->mapped_windows, node)
     {
-      list_for_each_entry (cur, &rp_current_group->mapped_windows, node)
+      if (!compare_names (name, window_name(cur->win), compare_length))
         {
-          if (!strcmp (name, window_name (cur->win)))
-            return cur->win;
+          if (may_return)
+            {
+              return cur->win;
+            }
+          else
+            {
+              if(cur->win->number == current_win_number)
+                may_return = 1;
+              if(!first_match)
+                first_match = cur->win;
+            }
         }
     }
-
-  /* didn't find it */
-  return NULL;
+  return first_match;
 }
 
+
 /* Return the previous window in the list. Assumes window is in the
    mapped window list. */
 rp_window*
diff --git a/src/window.h b/src/window.h
index fdd246c..ff8f867 100644
--- a/src/window.h
+++ b/src/window.h
@@ -40,7 +40,7 @@ char *window_name (rp_window *win);
 /* int goto_window_name (char *name); */
 rp_window *find_window_other (rp_screen *screen);
 rp_window *find_window_by_number (int n);
-rp_window *find_window_name (char *name, int exact_match);
+rp_window *find_window_name (char *name, int match_type);
 rp_window *find_window_prev (rp_window *w);
 rp_window *find_window_prev_with_frame (rp_window *w);
 rp_window *find_window_next (rp_window *w);