changes to Bindings and Keyboard API to support (mostly) GTK-free keyboard bindings

4 files changed, 204 insertions, 112 deletions

diff --git a/libs/gtkmm2ext/bindings.cc b/libs/gtkmm2ext/bindings.cc
index 8d77009347..6ef2e29196 100644
--- a/libs/gtkmm2ext/bindings.cc
+++ b/libs/gtkmm2ext/bindings.cc
@@ -20,13 +20,19 @@
 #include <iostream>
 
 #include "pbd/gstdio_compat.h"
+#include <gtkmm/accelmap.h>
+#include <gtkmm/uimanager.h>
 
-#include "pbd/xml++.h"
 #include "pbd/convert.h"
+#include "pbd/debug.h"
+#include "pbd/error.h"
+#include "pbd/xml++.h"
 
 #include "gtkmm2ext/actions.h"
 #include "gtkmm2ext/bindings.h"
+#include "gtkmm2ext/debug.h"
 #include "gtkmm2ext/keyboard.h"
+#include "gtkmm2ext/utils.h"
 
 #include "i18n.h"
 
@@ -34,6 +40,7 @@ using namespace std;
 using namespace Glib;
 using namespace Gtk;
 using namespace Gtkmm2ext;
+using namespace PBD;
 
 uint32_t Bindings::_ignored_state = 0;
 
@@ -130,12 +137,6 @@ KeyboardKey::KeyboardKey (uint32_t state, uint32_t keycode)
 {
         uint32_t ignore = Bindings::ignored_state();
 
-        if (gdk_keyval_is_upper (keycode) && gdk_keyval_is_lower (keycode)) {
-                /* key is not subject to case, so ignore SHIFT
-                 */
-                ignore |= GDK_SHIFT_MASK;
-        }
-
         _val = (state & ~ignore);
         _val <<= 32;
         _val |= keycode;
@@ -203,7 +204,7 @@ KeyboardKey::make_key (const string& str, KeyboardKey& k)
 
         string::size_type lastmod = str.find_last_of ('-');
         guint keyval;
-
+        
         if (lastmod == string::npos) {
                 keyval = gdk_keyval_from_name (str.c_str());
         } else {
@@ -211,10 +212,11 @@ KeyboardKey::make_key (const string& str, KeyboardKey& k)
         }
 
         if (keyval == GDK_VoidSymbol) {
-                return false;
+	        return false;
         }
 
         k = KeyboardKey (s, keyval);
+
         return true;
 }
 
@@ -266,16 +268,19 @@ Bindings::activate (KeyboardKey kb, Operation op)
                 kbm = &release_bindings;
                 break;
         }
-
+        
         KeybindingMap::iterator k = kbm->find (kb);
 
         if (k == kbm->end()) {
                 /* no entry for this key in the state map */
+	        DEBUG_TRACE (DEBUG::Bindings, string_compose ("no binding for %1\n", kb));
 	        return false;
         }
 
         /* lets do it ... */
 
+        DEBUG_TRACE (DEBUG::Bindings, string_compose ("binding for %1: %2\n", kb, k->second->get_name()));
+
         k->second->activate ();
         return true;
 }
