Basic refactoring: Action -> Action*

src/laybasic/laybasic/gsiDeclLayMenu.cc

@@ -25,6 +25,34 @@
 #include "gsiSignals.h"
 #include "layAbstractMenu.h"
 
+namespace {
+
+//  The Action stub to allow reimplementation of the triggered method
+class ActionStub
+  : public lay::Action
+{
+public:
+  virtual void triggered ()
+  {
+    if (triggered_cb.can_issue ()) {
+      triggered_cb.issue<lay::Action> (&lay::Action::triggered);
+    }
+    on_triggered_event ();
+  }
+
+  gsi::Callback triggered_cb;
+  tl::Event on_triggered_event;
+};
+
+}
+
+namespace tl
+{
+  template <> struct type_traits<ActionStub> : public type_traits<void> {
+    typedef tl::false_tag has_copy_constructor;
+  };
+}
+
 namespace gsi
 {
 
@@ -87,7 +115,7 @@ Class<lay::AbstractMenu> decl_AbstractMenu ("lay", "AbstractMenu",
     "\n"
     "This method has been introduced in version 0.26."
   ) +
-  method ("action", &lay::AbstractMenu::action,
+  method ("action", (lay::Action *(lay::AbstractMenu::*) (const std::string &path)) &lay::AbstractMenu::action,
     "@brief Get the reference to a Action object associated with the given path\n"
     "@args path\n"
     "\n"
@@ -359,23 +387,6 @@ Class<lay::Action> decl_ActionBase ("lay", "ActionBase",
   "@alias Action\n"
 );
   
-//  The Action stub to allow reimplementation of the triggered method
-class ActionStub
-  : public lay::Action
-{
-public:
-  virtual void triggered ()
-  {
-    if (triggered_cb.can_issue ()) {
-      triggered_cb.issue<lay::Action> (&lay::Action::triggered);
-    }
-    on_triggered_event ();
-  }
-
-  gsi::Callback triggered_cb;
-  tl::Event on_triggered_event;
-};
-
 Class<ActionStub> decl_Action (decl_ActionBase, "lay", "Action",
   gsi::callback ("triggered", &ActionStub::triggered, &ActionStub::triggered_cb,
     "@brief This method is called if the menu item is selected"

src/laybasic/laybasic/layAbstractMenu.h

@@ -75,83 +75,14 @@ LAYBASIC_PUBLIC std::vector<std::pair<std::string, bool> > unpack_menu_items_hid
 LAYBASIC_PUBLIC std::string pack_menu_items_hidden (const std::vector<std::pair<std::string, bool> > &unpacked);
 
 /**
- *  @brief A helper class that does reference counting for the QAction object
- */
-class ActionHandle
-  : public QObject
-{
-Q_OBJECT
-
-public:
-  ActionHandle ();
-  ActionHandle (QAction *action, bool owned = true);
-  ActionHandle (QMenu *menu, bool owned = true);
-  ~ActionHandle ();
-  void add_ref ();
-  void remove_ref ();
-  QAction *ptr () const;
-  QMenu *menu () const;
-  Dispatcher *dispatcher () { return mp_dispatcher; }
-
-  void set_symbol (const std::string &s) { m_symbol = s; }
-  const std::string &symbol () const { return m_symbol; }
-
-  void set_visible (bool v);
-  void set_hidden (bool h);
-  bool is_visible () const;
-  bool is_hidden () const;
-  bool is_effective_visible () const;
-
-  void set_default_shortcut (const std::string &sc);
-  void set_shortcut (const std::string &sc);
-  std::string get_default_shortcut () const;
-  std::string get_shortcut() const;
-  QKeySequence get_key_sequence () const;
-  QKeySequence get_key_sequence_for (const std::string &sc) const;
-
-signals:
-  void triggered ();
-
-protected slots:
-  void destroyed (QObject *obj);
-  void qaction_triggered ();
-
-private:
-  friend struct AbstractMenuItem;
-
-  QMenu *mp_menu;
-  QAction *mp_action;
-  lay::Dispatcher *mp_dispatcher;
-  int m_ref_count;
-  bool m_owned;
-  bool m_visible;
-  bool m_hidden;
-  std::string m_default_shortcut;
-  QKeySequence m_default_key_sequence;
-  std::string m_shortcut;
-  std::string m_symbol;
-  QKeySequence m_key_sequence;
-  bool m_no_key_sequence;
-
-  void set_dispatcher (Dispatcher *dispatcher);
-
-  //  no copying
-  ActionHandle (const ActionHandle &);
-  ActionHandle &operator= (const ActionHandle &);
-};
-
-/**
- *  @brief A QAction proxy 
+ *  @brief The basic Action object
  *
- *  This class is provided to be better suited to automation because it provides the corresponding interfaces
- *  and allows implementing the "trigger" event by reimplementation of the slot (after being connected).
- *  In addition, by acting as a proxy, it can act as a reference to a QAction object created by Qt itself (i.e.
- *  within a QMenu).
- *  To reimplement the "triggered" method, derive a class from Action and reimplement "triggered". Then
- *  pass a reference to this object to "insert_item" of AbstractMenu and store away the derived object.
- *  This object then will receive the triggered events, although the menu holds a pure Action object. 
- *  This works since the basic object is the QAction to which both the derived object and the Action 
- *  point to. A triggered event will be forwarded from this QAction to all referencing Action objects.
+ *  An "action" is the target of a menu action. It allows reimplementing the "triggered" method
+ *  to implement a specific action. The action encapsulates a QAction object.
+ *
+ *  Use this object in "insert_item" of AbstractMenu.
+ *
+ *  This object is typically owned by the AbstractMenu and cannot be copied or assigned.
  */
 class LAYBASIC_PUBLIC Action
   : public QObject,
@@ -162,21 +93,25 @@ Q_OBJECT
 public:
   /**
    *  @brief The main constructor
-   *
-   *  This constructor creates an QAction object internally which is owned by the Action object.
    */
   Action ();
 
   /**
-   *  @brief The copy constructor
-   *
-   *  This constructor creates an QAction object internally which is owned by the Action object.
+   *  @brief Creates an action from the given QAction
+   *  If "owned" is true, the QAction will become owned by the Action object.
    */
-  Action (const Action &action); 
+  Action (QAction *action, bool owned = true);
+
+  /**
+   *  @brief Creates an action from the given QMenu
+   *  If "owned" is true, the QAction will become owned by the Action object.
+   */
+  Action (QMenu *menu, bool owned = true);
 
   /**
    *  @brief Creates an action with the given title (icon, keyboard shortcut)
    *
+   *  The title will optionally encode the shortcut and/or icon resource.
    *  The format of the title string is: <text>["("shortcut")"]["<"icon-resource">"]
    *
    *  @param title The title string encoding icon and keyboard shortcut if applicable.
@@ -184,14 +119,9 @@ public:
   Action (const std::string &title);
 
   /**
-   *  @brief Assignement
+   *  @brief Destructor
    */
-  Action &operator= (const Action &action);
-
-  /**
-   *  @brief The destructor
-   */
-  ~Action (); 
+  ~Action ();
 
   /**
    *  @brief Set the title
@@ -316,7 +246,7 @@ public:
   void set_separator (bool s);
 
   /**
-   *  @brief Set the tool tip text 
+   *  @brief Set the tool tip text
    *
    *  @param text The text to display in the tool tip
    */
@@ -332,7 +262,7 @@ public:
   void set_icon (const std::string &filename);
 
   /**
-   *  @brief Set the icon's text 
+   *  @brief Set the icon's text
    *
    *  If an icon text is set, this will be used for the text below the icon.
    *  If no icon text is set, the normal text will be used for the icon.
@@ -340,7 +270,7 @@ public:
    */
   void set_icon_text (const std::string &icon_text);
 
-  /** 
+  /**
    *  @brief Set the action's QObject name (for testing for example)
    */
   void set_object_name (const std::string &name);
@@ -351,12 +281,12 @@ public:
   void add_to_exclusive_group (lay::AbstractMenu *menu, const std::string &group_name);
 
   /**
-   *  @brief Get the tool tip text 
+   *  @brief Get the tool tip text
    */
   std::string get_tool_tip () const;
 
   /**
-   *  @brief Get the icon's text 
+   *  @brief Get the icon's text
    */
   std::string get_icon_text () const;
 
@@ -382,50 +312,41 @@ public:
    */
   QMenu *menu () const;
 
-  /**
-   *  @brief Gets the handle
-   *  The handle is the representative of the menu entry which is shared
-   *  by all Action copies.
-   */
-  ActionHandle *handle ()
-  {
-    return mp_handle;
-  }
-
   /**
    *  @brief Gets the dispatcher object this action is connected to
    */
   Dispatcher *dispatcher () const
   {
-    return mp_handle->dispatcher ();
+    return mp_dispatcher;
   }
 
-  /**
-   *  @brief Compares two action objects 
-   *
-   *  Two action objects are equal when they refer to the same ActionHandle.
-   */
-  bool operator== (const Action &other) const
-  {
-    return mp_handle == other.mp_handle;
-  }
-
-public slots:
-  void triggered_slot ();
+protected slots:
+  void destroyed (QObject *obj);
+  void qaction_triggered ();
 
 private:
-  friend class AbstractMenu;
+  friend struct AbstractMenuItem;
 
-  ActionHandle *mp_handle;
+  QMenu *mp_menu;
+  QAction *mp_action;
+  lay::Dispatcher *mp_dispatcher;
+  bool m_owned;
+  bool m_visible;
+  bool m_hidden;
+  std::string m_default_shortcut;
+  QKeySequence m_default_key_sequence;
+  std::string m_shortcut;
+  std::string m_symbol;
+  QKeySequence m_key_sequence;
+  bool m_no_key_sequence;
 
-  /**
-   *  @brief The proxy constructor
-   *
-   *  This constructor takes a QAction object that it will refer to.
-   *  If the Action is copied, the copy will refer to the same QAction.
-   *  The QAction object is deleted if the last Action referring to QAction is deleted.
-   */
-  Action (ActionHandle *action);
+  void set_dispatcher (Dispatcher *dispatcher);
+  QKeySequence get_key_sequence () const;
+  QKeySequence get_key_sequence_for (const std::string &sc) const;
+
+  //  no copying
+  Action (const Action &);
+  Action &operator= (const Action &);
 };
 
 /**
@@ -444,7 +365,7 @@ public:
   /**
    *  @brief The default constructor
    */
-  ConfigureAction (lay::Dispatcher *dispatcher);
+  ConfigureAction ();
 
   /**
    *  @brief Create an configure action with the given name and value
@@ -455,14 +376,13 @@ public:
    *  The value can be "?" in which case the configuration action describes 
    *  a boolean parameter which is mapped to a checkable action.
    */
-  ConfigureAction (lay::Dispatcher *dispatcher, const std::string &cname, const std::string &value);
+  ConfigureAction (const std::string &cname, const std::string &value);
 
   /**
    *  @brief Create an configure action with the given title (icon, keyboard shortcut), name and value
    *
    *  The format of the title string is: <text>["("shortcut")"]["<"icon-resource">"]
    *
-   *  @param pr The reference to the plugin root object which receives the configure request 
    *  @param title The title string encoding icon and keyboard shortcut if applicable.
    *  @param cname The name of the configuration parameter to set
    *  @param cvalue The value to set "cname" to
@@ -470,7 +390,7 @@ public:
    *  The value can be "?" in which case the configuration action describes 
    *  a boolean parameter which is mapped to a checkable action.
    */
-  ConfigureAction (lay::Dispatcher *dispatcher, const std::string &title, const std::string &cname, const std::string &value);
+  ConfigureAction (const std::string &title, const std::string &cname, const std::string &value);
 
   /**
    *  @brief Destructor
@@ -519,14 +439,10 @@ public:
 protected:
   virtual void triggered ();
 
-  void reg ();
-  void unreg ();
-
 private:
   ConfigureAction (const ConfigureAction &action); 
   ConfigureAction &operator= (const ConfigureAction &action); 
 
-  lay::Dispatcher *mp_dispatcher;
   std::string m_cname, m_cvalue;
   type m_type;
 };
@@ -544,7 +460,7 @@ struct LAYBASIC_PUBLIC AbstractMenuItem
   /**
    *  @brief Internal method used to set up the item
    */
-  void setup_item (const std::string &pn, const std::string &n, const Action &a); 
+  void setup_item (const std::string &pn, const std::string &n, Action *a);
 
   Dispatcher *dispatcher () const
   {
@@ -561,18 +477,23 @@ struct LAYBASIC_PUBLIC AbstractMenuItem
     return m_groups;
   }
 
-  void set_action (const Action &a, bool copy_properties);
+  void set_action (Action *a, bool copy_properties);
 
   void set_action_title (const std::string &t);
 
-  const Action &action () const
+  Action *action ()
   {
-    return m_action;
+    return mp_action.get ();
   }
 
-  QMenu *menu () const
+  const Action *action () const
   {
-    return m_action.menu ();
+    return mp_action.get ();
+  }
+
+  QMenu *menu ()
+  {
+    return mp_action->menu ();
   }
 
   void set_has_submenu ();
@@ -592,7 +513,7 @@ struct LAYBASIC_PUBLIC AbstractMenuItem
   std::list <AbstractMenuItem> children;
 
 private:
-  Action m_action;
+  tl::shared_ptr<Action> mp_action;
   Dispatcher *mp_dispatcher;
   bool m_has_submenu;
   bool m_remove_on_empty;
@@ -694,7 +615,12 @@ public:
    *  @param path The path to the item. This must be a valid path.
    *  @return The action object
    */
-  Action action (const std::string &path) const;
+  Action *action (const std::string &path);
+
+  /**
+   *  @brief Get the Action object for a given item (const version)
+   */
+  const Action *action(const std::string &path) const;
 
   /**
    *  @brief Get the subitems for a given submenu
@@ -737,8 +663,10 @@ public:
    *  @param path The path to the item before which to insert the new item
    *  @param name The name of the item to insert 
    *  @param action The action associated with the item
+   *
+   *  NOTE: the abstract menu will take ownership of the Action object.
    */
-  void insert_item (const std::string &path, const std::string &name, const Action &action); 
+  void insert_item (const std::string &path, const std::string &name, Action *action);
 
   /**
    *  @brief Insert a new separator before the one item by the path
@@ -758,7 +686,7 @@ public:
    *  @param name The name of the submenu to insert 
    *  @param action The action associated with the submenu
    */
-  void insert_menu (const std::string &path, const std::string &name, const Action &action);
+  void insert_menu (const std::string &path, const std::string &name, Action *action);
 
   /**
    *  @brief Insert a new submenu before the item given by the path
@@ -789,7 +717,7 @@ public:
   /**
    *  @brief Delete the items referring to the given action
    */
-  void delete_items (const Action &action);
+  void delete_items (Action *action);
 
   /**
    *  @brief Get the group members
@@ -805,7 +733,14 @@ public:
    *  @param group The group name
    *  @param A vector of all members (as actions) of the group
    */
-  std::vector<lay::Action> group_actions (const std::string &name) const;
+  std::vector<Action *> group_actions(const std::string &name);
+
+  /**
+   *  @brief Get the configure actions for a given configuration name
+   *
+   *  @param The configuration actions for this given configuration name
+   */
+  std::vector<lay::ConfigureAction *> configure_actions (const std::string &name);
 
   /**
    *  @brief Get the detached menu
@@ -852,21 +787,15 @@ private:
   void build (QMenu *menu, std::list<AbstractMenuItem> &items);
   void build (QToolBar *tbar, std::list<AbstractMenuItem> &items);
   void collect_group (std::vector<std::string> &grp, const std::string &name, const AbstractMenuItem &item) const;
-
-  /**
-   *  @brief Create a action from a string
-   *
-   *  The format of the string is: <text>["("shortcut")"]["<"icon-resource">"]
-   *
-   *  @param s The title, key and icon resource string in the format given above
-   *  @return The ActionHandle object created
-   */
-  static ActionHandle *create_action (const std::string &s);
+  void collect_configure_actions (std::vector<ConfigureAction *> &ca, AbstractMenuItem &item);
+  void emit_changed ();
 
   Dispatcher *mp_dispatcher;
   AbstractMenuItem m_root;
   tl::stable_vector<QMenu> m_helper_menu_items;
   std::map<std::string, QActionGroup *> m_action_groups;
+  std::map<std::string, std::vector<ConfigureAction *> > m_config_action_by_name;
+  bool m_config_actions_valid;
 };
 
 }
@@ -877,6 +806,15 @@ namespace tl
     typedef tl::false_tag has_copy_constructor;
     typedef tl::false_tag has_default_constructor;
   };
+
+  template <> struct type_traits<lay::AbstractMenuItem> : public type_traits<void> {
+    typedef tl::false_tag has_copy_constructor;
+    typedef tl::false_tag has_default_constructor;
+  };
+
+  template <> struct type_traits<lay::Action> : public type_traits<void> {
+    typedef tl::false_tag has_copy_constructor;
+  };
 }
 
 #endif

