Improve registry visitation procedure so it doesn't require a project...

... though the specific visitations for Menu items (the only ones that exist
now) still do require a project, to construct the visitor.

src/Menus.cpp

@@ -110,18 +110,8 @@ ComputedItem::~ComputedItem() {}
 
 SingleItem::~SingleItem() {}
 
-GroupItem::GroupItem( const wxString &internalName, BaseItemPtrs &&items_ )
-: BaseItem{ internalName }, items{ std::move( items_ ) }
-{
-}
-void GroupItem::AppendOne( BaseItemPtr&& ptr )
-{
-   items.push_back( std::move( ptr ) );
-}
 GroupItem::~GroupItem() {}
 
-TransparentGroupItem::~TransparentGroupItem() {}
-
 Visitor::~Visitor(){}
 void Visitor::BeginGroup(GroupItem &, const Path &) {}
 void Visitor::EndGroup(GroupItem &, const Path &) {}
@@ -133,7 +123,8 @@ namespace MenuTable {
 
 MenuItem::MenuItem( const wxString &internalName,
    const TranslatableString &title_, BaseItemPtrs &&items_ )
-: GroupItem{ internalName, std::move( items_ ) }, title{ title_ }
+: ConcreteGroupItem< false, MenuVisitor >{
+   internalName, std::move( items_ ) }, title{ title_ }
 {
    wxASSERT( !title.empty() );
 }
@@ -141,7 +132,8 @@ MenuItem::~MenuItem() {}
 
 ConditionalGroupItem::ConditionalGroupItem(
    const wxString &internalName, Condition condition_, BaseItemPtrs &&items_ )
-: GroupItem{ internalName, std::move( items_ ) }, condition{ condition_ }
+: ConcreteGroupItem< false, MenuVisitor >{
+   internalName, std::move( items_ ) }, condition{ condition_ }
 {
 }
 ConditionalGroupItem::~ConditionalGroupItem() {}
@@ -203,15 +195,15 @@ struct CollectedItems
 // alternate as the entire tree is recursively visited.
 
 // forward declaration for mutually recursive functions
-void CollectItem( AudacityProject &project,
+void CollectItem( Registry::Visitor &visitor,
    CollectedItems &collection, BaseItem *Item );
-void CollectItems( AudacityProject &project,
+void CollectItems( Registry::Visitor &visitor,
    CollectedItems &collection, const BaseItemPtrs &items )
 {
    for ( auto &item : items )
-      CollectItem( project, collection, item.get() );
+      CollectItem( visitor, collection, item.get() );
 }
-void CollectItem( AudacityProject &project,
+void CollectItem( Registry::Visitor &visitor,
    CollectedItems &collection, BaseItem *pItem )
 {
    if (!pItem)
@@ -222,25 +214,25 @@ void CollectItem( AudacityProject &project,
        dynamic_cast<SharedItem*>( pItem )) {
       auto &delegate = pShared->ptr;
       // recursion
-      CollectItem( project, collection, delegate.get() );
+      CollectItem( visitor, collection, delegate.get() );
    }
    else
    if (const auto pComputed =
        dynamic_cast<ComputedItem*>( pItem )) {
-      auto result = pComputed->factory( project );
+      auto result = pComputed->factory( visitor );
       if (result) {
          // Guarantee long enough lifetime of the result
          collection.computedItems.push_back( result );
          // recursion
-         CollectItem( project, collection, result.get() );
+         CollectItem( visitor, collection, result.get() );
       }
    }
    else
    if (auto pGroup = dynamic_cast<GroupItem*>(pItem)) {
-      if (dynamic_cast<TransparentGroupItem*>(pItem) && pItem->name.empty())
+      if (pGroup->Transparent() && pItem->name.empty())
          // nameless grouping item is transparent to path calculations
          // recursion
-         CollectItems( project, collection, pGroup->items );
+         CollectItems( visitor, collection, pGroup->items );
       else
          // all other group items
          collection.items.push_back( pItem );
@@ -256,29 +248,26 @@ using Path = std::vector< Identifier >;
 
 // forward declaration for mutually recursive functions
 void VisitItem(
-   Registry::Visitor &visitor,
+   Registry::Visitor &visitor, CollectedItems &collection,
-   AudacityProject &project, CollectedItems &collection,
    Path &path, BaseItem *pItem );
 void VisitItems(
-   Registry::Visitor &visitor,
+   Registry::Visitor &visitor, CollectedItems &collection,
-   AudacityProject &project, CollectedItems &collection,
    Path &path, GroupItem *pGroup )
 {
    // Make a new collection for this subtree, sharing the memo cache
    CollectedItems newCollection{ {}, collection.computedItems };
 
    // Gather items at this level
-   CollectItems( project, newCollection, pGroup->items );
+   CollectItems( visitor, newCollection, pGroup->items );
 
    // Now visit them
    path.push_back( pGroup->name );
    for ( const auto &pSubItem : newCollection.items )
-      VisitItem( visitor, project, collection, path, pSubItem );
+      VisitItem( visitor, collection, path, pSubItem );
    path.pop_back();
 }
 void VisitItem(
-   Registry::Visitor &visitor,
+   Registry::Visitor &visitor, CollectedItems &collection,
-   AudacityProject &project, CollectedItems &collection,
    Path &path, BaseItem *pItem )
 {
    if (!pItem)
@@ -293,7 +282,7 @@ void VisitItem(
        dynamic_cast<GroupItem*>( pItem )) {
       visitor.BeginGroup( *pGroup, path );
       // recursion
-      VisitItems( visitor, project, collection, path, pGroup );
+      VisitItems( visitor, collection, path, pGroup );
       visitor.EndGroup( *pGroup, path );
    }
    else
@@ -304,14 +293,12 @@ void VisitItem(
 
 namespace Registry {
 
-void Visit(
+void Visit( Visitor &visitor,  BaseItem *pTopItem )
-   Visitor &visitor, AudacityProject &project,
-   BaseItem *pTopItem )
 {
    std::vector< BaseItemSharedPtr > computedItems;
    CollectedItems collection{ {}, computedItems };
    Path emptyPath;
-   VisitItem( visitor, project, collection, emptyPath, pTopItem );
+   VisitItem( visitor, collection, emptyPath, pTopItem );
 }
 
 }
@@ -362,10 +349,10 @@ static const auto menuTree = MenuTable::Items( MenuPathStart
 );
 
 namespace {
-struct MenuItemVisitor : Registry::Visitor
+struct MenuItemVisitor : MenuVisitor
 {
    MenuItemVisitor( AudacityProject &proj, CommandManager &man )
-      : project(proj), manager( man ) {}
+      : MenuVisitor(proj), manager( man ) {}
 
    void BeginGroup( GroupItem &item, const Path& ) override
    {
@@ -384,8 +371,8 @@ struct MenuItemVisitor : Registry::Visitor
          flags.push_back(flag);
       }
       else
-      if (const auto pGroup =
+      if (const auto pGroup = dynamic_cast<GroupItem*>( pItem )) {
-          dynamic_cast<TransparentGroupItem*>( pItem )) {
+         wxASSERT( pGroup->Transparent() );
       }
       else
          wxASSERT( false );
@@ -407,8 +394,8 @@ struct MenuItemVisitor : Registry::Visitor
          flags.pop_back();
       }
       else
-      if (const auto pGroup =
+      if (const auto pGroup = dynamic_cast<GroupItem*>( pItem )) {
-          dynamic_cast<TransparentGroupItem*>( pItem )) {
+         wxASSERT( pGroup->Transparent() );
       }
       else
          wxASSERT( false );
@@ -448,7 +435,6 @@ struct MenuItemVisitor : Registry::Visitor
          wxASSERT( false );
    }
 
-   AudacityProject &project;
    CommandManager &manager;
    std::vector<bool> flags;
 };
@@ -466,7 +452,7 @@ void MenuCreator::CreateMenusAndCommands(AudacityProject &project)
    wxASSERT(menubar);
 
    MenuItemVisitor visitor{ project, commandManager };
-   MenuManager::Visit( visitor, project );
+   MenuManager::Visit( visitor );
 
    GetProjectFrame( project ).SetMenuBar(menubar.release());
 
@@ -477,9 +463,9 @@ void MenuCreator::CreateMenusAndCommands(AudacityProject &project)
 #endif
 }
 
-void MenuManager::Visit( Registry::Visitor &visitor, AudacityProject &project )
+void MenuManager::Visit( MenuVisitor &visitor )
 {
-   Registry::Visit( visitor, project, menuTree.get() );
+   Registry::Visit( visitor, menuTree.get() );
 }
 
 // TODO: This surely belongs in CommandManager?

src/Menus.h

@@ -54,6 +54,8 @@ public:
    PluginID mLastEffect{};
 };
 
+struct MenuVisitor;
+
 class MenuManager final
    : public MenuCreator
    , public ClientData::Base
@@ -70,8 +72,7 @@ public:
    MenuManager &operator=( const MenuManager & ) PROHIBITED;
    ~MenuManager();
 
-   static void Visit(
+   static void Visit( MenuVisitor &visitor );
-      Registry::Visitor &visitor, AudacityProject &project );
 
    static void ModifyUndoMenuItems(AudacityProject &project);
    static void ModifyToolbarMenus(AudacityProject &project);

src/commands/CommandManager.h

@@ -427,6 +427,8 @@ namespace Registry {
    using BaseItemPtr = std::unique_ptr<BaseItem>;
    using BaseItemSharedPtr = std::shared_ptr<BaseItem>;
    using BaseItemPtrs = std::vector<BaseItemPtr>;
+
+   struct Visitor;
    
 
    // An item that delegates to another held in a shared pointer; this allows
@@ -454,15 +456,18 @@ namespace Registry {
       // Return type is a shared_ptr to let the function decide whether to
       // recycle the object or rebuild it on demand each time.
       // Return value from the factory may be null
-      using Factory = std::function< BaseItemSharedPtr( AudacityProject & ) >;
+      template< typename VisitorType >
+      using Factory = std::function< BaseItemSharedPtr( VisitorType & ) >;
 
-      explicit ComputedItem( const Factory &factory_ )
+      using DefaultVisitor = Visitor;
+
+      explicit ComputedItem( const Factory< DefaultVisitor > &factory_ )
          : BaseItem( wxEmptyString )
          , factory{ factory_ }
       {}
       ~ComputedItem() override;
 
-      Factory factory;
+      Factory< DefaultVisitor > factory;
    };
 
    // Common abstract base class for items that are not groups
@@ -473,17 +478,31 @@ namespace Registry {
 
    // Common abstract base class for items that group other items
    struct GroupItem : BaseItem {
+      using BaseItem::BaseItem;
+
       // Construction from an internal name and a previously built-up
       // vector of pointers
-      GroupItem( const wxString &internalName, BaseItemPtrs &&items_ );
+      GroupItem( const wxString &internalName, BaseItemPtrs &&items_ )
-      // In-line, variadic constructor that doesn't require building a vector
+         : BaseItem{ internalName }, items{ std::move( items_ ) }
-      template< typename... Args >
+      {}
-         GroupItem( const wxString &internalName, Args&&... args )
-         : BaseItem( internalName )
-         { Append( std::forward< Args >( args )... ); }
       ~GroupItem() override = 0;
 
+      // Whether the item is non-significant for path naming
+      // when it also has an empty name
+      virtual bool Transparent() const = 0;
+
       BaseItemPtrs items;
+   };
+   
+   // GroupItem adding variadic constructor conveniences
+   template< typename VisitorType = ComputedItem::DefaultVisitor >
+   struct InlineGroupItem : GroupItem {
+      using GroupItem::GroupItem;
+      // In-line, variadic constructor that doesn't require building a vector
+      template< typename... Args >
+         InlineGroupItem( const wxString &internalName, Args&&... args )
+         : GroupItem( internalName )
+         { Append( std::forward< Args >( args )... ); }
 
    private:
       // nullary overload grounds the recursion
@@ -502,27 +521,46 @@ namespace Registry {
          };
 
       // Move one unique_ptr to an item into our array
-      void AppendOne( BaseItemPtr&& ptr );
+      void AppendOne( BaseItemPtr&& ptr )
+      {
+         items.push_back( std::move( ptr ) );
+      }
       // This overload allows a lambda or function pointer in the variadic
       // argument lists without any other syntactic wrapping, and also
       // allows implicit conversions to type Factory.
       // (Thus, a lambda can return a unique_ptr<BaseItem> rvalue even though
       // Factory's return type is shared_ptr, and the needed conversion is
       // appled implicitly.)
-      void AppendOne( const ComputedItem::Factory &factory )
+      void AppendOne( const ComputedItem::Factory<VisitorType> &factory )
-      { AppendOne( std::make_unique<ComputedItem>( factory ) ); }
+      {
+         auto adaptedFactory = [factory]( Registry::Visitor &visitor ){
+            return factory( dynamic_cast< VisitorType& >( visitor ) );
+         };
+         AppendOne( std::make_unique<ComputedItem>( adaptedFactory ) );
+      }
       // This overload lets you supply a shared pointer to an item, directly
       template<typename Subtype>
       void AppendOne( const std::shared_ptr<Subtype> &ptr )
       { AppendOne( std::make_unique<SharedItem>(ptr) ); }
    };
 
+   // Inline group item also specifying transparency
+   template< bool transparent,
+      typename VisitorType = ComputedItem::DefaultVisitor >
+   struct ConcreteGroupItem : InlineGroupItem< VisitorType >
+   {
+      using InlineGroupItem< VisitorType >::InlineGroupItem;
+      ~ConcreteGroupItem() {}
+      bool Transparent() const override { return transparent; }
+   };
+
    // Concrete subclass of GroupItem that adds nothing else
    // TransparentGroupItem with an empty name is transparent to item path calculations
-   struct TransparentGroupItem final : GroupItem
+   template< typename VisitorType = ComputedItem::DefaultVisitor >
+   struct TransparentGroupItem final : ConcreteGroupItem< true, VisitorType >
    {
-      using GroupItem::GroupItem;
+      using ConcreteGroupItem< true, VisitorType >::ConcreteGroupItem;
-      ~TransparentGroupItem() override;
+      ~TransparentGroupItem() override {}
    };
 
    // Define actions to be done in Visit.
@@ -539,16 +577,23 @@ namespace Registry {
    };
 
    // Top-down visitation of all items and groups in a tree
-   void Visit(
+   void Visit( Visitor &visitor, BaseItem *pTopItem );
-      Visitor &visitor, AudacityProject &project, BaseItem *pTopItem );
 }
 
+struct MenuVisitor : Registry::Visitor
+{
+   explicit MenuVisitor( AudacityProject &p ) : project{ p } {}
+   operator AudacityProject & () const { return project; }
+
+   AudacityProject &project;
+};
+
 // Define items that populate tables that specifically describe menu trees
 namespace MenuTable {
    using namespace Registry;
 
    // Describes a main menu in the toolbar, or a sub-menu
-   struct MenuItem final : GroupItem {
+   struct MenuItem final : ConcreteGroupItem< false, MenuVisitor > {
       // Construction from an internal name and a previously built-up
       // vector of pointers
       MenuItem( const wxString &internalName,
@@ -557,7 +602,8 @@ namespace MenuTable {
       template< typename... Args >
          MenuItem( const wxString &internalName,
             const TranslatableString &title_, Args&&... args )
-            : GroupItem{ internalName, std::forward<Args>(args)... }
+            : ConcreteGroupItem< false, MenuVisitor >{
+               internalName, std::forward<Args>(args)... }
             , title{ title_ }
          {}
       ~MenuItem() override;
@@ -567,7 +613,7 @@ namespace MenuTable {
 
    // Collects other items that are conditionally shown or hidden, but are
    // always available to macro programming
-   struct ConditionalGroupItem final : GroupItem {
+   struct ConditionalGroupItem final : ConcreteGroupItem< false, MenuVisitor > {
       using Condition = std::function< bool() >;
 
       // Construction from an internal name and a previously built-up
@@ -578,7 +624,8 @@ namespace MenuTable {
       template< typename... Args >
          ConditionalGroupItem( const wxString &internalName,
             Condition condition_, Args&&... args )
-            : GroupItem{ internalName, std::forward<Args>(args)... }
+            : ConcreteGroupItem< false, MenuVisitor >{
+               internalName, std::forward<Args>(args)... }
             , condition{ condition_ }
          {}
       ~ConditionalGroupItem() override;
@@ -713,10 +760,10 @@ namespace MenuTable {
    // in identification of items by path.  Otherwise try to keep the name
    // stable across Audacity versions.
    template< typename... Args >
-   inline std::unique_ptr<TransparentGroupItem> Items(
+   inline std::unique_ptr<TransparentGroupItem< MenuVisitor > > Items(
       const wxString &internalName, Args&&... args )
-         { return std::make_unique<TransparentGroupItem>( internalName,
+         { return std::make_unique<TransparentGroupItem< MenuVisitor > >(
-            std::forward<Args>(args)... ); }
+            internalName, std::forward<Args>(args)... ); }
 
    // Menu items can be constructed two ways, as for group items
    // Items will appear in a main toolbar menu or in a sub-menu.

src/menus/HelpMenus.cpp

@@ -380,8 +380,10 @@ void OnMenuTree(const CommandContext &context)
    auto &project = context.project;
    
    using namespace MenuTable;
-   struct MyVisitor : Visitor
+   struct MyVisitor : MenuVisitor
    {
+      using MenuVisitor::MenuVisitor;
+
       enum : unsigned { TAB = 3 };
       void BeginGroup( GroupItem &item, const Path& ) override
       {
@@ -415,9 +417,9 @@ void OnMenuTree(const CommandContext &context)
       unsigned level{};
       wxString indentation;
       wxString info;
-   } visitor;
+   } visitor{ project };
 
-   MenuManager::Visit( visitor, project );
+   MenuManager::Visit( visitor );
 
    ShowDiagnostics( project, visitor.info,
       XO("Menu Tree"), wxT("menutree.txt"), true );