@@ -287,7 +292,7 @@ Bindings::add (KeyboardKey kb, Operation op, RefPtr<Action> what)
 
         switch (op) {
         case Press:
-                kbm = &press_bindings;
+	        kbm = &press_bindings;
                 break;
         case Release:
                 kbm = &release_bindings;
@@ -297,11 +302,36 @@ Bindings::add (KeyboardKey kb, Operation op, RefPtr<Action> what)
         KeybindingMap::iterator k = kbm->find (kb);
 
         if (k == kbm->end()) {
-                pair<KeyboardKey,RefPtr<Action> > newpair (kb, what);
+	        pair<KeyboardKey,RefPtr<Action> > newpair (kb, what);
                 kbm->insert (newpair);
         } else {
                 k->second = what;
         }
+
+        Gtk::AccelKey gtk_key;
+
+        /* tweak the binding so that GTK will accept it and display something
+         * acceptable 
+         */
+
+        uint32_t gtk_legal_keyval = kb.key();
+        possibly_translate_keyval_to_make_legal_accelerator (gtk_legal_keyval);
+        KeyboardKey gtk_binding (kb.state(), gtk_legal_keyval);
+        
+
+        bool entry_exists = Gtk::AccelMap::lookup_entry (what->get_accel_path(), gtk_key);
+
+        if (!entry_exists || gtk_key.get_key() == 0) {
+	        Gtk::AccelMap::add_entry (what->get_accel_path(),
+	                                  gtk_binding.key(),
+	                                  (Gdk::ModifierType) gtk_binding.state());
+        } else {
+	        warning << string_compose (_("There is more than one key binding defined for %1. Both will work, but only the first will be visible in menus"), what->get_accel_path()) << endmsg;
+        }
+
+        if (!Gtk::AccelMap::lookup_entry (what->get_accel_path(), gtk_key) || gtk_key.get_key() == 0) {
+	        cerr << "GTK binding using " << gtk_binding << " failed for " << what->get_accel_path() << " existing = " << gtk_key.get_key() << " + " << gtk_key.get_mod() << endl;
+        }
 }
 
 void
@@ -463,7 +493,7 @@ Bindings::save (XMLNode& root)
 }
 
 bool
-Bindings::load (const string& path)
+Bindings::load (string const & name)
 {
         XMLTree tree;
 
@@ -471,18 +501,46 @@ Bindings::load (const string& path)
                 return false;
         }
 
