2009/2/8 Dominik Vogt <dominik.v...@gmx.de>:
> On Sun, Feb 08, 2009 at 02:44:15AM +0000, Thomas Adam wrote:
>> Comment/suggestions welcome as always.
>
> See below.

Thanks.

>> +void print_bindings(int verbose)
>
> If the verbose flag has no effect, why is it an argument to the
> function?

I had thought about trying to utilise verbose at one point, but
couldn't think of the need.  Changed to void instead.

>> +             char type[20];
>> +             switch (b->type)
>> +             {
>> +                     case BIND_KEYPRESS:
>> +                             binding_type = "Key";
>> +                             sprintf(type, "%s", b->key_name);
>> +                             break;
>> +                     case BIND_PKEYPRESS:
>> +                             binding_type = "PointerKey";
>> +                             sprintf(type, "%s", b->key_name);
>> +                             break;
>> +                     case BIND_BUTTONPRESS:
>> +                     case BIND_BUTTONRELEASE:
>> +                            binding_type = "Mouse";
>> +                            sprintf(type, "%d", b->Button_Key);
>> +                            break;
>> +                     case BIND_STROKE:
>> +                           binding_type = "Stroke";
>> +                           sprintf(type, "%s\t%d", (char *)b->Stroke_Seq,
>> +                                           b->Button_Key);
>
> I think we shouldn't copy arbitrary strings into a fixed length
> buffer.  I'm sure there are key names that are longer than 19
> characters.

I checked in bindings.c:ParseBindings() -- key_string is defined as:

char key_string[201] = "";

I've mimicked this here as well for char[] type.  If you think that's
still going to cause problems, I will really make it dynamic, although
from testing it locally, it's not yet caused any problems, and my
Thinkpad does have some nice XF86LongNameKeys defined, for instance.

>> +                           break;
>> +             }
>> +
>> +             char *mod_string = charmap_table_to_string(
>> +                             MaskUsedModifiers(b->Modifier),key_modifiers);
>> +             char *context_string = charmap_table_to_string(
>> +                             b->Context, win_contexts);
>> +             char win_name[260] = "";
>
> Uh, I know we have hard coded some length limit to the window
> name, but that's just because there was or is a bug in the X
> system that caused a crash.

OK.  Changed to be dynamic, although I will note that again, according
to ParseBindings(), the max length of a window name is capped at 79
chars.  But I've ignored that again here, and allocated win_name with
malloc().

>>  #include "config.h"
>>  #include <stdio.h>
>>  #include <ctype.h>
>> +#include <string.h>
>
> You can't rely on string.h being present.  On some systems, it's
> callend strings.h.  Luckily you don't have to take care of that
> yourself:  config.h already includes the proper file, so you can
> just delete this line.

I'm sure I had problems with that before -- but you're right.  Removed
without any problems.

>>
>>  #include "charmap.h"
>> +#include "safemalloc.h"
>
> Should be libs/safemalloc.h.  Maybe I should remove libs from the
> include path.

Fixed.   From doing a quick cursive glance through some of the other
header files in libs/, there's a real mixture of some header files
being included as "libs/foo.h" and others just as "foo.h".  If you
don't get around to this some point next week, I will send in a patch
to clean this up on your behalf.

