To remove a key binding fail with -

2013-12-18 Thread Dominique Michel
Hi,

In fvwm man page:

31.6.5. Key

Key [(window)] Keyname Context Modifiers Function

Binds a keyboard key to a specified fvwm command, or removes the
binding if Function is '-'.

If I have ardour running, its class is Ardour, I get the following into
the fvwm console:

Key (Ardour) B  W M ToggleFail
[fvwm][__execute_function]: ERROR No such command 'ToggleFail'

# The ToggleFail function doesn't exist

Key (Ardour) B  W M -
[fvwm][__execute_function]: ERROR No such command 'ToggleFail'

# To remove the binding fail, but the following work:
Key (Ardour) B  W M

More strange is the following sequence:
Key (Ardour) B  W M ToggleFail
[fvwm][__execute_function]: ERROR No such command 'ToggleFail'
Key (Ardour) B  W M ToggleWork
[fvwm][__execute_function]: ERROR No such command 'ToggleWork'
Key (Ardour) B  W M -
[fvwm][__execute_function]: ERROR No such command 'ToggleFail'
Key (Ardour) B  W M -
[fvwm][__execute_function]: ERROR No such command 'ToggleFail'
Key (Ardour) B  W M
# work

Cheers,
Dominique



Re: To remove a key binding fail with -

2013-12-18 Thread Dominique Michel
Le Wed, 18 Dec 2013 12:52:59 +0100,
Dominique Michel dominique.mic...@vtxnet.ch a écrit :

 Hi,
 
 In fvwm man page:
 
 31.6.5. Key
 
 Key [(window)] Keyname Context Modifiers Function
 
 Binds a keyboard key to a specified fvwm command, or removes the
 binding if Function is '-'.
 
 If I have ardour running, its class is Ardour, I get the following
 into the fvwm console:
 
 Key (Ardour) B  W M ToggleFail

Maybe I was not clear enough. When ardour have the focus and I press
Alt+B, I get the following:
 [fvwm][__execute_function]: ERROR No such command 'ToggleFail'
That imply the binding work fine.

 # The ToggleFail function doesn't exist
 
 Key (Ardour) B  W M -
Idem, that imply the binding was not removed:
 [fvwm][__execute_function]: ERROR No such command 'ToggleFail'
 
 # To remove the binding fail, but the following work:
 Key (Ardour) B  W M
 
 More strange is the following sequence:
 Key (Ardour) B  W M ToggleFail
The binding work:
 [fvwm][__execute_function]: ERROR No such command 'ToggleFail'
 Key (Ardour) B  W M ToggleWork
The new binding work:
 [fvwm][__execute_function]: ERROR No such command 'ToggleWork'
 Key (Ardour) B  W M -
The binding was not removed, but put back to its preceding state:
 [fvwm][__execute_function]: ERROR No such command 'ToggleFail'
The way to remove a binding as stated into the man page doesn't work:
 Key (Ardour) B  W M -
 [fvwm][__execute_function]: ERROR No such command 'ToggleFail'
The only way to remove a binding:
 Key (Ardour) B  W M
 # work
 
 Cheers,
 Dominique
 



Re: To remove a key binding fail with -

2013-12-18 Thread Dominique Michel
Le Wed, 18 Dec 2013 23:05:23 +0100,
Dominik Vogt dominik.v...@gmx.de a écrit :

 On Wed, Dec 18, 2013 at 10:47:59PM +0100, Dominique Michel wrote:
  Le Wed, 18 Dec 2013 12:52:59 +0100,
  Dominique Michel dominique.mic...@vtxnet.ch a écrit :
   31.6.5. Key
   
   Key [(window)] Keyname Context Modifiers Function
   
   Binds a keyboard key to a specified fvwm command, or removes the
   binding if Function is '-'.
   
   If I have ardour running, its class is Ardour, I get the following
   into the fvwm console:
   
   Key (Ardour) B  W M ToggleFail
  
  Maybe I was not clear enough. When ardour have the focus and I press
  Alt+B, I get the following:
   [fvwm][__execute_function]: ERROR No such command 'ToggleFail'
 
 ...
 
 I can see that soething is amiss with the binding removal code,
 but I cannot reproduce your specific problem.  Could you please
 make a minimal config file that shows this behaviour?

