Bugs 2778, 2339, Issue 887: Translation of some effect names...

... Those for which the internal name and the user-visible English names differ,
or that needed disambiguating context for i18n,  were always shown as English
in the menus.

Silence and Filter Curve were the only two examples. There are others.

include/audacity/PluginInterface.h

@@ -60,7 +60,15 @@ public:
    static const PluginID &AudacityCommandRegistrationCallback(
       ModuleInterface *provider, ComponentInterface *ident );
 
-   virtual bool IsPluginRegistered(const PluginPath & path) = 0;
+   //! Was the plugin registry already populated for a path (maybe from loading the config file)?
+   /*!
+   @param path an identifier for the plug-in with meaning defined by provider; not always a file path
+   @param pName if supplied, a correction for the user visible name associated with the plug-in, if it is
+    registered already.  (Needed because the configuration file only stores an internal name.)
+    */
+   virtual bool IsPluginRegistered(
+      const PluginPath & path,
+      const TranslatableString *pName = nullptr) = 0;
 
    virtual const PluginID & RegisterPlugin(ModuleInterface *module) = 0;
    virtual const PluginID & RegisterPlugin(ModuleInterface *provider, EffectDefinitionInterface *effect, int type) = 0;

src/PluginManager.cpp

@@ -1435,12 +1435,17 @@ RegistryPath PluginManager::GetPluginEnabledSetting(
    }
 }
 
