CommandManager keys by CommandId, not plain wxString

include/audacity/Types.h

@@ -286,8 +286,11 @@ using FileExtensions = wxArrayStringEx;
 using FilePath = wxString;
 using FilePaths = wxArrayStringEx;
 
-using CommandID = wxString;
+// Identifies a menu command or macro.
-using CommandIDs = std::vector< CommandID >;
+// Case-insensitive comparison
+struct CommandIdTag;
+using CommandID = TaggedIdentifier< CommandIdTag, false >;
+using CommandIDs = std::vector<CommandID>;
 
 // ----------------------------------------------------------------------------
 // A native 64-bit integer...used when referring to any number of samples

src/BatchCommandDialog.cpp

@@ -154,7 +154,11 @@ void MacroCommandDialog::OnChoice(wxCommandEvent & WXUNUSED(event))
 
 void MacroCommandDialog::OnOk(wxCommandEvent & WXUNUSED(event))
 {
-   mSelectedCommand = mInternalCommandName.Strip(wxString::both);
+   mSelectedCommand = mInternalCommandName
+      // .Strip(wxString::both) // PRL: used to do this, here only,
+         // but ultimately mSelectedCommand is looked up in the catalog without
+         // similar adjustment of whitespace in the comparison
+   ;
    mSelectedParameters = mParameters->GetValue().Strip(wxString::trailing);
    EndModal(true);
 }
@@ -195,9 +199,12 @@ void MacroCommandDialog::OnItemSelected(wxListEvent &event)
       params = em.GetDefaultPreset(ID);
    }
 
+   // using GET to expose a CommandID to the user!
    // Cryptic command and category.
    // Later we can put help information there, perhaps.
-   mDetails->SetValue( mInternalCommandName + "\r\n" + command.category  );
+   // Macro command details are one place that we do expose Identifier
+   // to (more sophisticated) users
+   mDetails->SetValue( mInternalCommandName.GET() + "\r\n" + command.category  );
    mParameters->SetValue(params);
 }
 
