more changes to Bindings, Keyboard APIs

6 files changed, 187 insertions, 68 deletions

diff --git a/libs/gtkmm2ext/bindings.cc b/libs/gtkmm2ext/bindings.cc
index 232405bb97..4c9184c0c2 100644
--- a/libs/gtkmm2ext/bindings.cc
+++ b/libs/gtkmm2ext/bindings.cc
@@ -42,9 +42,10 @@ using namespace Gtk;
 using namespace Gtkmm2ext;
 using namespace PBD;
 
-ActionMap Gtkmm2ext::Actions; /* global. Gulp */
 list<Bindings*> Bindings::bindings; /* global. Gulp */
 uint32_t Bindings::_ignored_state = 0;
+list<ActionMap*> ActionMap::action_maps; /* global. Gulp */
+PBD::Signal1<void,Bindings*> Bindings::BindingsChanged;
 
 MouseButton::MouseButton (uint32_t state, uint32_t keycode)
 {
@@ -142,8 +143,21 @@ KeyboardKey::KeyboardKey (uint32_t state, uint32_t keycode)
         _val = (state & ~ignore);
         _val <<= 32;
         _val |= keycode;
-};
+}
 
+string
+KeyboardKey::display_label () const
+{
+	if (key() == 0) {
+		return string();
+	}
+
+	/* This magically returns a string that will display the right thing
+	 *  on all platforms, notably the command key on OS X.
+	 */
+	
+	return gtk_accelerator_get_label (key(), (GdkModifierType) state());
+}
 
 string
 KeyboardKey::name () const
@@ -231,7 +245,7 @@ KeyboardKey::make_key (const string& str, KeyboardKey& k)
 
 Bindings::Bindings (std::string const& name)
 	: _name (name)
-	, _action_map (Actions)
+	, _action_map (0)
 {
 	bindings.push_back (this);
 }
@@ -241,10 +255,67 @@ Bindings::~Bindings()
 	bindings.remove (this);
 }
 
+string
+Bindings::ardour_action_name (RefPtr<Action> action)
+{
+	/* Skip "<Actions>/" */
+	return action->get_accel_path ().substr (10);
+}
+
+KeyboardKey
+Bindings::get_binding_for_action (RefPtr<Action> action, Operation& op)
+{
+	const string action_name = ardour_action_name (action);
+	
+        for (KeybindingMap::iterator k = press_bindings.begin(); k != press_bindings.end(); ++k) {
+
+	        /* option one: action has already been associated with the
+	         * binding
+	         */
+
+	        if (k->second.action == action) {
+		        return k->first;
+	        }
+
+	        /* option two: action name matches, so lookup the action,
+	         * setup the association while we're here, and return the binding.
+	         */
+
+	        if (_action_map && k->second.action_name == action_name) {
+		        k->second.action = _action_map->find_action (action_name);
+		        return k->first;
+	        }
+	         
+        }
+
+        for (KeybindingMap::iterator k = release_bindings.begin(); k != release_bindings.end(); ++k) {
+	        
+	        /* option one: action has already been associated with the
+	         * binding
+	         */
+
+	        if (k->second.action == action) {
+		        return k->first;
+	        }
+
+	        /* option two: action name matches, so lookup the action,
+	         * setup the association while we're here, and return the binding.
+	         */
+
+	        if (_action_map && k->second.action_name == action_name) {
+		         k->second.action = _action_map->find_action (action_name);
+		         return k->first;
+	         }
+	         
+        }
+        
+        return KeyboardKey::null_key();
+}
+
 void
 Bindings::set_action_map (ActionMap& actions)
 {
-	_action_map = actions;
+	_action_map = &actions;
 	dissociate ();
 	associate ();
 }
@@ -291,8 +362,12 @@ Bindings::activate (KeyboardKey kb, Operation op)
 
         RefPtr<Action> action;
         
