Reworked bookmark menu implementation to avoid MacOS menu issue

Without this commit, bookmark menus got grayed out
on MacOS with Qt5 sometimes. Now, the implementation
of recent file menu, bookmark menu, macro menu and
the static main menu use the same framewkork which
includes a workaround for the disabled menu issue.

src/lay/lay/layMainWindow.cc

@@ -609,11 +609,6 @@ MainWindow::MainWindow (QApplication *app, const char *name)
   cp_frame_ly->addWidget (mp_cpy_label);
   cp_frame_ly->insertSpacing (-1, 6);
 
-  //  connect to the menus to provide the dynamic parts
-  QMenu *bookmark_menu = mp_menu->menu ("bookmark_menu");
-  tl_assert (bookmark_menu != 0);
-  connect (bookmark_menu, SIGNAL (aboutToShow ()), this, SLOT (bookmark_menu_show ()));
-
   //  select the default mode
   select_mode (lay::LayoutView::default_mode ());
 
@@ -1798,6 +1793,12 @@ MainWindow::edits_enabled_changed ()
   }
 }
 
+void
+MainWindow::menu_needs_update ()
+{
+  lay::LayoutView::update_menu (current_view (), *mp_menu);
+}
+
 void
 MainWindow::libraries_changed ()
 {
@@ -2426,57 +2427,6 @@ MainWindow::cm_redo ()
   END_PROTECTED
 }
 
