Bug2302: Don't make separators at bottoms of menus on Win & Linux...

... Fix it by demoting the logic for separators from the menu tree diagnostic
into the common MenuVisitor class, then reusing that.

src/Menus.cpp

@@ -120,6 +120,81 @@ void Visitor::Visit(SingleItem &, const Path &) {}
 
 }
 
+void MenuVisitor::BeginGroup( Registry::GroupItem &item, const Path &path )
+{
+   bool isMenu = false;
+   auto pItem = &item;
+   if ( pItem->Transparent() ) {
+   }
+   else if ( dynamic_cast<MenuTable::MenuSection*>( pItem ) ) {
+      if ( !needSeparator.empty() )
+         needSeparator.back() = true;
+   }
+   else {
+      isMenu = true;
+      MaybeDoSeparator();
+   }
+
+   DoBeginGroup( item, path );
+
+   if ( isMenu ) {
+      needSeparator.push_back( false );
+      firstItem.push_back( true );
+   }
+}
+
+void MenuVisitor::EndGroup( Registry::GroupItem &item, const Path &path )
+{
+   auto pItem = &item;
+   if ( pItem->Transparent() ) {
+   }
+   else if ( dynamic_cast<MenuTable::MenuSection*>( pItem ) ) {
+      if ( !needSeparator.empty() )
+         needSeparator.back() = true;
+   }
+   else {
+      firstItem.pop_back();
+      needSeparator.pop_back();
+   }
+
+   DoEndGroup( item, path );
+}
+
+void MenuVisitor::Visit( Registry::SingleItem &item, const Path &path )
+{
+   MaybeDoSeparator();
+   DoVisit( item, path );
+}
+
+void MenuVisitor::MaybeDoSeparator()
+{
+   bool separate = false;
+   if ( !needSeparator.empty() ) {
+      separate = needSeparator.back() && !firstItem.back();
+      needSeparator.back() = false;
+      firstItem.back() = false;
+   }
+
+   if ( separate )
+      DoSeparator();
+}
+
+void MenuVisitor::DoBeginGroup( Registry::GroupItem &, const Path & )
+{
+}
+
+void MenuVisitor::DoEndGroup( Registry::GroupItem &, const Path & )
+{
+}
+
+void MenuVisitor::DoVisit( Registry::SingleItem &, const Path & )
+{
+}
+
+void MenuVisitor::DoSeparator()
+{
+}
+
 namespace MenuTable {
 
 MenuItem::MenuItem( const wxString &internalName,
@@ -988,7 +1063,7 @@ struct MenuItemVisitor : MenuVisitor
    MenuItemVisitor( AudacityProject &proj, CommandManager &man )
       : MenuVisitor(proj), manager( man ) {}
 
-   void BeginGroup( GroupItem &item, const Path& ) override
+   void DoBeginGroup( GroupItem &item, const Path& ) override
    {
       auto pItem = &item;
       if (const auto pMenu =
@@ -1009,13 +1084,12 @@ struct MenuItemVisitor : MenuVisitor
       }
       else
       if ( const auto pGroup = dynamic_cast<MenuSection*>( pItem ) ) {
-         manager.AddSeparator();
       }
       else
          wxASSERT( false );
    }
 
-   void EndGroup( GroupItem &item, const Path& ) override
+   void DoEndGroup( GroupItem &item, const Path& ) override
    {
       auto pItem = &item;
       if (const auto pMenu =
@@ -1035,13 +1109,12 @@ struct MenuItemVisitor : MenuVisitor
       }
       else
       if ( const auto pGroup = dynamic_cast<MenuSection*>( pItem ) ) {
-         manager.AddSeparator();
       }
       else
          wxASSERT( false );
    }
 
-   void Visit( SingleItem &item, const Path& ) override
+   void DoVisit( SingleItem &item, const Path& ) override
    {
       const auto pCurrentMenu = manager.CurrentMenu();
       if ( !pCurrentMenu ) {
@@ -1077,6 +1150,11 @@ struct MenuItemVisitor : MenuVisitor
          wxASSERT( false );
    }
 
+   void DoSeparator() override
+   {
+      manager.AddSeparator();
+   }
+
    CommandManager &manager;
    std::vector<bool> flags;
 };

src/commands/CommandManager.h

@@ -670,7 +670,23 @@ struct MenuVisitor : Registry::Visitor
    explicit MenuVisitor( AudacityProject &p ) : project{ p } {}
    operator AudacityProject & () const { return project; }
 
+   // final overrides
+   void BeginGroup( Registry::GroupItem &item, const Path &path ) final;
+   void EndGroup( Registry::GroupItem &item, const Path& ) final;
+   void Visit( Registry::SingleItem &item, const Path &path ) final;
+
+   // added virtuals
+   virtual void DoBeginGroup( Registry::GroupItem &item, const Path &path );
+   virtual void DoEndGroup( Registry::GroupItem &item, const Path &path );
+   virtual void DoVisit( Registry::SingleItem &item, const Path &path );
+   virtual void DoSeparator();
+
    AudacityProject &project;
+
+private:
+   void MaybeDoSeparator();
+   std::vector<bool> firstItem;
+   std::vector<bool> needSeparator;
 };
 
 // Define items that populate tables that specifically describe menu trees

src/menus/HelpMenus.cpp

@@ -385,70 +385,38 @@ void OnMenuTree(const CommandContext &context)
       using MenuVisitor::MenuVisitor;
 
       enum : unsigned { TAB = 3 };
-      void BeginGroup( GroupItem &item, const Path& ) override
+      void DoBeginGroup( GroupItem &item, const Path& ) override
       {
-         auto pItem = &item;
+         if ( dynamic_cast<MenuItem*>( &item ) ) {
-         if ( pItem->Transparent() ) {
-         }
-         else if ( const auto pGroup = dynamic_cast<MenuSection*>( pItem ) ) {
-            if ( !needSeparator.empty() )
-               needSeparator.back() = true;
-         }
-         else {
-            MaybeEmitSeparator();
             Indent();
             // using GET for alpha only diagnostic tool
             info += item.name.GET();
             Return();
             indentation = wxString{ ' ', TAB * ++level };
-            needSeparator.push_back( false );
-            firstItem.push_back( true );
          }
       }
 
-      void EndGroup( GroupItem &item, const Path& ) override
+      void DoEndGroup( GroupItem &item, const Path& ) override
       {
-         auto pItem = &item;
+         if ( dynamic_cast<MenuItem*>( &item ) )
-         if ( pItem->Transparent() ) {
-         }
-         else if ( const auto pGroup = dynamic_cast<MenuSection*>( pItem ) ) {
-            if ( !needSeparator.empty() )
-               needSeparator.back() = true;
-         }
-         else {
-            firstItem.pop_back();
-            needSeparator.pop_back();
             indentation = wxString{ ' ', TAB * --level };
       }
-      }
 
-      void Visit( SingleItem &item, const Path& ) override
+      void DoVisit( SingleItem &item, const Path& ) override
       {
-         MaybeEmitSeparator();
-
          // using GET for alpha only diagnostic tool
          Indent();
          info += item.name.GET();
          Return();
       }
 
-      void MaybeEmitSeparator()
+      void DoSeparator() override
       {
          static const wxString separatorName{ '=', 20 };
-
-         bool separate = false;
-         if ( !needSeparator.empty() ) {
-            separate = needSeparator.back() && !firstItem.back();
-            needSeparator.back() = false;
-            firstItem.back() = false;
-         }
-
-         if ( separate ) {
          Indent();
          info += separatorName;
          Return();
       }
-      }
 
       void Indent() { info += indentation; }
       void Return() { info += '\n'; }
@@ -456,9 +424,6 @@ void OnMenuTree(const CommandContext &context)
       unsigned level{};
       wxString indentation;
       wxString info;
-
-      std::vector<bool> firstItem;
-      std::vector<bool> needSeparator;
    } visitor{ project };
 
    MenuManager::Visit( visitor );