Fwd: PATCH - Bug 5888

Erik Ackermann <[email protected]>
Newsgroups gmane.comp.audio.ardour.devel
Message-ID <CAN5Z5QGPQPJbrcL8DuNmiuP4UE3iz_urzUyc9tDL7rMx_8wbHA@mail.gmail.com>
This fixes the keybinding window setting behavior to prevent the capture of
a completely invalid setting, Bug 5888

E.g.  Alt(down) M(down) Alt(up) M(up) == Alt+Alt_L (Which is completely
impossible to trigger).

The new behavior is simpler explain:
1)  On any keyup event, if a non-modifier key was down pressed since the
last-key up event, use that key and all currently pressed modifier keys as
the captured binding.

To be honest this is still slightly less than ideal (and also different
from other keybinding captures I've seen) in that it does not handle
keyboard auto-repeat gracefully. However this seems a matter of design of
the Keybindings window and would require GUI change. Every other keycapture
I've seen uses some explicit event to trigger capture (e.g. mouse click or
double click), while Ardour just uses keypress. The keypress however is
emulated by the window manager during autorepeat. The result is that
performing the following Alt(down) M(down) Alt(up) [Wait 1 second] M(up)
will result in the binding first being set to Alt-M, but during the wait
the autorepeat will Trigger M again changing the setting to M. I think this
code change (or similar) should be accepted now with the possible debate
opened for a more complex change to ignore keyboard autorepeat if warranted.

Patch 0002 is to enable the clear button when a new keybinding is set
(different unreported bug).

_______________________________________________
ardour-dev mailing list
[email protected]
http://lists.ardour.org/listinfo.cgi/ardour-dev-ardour.org
0001-Fix-keybinding-setting-but-5888.patch (text/x-patch, 2 KB)
From 938084d91b661b578fb536e9dd35cecd6b8ad603 Mon Sep 17 00:00:00 2001
From: Erik Ackermann <[email protected]>
Date: Thu, 20 Mar 2014 02:22:21 -0700
Subject: [PATCH - fix keybinding setting 1/2] Fix keybinding setting, but 5888

Signed-off-by: Erik Ackermann <[email protected]>

diff --git a/gtk2_ardour/keyeditor.cc b/gtk2_ardour/keyeditor.cc
index f23e246..21fea49 100644
--- a/gtk2_ardour/keyeditor.cc
+++ b/gtk2_ardour/keyeditor.cc
@@ -55,8 +55,7 @@ KeyEditor::KeyEditor ()
 	, unbind_box (BUTTONBOX_END)
 
 {
-	can_bind = false;
-	last_state = 0;
+	last_keyval = 0;
 
 	model = TreeStore::create(columns);
 
@@ -173,15 +172,16 @@ KeyEditor::action_selected ()
 bool
 KeyEditor::on_key_press_event (GdkEventKey* ev)
 {
-	can_bind = true;
-	last_state = ev->state;
+	if (!ev->is_modifier) {
+		last_keyval = ev->keyval;
+	}
 	return false;
 }
 
 bool
 KeyEditor::on_key_release_event (GdkEventKey* ev)
 {
-	if (ARDOUR::Profile->get_sae() || !can_bind || ev->state != last_state) {
+	if (ARDOUR::Profile->get_sae() || last_keyval == 0) {
 		return false;
 	}
 
@@ -194,11 +194,10 @@ KeyEditor::on_key_release_event (GdkEventKey* ev)
 			goto out;
 		}
 
-                Gtkmm2ext::possibly_translate_keyval_to_make_legal_accelerator (ev->keyval);
-
+		Gtkmm2ext::possibly_translate_keyval_to_make_legal_accelerator (ev->keyval);
 
 		bool result = AccelMap::change_entry (path,
-						      ev->keyval,
+						      last_keyval,
 						      ModifierType (Keyboard::RelevantModifierKeyMask & ev->state),
 						      true);
 
@@ -209,7 +208,7 @@ KeyEditor::on_key_release_event (GdkEventKey* ev)
 	}
 
   out:
-	can_bind = false;
+    last_keyval = 0;
 	return true;
 }
 
diff --git a/gtk2_ardour/keyeditor.h b/gtk2_ardour/keyeditor.h
index 51cf1dc..babc471 100644
--- a/gtk2_ardour/keyeditor.h
+++ b/gtk2_ardour/keyeditor.h
@@ -64,8 +64,7 @@ class KeyEditor : public ArdourWindow
 
 	void unbind ();
 
-	bool can_bind;
-	guint last_state;
+	guint last_keyval;
 
 	void action_selected ();
 	void populate ();
-- 
1.8.3.2
0002-Enable-Remove-shortcut-button-when-new-keybinding-is.patch (text/x-patch, 735 B)
From f322e0d89b2213ea068ae4be4707e9d7b82ca44e Mon Sep 17 00:00:00 2001
From: Erik Ackermann <[email protected]>
Date: Thu, 20 Mar 2014 02:23:11 -0700
Subject: [PATCH - fix keybinding setting 2/2] Enable "Remove shortcut" button
 when new keybinding is added

Signed-off-by: Erik Ackermann <[email protected]>

diff --git a/gtk2_ardour/keyeditor.cc b/gtk2_ardour/keyeditor.cc
index 21fea49..114d96d 100644
--- a/gtk2_ardour/keyeditor.cc
+++ b/gtk2_ardour/keyeditor.cc
@@ -204,6 +204,7 @@ KeyEditor::on_key_release_event (GdkEventKey* ev)
 		if (result) {
 			AccelKey key;
 			(*i)[columns.binding] = ActionManager::get_key_representation (path, key);
+			unbind_button.set_sensitive (true);
 		}
 	}
 
-- 
1.8.3.2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.