>> +
>> +/* Used from "PrintInfo Bindings". */
>> +char *charmap_table_to_string(int mask, charmap_t *table)
>> +{
>> +     char *allmods = safemalloc (sizeof(table));
>> +     memset (allmods, '\0', sizeof(table));
>
> Are you sure you want to reserve sizeof(table) bytes?  This
> would reserve space to store a pointer, not space to store a
> charmap_t.

Oops.  I meant:  "sizeof(*table)" here.  Feel free to replace it with:
 sizeof(charmap_t) -- I know some prefer the latter over the former.

>> +
>> +     int modmask = mask;
>
> Please don't declare variables in the middle of a block.  This
> does not compile with oplder compilers.

I didn't quite understand what you meant by this -- the declaration
wasn't in a block, it was just after the function had been declared.
I've tried to change it, but if it's still wrong, do say and I'll
correct it again.

Please see print-bindings2.patch attached.

Kindly,

-- Thomas Adam
Index: AUTHORS
===================================================================
RCS file: /home/cvs/fvwm/fvwm/AUTHORS,v
retrieving revision 1.130
diff -u -r1.130 AUTHORS
--- AUTHORS	19 Oct 2008 12:04:12 -0000	1.130
+++ AUTHORS	8 Feb 2009 17:35:08 -0000
@@ -15,6 +15,7 @@
 StartShaded style option.
 Introduce the command expansion placeholder:  $[w.visiblename]
 Make style matching honour a window's visible name (c.f. $[w.visiblename])
+Added "bindings" option to PrintInfo command useful for debugging.
 
 Serge (gentoosiast) Koksharov:
 Documentation fixes, bug fixes.
Index: NEWS
===================================================================
RCS file: /home/cvs/fvwm/fvwm/NEWS,v
retrieving revision 1.771
diff -u -r1.771 NEWS
--- NEWS	8 Feb 2009 11:01:45 -0000	1.771
+++ NEWS	8 Feb 2009 17:35:10 -0000
@@ -21,6 +21,9 @@
    - New style InitialMapCommand allows to execute any command
      when a window is mapped first.
 
+   - New option to PrintInfo, "bindings" which prints out all of the Key,
+     PointerKey, Mouse and Stroke bindings which fvwm knows about.
+
 * Bug fixes:
 
    - Fixed compilation without XRender support.
Index: doc/commands/PrintInfo.xml
===================================================================
RCS file: /home/cvs/fvwm/fvwm/doc/commands/PrintInfo.xml,v
retrieving revision 1.4
diff -u -r1.4 PrintInfo.xml
--- doc/commands/PrintInfo.xml	16 Jun 2007 12:38:46 -0000	1.4
+++ doc/commands/PrintInfo.xml	8 Feb 2009 17:35:10 -0000
@@ -57,4 +57,10 @@
 <replaceable>verbose</replaceable>
 can be 1.</para>
 
+<para><fvwmopt cms="PrintInfo" opt="bindings"/>
+which prints information on all the bindings fvwm has:  key, mouse and
+stroke bindings.
+<replaceable>verbose</replaceable>
+has no effect with this option.</para>
+
 </section>
Index: fvwm/bindings.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/bindings.c,v
retrieving revision 1.103
diff -u -r1.103 bindings.c
--- fvwm/bindings.c	23 Nov 2007 08:49:11 -0000	1.103
+++ fvwm/bindings.c	8 Feb 2009 17:35:11 -0000
@@ -639,6 +639,66 @@
 	return;
 }
 
+void print_bindings(void)
+{
+	Binding *b;
+	
+	fprintf(stderr, "Current list of bindings:\n\n");
+	
+	for (b = Scr.AllBindings; b != NULL; b = b->NextBinding)
+	{
+		/* Key, PointerKey, Mouse, Stroke. */
+		char *binding_type = NULL;
+		char type[201];
+		
+		switch (b->type)
+		{
+			case BIND_KEYPRESS:
+				binding_type = "Key";
+				sprintf(type, "%s", b->key_name);
+				break;
+			case BIND_PKEYPRESS:
+				binding_type = "PointerKey";
+				sprintf(type, "%s", b->key_name);
+				break;
+			case BIND_BUTTONPRESS:
+			case BIND_BUTTONRELEASE:
+			       binding_type = "Mouse";
+			       sprintf(type, "%d", b->Button_Key);
+			       break;
+			case BIND_STROKE:
+			      binding_type = "Stroke";
+			      sprintf(type, "%s\t%d", (char *)b->Stroke_Seq,
+					      b->Button_Key);
+			      break;
+		}
+
+		char *mod_string = charmap_table_to_string(
+				MaskUsedModifiers(b->Modifier),key_modifiers);
+		char *context_string = charmap_table_to_string(
+				b->Context, win_contexts);
+		char *win_name = NULL;
+
+		if (b->windowName != NULL)
+		{
+			win_name = safemalloc (strlen(b->windowName) + 3);
+			win_name = memset (win_name, '\0', sizeof(win_name));
+			sprintf(win_name, "(%s)", b->windowName);
+		}
+
+		fprintf(stderr, "%s %s\t%s\t%s\t%s\t%s\n",
+				binding_type, win_name == NULL ? "" : win_name, 
+				type, context_string, 
+				mod_string, (char *)b->Action);
+		
+		free (mod_string);
+		free (context_string);
+		free (win_name);
+	}
+
+
+}
+
 /* ---------------------------- interface functions ------------------------ */
 
 /* Removes all unused modifiers from in_modifiers */
