Re: [cwm 1/1] Add a menu to search and call internal functions.

2012-11-07 Thread Thomas Pfaff
  Why not
  
  for (iter = 0; iter  nitems(name_to_kbfunc); iter++) {
  func = name_to_kbfunc[iter]; /* if you want to use func */
  
  then you don't need the last NULL entry in the array or add code
  to not break existing code.
 
 See: http://c-faq.com/decl/extarraysize.html
 
 The diff moves the declaration of name_to_kbfunc[] to calmwm.h and
 externs it for use in kbfunc.c, but it's still defined in conf.c.


Yeah, I see that now, though the use of nitems is used throughout
the code already and deviating from that and making that particular
array a special case seems kind of messy and somewhat confusing.



Re: [cwm 1/1] Add a menu to search and call internal functions.

2012-11-06 Thread Thomas Pfaff
Can you please provide a unified cvs diff?

I've not tried this patch but I have some comments (see below).

 Useful for those times you want to use an unbound function, but even
 when the function is bound to something you haven't memorized yet it
 can be faster than lookup up the keybinding in the manual.  Bound to
 CM-/ by default.

[...]

 +struct func name_to_kbfunc[] = {
   { lower, kbfunc_client_lower, KBFLAG_NEEDCLIENT, {0} },
   { raise, kbfunc_client_raise, KBFLAG_NEEDCLIENT, {0} },
   { search, kbfunc_client_search, 0, {0} },
 + { funcsearch, kbfunc_func_search, KBFLAG_NEEDCLIENT, {0} },
   { menusearch, kbfunc_menu_search, 0, {0} },
   { hide, kbfunc_client_hide, KBFLAG_NEEDCLIENT, {0} },
   { cycle, kbfunc_client_cycle, 0, {.i = CWM_CYCLE} },
 @@ -399,6 +396,7 @@
   {.i = (CWM_LEFT|CWM_PTRMOVE|CWM_BIGMOVE)} },
   { bigptrmoveright, kbfunc_moveresize, 0,
   {.i = (CWM_RIGHT|CWM_PTRMOVE|CWM_BIGMOVE)} },
 + { NULL, NULL, 0, {0} }

Why do you insist on adding this?  Why not just use nitems?

 @@ -488,7 +486,8 @@
   return;
  
   for (iter = 0; iter  nitems(name_to_kbfunc); iter++) {
 - if (strcmp(name_to_kbfunc[iter].tag, binding) != 0)
 + if (name_to_kbfunc[iter].tag == NULL ||
 + strcmp(name_to_kbfunc[iter].tag, binding) != 0)
   continue;

See?  Now you have to add more code because you broke something.

 Index: cwm/kbfunc.c
 ===
 --- cwm.orig/kbfunc.c 2012-11-05 10:25:37.577424801 -0600
 +++ cwm/kbfunc.c  2012-11-05 14:17:27.164487245 -0600
 @@ -174,6 +174,35 @@
  }
  
  void
 +kbfunc_func_search(struct client_ctx *cc, union arg *arg)
 +{
 + struct screen_ctx   *sc = cc-sc;
 + struct func *func;
 + struct menu *mi;
 + struct menu_qmenuq;
 +
 + TAILQ_INIT(menuq);
 +
 + for (func = name_to_kbfunc; func-tag != NULL; func++) {

Why not

for (iter = 0; iter  nitems(name_to_kbfunc); iter++) {
func = name_to_kbfunc[iter]; /* if you want to use func */

then you don't need the last NULL entry in the array or add code
to not break existing code.

 + if ((mi = menu_filter(sc, menuq, function, NULL, 0,
 + search_match_text, NULL)) != NULL) {
 + func = (struct func *)mi-ctx;
 + (*func-handler)(cc, func-argument);

func-handler(... is fine but some people prefer the pre-ANSI
C style, (*func-handler)(... ... Just a tiny nit.  YMMV.



Re: [cwm 1/1] Add a menu to search and call internal functions.

2012-11-06 Thread Okan Demirmen
I haven't caught-up, nor reviewed anything else yet, but comments
inline:

On Tue 2012.11.06 at 16:11 +0100, Thomas Pfaff wrote:
 Can you please provide a unified cvs diff?

'cvs diff -uNp' is best...

 [...]
  for (iter = 0; iter  nitems(name_to_kbfunc); iter++) {
  -   if (strcmp(name_to_kbfunc[iter].tag, binding) != 0)
  +   if (name_to_kbfunc[iter].tag == NULL ||
  +   strcmp(name_to_kbfunc[iter].tag, binding) != 0)
  continue;

I too would prefer to use nitems, to be consistent with the rest of the
code.  Also reduces the number of gratuitous changes, and of course the
size of the diff.

  +   if ((mi = menu_filter(sc, menuq, function, NULL, 0,
  +   search_match_text, NULL)) != NULL) {
  +   func = (struct func *)mi-ctx;
  +   (*func-handler)(cc, func-argument);

Likewise, as mentioned, consistency with the rest is best.



Re: [cwm 1/1] Add a menu to search and call internal functions.

2012-11-06 Thread Kent R. Spillner
Hey, dude-

 Can you please provide a unified cvs diff?

The first patches I sent last week were cvs diffs, but I saw quilt
was recently added to ports to I switched to using it to manage all
of the patches (I have a few other things I'm working on, too, which
I haven't sent out for review yet).

Does anyone know if quilt can delegate diffing to cvs?  I'll
investigate some more and see if I can get quilt to do this
automatically.

 Why do you insist on adding this?  Why not just use nitems?
...
 See?  Now you have to add more code because you broke something.
...
 Why not
 
 for (iter = 0; iter  nitems(name_to_kbfunc); iter++) {
 func = name_to_kbfunc[iter]; /* if you want to use func */
 
 then you don't need the last NULL entry in the array or add code
 to not break existing code.

See: http://c-faq.com/decl/extarraysize.html

The diff moves the declaration of name_to_kbfunc[] to calmwm.h and
externs it for use in kbfunc.c, but it's still defined in conf.c.

  +   if ((mi = menu_filter(sc, menuq, function, NULL, 0,
  +   search_match_text, NULL)) != NULL) {
  +   func = (struct func *)mi-ctx;
  +   (*func-handler)(cc, func-argument);
 
 func-handler(... is fine but some people prefer the pre-ANSI
 C style, (*func-handler)(... ... Just a tiny nit.  YMMV.

I don't have a preference, I just followed the style used elsewhere
in cwm.

Thanks for the feedback!

Best,
Kent



Re: [cwm 1/1] Add a menu to search and call internal functions.

2012-11-06 Thread Kent R. Spillner
Hey, dude-

 I too would prefer to use nitems, to be consistent with the rest of the
 code.  Also reduces the number of gratuitous changes, and of course the
 size of the diff.

I chose the guard element approach because it leads to the smallest diff,
but I can move the definition of name_to_kbfunc[] from conf.c to a header
if you prefer.

   + if ((mi = menu_filter(sc, menuq, function, NULL, 0,
   + search_match_text, NULL)) != NULL) {
   + func = (struct func *)mi-ctx;
   + (*func-handler)(cc, func-argument);
 
 Likewise, as mentioned, consistency with the rest is best.

I wrote the above specifically to be consistent with other parts of cwm;
see: xevents.c:261 and xevents.c:314.

Thanks for the feedback!

Best,
Kent



[cwm 1/1] Add a menu to search and call internal functions.

2012-11-05 Thread kspillner
Useful for those times you want to use an unbound function, but even
when the function is bound to something you haven't memorized yet it
can be faster than lookup up the keybinding in the manual.  Bound to
CM-/ by default.

Index: cwm/calmwm.h
===
--- cwm.orig/calmwm.h   2012-11-05 13:50:58.468179916 -0600
+++ cwm/calmwm.h2012-11-05 14:17:27.164487245 -0600
@@ -225,6 +225,13 @@
 };
 TAILQ_HEAD(screen_ctx_q, screen_ctx);
 
+struct func {
+   char*tag;
+   void (*handler)(struct client_ctx *, union arg *);
+   int  flags;
+   union argargument;
+};
+
 struct keybinding {
TAILQ_ENTRY(keybinding)  entry;
void(*callback)(struct client_ctx *, union arg *);
@@ -392,6 +399,7 @@
 union arg *);
 voidkbfunc_cmdexec(struct client_ctx *, union arg *);
 voidkbfunc_exec(struct client_ctx *, union arg *);
+voidkbfunc_func_search(struct client_ctx *, union arg *);
 voidkbfunc_lock(struct client_ctx *, union arg *);
 voidkbfunc_menu_search(struct client_ctx *, union arg *);
 voidkbfunc_moveresize(struct client_ctx *, union arg *);
@@ -498,6 +506,7 @@
 extern struct screen_ctx_q  Screenq;
 extern struct client_ctx_q  Clientq;
 extern struct conf  Conf;
+extern struct func  name_to_kbfunc[];
 
 extern int  HasXinerama, HasRandr, Randr_ev;
 
Index: cwm/conf.c
===
--- cwm.orig/conf.c 2012-11-05 10:25:37.577424801 -0600
+++ cwm/conf.c  2012-11-05 14:17:27.164487245 -0600
@@ -99,6 +99,7 @@
{ M-Up,   raise },
{ M-slash,search },
{ C-slash,menusearch },
+   { CM-slash,   funcsearch },
{ M-Tab,  cycle },
{ MS-Tab, rcycle },
{ CM-n,   label },
@@ -284,15 +285,11 @@
cc-flags |= ignore ? CLIENT_IGNORE : 0;
 }
 
-static struct {
-   char*tag;
-   void (*handler)(struct client_ctx *, union arg *);
-   int  flags;
-   union argargument;
-} name_to_kbfunc[] = {
+struct func name_to_kbfunc[] = {
{ lower, kbfunc_client_lower, KBFLAG_NEEDCLIENT, {0} },
{ raise, kbfunc_client_raise, KBFLAG_NEEDCLIENT, {0} },
{ search, kbfunc_client_search, 0, {0} },
+   { funcsearch, kbfunc_func_search, KBFLAG_NEEDCLIENT, {0} },
{ menusearch, kbfunc_menu_search, 0, {0} },
{ hide, kbfunc_client_hide, KBFLAG_NEEDCLIENT, {0} },
{ cycle, kbfunc_client_cycle, 0, {.i = CWM_CYCLE} },
@@ -399,6 +396,7 @@
{.i = (CWM_LEFT|CWM_PTRMOVE|CWM_BIGMOVE)} },
{ bigptrmoveright, kbfunc_moveresize, 0,
{.i = (CWM_RIGHT|CWM_PTRMOVE|CWM_BIGMOVE)} },
+   { NULL, NULL, 0, {0} }
 };
 
 /*
@@ -488,7 +486,8 @@
return;
 
for (iter = 0; iter  nitems(name_to_kbfunc); iter++) {
-   if (strcmp(name_to_kbfunc[iter].tag, binding) != 0)
+   if (name_to_kbfunc[iter].tag == NULL ||
+   strcmp(name_to_kbfunc[iter].tag, binding) != 0)
continue;
 
current_binding-callback = name_to_kbfunc[iter].handler;
Index: cwm/kbfunc.c
===
--- cwm.orig/kbfunc.c   2012-11-05 10:25:37.577424801 -0600
+++ cwm/kbfunc.c2012-11-05 14:17:27.164487245 -0600
@@ -174,6 +174,35 @@
 }
 
 void
+kbfunc_func_search(struct client_ctx *cc, union arg *arg)
+{
+   struct screen_ctx   *sc = cc-sc;
+   struct func *func;
+   struct menu *mi;
+   struct menu_qmenuq;
+
+   TAILQ_INIT(menuq);
+
+   for (func = name_to_kbfunc; func-tag != NULL; func++) {
+   mi = xcalloc(1, sizeof(*mi));
+   (void)strlcpy(mi-text, func-tag, sizeof(mi-text));
+   mi-ctx = func;
+   TAILQ_INSERT_TAIL(menuq, mi, entry);
+   }
+
+   if ((mi = menu_filter(sc, menuq, function, NULL, 0,
+   search_match_text, NULL)) != NULL) {
+   func = (struct func *)mi-ctx;
+   (*func-handler)(cc, func-argument);
+   }
+
+   while ((mi = TAILQ_FIRST(menuq)) != NULL) {
+   TAILQ_REMOVE(menuq, mi, entry);
+   xfree(mi);
+   }
+}
+
+void
 kbfunc_menu_search(struct client_ctx *cc, union arg *arg)
 {
struct screen_ctx   *sc = cc-sc;
Index: cwm/cwm.1
===
--- cwm.orig/cwm.1  2012-10-31 17:12:11.71058 -0500
+++ cwm/cwm.1   2012-11-05 14:21:15.746828265 -0600
@@ -70,6 +70,8 @@
 Search for windows.
 .It Ic C-/
 Search for applications.
+.It Ic CM-/