-        if (!tree.read (path)) {
-                return false;
+        XMLNode const * node = Keyboard::bindings_node();
+
+        if (!node) {
+	        error << string_compose (_("No keyboard binding information when loading bindings for \"%1\""), name) << endmsg;
+	        return false;
+        }
+        
+        const XMLNodeList& children (node->children());
+        bool found = false;
+        
+        for (XMLNodeList::const_iterator i = children.begin(); i != children.end(); ++i) {
+
+	        if ((*i)->name() == X_("Bindings")) {
+		        XMLProperty const * prop = (*i)->property (X_("name"));
+
+		        if (!prop) {
+			        continue;
+		        }
+
+		        if (prop->value() == name) {
+			        found = true;
+			        node = *i;
+			        break;
+		        }
+	        }
+        }
+        
+        if (!found) {
+	        error << string_compose (_("Bindings for \"%1\" not found in keyboard binding node\n"), name) << endmsg;
+	        return false;
         }
 
         press_bindings.clear ();
         release_bindings.clear ();
 
-        XMLNode& root (*tree.root());
-        const XMLNodeList& children (root.children());
+        const XMLNodeList& bindings (node->children());
 
-        for (XMLNodeList::const_iterator i = children.begin(); i != children.end(); ++i) {
-                load (**i);
+        for (XMLNodeList::const_iterator i = bindings.begin(); i != bindings.end(); ++i) {
+	        /* each node could be Press or Release */
+	        load (**i);
         }
 
         return true;
@@ -520,8 +578,8 @@ Bindings::load (const XMLNode& node)
                         RefPtr<Action> act;
 
                         if (action_map) {
-                                act = action_map->find_action (ap->value());
-                        }
+	                        act = action_map->find_action (ap->value());
+                        } 
 
                         if (!act) {
                                 string::size_type slash = ap->value().find ('/');
@@ -565,8 +623,38 @@ ActionMap::find_action (const string& name)
         return RefPtr<Action>();
 }
 
-RefPtr<Action>
-ActionMap::register_action (const char* path,
+RefPtr<ActionGroup>
+ActionMap::create_action_group (const string& name)
+{
+	RefPtr<ActionGroup> g = ActionGroup::create (name);
+	return g;
+}
+
+void
+ActionMap::install_action_group (RefPtr<ActionGroup> group)
+{
+	ActionManager::ui_manager->insert_action_group (group);
+}
+
+RefPtr<Action> 
+ActionMap::register_action (RefPtr<ActionGroup> group, const char* name, const char* label)
+{
+        string fullpath;
+
+        RefPtr<Action> act = Action::create (name, label);
+
+        fullpath = group->get_name();
+        fullpath += '/';
+        fullpath += name;
+        
+        actions.insert (_ActionMap::value_type (fullpath, act));
+        group->add (act);
+        
+        return act;
+}
+
+RefPtr<Action> 
+ActionMap::register_action (RefPtr<ActionGroup> group,
                             const char* name, const char* label, sigc::slot<void> sl)
 {
         string fullpath;
@@ -575,17 +663,42 @@ ActionMap::register_action (const char* path,
 
         act->signal_activate().connect (sl);
 
-        fullpath = path;
+        fullpath = group->get_name();
         fullpath += '/';
         fullpath += name;
 
         actions.insert (_ActionMap::value_type (fullpath, act));
+        group->add (act, sl);
+
         return act;
 }
 
-RefPtr<Action>
-ActionMap::register_radio_action (const char* path, Gtk::RadioAction::Group& rgroup,
-                                  const char* name, const char* label,
+RefPtr<Action> 
+ActionMap::register_radio_action (RefPtr<ActionGroup> group,
+                                  Gtk::RadioAction::Group& rgroup,
+                                  const char* name, const char* label, 
+                                  sigc::slot<void> sl)
+{
+        string fullpath;
+
+        RefPtr<Action> act = RadioAction::create (rgroup, name, label);
+        RefPtr<RadioAction> ract = RefPtr<RadioAction>::cast_dynamic(act);
+        
+        act->signal_activate().connect (sl);
+
+        fullpath = group->get_name();
+        fullpath += '/';
+        fullpath += name;
+
+        actions.insert (_ActionMap::value_type (fullpath, act));
+        group->add (act, sl);
+        return act;
+}
+
+RefPtr<Action> 
+ActionMap::register_radio_action (RefPtr<ActionGroup> group,
+                                  Gtk::RadioAction::Group& rgroup,
+                                  const char* name, const char* label, 
                                   sigc::slot<void,GtkAction*> sl,
                                   int value)
 {
@@ -597,16 +710,17 @@ ActionMap::register_radio_action (const char* path, Gtk::RadioAction::Group& rgr
 
         act->signal_activate().connect (sigc::bind (sl, act->gobj()));
 
-        fullpath = path;
+        fullpath = group->get_name();
         fullpath += '/';
         fullpath += name;
 
         actions.insert (_ActionMap::value_type (fullpath, act));
+        group->add (act, sigc::bind (sl, act->gobj()));
         return act;
 }
 
-RefPtr<Action>
-ActionMap::register_toggle_action (const char* path,
+RefPtr<Action> 
+ActionMap::register_toggle_action (RefPtr<ActionGroup> group,
                                    const char* name, const char* label, sigc::slot<void> sl)
 {
         string fullpath;
@@ -615,15 +729,16 @@ ActionMap::register_toggle_action (const char* path,
 
         act->signal_activate().connect (sl);
 
-        fullpath = path;
+        fullpath = group->get_name();
         fullpath += '/';
         fullpath += name;
 
         actions.insert (_ActionMap::value_type (fullpath, act));
+        group->add (act, sl);
         return act;
 }
 
-std::ostream& operator<<(std::ostream& out, Gtkmm2ext::KeyboardKey& k) {
-	return out << "Key " << k.key() << " state " << k.state();
+std::ostream& operator<<(std::ostream& out, Gtkmm2ext::KeyboardKey const & k) {
+	return out << "Key " << k.key() << " (" << (k.key() > 0 ? gdk_keyval_name (k.key()) : "no-key") << ") state " << k.state();
 }
 
diff --git a/libs/gtkmm2ext/gtkmm2ext/bindings.h b/libs/gtkmm2ext/gtkmm2ext/bindings.h
index 413e80655c..7685d8cf32 100644
--- a/libs/gtkmm2ext/gtkmm2ext/bindings.h
+++ b/libs/gtkmm2ext/gtkmm2ext/bindings.h
@@ -5,7 +5,6 @@
 #include <stdint.h>
 #include <gdk/gdkkeysyms.h>
 #include <gtkmm/action.h>
-#include <gtkmm/action.h>
 #include <gtkmm/radioaction.h>
 #include <gtkmm/toggleaction.h>
 
@@ -72,13 +71,27 @@ class LIBGTKMM2EXT_API ActionMap {
         ActionMap() {}
         ~ActionMap() {}
 
-	Glib::RefPtr<Gtk::Action> register_action (const char* path,
+        Glib::RefPtr<Gtk::ActionGroup> create_action_group (const std::string& group_name);
+        void install_action_group (Glib::RefPtr<Gtk::ActionGroup>);
+        
+        Glib::RefPtr<Gtk::Action> register_action (Glib::RefPtr<Gtk::ActionGroup> group, const char* name, const char* label);
+        Glib::RefPtr<Gtk::Action> register_action (Glib::RefPtr<Gtk::ActionGroup> group,
 						   const char* name, const char* label, sigc::slot<void> sl);
+<<<<<<< HEAD
 	Glib::RefPtr<Gtk::Action> register_radio_action (const char* path, Gtk::RadioAction::Group&,
 							 const char* name, const char* label,
+=======
+        Glib::RefPtr<Gtk::Action> register_radio_action (Glib::RefPtr<Gtk::ActionGroup> group,
+							 Gtk::RadioAction::Group&,
+							 const char* name, const char* label, 
+>>>>>>> changes to Bindings and Keyboard API to support (mostly) GTK-free keyboard bindings
                                                          sigc::slot<void,GtkAction*> sl,
                                                          int value);
-	Glib::RefPtr<Gtk::Action> register_toggle_action (const char*path,
+        Glib::RefPtr<Gtk::Action> register_radio_action (Glib::RefPtr<Gtk::ActionGroup> group,
+							 Gtk::RadioAction::Group&,
+							 const char* name, const char* label, 
+                                                         sigc::slot<void> sl);
+	Glib::RefPtr<Gtk::Action> register_toggle_action (Glib::RefPtr<Gtk::ActionGroup> group,
 							  const char* name, const char* label, sigc::slot<void> sl);
 
         Glib::RefPtr<Gtk::Action> find_action (const std::string& name);
@@ -127,7 +140,7 @@ class LIBGTKMM2EXT_API Bindings {
 
         KeybindingMap press_bindings;
         KeybindingMap release_bindings;
-
+        
         typedef std::map<MouseButton,Glib::RefPtr<Gtk::Action> > MouseButtonBindingMap;
         MouseButtonBindingMap button_press_bindings;
         MouseButtonBindingMap button_release_bindings;
@@ -138,6 +151,6 @@ class LIBGTKMM2EXT_API Bindings {
 
 } // namespace
 
-std::ostream& operator<<(std::ostream& out, Gtkmm2ext::KeyboardKey& k);
+std::ostream& operator<<(std::ostream& out, Gtkmm2ext::KeyboardKey const & k);
 
 #endif /* __libgtkmm2ext_bindings_h__ */
diff --git a/libs/gtkmm2ext/gtkmm2ext/keyboard.h b/libs/gtkmm2ext/gtkmm2ext/keyboard.h
index 5b4e6c50c6..97d42f5cc8 100644
--- a/libs/gtkmm2ext/gtkmm2ext/keyboard.h
+++ b/libs/gtkmm2ext/gtkmm2ext/keyboard.h
@@ -161,24 +161,18 @@ class LIBGTKMM2EXT_API Keyboard : public sigc::trackable, PBD::Stateful
 
 	static void keybindings_changed ();
 	static void save_keybindings ();
-	static bool load_keybindings (std::string path);
 	static void set_can_save_keybindings (bool yn);
 	static std::string current_binding_name () { return _current_binding_name; }
 	static std::map<std::string,std::string> binding_files;
 
         static bool catch_user_event_for_pre_dialog_focus (GdkEvent* ev, Gtk::Window* w);
 
-	int reset_bindings ();
+	static bool load_keybindings (std::string const& path);
+	static void save_keybindings (std::string const& path);
+
+	static XMLNode const * bindings_node() { return _bindings_node; }
 
-	struct AccelKeyLess {
-	    bool operator() (const Gtk::AccelKey a, const Gtk::AccelKey b) const {
-		    if (a.get_key() != b.get_key()) {
-			    return a.get_key() < b.get_key();
-		    } else {
-			    return a.get_mod() < b.get_mod();
-		    }
-	    }
-	};
+	int reset_bindings ();
 
 	sigc::signal0<void> ZoomVerticalModifierReleased;
 
@@ -202,18 +196,21 @@ class LIBGTKMM2EXT_API Keyboard : public sigc::trackable, PBD::Stateful
 	static bool can_save_keybindings;
 	static bool bindings_changed_after_save_became_legal;
 	static std::string _current_binding_name;
+	static XMLNode* _bindings_node;
 
 	typedef std::pair<std::string,std::string> two_strings;
 
-	static std::map<Gtk::AccelKey,two_strings,AccelKeyLess> release_keys;
-
 	static gint _snooper (GtkWidget*, GdkEventKey*, gpointer);
 	gint snooper (GtkWidget*, GdkEventKey*);
 
 	static void set_modifier (uint32_t newval, uint32_t& variable);
 
 	static bool _some_magic_widget_has_focus;
+
         static Gtk::Window* pre_dialog_active_window;
+
+	static int read_keybindings (std::string const& path);
+	static int store_keybindings (std::string const& path);
 };
 
 } /* namespace */
diff --git a/libs/gtkmm2ext/keyboard.cc b/libs/gtkmm2ext/keyboard.cc
index 7217544ea6..7e1473931f 100644
--- a/libs/gtkmm2ext/keyboard.cc
+++ b/libs/gtkmm2ext/keyboard.cc
@@ -110,8 +110,8 @@ bool Keyboard::can_save_keybindings = false;
 bool Keyboard::bindings_changed_after_save_became_legal = false;
 map<string,string> Keyboard::binding_files;
 string Keyboard::_current_binding_name;
-map<AccelKey,pair<string,string>,Keyboard::AccelKeyLess> Keyboard::release_keys;
 Gtk::Window* Keyboard::pre_dialog_active_window = 0;
+XMLNode* Keyboard::_bindings_node = 0;
 
 /* set this to initially contain the modifiers we care about, then track changes in ::set_edit_modifier() etc. */
 GdkModifierType Keyboard::RelevantModifierKeyMask;
@@ -368,6 +368,10 @@ Keyboard::snooper (GtkWidget *widget, GdkEventKey *event)
 			   prevent auto-repeat events.
 			*/
 
+#if 0
+			/* August 2015: we don't have any release bindings
+			 */
+			
 			for (map<AccelKey,two_strings,AccelKeyLess>::iterator k = release_keys.begin(); k != release_keys.end(); ++k) {
 
 				const AccelKey& ak (k->first);
@@ -378,32 +382,7 @@ Keyboard::snooper (GtkWidget *widget, GdkEventKey *event)
 					break;
 				}
 			}
-		}
-
-	} else if (event->type == GDK_KEY_RELEASE) {
-
-		State::iterator i;
-
-		if ((i = find (state.begin(), state.end(), keyval)) != state.end()) {
-			state.erase (i);
-			sort (state.begin(), state.end());
-		}
-
-		for (map<AccelKey,two_strings,AccelKeyLess>::iterator k = release_keys.begin(); k != release_keys.end(); ++k) {
-
-			const AccelKey& ak (k->first);
-			two_strings ts (k->second);
-
-			if (keyval == ak.get_key() && (Gdk::ModifierType)((event->state & Keyboard::RelevantModifierKeyMask) | Gdk::RELEASE_MASK) == ak.get_mod()) {
-				Glib::RefPtr<Gtk::Action> act = ActionManager::get_action (ts.first.c_str(), ts.second.c_str());
-				if (act) {
-					DEBUG_TRACE (DEBUG::Keyboard, string_compose ("Activate %1 %2\n", ts.first, ts.second));
-					act->activate();
-					DEBUG_TRACE (DEBUG::Keyboard, string_compose ("Use repeat, suppress other\n", ts.first, ts.second));
-					ret = true;
-				}
-				break;
-			}
+#endif
 		}
 	}
 
@@ -675,18 +654,20 @@ void
 Keyboard::save_keybindings ()
 {
 	if (can_save_keybindings && bindings_changed_after_save_became_legal) {
-		Gtk::AccelMap::save (user_keybindings_path);
+		/* Call to specific implementation to save bindings to path */
+		store_keybindings (user_keybindings_path);
 	}
 }
 
 bool
-Keyboard::load_keybindings (string path)
+Keyboard::load_keybindings (string const & path)
 {
 	try {
 		info << "Loading bindings from " << path << endl;
 
-		Gtk::AccelMap::load (path);
-
+		/* Call to specific implementation to load bindings from path */
+		read_keybindings (path);
+		
 		_current_binding_name = _("Unknown");
 
 		for (map<string,string>::iterator x = binding_files.begin(); x != binding_files.end(); ++x) {
@@ -703,41 +684,27 @@ Keyboard::load_keybindings (string path)
 		return false;
 	}
 
-	/* now find all release-driven bindings */
-
-	vector<string> groups;
-	vector<string> names;
-	vector<string> tooltips;
-	vector<AccelKey> bindings;
-
-	ActionManager::get_all_actions (groups, names, tooltips, bindings);
-
-	vector<string>::iterator g;
-	vector<AccelKey>::iterator b;
-	vector<string>::iterator n;
-
-	release_keys.clear ();
-
-	for (n = names.begin(), b = bindings.begin(), g = groups.begin(); n != names.end(); ++n, ++b, ++g) {
-		stringstream s;
-		s << "Action: " << *n << " Group: " << *g << " Binding: ";
+	return true;
+}
 
-		if ((*b).get_key() != GDK_VoidSymbol) {
-			s << b->get_key() << " w/mod " << hex << b->get_mod() << dec << " = " << b->get_abbrev () << "\n";
-		} else {
-			s << "unbound\n";
-		}
+int
+Keyboard::read_keybindings (string const & path)
+{
+	XMLTree tree;
 
-		DEBUG_TRACE (DEBUG::Bindings, s.str ());
+	if (!tree.read (path.c_str())) {
+		return -1;
 	}
 
-	for (n = names.begin(), b = bindings.begin(), g = groups.begin(); n != names.end(); ++n, ++b, ++g) {
-		if ((*b).get_mod() & Gdk::RELEASE_MASK) {
-			release_keys.insert (pair<AccelKey,two_strings> (*b, two_strings (*g, *n)));
-		}
-	}
+	_bindings_node = new XMLNode (*tree.root ()); /* copy operation. Sorry */
+	
+	return 0;
+}
 
-	return true;
+int
+Keyboard::store_keybindings (string const & path)
+{
+	return 0;
 }
 
 int