-        if (!k->second.action) {
-	        action = _action_map.find_action (k->second.action_name);
+        if (k->second.action) {
+	        action = k->second.action;
+        } else {
+	        if (_action_map) {
+		        action = _action_map->find_action (k->second.action_name);
+	        }
         }
 
         if (action) {
@@ -311,26 +386,33 @@ Bindings::associate ()
 {
 	KeybindingMap::iterator k;
 
+	if (!_action_map) {
+		return;
+	}
+	
 	for (k = press_bindings.begin(); k != press_bindings.end(); ++k) {
-		k->second.action = _action_map.find_action (k->second.action_name);
+		k->second.action = _action_map->find_action (k->second.action_name);
 		if (k->second.action) {
+			cerr << "push to GTK " << k->first << ' ' << k->second.action_name << endl;
 			push_to_gtk (k->first, k->second.action);
+		} else {
+			cerr << "didn't find " << k->second.action_name << endl;
 		}
 	}
 
 	for (k = release_bindings.begin(); k != release_bindings.end(); ++k) {
-		k->second.action = _action_map.find_action (k->second.action_name);
+		k->second.action = _action_map->find_action (k->second.action_name);
 		/* no working support in GTK for release bindings */
 	}
 
 	MouseButtonBindingMap::iterator b;
 	
 	for (b = button_press_bindings.begin(); b != button_press_bindings.end(); ++b) {
-		b->second.action = _action_map.find_action (b->second.action_name);
+		b->second.action = _action_map->find_action (b->second.action_name);
 	}
 
 	for (b = button_release_bindings.begin(); b != button_release_bindings.end(); ++b) {
-		b->second.action = _action_map.find_action (b->second.action_name);
+		b->second.action = _action_map->find_action (b->second.action_name);
 	}
 }
 
@@ -358,22 +440,14 @@ Bindings::push_to_gtk (KeyboardKey kb, RefPtr<Action> what)
          * up with all bindings/actions. 
          */
 
-        Gtk::AccelKey gtk_key;
-
-        /* tweak the modifier used in the binding so that GTK will accept it
-         * and display something acceptable. The actual keyval should display
-         * correctly even if it involves a key that GTK would not allow
-         * as an accelerator.
-         */
-
-        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);
+	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);
+	Gtk::AccelKey gtk_key;
+	
+	bool entry_exists = Gtk::AccelMap::lookup_entry (what->get_accel_path(), gtk_key);
 
-        if (!entry_exists || gtk_key.get_key() == 0) {
+        if (!entry_exists) {
 
 	        /* there is a trick happening here. It turns out that
 	         * gtk_accel_map_add_entry() performs no validation checks on
@@ -386,21 +460,17 @@ Bindings::push_to_gtk (KeyboardKey kb, RefPtr<Action> what)
 	         * happens.
 	         */
 
-	        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;
+	        Gtk::AccelMap::add_entry (what->get_accel_path(), gtk_binding.key(), (Gdk::ModifierType) gtk_binding.state());
         }
 }
 
 bool
 Bindings::replace (KeyboardKey kb, Operation op, string const & action_name, bool can_save)
 {
+	if (!_action_map) {
+		return false;
+	}
+
 	/* We have to search the existing binding map by both action and
 	 * keybinding, because the following are possible:
 	 *
@@ -410,7 +480,7 @@ Bindings::replace (KeyboardKey kb, Operation op, string const & action_name, boo
 	 *   - action is not bound
 	 */
 	
-	RefPtr<Action> action = Actions.find_action (action_name);
+	RefPtr<Action> action = _action_map->find_action (action_name);
 
         if (!action) {
 	        return false;
@@ -475,6 +545,8 @@ Bindings::add (KeyboardKey kb, Operation op, string const& action_name, bool can
         if (can_save) {
 	        Keyboard::keybindings_changed ();
         }
+
+        BindingsChanged (this); /* EMIT SIGNAL */
 }
 
 void
@@ -500,6 +572,8 @@ Bindings::remove (KeyboardKey kb, Operation op, bool can_save)
         if (can_save) {
 	        Keyboard::keybindings_changed ();
         }
+
+        BindingsChanged (this); /* EMIT SIGNAL */
 }
 
 void
@@ -526,6 +600,8 @@ Bindings::remove (RefPtr<Action> action, Operation op, bool can_save)
         if (can_save) {
 	        Keyboard::keybindings_changed ();
         }
+
+        BindingsChanged (this); /* EMIT SIGNAL */
 }
 
 bool
@@ -551,17 +627,22 @@ Bindings::activate (MouseButton bb, Operation op)
 
         RefPtr<Action> action;
         