src/laybasic/laybasic/layDispatcher.cc

@@ -50,66 +50,13 @@ Dispatcher::~Dispatcher ()
   }
 }
 
-lay::ConfigureAction *
-Dispatcher::create_config_action (const std::string &title, const std::string &cname, const std::string &cvalue)
-{
-  lay::ConfigureAction *ca = new lay::ConfigureAction (this, title, cname, cvalue);
-  m_ca_collection.push_back (ca);
-  return ca;
-}
-
-lay::ConfigureAction *
-Dispatcher::create_config_action (const std::string &cname, const std::string &cvalue)
-{
-  lay::ConfigureAction *ca = new lay::ConfigureAction (this, std::string (), cname, cvalue);
-  m_ca_collection.push_back (ca);
-  return ca;
-}
-
-void
-Dispatcher::register_config_action (const std::string &name, lay::ConfigureAction *action)
-{
-  std::map<std::string, std::vector<lay::ConfigureAction *> >::iterator ca = m_configuration_actions.insert (std::make_pair (name, std::vector<lay::ConfigureAction *> ())).first;
-  for (std::vector<lay::ConfigureAction *>::iterator a = ca->second.begin (); a != ca->second.end (); ++a) {
-    if (*a == action) {
-      return; // already registered
-    }
-  }
-
-  ca->second.push_back (action);
-}
-
-void
-Dispatcher::unregister_config_action (const std::string &name, lay::ConfigureAction *action)
-{
-  std::map<std::string, std::vector<lay::ConfigureAction *> >::iterator ca = m_configuration_actions.find (name);
-  if (ca != m_configuration_actions.end ()) {
-    for (std::vector<lay::ConfigureAction *>::iterator a = ca->second.begin (); a != ca->second.end (); ++a) {
-      if (*a == action) {
-        ca->second.erase (a);
-        return;
-      }
-    }
-  }
-}
-
-void
-Dispatcher::clear_configuration_actions ()
-{
-  m_ca_collection.clear ();
-  m_configuration_actions.clear ();
-}
-
 bool
 Dispatcher::configure (const std::string &name, const std::string &value)
 {
-  std::map<std::string, std::vector<lay::ConfigureAction *> >::iterator ca = m_configuration_actions.find (name);
-  if (ca != m_configuration_actions.end ()) {
-    for (std::vector<lay::ConfigureAction *>::const_iterator a = ca->second.begin (); a != ca->second.end (); ++a) {
-      (*a)->configure (value);
-    }
+  std::vector<lay::ConfigureAction *> ca = m_menu.configure_actions (name);
+  for (std::vector<lay::ConfigureAction *>::const_iterator a = ca.begin (); a != ca.end (); ++a) {
+    (*a)->configure (value);
   }
-
   return false;
 }
 

src/laybasic/laybasic/layDispatcher.h

@@ -129,44 +129,15 @@ public:
     return (dispatcher () == this) ? &m_menu : dispatcher ()->menu ();
   }
 