That will be hard, my minimal config is fvwm-crystal. I will think
about what I can do to simplify it. Binding removal was working it was
a few years ago inside fvwm-crystal, so maybe I can find a clue
about which commit changed it. But it will take a lot of time, and I
don't have much time these days.

Ciao,
Dominique

 
 Ciao
 
 Dominik ^_^  ^_^
 



Re: To remove a key binding fail with -

2013-12-18 Thread Dominik Vogt
On Wed, Dec 18, 2013 at 04:39:42PM -0500, Dan Espen wrote:
  Key (Ardour) B  W M -
  Idem, that imply the binding was not removed:
  [fvwm][__execute_function]: ERROR No such command 'ToggleFail'
 
 It's pretty clear to me Fvwm doesn't work as expected when you
 try to remove a binding with '-'.
 During the removal process it should NOT be validating the command
 in the binding.

The algorithm when adding a binding is:

 * Parse the binding and collect info about it in local variables
 * Collect all bindings that are made obsolete by the new binding
   in a list.
 * Ungrab the keys and buttons for the bindings in the remove
   list.
 * Delete the remove list.
 * If the action is just - that's all that is to do.
 * Otherwise append the new binding to the list of all bindings.

However, something is going wrong.  With

  key (w1) q w m echo w1
  key (w2) q w m echo w2

both bindings work as expected, but after

  key (w2) q w m -

the w1 binding does not work either.  It turns out that removing
the second binding removes the grab for meta-q on all windows
although it is still needed.

That does not explain the bug described earlier, though.

  The only way to remove a binding:
  Key (Ardour) B  W M
  # work
 
 I don't think that actually works (haven't looked yet).
 It might be setting the binding to an empty command.
 Empty commands are skipped so they appear not to be there.

Looking at the code in ParseBinding, I think fvwm would crash
with a NULL pointer access if the action is omitted.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt



Re: To remove a key binding fail with -

2013-12-18 Thread Dominik Vogt
The attached patch should fix the problems.  For some reason my CVS
connection hangs forever, so I cannot apply the patch myself.  :-/

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
From d36cc99975d99dae7305ae49b8d1d19aab82028d Mon Sep 17 00:00:00 2001
From: Dominik Vogt dominik.v...@gmx.de
Date: Thu, 19 Dec 2013 01:23:41 +0100
Subject: [PATCH] * Fix removal of bindings.

---
 ChangeLog   |   16 +
 NEWS|3 ++
 fvwm/bindings.c |   38 +
 fvwm/menubindings.c |3 +-
 libs/Bindings.c |   93 +--
 libs/Bindings.h |   10 +++---
 6 files changed, 102 insertions(+), 61 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 03c3c9d..f82f854 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2013-12-19  Dominik Vogt  dominik(dot)vogt(at)gmx(dot)de
+
+	* fvwm/menubindings.c (menu_binding):
+	Use new signature of CollectBindingList
+	* fvwm/bindings.c (ParseBinding):
+	Cleaned up some code
+	Do not remove window specific bindings that are still needed for other
+	windows.
+	* libs/Bindings.h (CollectBindingList):
+	Added ret_are_similar_bindings_left
+	* libs/Bindings.c (CollectBindingList):
+	indicates whether similar bindings are left, i.e. bindings that differ
+	in window name only
+	(replacesBinding, compare_bindings):
+	Renamed function, returns 2 if bindings only differ in window name
+
 2013-11-09  Dan Espen  despen(at)1verizon.net
 
 	* fvwm/events.c (HandlePropertyNotify): Disable prior fix suspected of causing
diff --git a/NEWS b/NEWS
index 8e36864..7a7500c 100644
--- a/NEWS
+++ b/NEWS
@@ -26,6 +26,9 @@ Changes in release 2.6.6 (not released yet)
   - Windows no longer jump from one position to the other which
 could happen in some cases with SnapAttraction.  Windows now
 snap to the closest window (or screen edge).
+  - Removing bindings had several strange side effects that are
+fixed now (removing too many bindings; old bindings showing up
+again after another is removed; possibly other effects).
 
 ---
 Changes in release 2.6.5 (20-Apr-2012)
