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);