-  /**
-   *  @brief Creates a configuration action with the given title, parameter name and value
-   *
-   *  The action will be owned by the abstract menu provider but can be deleted to remove it from there.
-   */
-  lay::ConfigureAction *create_config_action (const std::string &title, const std::string &cname, const std::string &cvalue);
-
-  /**
-   *  @brief Creates a configuration action with the given parameter name and value
-   *
-   *  The action will be owned by the abstract menu provider but can be deleted to remove it from there.
-   *  This version is provided for applications, where the title is set later.
-   */
-  lay::ConfigureAction *create_config_action (const std::string &cname, const std::string &cvalue);
-
-  /**
-   *  @brief Registers a configuration action with the given name
-   */
-  void register_config_action (const std::string &name, lay::ConfigureAction *action);
-
-  /**
-   *  @brief Unregisters a configuration action with the given name
-   */
-  void unregister_config_action (const std::string &name, lay::ConfigureAction *action);
-
 protected:
   //  capture the configuration events so we can change the value of the configuration actions
   virtual bool configure (const std::string &name, const std::string &value);
 
-  void clear_configuration_actions ();
-
 private:
   Dispatcher (const Dispatcher &);
   Dispatcher &operator= (const Dispatcher &);
 
   lay::AbstractMenu m_menu;