-        if (!b->second.action) {
-	        action = _action_map.find_action (b->second.action_name);
+        if (b->second.action) {
+	        action = b->second.action;
+        } else {
+	        if (_action_map) {
+		        action = _action_map->find_action (b->second.action_name);
+	        }
         }
 
         if (action) {
 	        /* lets do it ... */
+	        DEBUG_TRACE (DEBUG::Bindings, string_compose ("activating action %1\n", ardour_action_name (action)));
 	        action->activate ();
         }
 
         /* return true even if the action could not be found */
-        
+
         return true;
 }
 
@@ -726,6 +807,10 @@ Bindings::get_all_actions (std::vector<std::string>& paths,
                            std::vector<std::string>& keys,
                            std::vector<RefPtr<Action> >& actions)
 {
+	if (!_action_map) {
+		return;
+	}
+
 	/* build a reverse map from actions to bindings */
 
 	typedef map<Glib::RefPtr<Gtk::Action>,KeyboardKey> ReverseMap;
@@ -738,7 +823,7 @@ Bindings::get_all_actions (std::vector<std::string>& paths,
 	/* get a list of all actions */
 
 	ActionMap::Actions all_actions;
-	_action_map.get_actions (all_actions);
+	_action_map->get_actions (all_actions);
 	
 	for (ActionMap::Actions::const_iterator act = all_actions.begin(); act != all_actions.end(); ++act) {
 
@@ -749,7 +834,7 @@ Bindings::get_all_actions (std::vector<std::string>& paths,
 		ReverseMap::iterator r = rmap.find (*act);
 
 		if (r != rmap.end()) {
-			keys.push_back (gtk_accelerator_get_label (r->second.key(), (GdkModifierType) r->second.state()));
+			keys.push_back (r->second.display_label());
 		} else {
 			keys.push_back (string());
 		}
@@ -763,6 +848,10 @@ Bindings::get_all_actions (std::vector<std::string>& names,
                            std::vector<std::string>& paths,
                            std::vector<std::string>& keys)
 {
+	if (!_action_map) {
+		return;
+	}
+
 	/* build a reverse map from actions to bindings */
 
 	typedef map<Glib::RefPtr<Gtk::Action>,KeyboardKey> ReverseMap;
@@ -775,7 +864,7 @@ Bindings::get_all_actions (std::vector<std::string>& names,
 	/* get a list of all actions */
 
 	ActionMap::Actions all_actions;
-	_action_map.get_actions (all_actions);
+	_action_map->get_actions (all_actions);
 	
 	for (ActionMap::Actions::const_iterator act = all_actions.begin(); act != all_actions.end(); ++act) {
 		
@@ -784,7 +873,7 @@ Bindings::get_all_actions (std::vector<std::string>& names,
 
 		ReverseMap::iterator r = rmap.find (*act);
 		if (r != rmap.end()) {
-			keys.push_back (gtk_accelerator_get_label (r->second.key(), (GdkModifierType) r->second.state()));
+			keys.push_back (r->second.display_label());
 		} else {
 			keys.push_back (string());
 		}
@@ -792,10 +881,11 @@ Bindings::get_all_actions (std::vector<std::string>& names,
 }
 
 Bindings*
-Bindings::get_bindings (string const& name)
+Bindings::get_bindings (string const& name, ActionMap& map)
 {
 	for (list<Bindings*>::iterator b = bindings.begin(); b != bindings.end(); b++) {
 		if ((*b)->name() == name) {
+			(*b)->set_action_map (map);
 			return *b;
 		}
 	}
diff --git a/libs/gtkmm2ext/gtk_ui.cc b/libs/gtkmm2ext/gtk_ui.cc
index 6ae50db9a8..6b5a9c230a 100644
--- a/libs/gtkmm2ext/gtk_ui.cc
+++ b/libs/gtkmm2ext/gtk_ui.cc
@@ -35,6 +35,7 @@
 #include "pbd/replace_all.h"
 
 #include "gtkmm2ext/application.h"
+#include "gtkmm2ext/bindings.h"
 #include "gtkmm2ext/gtk_ui.h"
 #include "gtkmm2ext/textviewer.h"
 #include "gtkmm2ext/popup.h"
@@ -71,8 +72,8 @@ template class AbstractUI<Gtkmm2ext::UIRequest>;
 UI::UI (string application_name, string thread_name, int *argc, char ***argv)
 	: AbstractUI<UIRequest> (thread_name)
 	, _receiver (*this)
+	, global_bindings (0)
 	, errors (0)
-
 {
 	theMain = new Main (argc, argv);
 
@@ -365,11 +366,22 @@ UI::set_tip (Widget *w, const gchar *tip, const gchar *hlp)
 	}
 
 	if (action) {
-#warning Paul fix this before you think tabbed is done
-		Gtk::AccelKey key;
-		ustring ap = action->get_accel_path();
-		if (!ap.empty()) {
-			string shortcut = string(); // ActionManager::get_key_representation (ap, key);
+		Bindings* bindings = (Bindings*) w->get_data ("ardour-bindings");
+		if (!bindings) {
+			Gtk::Window* win = (Gtk::Window*) w->get_toplevel();
+			if (win) {
+				bindings = (Bindings*) win->get_data ("ardour-bindings");
+			}
+		}
+
+		if (!bindings) {
+			bindings = global_bindings;
+		}
+		
+		if (bindings) {
+			Bindings::Operation op;
+			KeyboardKey kb = bindings->get_binding_for_action (action, op);
+			string shortcut = kb.display_label ();
 			if (!shortcut.empty()) {
 				replace_all (shortcut, "<", "");
 				replace_all (shortcut, ">", "-");
diff --git a/libs/gtkmm2ext/gtkmm2ext/bindings.h b/libs/gtkmm2ext/gtkmm2ext/bindings.h
index c586676e3c..4886ebb73c 100644
--- a/libs/gtkmm2ext/gtkmm2ext/bindings.h
+++ b/libs/gtkmm2ext/gtkmm2ext/bindings.h
@@ -12,6 +12,8 @@
 #include <gtkmm/radioaction.h>
 #include <gtkmm/toggleaction.h>
 
+#include "pbd/signals.h"
+
 #include "gtkmm2ext/visibility.h"
 
 class XMLNode;
@@ -43,6 +45,8 @@ class LIBGTKMM2EXT_API KeyboardKey
         std::string name() const;
         static bool make_key (const std::string&, KeyboardKey&);
 
+        std::string display_label() const;
+        
   private:
         uint64_t _val;
 };
@@ -99,6 +103,8 @@ class LIBGTKMM2EXT_API ActionMap {
         typedef std::vector<Glib::RefPtr<Gtk::Action> > Actions;
 	void get_actions (Actions&);
 
+	static std::list<ActionMap*> action_maps;
+	
   private:
 <<<<<<< HEAD
         typedef std::map<std::string, Glib::RefPtr<Gtk::Action> > _ActionMap;
@@ -112,15 +118,6 @@ class LIBGTKMM2EXT_API ActionMap {
 };        
 >>>>>>> radically change Keyboard/Binding API design to disconnect Gtk::Action lookup from binding definition
 
-/* single global action map for entire application. 
- * 
- * Actions are name-spaced by group, and it makes things
- * much easier if there is a single place to look up
- * any action.
- */
-
-LIBGTKMM2EXT_API extern ActionMap Actions;
-
 class LIBGTKMM2EXT_API Bindings {
   public:
         enum Operation {
@@ -162,6 +159,8 @@ class LIBGTKMM2EXT_API Bindings {
         void remove (MouseButton, Operation);
         bool activate (MouseButton, Operation);
 
+        KeyboardKey get_binding_for_action (Glib::RefPtr<Gtk::Action>, Operation& op);
+        
         bool load (XMLNode const& node);
         void load_operation (XMLNode const& node);
         void save (XMLNode& root);
@@ -171,11 +170,31 @@ class LIBGTKMM2EXT_API Bindings {
 =======
 >>>>>>> radically change Keyboard/Binding API design to disconnect Gtk::Action lookup from binding definition
         
+        /* There are modifiers that we just don't care about
+           when it comes to defining bindings. This sets the modifiers
+           that will be ignored when comparing a key event with
+           existing bindings.
+        */
         static void set_ignored_state (int mask) {
                 _ignored_state = mask;
         }
         static uint32_t ignored_state() { return _ignored_state; }
 
+        /* GTK has the following position a Gtk::Action:
+         * 
+         *  accel_path: <Actions>/GroupName/ActionName
+         *  name: ActionName
+         *
+         * We want proper namespacing and we're not interested in
+         * the silly <Actions> "extra" namespace. So in Ardour:
+         *
+         * accel_path: <Actions>/GroupName/ActionName
+         * name: GroupName/ActionName
+         *
+         * This (static) method returns the "ardour" name for the action.
+         */
+        static std::string ardour_action_name (Glib::RefPtr<Gtk::Action>);
+        
         void set_action_map (ActionMap&);
         
         /* used to list all actions */
@@ -192,14 +211,16 @@ class LIBGTKMM2EXT_API Bindings {
 
 	/* all bindings currently in existence, as grouped into Bindings */
 	static std::list<Bindings*> bindings;
-	static Bindings* get_bindings (std::string const& name);
+	static Bindings* get_bindings (std::string const& name, ActionMap&);
 	static void associate_all ();
+
+	static PBD::Signal1<void,Bindings*> BindingsChanged;
 	
   private:
         typedef std::map<KeyboardKey,ActionInfo> KeybindingMap;
 
         std::string  _name;
-        ActionMap&   _action_map;
+        ActionMap*   _action_map;
         KeybindingMap press_bindings;
         KeybindingMap release_bindings;
         
diff --git a/libs/gtkmm2ext/gtkmm2ext/gtk_ui.h b/libs/gtkmm2ext/gtkmm2ext/gtk_ui.h
index f0f34fb4d8..b9c80a34e0 100644
--- a/libs/gtkmm2ext/gtkmm2ext/gtk_ui.h
+++ b/libs/gtkmm2ext/gtkmm2ext/gtk_ui.h
@@ -58,6 +58,7 @@ class Touchable;
 namespace Gtkmm2ext {
 
 class TextViewer;
+class Bindings;
 
 extern BaseUI::RequestType NullMessage;
 extern BaseUI::RequestType ErrorMessage;
@@ -169,6 +170,8 @@ class LIBGTKMM2EXT_API UI : public AbstractUI<UIRequest>
 	static bool just_hide_it (GdkEventAny *, Gtk::Window *);
 	static float ui_scale;
 
+	Gtkmm2ext::Bindings* global_bindings;
+
   protected:
 	virtual void handle_fatal (const char *);
 	virtual void display_message (const char *prefix, gint prefix_len,
diff --git a/libs/gtkmm2ext/gtkmm2ext/keyboard.h b/libs/gtkmm2ext/gtkmm2ext/keyboard.h
index 48a1bdcaad..fdea0416c3 100644
--- a/libs/gtkmm2ext/gtkmm2ext/keyboard.h
+++ b/libs/gtkmm2ext/gtkmm2ext/keyboard.h
@@ -24,7 +24,6 @@
 #include <vector>
 #include <string>
 
-#include <sigc++/signal.h>
 #include <gtk/gtk.h>
 #include <gtkmm/accelkey.h>
 
@@ -40,6 +39,7 @@ namespace Gtk {
 namespace Gtkmm2ext {
 
 class Bindings;
+class ActionMap;
 
 class LIBGTKMM2EXT_API Keyboard : public sigc::trackable, PBD::Stateful
 {
@@ -177,10 +177,6 @@ class LIBGTKMM2EXT_API Keyboard : public sigc::trackable, PBD::Stateful
 
 	sigc::signal0<void> ZoomVerticalModifierReleased;
 
-	static std::vector<Bindings*> bindings;
-	static Bindings* get_bindings (std::string const& name);
-	static PBD::Signal0<void> BindingsChanged;
-
   protected:
 	static Keyboard* _the_keyboard;
 
diff --git a/libs/gtkmm2ext/keyboard.cc b/libs/gtkmm2ext/keyboard.cc
index 1b3f661e60..cd6c179b1a 100644
--- a/libs/gtkmm2ext/keyboard.cc
+++ b/libs/gtkmm2ext/keyboard.cc
@@ -111,8 +111,6 @@ 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;
-Gtk::Window* Keyboard::pre_dialog_active_window = 0;
-PBD::Signal0<void> Keyboard::BindingsChanged;
 
 /* set this to initially contain the modifiers we care about, then track changes in ::set_edit_modifier() etc. */
 GdkModifierType Keyboard::RelevantModifierKeyMask;
@@ -643,7 +641,6 @@ Keyboard::keybindings_changed ()
 	}
 
 	Keyboard::save_keybindings ();
-	BindingsChanged (); /* EMIT SIGNAL */
 }
 
 void