-void
-MainWindow::bookmark_menu_show ()
-{
-  if (mp_menu->is_valid ("bookmark_menu.goto_bookmark_menu")) {
-
-    Action goto_bookmark_action = mp_menu->action ("bookmark_menu.goto_bookmark_menu");
-    bool has_bookmarks = current_view () && current_view ()->bookmarks ().size () > 0;
-
-    if (has_bookmarks && edits_enabled ()) {
-
-      goto_bookmark_action.set_enabled (true);
-
-      QMenu *goto_bookmark_menu = goto_bookmark_action.qaction ()->menu ();
-      if (goto_bookmark_menu) {
-
-        goto_bookmark_menu->clear ();
-
-        if (current_view ()) {
-          const lay::BookmarkList &bookmarks = current_view ()->bookmarks ();
-          for (size_t i = 0; i < bookmarks.size (); ++i) {
-            QAction *action = goto_bookmark_menu->addAction (tl::to_qstring (bookmarks.name (i)));
-            action->setObjectName (tl::to_qstring (tl::sprintf ("bookmark_%d", i + 1)));
-            gtf::action_connect (action, SIGNAL (triggered ()), this, SLOT (goto_bookmark ()));
-            action->setData (QVariant (int (i)));
-          }
-        }
-
-      }
-
-    } else {
-      goto_bookmark_action.set_enabled (false);
-    }
-
-  }
-}
-
-void
-MainWindow::goto_bookmark ()
-{
-  BEGIN_PROTECTED
-
-  QAction *action = dynamic_cast <QAction *> (sender ());
-  tl_assert (action);
-  size_t id = size_t (action->data ().toInt ());
-  if (current_view () && current_view ()->bookmarks ().size () > id) {
-    current_view ()->goto_view (current_view ()->bookmarks ().state (id));
-  }
-
-  END_PROTECTED
-}
-
 void
 MainWindow::cm_goto_position ()
 {
@@ -2580,19 +2530,7 @@ MainWindow::cm_bookmark_view ()
   BEGIN_PROTECTED
 
   if (current_view ()) {
-    while (true) {
-      bool ok = false;
-      QString text = QInputDialog::getText (this, QObject::tr ("Enter Bookmark Name"), QObject::tr ("Bookmark name"),
-                                            QLineEdit::Normal, QString::null, &ok);
-      if (! ok) {
-        break;
-      } else if (text.isEmpty ()) {
-        QMessageBox::critical (this, QObject::tr ("Error"), QObject::tr ("Enter a name for the bookmark"));
-      } else {
-        current_view ()->bookmark_view (tl::to_string (text));
-        break;
-      }
-    }
+    current_view ()->bookmark_current_view ();
   }
 
   END_PROTECTED
@@ -3359,6 +3297,7 @@ MainWindow::select_view (int index)
     clear_current_pos ();
     edits_enabled_changed ();
     clear_message ();
+    menu_needs_update ();
 
     m_disable_tab_selected = dis;
 
@@ -3738,6 +3677,7 @@ MainWindow::clone_current_view ()
   connect (view, SIGNAL (title_changed ()), this, SLOT (view_title_changed ()));
   connect (view, SIGNAL (dirty_changed ()), this, SLOT (view_title_changed ()));
   connect (view, SIGNAL (edits_enabled_changed ()), this, SLOT (edits_enabled_changed ()));
+  connect (view, SIGNAL (menu_needs_update ()), this, SLOT (menu_needs_update ()));
   connect (view, SIGNAL (show_message (const std::string &, int)), this, SLOT (message (const std::string &, int)));
   connect (view, SIGNAL (current_pos_changed (double, double, bool)), this, SLOT (current_pos (double, double, bool)));
   connect (view, SIGNAL (clear_current_pos ()), this, SLOT (clear_current_pos ()));
@@ -4041,6 +3981,7 @@ MainWindow::close_view (int index)
 
         clear_current_pos ();
         edits_enabled_changed ();
+        menu_needs_update ();
         clear_message ();
 
         update_dock_widget_state ();
@@ -4184,10 +4125,7 @@ MainWindow::do_update_file_menu ()
     if (m_mru.size () > 0 && edits_enabled ()) {
 
       //  rebuild MRU menu
-      std::vector<std::string> items = mp_menu->items (mru_menu);
-      for (std::vector<std::string>::const_iterator i = items.begin (); i != items.end (); ++i) {
-          mp_menu->delete_item (mru_menu + "." + *i);
-      }
+      mp_menu->clear_menu (mru_menu);
 
       for (std::vector<std::pair<std::string, std::string> >::iterator mru = m_mru.end (); mru != m_mru.begin (); ) {
         --mru;
@@ -4349,6 +4287,7 @@ MainWindow::do_create_view ()
   connect (view, SIGNAL (title_changed ()), this, SLOT (view_title_changed ()));
   connect (view, SIGNAL (dirty_changed ()), this, SLOT (view_title_changed ()));
   connect (view, SIGNAL (edits_enabled_changed ()), this, SLOT (edits_enabled_changed ()));
+  connect (view, SIGNAL (menu_needs_update ()), this, SLOT (menu_needs_update ()));
   connect (view, SIGNAL (show_message (const std::string &, int)), this, SLOT (message (const std::string &, int)));
   connect (view, SIGNAL (current_pos_changed (double, double, bool)), this, SLOT (current_pos (double, double, bool)));
   connect (view, SIGNAL (clear_current_pos ()), this, SLOT (clear_current_pos ()));

src/lay/lay/layMainWindow.h

@@ -689,7 +689,6 @@ public slots:
   void tab_close_requested (int);
   void enable_all ();
   void disable_all ();
-  void goto_bookmark ();
   void open_recent ();
   void view_selected (int index);
   void view_title_changed ();
@@ -858,8 +857,9 @@ public slots:
 protected slots:
   void menu_changed ();
   void message_timer ();
-  void bookmark_menu_show ();
   void edits_enabled_changed ();
+  void menu_needs_update ();
+
   void file_changed_timer ();
   void file_changed (const QString &path);
   void file_removed (const QString &path);
@@ -874,7 +874,6 @@ private:
 
   //  Main menu
   AbstractMenu *mp_menu;
-  QMenu *mp_goto_bookmark_menu;
   QTabBar *mp_tab_bar;
   QToolBar *mp_tool_bar;
   QDockWidget *mp_navigator_dock_widget;

src/laybasic/laybasic/layAbstractMenu.cc

@@ -1354,6 +1354,17 @@ AbstractMenu::insert_menu (const std::string &path, const std::string &name, con
   insert_menu (path, name, create_action (title));
 }
 
+void
+AbstractMenu::clear_menu (const std::string &p)
+{
+  typedef std::vector<std::pair<AbstractMenuItem *, std::list<AbstractMenuItem>::iterator > > path_type;
+  path_type path = find_item (p);
+  if (! path.empty () && ! path.back ().second->children.empty ()) {
+    path.back ().second->children.clear ();
+    emit changed ();
+  }
+}
+
 void
 AbstractMenu::delete_item (const std::string &p)
 {

src/laybasic/laybasic/layAbstractMenu.h

@@ -784,6 +784,13 @@ public:
    */
   void insert_menu (const std::string &path, const std::string &name, const std::string &title); 
 
+  /**
+   *  @brief Deletes the children of the item with the given path
+   *
+   *  If the item does not exist or is not a menu, this method does nothing.
+   */
+  void clear_menu (const std::string &path);
+
   /**
    *  @brief Delete the item given by the path
    *

src/laybasic/laybasic/layLayoutView.cc

@@ -42,6 +42,7 @@
 #include "tlString.h"
 #include "tlLog.h"
 #include "tlAssert.h"
+#include "tlExceptions.h"
 #include "layLayoutView.h"
 #include "layViewOp.h"
 #include "layViewObject.h"
@@ -74,6 +75,7 @@
 #include "rdbMarkerBrowserDialog.h"
 #include "tlXMLParser.h"
 #include "gsi.h"
+#include "gtf.h"
 
 #include <limits>
 
@@ -772,6 +774,37 @@ LayoutView::init_menu (lay::AbstractMenu &menu)
   lay::HierarchyControlPanel::init_menu (menu);
 }
 
+void
+LayoutView::update_menu (lay::LayoutView *view, lay::AbstractMenu &menu)
+{
+  std::string bm_menu = "bookmark_menu.goto_bookmark_menu";
+
+  if (menu.is_valid (bm_menu)) {
+
+    menu.clear_menu (bm_menu);
+
+    Action goto_bookmark_action = menu.action (bm_menu);
+
+    if (view && view->bookmarks ().size () > 0) {
+
+      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)));
+        menu.insert_item (bm_menu + ".end", tl::sprintf ("bookmark_%d", i + 1), action);
+      }
+
+    } else {
+      goto_bookmark_action.set_enabled (false);
+    }
+
+  }
+}
+
 void
 LayoutView::set_drawing_workers (int workers)
 {
@@ -3650,6 +3683,24 @@ LayoutView::cancel ()
   clear_selection ();
 }
 
+void
+LayoutView::bookmark_current_view ()
+{
+  while (true) {
+    bool ok = false;
+    QString text = QInputDialog::getText (this, QObject::tr ("Enter Bookmark Name"), QObject::tr ("Bookmark name"),
+                                          QLineEdit::Normal, QString::null, &ok);
+    if (! ok) {
+      break;
+    } else if (text.isEmpty ()) {
+      QMessageBox::critical (this, QObject::tr ("Error"), QObject::tr ("Enter a name for the bookmark"));
+    } else {
+      bookmark_view (tl::to_string (text));
+      break;
+    }
+  }
+}
+
 void
 LayoutView::manage_bookmarks ()
 {
@@ -3663,6 +3714,7 @@ void
 LayoutView::bookmarks (const BookmarkList &b)
 {
   m_bookmarks = b;
+  emit menu_needs_update ();
 }
 
 void
@@ -3670,6 +3722,22 @@ LayoutView::bookmark_view (const std::string &name)
 {
   DisplayState state (box (), get_min_hier_levels (), get_max_hier_levels (), m_cellviews);
   m_bookmarks.add (name, state);
+  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

src/laybasic/laybasic/layLayoutView.h

@@ -1557,6 +1557,11 @@ public:
    */
   void bookmark_view (const std::string &name);
 
+  /**
+   *  @brief Asks for a bookmark name and bookmark the current view under this name
+   */
+  void bookmark_current_view ();
+
   /**
    *  @brief Show the bookmark management form
    */
@@ -1603,6 +1608,12 @@ public:
    */
   static void init_menu (lay::AbstractMenu &menu);
 
+  /**
+   *  @brief Updates the menu for the given view
+   *  If the view is 0, the menu shall be updated to reflect "no view active"
+   */
+  static void update_menu (lay::LayoutView *view, lay::AbstractMenu &menu);
+
   /**
    *  @brief Query the default mode
    */
@@ -2530,6 +2541,7 @@ public slots:
 
 private slots:
   void active_cellview_changed (int index);
+  void goto_bookmark ();
 
 signals:
   /**
@@ -2562,6 +2574,11 @@ signals:
    */
   void edits_enabled_changed ();
 
+  /**
+   *  @brief This signal is sent when the view wants to update the menu
+   */
+  void menu_needs_update ();
+
 protected:
   /**
    *  @brief Establish the view operations

src/laybasic/unit_tests/layAbstractMenu.cc

@@ -124,5 +124,14 @@ TEST(1)
 
   menu.delete_item ("n1.c2");
   EXPECT_EQ (menu_to_string (menu), "(n2)");
-}
 
+  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"));
+  EXPECT_EQ (menu_to_string (menu), "(n2,n1(n1.c1,n1.c2))");
+  menu.clear_menu ("n1");
+  EXPECT_EQ (menu_to_string (menu), "(n2,n1)");
+}