-  std::map<std::string, std::vector<lay::ConfigureAction *> > m_configuration_actions;
-  tl::shared_collection<lay::ConfigureAction> m_ca_collection;
 };
 
 }

src/laybasic/laybasic/layLayoutView.cc

@@ -366,11 +366,11 @@ LayoutView::init_menu ()
   //  if not in editable mode, hide all entries from "edit_mode" group and show all from the "view_mode" group and vice versa
   std::vector<std::string> edit_mode_grp = menu ()->group ("edit_mode");
   for (std::vector<std::string>::const_iterator g = edit_mode_grp.begin (); g != edit_mode_grp.end (); ++g) {
-    menu ()->action (*g).set_visible (is_editable ());
+    menu ()->action (*g)->set_visible (is_editable ());
   }
   std::vector<std::string> view_mode_grp = menu ()->group ("view_mode");
   for (std::vector<std::string>::const_iterator g = view_mode_grp.begin (); g != view_mode_grp.end (); ++g) {
-    menu ()->action (*g).set_visible (! is_editable ());
+    menu ()->action (*g)->set_visible (! is_editable ());
   }
 }
 
@@ -896,6 +896,32 @@ Plugin *LayoutView::get_plugin_by_name (const std::string &name) const
   return 0;
 }
 
+namespace {
+
+class GotoBookmarkAction
+  : public lay::Action
+{
+public:
+  GotoBookmarkAction (lay::LayoutView *view, size_t id, const std::string &title)
+    : Action (), mp_view (view), m_id (id)
+  {
+    set_title (title);
+  }
+
+  void triggered ()
+  {
+    if (mp_view) {
+      mp_view->goto_view (mp_view->bookmarks ().state (m_id));
+    }
+  }
+
+private:
+  tl::weak_ptr<lay::LayoutView> mp_view;
+  size_t m_id;
+};
+
+}
+
 void
 LayoutView::update_menu (lay::LayoutView *view, lay::AbstractMenu &menu)
 {
@@ -905,23 +931,20 @@ LayoutView::update_menu (lay::LayoutView *view, lay::AbstractMenu &menu)
 
     menu.clear_menu (bm_menu);
 
-    Action goto_bookmark_action = menu.action (bm_menu);
+    Action *goto_bookmark_action = menu.action (bm_menu);
 
     if (view && view->bookmarks ().size () > 0) {
 
-      goto_bookmark_action.set_enabled (true);
+      goto_bookmark_action->set_enabled (true);
 
       const lay::BookmarkList &bookmarks = view->bookmarks ();
       for (size_t i = 0; i < bookmarks.size (); ++i) {
-        Action action;
-        gtf::action_connect (action.qaction (), SIGNAL (triggered ()), view, SLOT (goto_bookmark ()));
-        action.set_title (bookmarks.name (i));
-        action.qaction ()->setData (QVariant (int (i)));
+        Action *action = new GotoBookmarkAction (view, i, bookmarks.name (i));
         menu.insert_item (bm_menu + ".end", tl::sprintf ("bookmark_%d", i + 1), action);
       }
 
     } else {
-      goto_bookmark_action.set_enabled (false);
+      goto_bookmark_action->set_enabled (false);
     }
 
   }