diff --git a/fvwm/bindings.c b/fvwm/bindings.c
index 4cde680..c1aa6af 100644
--- a/fvwm/bindings.c
+++ b/fvwm/bindings.c
@@ -262,7 +262,7 @@ static int ParseBinding(
 	KeySym keysym = NoSymbol;
 	Bool is_unbind_request = False;
 	Bool is_pass_through = False;
-	Bool is_binding_removed = False;
+	Bool are_similar_bindings_left;
 	Binding *b;
 	Binding *rmlist = NULL;
 	STROKE_CODE(char stroke[STROKE_MAX_SEQUENCE + 1] = );
@@ -477,15 +477,17 @@ static int ParseBinding(
 		}
 	}
 
-	/*
-	** strip leading whitespace from action if necessary
-	*/
-	while (*action  (*action == ' ' || *action == '\t'))
+	if (action != NULL)
 	{
-		action++;
+		action = SkipSpaces(action, NULL, 0);
 	}
-
-	if (action)
+	if (
+		action == NULL || *action == 0 ||
+		(action[0] == '-'  !is_pass_through))
+	{
+		is_unbind_request = True;
+	}
+	else
 	{
 		is_pass_through = is_pass_through_action(action);
 		if (is_pass_through)
@@ -508,11 +510,6 @@ static int ParseBinding(
 			}
 		}
 	}
-	/* see if it is an unbind request */
-	if (!action || (action[0] == '-'  !is_pass_through))
-	{
-		is_unbind_request = True;
-	}
 
 	/* short circuit menu bindings for now. */
 	if ((context  C_MENU) == C_MENU)
@@ -540,17 +537,17 @@ static int ParseBinding(
 	*/
 	/* BEGIN remove */
 	CollectBindingList(
-		dpy, pblist, rmlist, type, STROKE_ARG((void *)stroke)
+		dpy, pblist, rmlist, are_similar_bindings_left, type,
+		STROKE_ARG((void *)stroke)
 		button, keysym, modifier, context, window_name);
 	if (rmlist != NULL)
 	{
-		is_binding_removed = True;
-		if (is_unbind_request)
+		int bcontext;
+
+		if (is_unbind_request  are_similar_bindings_left == False)
 		{
 			int rc = 0;
 
-			/* remove the grabs for the key for unbind
-			 * requests */
 			for (b = rmlist; b != NULL; b = b-NextBinding)
 			{
 /* release the grab */
@@ -563,11 +560,6 @@ static int ParseBinding(
 			}
 		}
 		FreeBindingList(rmlist);
-	}
-	if (is_binding_removed)
-	{
-		int bcontext;
-
 		bcontext = bind_get_bound_button_contexts(
 			pblist, buttons_grabbed);
 		update_nr_buttons(
diff --git a/fvwm/menubindings.c b/fvwm/menubindings.c
index c45c449..3b422c2 100644
--- a/fvwm/menubindings.c
+++ b/fvwm/menubindings.c
@@ -402,6 +402,7 @@ int menu_binding(
 {
 	Binding *rmlist;
 	int rc;
+	Bool dummy;
 
 	if (menu_bindings == NULL)
 	{
@@ -430,7 +431,7 @@ int menu_binding(
 	 */
 	/* BEGIN remove */
 	CollectBindingList(
-		dpy, menu_bindings, rmlist, type, STROKE_ARG(NULL)
+		dpy, menu_bindings, rmlist, dummy, type, STROKE_ARG(NULL)
 		button, keysym, modifier, context, menu_style);
 	if (rmlist != NULL)
 	{
diff --git a/libs/Bindings.c b/libs/Bindings.c
index d7f76e6..ab2fb08 100644
--- a/libs/Bindings.c
+++ b/libs/Bindings.c
@@ -269,27 +269,28 @@ int AddBinding(
 	return count;
 }
 
-/*
- * replacesBinding() - does the new binding, b1, 

Re: To remove a key binding fail with -

2013-12-18 Thread Dan Espen
Dominik Vogt dominik.v...@gmx.de writes:

 The attached patch should fix the problems.  For some reason my CVS
 connection hangs forever, so I cannot apply the patch myself.  :-/

Works from here.
Not sure why you would have a problem.

-- 
Dan Espen