@@ -231,12 +238,15 @@ void MacroCommandDialog::SetCommandAndParams(const CommandID &Command, const wxS
 
    mInternalCommandName = Command;
    if (iter == mCatalog.end())
-      // Expose an internal name to the user in default of any friendly name
+      // uh oh, using GET to expose an internal name to the user!
-      // -- AVOID THIS!
+      // in default of any better friendly name
-      mCommand->SetValue( Command );
+      mCommand->SetValue( Command.GET() );
    else {
       mCommand->SetValue( iter->name.Translated() );
-      mDetails->SetValue( iter->name.Internal() + "\r\n" + iter->category  );
+      // using GET to expose a CommandID to the user!
+      // Macro command details are one place that we do expose Identifier
+      // to (more sophisticated) users
+      mDetails->SetValue( iter->name.Internal().GET() + "\r\n" + iter->category  );
       mChoices->SetItemState(iter - mCatalog.begin(),
                              wxLIST_STATE_SELECTED, wxLIST_STATE_SELECTED);
 

src/BatchCommands.cpp

@@ -235,7 +235,8 @@ bool MacroCommands::WriteMacro(const wxString & macro)
    // Copy over the commands
    int lines = mCommandMacro.size();
    for (int i = 0; i < lines; i++) {
-      tf.AddLine(mCommandMacro[i] + wxT(":") + mParamsMacro[ i ]);
+      // using GET to serialize macro definition to a text file
+      tf.AddLine(mCommandMacro[i].GET() + wxT(":") + mParamsMacro[ i ]);
    }
 
    // Write the macro
@@ -346,11 +347,17 @@ MacroCommandsCatalog::MacroCommandsCatalog( const AudacityProject *project )
             wxString squashed = label;
             squashed.Replace( " ", "" );
 
-            suffix = squashed.length() < wxMin( 18, mNames[i].length());
+            // uh oh, using GET for dubious comparison of (lengths of)
+            // user-visible name and internal CommandID!
+            // and doing this only for English locale!
+            suffix = squashed.length() < wxMin( 18, mNames[i].GET().length());
          }
 
          if( suffix )
-            label = label + " (" + mNames[i] + ")";
+            // uh oh, using GET to expose CommandID to the user, as a
+            // disambiguating suffix on a name, but this is only ever done if
+            // the locale is English!
+            label = label + " (" + mNames[i].GET() + ")";
 
          commands.push_back(
             {
@@ -759,11 +766,10 @@ bool MacroCommands::ApplyCommand( const wxString &friendlyCommand,
    CommandContext const * pContext)
 {
 
-   unsigned int i;
    // Test for a special command.
    // CLEANSPEECH remnant
-   for( i = 0; i < sizeof(SpecialCommands)/sizeof(*SpecialCommands); ++i ) {
+   for( size_t i = 0; i < WXSIZEOF( SpecialCommands ); ++i ) {
-      if( command.IsSameAs( SpecialCommands[i].second, false) )
+      if( command == SpecialCommands[i].second )
          return ApplySpecialCommand( i, friendlyCommand, command, params );
    }
    // end CLEANSPEECH remnant
@@ -860,7 +866,10 @@ bool MacroCommands::ApplyMacro(
       const auto &command = mCommandMacro[i];
       auto iter = catalog.ByCommandId(command);
       auto friendly = (iter == catalog.end())
-         ? command // Expose internal name to user, in default of a better one!
+         ?
+           // uh oh, using GET to expose an internal name to the user!
+           // in default of any better friendly name
+           command.GET()
          : iter->name.Translated();
       if (!ApplyCommandInBatchMode(friendly, command, mParamsMacro[i]) || mAbort)
          break;

src/BatchProcessDialog.cpp

@@ -739,9 +739,9 @@ void MacrosWindow::AddItem(const CommandID &Action, const wxString &Params)
    auto friendlyName = entry != mCatalog.end()
       ? entry->name.Translated()
       :
-         // Expose an internal name to the user in default of any friendly name
+         // uh oh, using GET to expose an internal name to the user!
-         // -- AVOID THIS!
+         // in default of any better friendly name
-        Action;
+        Action.GET();
 
    int i = mList->GetItemCount();
 

src/commands/CommandManager.cpp

@@ -996,7 +996,7 @@ CommandListEntry *CommandManager::NewIdentifier(const CommandID & nameIn,
       // the name in prefs is the category name plus the effect name.
       // This feature is not used for built-in effects.
       if (multi)
-         name = wxString::Format(wxT("%s_%s"), name, nameSuffix);
+         name = CommandID{ { name, nameSuffix }, wxT('_') };
 
       // wxMac 2.5 and higher will do special things with the
       // Preferences, Exit (Quit), and About menu items,
@@ -1052,9 +1052,11 @@ CommandListEntry *CommandManager::NewIdentifier(const CommandID & nameIn,
 
       // Key from preferences overrides the default key given
       gPrefs->SetPath(wxT("/NewKeys"));
-      if (gPrefs->HasEntry(entry->name)) {
+      // using GET to interpret CommandID as a config path component
+      const auto &path = entry->name.GET();
+      if (gPrefs->HasEntry(path)) {
          entry->key =
-            NormalizedKeyString{ gPrefs->ReadObject(entry->name, entry->key) };
+            NormalizedKeyString{ gPrefs->ReadObject(path, entry->key) };
       }
       gPrefs->SetPath(wxT("/"));
 
@@ -1075,11 +1077,14 @@ CommandListEntry *CommandManager::NewIdentifier(const CommandID & nameIn,
       if( prev->label != entry->label )
       {
          wxLogDebug(wxT("Command '%s' defined by '%s' and '%s'"),
-                    entry->name,
+                    // using GET in a log message for devs' eyes only
+                    entry->name.GET(),
                     prev->label.BeforeFirst(wxT('\t')),
                     entry->label.BeforeFirst(wxT('\t')));
          wxFAIL_MSG(wxString::Format(wxT("Command '%s' defined by '%s' and '%s'"),
-                    entry->name,
+                    // using GET in an assertion violation message for devs'
+                    // eyes only
+                    entry->name.GET(),
                     prev->label.BeforeFirst(wxT('\t')),
                     entry->label.BeforeFirst(wxT('\t'))));
       }
@@ -1199,8 +1204,9 @@ void CommandManager::Enable(CommandListEntry *entry, bool enabled)
          if (item) {
             item->Enable(enabled);
          } else {
+            // using GET in a log message for devs' eyes only
             wxLogDebug(wxT("Warning: Menu entry with id %i in %s not found"),
-                ID, (const wxChar*)entry->name);
+                ID, entry->name.GET());
          }
          } else {
             wxLogDebug(wxT("Warning: Menu entry with id %i not in hash"), ID);
@@ -1242,8 +1248,9 @@ bool CommandManager::GetEnabled(const CommandID &name)
 {
    CommandListEntry *entry = mCommandNameHash[name];
    if (!entry || !entry->menu) {
+      // using GET in a log message for devs' eyes only
       wxLogDebug(wxT("Warning: command doesn't exist: '%s'"),
-                 (const wxChar*)name);
+                 name.GET());
       return false;
    }
    return entry->enabled;
@@ -1584,7 +1591,11 @@ bool CommandManager::HandleTextualCommand(const CommandID & Str, const CommandCo
       if (!entry->multi)
       {
          // Testing against labelPrefix too allows us to call Nyquist functions by name.
-         if( Str.IsSameAs( entry->name, false ) || Str.IsSameAs( entry->labelPrefix, false ))
+         if( Str == entry->name ||
+            // PRL:  uh oh, mixing internal string (Str) with user-visible
+            // (labelPrefix, which was initialized from a user-visible
+            // sub-menu name)
+            Str == entry->labelPrefix )
          {
             return HandleCommandEntry( entry.get(), flags, mask);
          }
@@ -1592,7 +1603,7 @@ bool CommandManager::HandleTextualCommand(const CommandID & Str, const CommandCo
       else
       {
          // Handle multis too...
-         if( Str.IsSameAs( entry->name, false ) )
+         if( Str == entry->name )
          {
             return HandleCommandEntry( entry.get(), flags, mask);
          }
@@ -1612,7 +1623,7 @@ bool CommandManager::HandleTextualCommand(const CommandID & Str, const CommandCo
    const PluginDescriptor *plug = pm.GetFirstPlugin(PluginTypeEffect);
    while (plug)
    {
-      if (em.GetCommandIdentifier(plug->GetID()).IsSameAs(Str, false))
+      if (em.GetCommandIdentifier(plug->GetID()) == Str)
       {
          return PluginActions::DoEffect(
             plug->GetID(), context,

src/commands/CommandManager.h

@@ -94,7 +94,7 @@ using SubMenuList = std::vector < std::unique_ptr<SubMenuListEntry> >;
 using CommandList = std::vector<std::unique_ptr<CommandListEntry>>;
 
 using CommandKeyHash = std::unordered_map<NormalizedKeyString, CommandListEntry*>;
-using CommandNameHash = std::unordered_map<wxString, CommandListEntry*>;
+using CommandNameHash = std::unordered_map<CommandID, CommandListEntry*>;
 using CommandNumericIDHash = std::unordered_map<int, CommandListEntry*>;
 
 class AudacityProject;

src/commands/GetInfoCommand.cpp

@@ -673,7 +673,9 @@ void GetInfoCommand::ExploreMenu( const CommandContext &context, wxMenu * pMenu,
       context.AddItem( Label, "label" );
       context.AddItem( Accel, "accel" );
       if( !Name.empty() )
-         context.AddItem( Name, "id" );// It is called Scripting ID outside Audacity.
+         // using GET to expose CommandID in results of GetInfoCommand...
+         // PRL asks, is that all right?
+         context.AddItem( Name.GET(), "id" );// It is called Scripting ID outside Audacity.
       context.EndStruct();
 
       if (item->IsSubMenu()) {

src/commands/ScreenshotCommand.cpp

@@ -441,7 +441,8 @@ void ScreenshotCommand::CapturePreferences(
       const CommandContext projectContext( *pProject );
       if( !pMan->HandleTextualCommand( Command, projectContext, AlwaysEnabledFlag, AlwaysEnabledFlag ) )
       {
-         wxLogDebug("Command %s not found", Command );
+         // using GET in a log message for devs' eyes only
+         wxLogDebug("Command %s not found", Command.GET() );
       }
       // This sleep is not needed, but gives user a chance to see the
       // dialogs as they whizz by.

src/effects/EffectManager.cpp

@@ -250,7 +250,10 @@ void EffectManager::GetCommandDefinition(const PluginID & ID, const CommandConte
    // This is capturing the output context into the shuttle.
    ShuttleGetDefinition S(  *context.pOutput.get()->mStatusTarget.get() );
    S.StartStruct();
-   S.AddItem( GetCommandIdentifier( ID ), "id" );
+   // using GET to expose a CommandID to the user!
+   // Macro command details are one place that we do expose Identifier
+   // to (more sophisticated) users
+   S.AddItem( GetCommandIdentifier( ID ).GET(), "id" );
    S.AddItem( GetCommandName( ID ), "name" );
    if( bHasParams ){
       S.StartField( "params" );
@@ -260,6 +263,7 @@ void EffectManager::GetCommandDefinition(const PluginID & ID, const CommandConte
       S.EndField();
    }
    S.AddItem( GetCommandUrl( ID ), "url" );
+   // The tip is a translated string!
    S.AddItem( GetCommandTip( ID ), "tip" );
    S.EndStruct();
 }
@@ -960,7 +964,7 @@ const PluginID & EffectManager::GetEffectByIdentifier(const CommandID & strTarge
    const PluginDescriptor *plug = pm.GetFirstPlugin(PluginTypeEffect | PluginTypeAudacityCommand);
    while (plug)
    {
-      if (GetCommandIdentifier(plug->GetID()).IsSameAs(strTarget, false))
+      if (GetCommandIdentifier(plug->GetID()) == strTarget)
       {
          return plug->GetID();
       }

src/menus/PluginMenus.cpp

@@ -559,7 +559,8 @@ void OnManageGenerators(const CommandContext &context)
 
 void OnEffect(const CommandContext &context)
 {
-   DoEffect(context.parameter, context, 0);
+   // using GET to interpret parameter as a PluginID
+   DoEffect(context.parameter.GET(), context, 0);
 }
 
 void OnManageEffects(const CommandContext &context)
@@ -680,7 +681,8 @@ void OnApplyMacroDirectly(const CommandContext &context )
 
 void OnAudacityCommand(const CommandContext & ctx)
 {
-   wxLogDebug( "Command was: %s", ctx.parameter);
+   // using GET in a log message for devs' eyes only
+   wxLogDebug( "Command was: %s", ctx.parameter.GET());
    // Not configured, so prompt user.
    DoAudacityCommand(EffectManager::Get().GetEffectByIdentifier(ctx.parameter),
       ctx, kNone);

src/prefs/KeyConfigPrefs.cpp

@@ -657,7 +657,8 @@ bool KeyConfigPrefs::Commit()
    bool bFull = gPrefs->ReadBool(wxT("/GUI/Shortcuts/FullDefaults"), false);
    for (size_t i = 0; i < mNames.size(); i++) {
       const auto &dkey = bFull ? mDefaultKeys[i] : mStandardDefaultKeys[i];
-      wxString name = wxT("/NewKeys/") + mNames[i];
+      // using GET to interpret CommandID as a config path component
+      auto name = wxT("/NewKeys/") + mNames[i].GET();
       const auto &key = mNewKeys[i];
 
       if (gPrefs->HasEntry(name)) {

src/widgets/KeyView.cpp

@@ -252,7 +252,7 @@ KeyView::GetIndexByName(const CommandID & name) const
    // Search the nodes for the key
    for (int i = 0; i < cnt; i++)
    {
-      if (name.CmpNoCase(mNodes[i].name) == 0)
+      if (name == mNodes[i].name)
       {
          return mNodes[i].index;
       }