@@ -3917,21 +3940,6 @@ LayoutView::bookmark_view (const std::string &name)
   emit menu_needs_update ();
 }
 
-void
-LayoutView::goto_bookmark ()
-{
-  BEGIN_PROTECTED
-
-  QAction *action = dynamic_cast <QAction *> (sender ());
-  tl_assert (action);
-  size_t id = size_t (action->data ().toInt ());
-  if (bookmarks ().size () > id) {
-    goto_view (bookmarks ().state (id));
-  }
-
-  END_PROTECTED
-}
-
 void
 LayoutView::goto_view (const DisplayState &state)
 {

src/laybasic/laybasic/layLayoutView.h

@@ -2684,7 +2684,6 @@ public slots:
 private slots:
   void active_cellview_changed (int index);
   void active_library_changed (int index);
-  void goto_bookmark ();
   void side_panel_destroyed ();
 
 signals:

src/laybasic/laybasic/layPlugin.cc

@@ -56,8 +56,6 @@ PluginDeclaration::PluginDeclaration ()
 
 PluginDeclaration::~PluginDeclaration ()
 {
-  clear_menu_items ();
-
   if (Dispatcher::instance ()) {
     Dispatcher::instance ()->plugin_removed (this);
   }
@@ -95,60 +93,58 @@ PluginDeclaration::menu_symbols ()
   return symbols;
 }
 
-void 
-PluginDeclaration::generic_menu ()
+namespace {
+
+class GenericMenuAction
+  : public Action
 {
-  BEGIN_PROTECTED
+public:
+  GenericMenuAction (Dispatcher *dispatcher, const std::string &title, const std::string &symbol)
+    : Action (title), mp_dispatcher (dispatcher), m_symbol (symbol)
+  { }
 
-  ActionHandle *action_handle = dynamic_cast <ActionHandle *> (sender ());
-  tl_assert (action_handle);
-  tl_assert (action_handle->dispatcher () != 0);
-
-  //  Global handler: give the declaration a chance to handle the menu request globally
-  if (menu_activated (action_handle->symbol ())) {
-    return;
+  void triggered ()
+  {
+    if (mp_dispatcher) {
+      mp_dispatcher->menu_activated (m_symbol);
+    }
   }
 
-  //  Forward the request to the plugin root which will propagate it down to the plugins
-  action_handle->dispatcher ()->menu_activated (action_handle->symbol ());
+private:
+  Dispatcher *mp_dispatcher;
+  std::string m_symbol;
+};
 
-  END_PROTECTED
-}
-
-void 
-PluginDeclaration::mode_triggered ()
+class ModeAction
+  : public Action
 {
-  BEGIN_PROTECTED
+public:
+  ModeAction (Dispatcher *dispatcher, const std::string &title, int mode)
+    : Action (title), mp_dispatcher (dispatcher), m_mode (mode)
+  { }
 
-  ActionHandle *action_handle = dynamic_cast<ActionHandle *> (sender ());
-  tl_assert (action_handle != 0);
-  tl_assert (action_handle->dispatcher () != 0);
+  void triggered ()
+  {
+    if (mp_dispatcher) {
+      mp_dispatcher->select_mode (m_mode);
+      set_checked (true);
+    }
+  }
 
-  int mode = 0;
-  tl::from_string (action_handle->symbol (), mode);
+private:
+  Dispatcher *mp_dispatcher;
+  int m_mode;
+};
 
-  action_handle->dispatcher ()->select_mode (mode);
-  action_handle->ptr ()->setChecked (true);
-
-  END_PROTECTED
 }
 
 void
-PluginDeclaration::clear_menu_items ()
-{
-  m_menu_actions.clear ();
-  m_our_menu_actions.clear ();
-}
-
-void 
 PluginDeclaration::init_menu (lay::Dispatcher *dispatcher)
 {
   lay::AbstractMenu &menu = *dispatcher->menu ();
 
-  //  pre-initialize to allow multiple init_menu calls
-  m_editable_mode_action = lay::Action ();
-  m_mouse_mode_action = lay::Action ();
-  clear_menu_items ();
+  mp_editable_mode_action.reset ((Action *) 0);
+  mp_mouse_mode_action.reset ((Action *) 0);
 
   std::string title;
 
@@ -166,12 +162,12 @@ PluginDeclaration::init_menu (lay::Dispatcher *dispatcher)
       title = tab + 1;
     } 
 
-    m_editable_mode_action = Action (title);
-    gtf::action_connect (m_editable_mode_action.qaction (), SIGNAL (triggered ()), this, SLOT (toggle_editable_enabled ()));
-    m_editable_mode_action.set_checkable (true);
-    m_editable_mode_action.set_checked (m_editable_enabled);
+    mp_editable_mode_action.reset (new Action (title));
+    gtf::action_connect (mp_editable_mode_action->qaction (), SIGNAL (triggered ()), this, SLOT (toggle_editable_enabled ()));
+    mp_editable_mode_action->set_checkable (true);
+    mp_editable_mode_action->set_checked (m_editable_enabled);
 
-    menu.insert_item ("edit_menu.select_menu.end", name, m_editable_mode_action);
+    menu.insert_item ("edit_menu.select_menu.end", name, mp_editable_mode_action.get ());
 
   }
 