Index: fvwm/builtins.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/builtins.c,v
retrieving revision 1.432
diff -u -r1.432 builtins.c
--- fvwm/builtins.c	28 Nov 2008 23:29:27 -0000	1.432
+++ fvwm/builtins.c	8 Feb 2009 17:35:13 -0000
@@ -2700,6 +2700,10 @@
 	{
 		PicturePrintImageCache(verbose);
 	}
+	else if (StrEquals(subject, "Bindings"))
+	{
+		print_bindings();
+	}
 	else
 	{
 		fvwm_msg(ERR, "PrintInfo",
Index: fvwm/builtins.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/fvwm/builtins.h,v
retrieving revision 1.22
diff -u -r1.22 builtins.h
--- fvwm/builtins.h	29 Jun 2003 19:53:23 -0000	1.22
+++ fvwm/builtins.h	8 Feb 2009 17:35:13 -0000
@@ -10,5 +10,6 @@
 Bool ReadDecorFace(char *s, DecorFace *df, int button, int verbose);
 void FreeDecorFace(Display *dpy, DecorFace *df);
 void update_fvwm_colorset(int cset);
+void print_bindings(void);
 
 #endif /* BUILTINS_H */
Index: libs/charmap.c
===================================================================
RCS file: /home/cvs/fvwm/fvwm/libs/charmap.c,v
retrieving revision 1.2
diff -u -r1.2 charmap.c
--- libs/charmap.c	2 Nov 2003 17:04:44 -0000	1.2
+++ libs/charmap.c	8 Feb 2009 17:35:13 -0000
@@ -21,6 +21,7 @@
 #include <ctype.h>
 
 #include "charmap.h"
+#include "libs/safemalloc.h"
 
 /* ---------------------------- local definitions -------------------------- */
 
@@ -102,3 +103,30 @@
 
 	return c;
 }
+
+/* Used from "PrintInfo Bindings". */
+char *charmap_table_to_string(int mask, charmap_t *table)
+{
+	char *allmods = safemalloc (sizeof(*table));
+	memset (allmods, '\0', sizeof(*table));	
+	int modmask;
+
+	modmask = mask;
+	for (; table->key !=0; table++)
+	{
+		char *c = safemalloc (sizeof(table->key));
+		memset (c, '\0', sizeof(c));
+		
+		strcpy(c, (char *)&table->key);
+		*c = toupper(*c);
+		
+		if (modmask & table->value)
+		{
+			modmask |= table->value;
+			strcat(allmods, c);
+		}
+		modmask &= ~table->value;
+		free(c);
+	}
+	return allmods;
+}
Index: libs/charmap.h
===================================================================
RCS file: /home/cvs/fvwm/fvwm/libs/charmap.h,v
retrieving revision 1.1
diff -u -r1.1 charmap.h
--- libs/charmap.h	5 Jul 2003 01:37:47 -0000	1.1
+++ libs/charmap.h	8 Feb 2009 17:35:13 -0000
@@ -27,4 +27,6 @@
 	int *ret, const char *string, charmap_t *table, char *errstring);
 char charmap_mask_to_char(int mask, charmap_t *table);
 
+char *charmap_table_to_string(int mask, charmap_t *table);
+
 #endif /* CHARMAP_H */

Reply via email to