-bool PluginManager::IsPluginRegistered(const PluginPath &path)
+bool PluginManager::IsPluginRegistered(
+   const PluginPath &path, const TranslatableString *pName)
 {
    for (PluginMap::iterator iter = mPlugins.begin(); iter != mPlugins.end(); ++iter)
    {
-      if (iter->second.GetPath() == path)
+      auto &descriptor = iter->second;
+      if (descriptor.GetPath() == path)
       {
+         if (pName)
+            descriptor.SetSymbol(
+               { descriptor.GetSymbol().Internal(), *pName });
          return true;
       }
    }
@@ -2116,6 +2121,10 @@ void PluginManager::LoadGroup(FileConfig *pRegistry, PluginType type)
       // effects.
       if (!pRegistry->Read(KEY_SYMBOL, &strVal))
          continue;
+
+      // Related to Bug2778: config file only remembered an internal name,
+      // so this symbol may not contain the correct TranslatableString.
+      // See calls to IsPluginRegistered which can correct that.
       plug.SetSymbol(strVal);
 
       // Get the version and bypass group if not found
@@ -2315,6 +2324,8 @@ void PluginManager::SaveGroup(FileConfig *pRegistry, PluginType type)
       pRegistry->SetPath(REGROOT + group + wxCONFIG_PATH_SEPARATOR + ConvertID(plug.GetID()));
 
       pRegistry->Write(KEY_PATH, plug.GetPath());
+
+      // See comments with the corresponding load-time call to SetSymbol().
       pRegistry->Write(KEY_SYMBOL, plug.GetSymbol().Internal());
 
       // PRL:  Writing KEY_NAME which is no longer read, but older Audacity

src/PluginManager.h

@@ -177,7 +177,8 @@ public:
 
    // PluginManagerInterface implementation
 
-   bool IsPluginRegistered(const PluginPath &path) override;
+   bool IsPluginRegistered(
+      const PluginPath &path, const TranslatableString *pSymbol) override;
 
    const PluginID & RegisterPlugin(ModuleInterface *module) override;
    const PluginID & RegisterPlugin(ModuleInterface *provider, ComponentInterface *command);

src/commands/LoadCommands.cpp

@@ -24,7 +24,7 @@ bool sInitialized = false;
 }
 
 struct BuiltinCommandsModule::Entry {
-   wxString name;
+   ComponentInterfaceSymbol name;
    Factory factory;
 
    using Entries = std::vector< Entry >;
@@ -39,7 +39,7 @@ void BuiltinCommandsModule::DoRegistration(
    const ComponentInterfaceSymbol &name, const Factory &factory )
 {
    wxASSERT( !sInitialized );
-   Entry::Registry().emplace_back( Entry{ name.Internal(), factory } );
+   Entry::Registry().emplace_back( Entry{ name, factory } );
 }
 
 // ============================================================================
@@ -119,7 +119,8 @@ TranslatableString BuiltinCommandsModule::GetDescription()
 bool BuiltinCommandsModule::Initialize()
 {
    for ( const auto &entry : Entry::Registry() ) {
-      auto path = wxString(BUILTIN_GENERIC_COMMAND_PREFIX) + entry.name;
+      auto path = wxString(BUILTIN_GENERIC_COMMAND_PREFIX)
+         + entry.name.Internal();
       mCommands[ path ] = &entry;
    }
    sInitialized = true;
@@ -150,7 +151,7 @@ bool BuiltinCommandsModule::AutoRegisterPlugins(PluginManagerInterface & pm)
    for (const auto &pair : mCommands)
    {
       const auto &path = pair.first;
-      if (!pm.IsPluginRegistered(path))
+      if (!pm.IsPluginRegistered(path, &pair.second->name.Msgid()))
       {
          // No checking of error ?
          // Uses Generic Registration, not Default.

src/effects/LoadEffects.cpp

@@ -21,7 +21,7 @@
 static bool sInitialized = false;
 
 struct BuiltinEffectsModule::Entry {
-   wxString name;
+   ComponentInterfaceSymbol name;
    BuiltinEffectsModule::Factory factory;
    bool excluded;
 
@@ -37,7 +37,7 @@ void BuiltinEffectsModule::DoRegistration(
    const ComponentInterfaceSymbol &name, const Factory &factory, bool excluded )
 {
    wxASSERT( !sInitialized );
-   Entry::Registry().emplace_back( Entry{ name.Internal(), factory, excluded } );
+   Entry::Registry().emplace_back( Entry{ name, factory, excluded } );
 }
 
 // ============================================================================
@@ -117,7 +117,7 @@ TranslatableString BuiltinEffectsModule::GetDescription()
 bool BuiltinEffectsModule::Initialize()
 {
    for ( const auto &entry : Entry::Registry() ) {
-      auto path = wxString(BUILTIN_EFFECT_PREFIX) + entry.name;
+      auto path = wxString(BUILTIN_EFFECT_PREFIX) + entry.name.Internal();
       mEffects[ path ] = &entry;
    }
    sInitialized = true;
@@ -148,11 +148,11 @@ bool BuiltinEffectsModule::AutoRegisterPlugins(PluginManagerInterface & pm)
    TranslatableString ignoredErrMsg;
    for (const auto &pair : mEffects)
    {
-      if ( pair.second->excluded )
-         continue;
       const auto &path = pair.first;
-      if (!pm.IsPluginRegistered(path))
+      if (!pm.IsPluginRegistered(path, &pair.second->name.Msgid()))
       {
+         if ( pair.second->excluded )
+            continue;
          // No checking of error ?
          DiscoverPluginsAtPath(path, ignoredErrMsg,
             PluginManagerInterface::DefaultRegistrationCallback);

src/effects/nyquist/LoadNyquist.cpp

@@ -184,7 +184,8 @@ bool NyquistEffectsModule::AutoRegisterPlugins(PluginManagerInterface & pm)
    FilePaths files;
    TranslatableString ignoredErrMsg;
 
-   if (!pm.IsPluginRegistered(NYQUIST_PROMPT_ID))
+   auto name = NYQUIST_PROMPT_NAME;
+   if (!pm.IsPluginRegistered(NYQUIST_PROMPT_ID, &name))
    {
       // No checking of error ?
       DiscoverPluginsAtPath(NYQUIST_PROMPT_ID, ignoredErrMsg,
@@ -197,6 +198,19 @@ bool NyquistEffectsModule::AutoRegisterPlugins(PluginManagerInterface & pm)
       pm.FindFilesInPathList(kShippedEffects[i], pathList, files);
       for (size_t j = 0, cnt = files.size(); j < cnt; j++)
       {
+         /*
+           TODO: Currently the names of Nyquist plug-ins cannot have
+          context specific translations or internal names different from
+          the visible English names.
+   
+          This makes it unnecessary to pass a second argument to
+          IsPluginRegistered for correction of the registry (as is needed
+          in the case of built-in effects).
+
+          If it does become necessary in the future, we will need to open the
+          .ny files to access their $name lines so that this argument could
+          be supplied.
+          */
          if (!pm.IsPluginRegistered(files[j]))
          {
             // No checking of error ?

src/effects/nyquist/Nyquist.cpp

@@ -1794,6 +1794,7 @@ TranslatableString NyquistEffect::UnQuoteMsgid(const wxString &s, bool allowPare
    if (len >= 2 && s[0] == wxT('\"') && s[len - 1] == wxT('\"')) {
       auto unquoted = s.Mid(1, len - 2);
       // Sorry, no context strings, yet
+      // (See also comments in NyquistEffectsModule::AutoRegisterPlugins)
       return TranslatableString{ unquoted, {} };
    }
    else if (allowParens &&
@@ -2052,6 +2053,9 @@ bool NyquistEffect::Parse(
    }
 
    if (len >= 2 && tokens[0] == wxT("name")) {
+      // Names do not yet support context strings for translations, or
+      // internal names distinct from visible English names.
+      // (See also comments in NyquistEffectsModule::AutoRegisterPlugins)
       auto name = UnQuote(tokens[1]);
       // Strip ... from name if it's present, perhaps in third party plug-ins
       // Menu system puts ... back if there are any controls