@@ -198,20 +194,13 @@ PluginDeclaration::init_menu (lay::Dispatcher *dispatcher)
       Action *action = 0;
 
       if (! m->cname.empty ()) {
-
-        action = dispatcher->create_config_action (m->title, m->cname, m->cvalue);
-
+        action = new ConfigureAction (m->title, m->cname, m->cvalue);
       } else {
-
-        action = new Action (m->title);
-        action->handle ()->set_symbol (m->symbol);
-        connect (action->handle (), SIGNAL (triggered ()), this, SLOT (generic_menu ()));
-        m_our_menu_actions.push_back (action);
-
+        action = new GenericMenuAction (dispatcher, m->title, m->symbol);
       }
 
       m_menu_actions.push_back (action);
-      menu.insert_item (m->insert_pos, m->menu_name, *action);
+      menu.insert_item (m->insert_pos, m->menu_name, action);
 
       if (! m->exclusive_group.empty ()) {
         action->add_to_exclusive_group (&menu, m->exclusive_group);
@@ -254,14 +243,11 @@ PluginDeclaration::init_menu (lay::Dispatcher *dispatcher)
       title = std::string (tab + 1);
     } 
 
-    m_mouse_mode_action = Action (title);
-    m_mouse_mode_action.add_to_exclusive_group (&menu, "mouse_mode_exclusive_group");
-    m_mouse_mode_action.set_checkable (true);
-    m_mouse_mode_action.handle ()->set_symbol (tl::to_string (m->second.second));
+    mp_mouse_mode_action.reset (new ModeAction (dispatcher, title, m->second.second));
+    mp_mouse_mode_action->add_to_exclusive_group (&menu, "mouse_mode_exclusive_group");
+    mp_mouse_mode_action->set_checkable (true);
 
-    menu.insert_item (m->second.first, name + ":mode_group", m_mouse_mode_action);
-
-    connect (m_mouse_mode_action.handle (), SIGNAL (triggered ()), this, SLOT (mode_triggered ()));
+    menu.insert_item (m->second.first, name + ":mode_group", mp_mouse_mode_action.get ());
 
   }
 }
@@ -270,13 +256,19 @@ void
 PluginDeclaration::remove_menu_items (Dispatcher *dispatcher)
 {
   lay::AbstractMenu *menu = dispatcher->menu ();
-  menu->delete_items (m_editable_mode_action);
-  menu->delete_items (m_mouse_mode_action);
+  menu->delete_items (mp_editable_mode_action.get ());
+  menu->delete_items (mp_mouse_mode_action.get ());
+
+  std::vector<lay::Action *> actions;
   for (tl::weak_collection <lay::Action>::iterator a = m_menu_actions.begin (); a != m_menu_actions.end (); ++a) {
     if (a.operator-> ()) {
-      menu->delete_items (*a);
+      actions.push_back (a.operator-> ());
     }
   }
+  for (std::vector<lay::Action *>::const_iterator a = actions.begin (); a != actions.end (); ++a) {
+    menu->delete_items (*a);
+  }
+  m_menu_actions.clear ();
 }
 
 void 
@@ -284,7 +276,9 @@ PluginDeclaration::set_editable_enabled (bool f)
 {
   if (f != m_editable_enabled) {
     m_editable_enabled = f;
-    m_editable_mode_action.set_checked (f);
+    if (mp_editable_mode_action.get ()) {
+      mp_editable_mode_action->set_checked (f);
+    }
     editable_enabled_changed_event ();
   }
 }

src/laybasic/laybasic/layPlugin.h

@@ -471,18 +471,13 @@ public:
 
 private slots:
   void toggle_editable_enabled ();
-  void generic_menu ();
-  void mode_triggered ();
 
 private:
   int m_id;
-  tl::weak_collection <lay::Action> m_menu_actions;
-  tl::shared_collection <lay::Action> m_our_menu_actions;
-  lay::Action m_editable_mode_action;
-  lay::Action m_mouse_mode_action;
+  tl::weak_ptr<lay::Action> mp_editable_mode_action;
+  tl::weak_ptr<lay::Action> mp_mouse_mode_action;
+  tl::weak_collection<lay::Action> m_menu_actions;
   bool m_editable_enabled;
-
-  void clear_menu_items ();
 };
 
 /**

src/laybasic/unit_tests/layAbstractMenu.cc

@@ -51,57 +51,57 @@ TEST(1)
   EXPECT_EQ (menu_to_string (menu), "");
 
   try {
-    EXPECT_EQ (menu.action ("n1").get_title (), "");
+    EXPECT_EQ (menu.action ("n1")->get_title (), "");
     EXPECT_EQ (true, false);
   } catch (...) {
   }
 
   EXPECT_EQ (menu.is_valid ("n1"), false);
 
-  menu.insert_menu ("end", "n1", lay::Action ("title:n1"));
+  menu.insert_menu ("end", "n1", new lay::Action ("title:n1"));
   EXPECT_EQ (menu_to_string (menu), "(n1)");
   EXPECT_EQ (tl::join (menu.items (""), ","), "n1");
   EXPECT_EQ (menu.is_menu ("n1"), true);
-  EXPECT_EQ (menu.action ("n1").get_title (), "title:n1");
+  EXPECT_EQ (menu.action ("n1")->get_title (), "title:n1");
 
   EXPECT_EQ (menu.is_valid ("n1"), true);
   EXPECT_EQ (menu.is_valid ("n2"), false);
 
-  menu.insert_menu ("end", "n2", lay::Action ("title:n2"));
+  menu.insert_menu ("end", "n2", new lay::Action ("title:n2"));
   EXPECT_EQ (menu_to_string (menu), "(n1,n2)");
   EXPECT_EQ (tl::join (menu.items (""), ","), "n1,n2");
   EXPECT_EQ (menu.is_menu ("n2"), true);
-  EXPECT_EQ (menu.action ("n2").get_title (), "title:n2");
+  EXPECT_EQ (menu.action ("n2")->get_title (), "title:n2");
 
   EXPECT_EQ (menu.is_valid ("n2"), true);
 
-  menu.insert_menu ("end", "n1", lay::Action ("title:n1"));
+  menu.insert_menu ("end", "n1", new lay::Action ("title:n1"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1)");
   EXPECT_EQ (menu.is_menu ("n1"), true);
-  EXPECT_EQ (menu.action ("n1").get_title (), "title:n1");
+  EXPECT_EQ (menu.action ("n1")->get_title (), "title:n1");
 
-  menu.insert_item ("n1.begin", "c1", lay::Action ("title:c1"));
+  menu.insert_item ("n1.begin", "c1", new lay::Action ("title:c1"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1))");
   EXPECT_EQ (tl::join (menu.items ("n1"), ","), "n1.c1");
-  EXPECT_EQ (menu.action ("n1.c1").get_title (), "title:c1");
+  EXPECT_EQ (menu.action ("n1.c1")->get_title (), "title:c1");
 
-  menu.insert_item ("n1.end", "c2", lay::Action ("title:c2"));
+  menu.insert_item ("n1.end", "c2", new lay::Action ("title:c2"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1,n1.c2))");
   EXPECT_EQ (tl::join (menu.items ("n1"), ","), "n1.c1,n1.c2");
   EXPECT_EQ (menu.is_menu ("n1.c2"), false);
-  EXPECT_EQ (menu.action ("n1.c2").get_title (), "title:c2");
+  EXPECT_EQ (menu.action ("n1.c2")->get_title (), "title:c2");
 
-  menu.insert_item ("n1.begin", "c1", lay::Action ("title:c1a"));
+  menu.insert_item ("n1.begin", "c1", new lay::Action ("title:c1a"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1,n1.c2))");
   EXPECT_EQ (tl::join (menu.items ("n1"), ","), "n1.c1,n1.c2");
-  EXPECT_EQ (menu.action ("n1.c1").get_title (), "title:c1a");
+  EXPECT_EQ (menu.action ("n1.c1")->get_title (), "title:c1a");
 
-  menu.insert_item ("n1.c1", "c3", lay::Action ("title:c3"));
+  menu.insert_item ("n1.c1", "c3", new lay::Action ("title:c3"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c3,n1.c1,n1.c2))");
 
-  menu.insert_item ("n1.c1+", "c4", lay::Action ("title:c4"));
+  menu.insert_item ("n1.c1+", "c4", new lay::Action ("title:c4"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c3,n1.c1,n1.c4,n1.c2))");
-  EXPECT_EQ (menu.action ("n1.c4").get_title (), "title:c4");
+  EXPECT_EQ (menu.action ("n1.c4")->get_title (), "title:c4");
 
   menu.delete_item ("n1.c1");
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c3,n1.c4,n1.c2))");
@@ -109,11 +109,11 @@ TEST(1)
   menu.delete_item ("n1");
   EXPECT_EQ (menu_to_string (menu), "(n2)");
 
-  menu.insert_item ("n1>end(title).end", "c1", lay::Action ("title:c1"));
+  menu.insert_item ("n1>end(title).end", "c1", new lay::Action ("title:c1"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1))");
-  EXPECT_EQ (menu.action ("n1.c1").get_title (), "title:c1");
+  EXPECT_EQ (menu.action ("n1.c1")->get_title (), "title:c1");
 
-  menu.insert_item ("n1>end(title).end", "c2", lay::Action ("title:c2"));
+  menu.insert_item ("n1>end(title).end", "c2", new lay::Action ("title:c2"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1,n1.c2))");
 
   menu.delete_item ("n1.c1");
@@ -128,9 +128,9 @@ TEST(1)
   menu.clear_menu ("n1");
   EXPECT_EQ (menu_to_string (menu), "(n2)");
 
-  menu.insert_menu ("end", "n1", lay::Action ("title:n1"));
-  menu.insert_item ("n1.begin", "c1", lay::Action ("title:c1"));
-  menu.insert_item ("n1.end", "c2", lay::Action ("title:c2"));
+  menu.insert_menu ("end", "n1", new lay::Action ("title:n1"));
+  menu.insert_item ("n1.begin", "c1", new lay::Action ("title:c1"));
+  menu.insert_item ("n1.end", "c2", new lay::Action ("title:c2"));
   EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1,n1.c2))");
   menu.clear_menu ("n1");
   EXPECT_EQ (menu_to_string (menu), "(n2,n1)");