Merge pull request #894 from Paul-Licameli/Bug2778

Bug 2778 - In German in the Erzeugen menu (Generator menu) "Silence..." is untranslated

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;
       }
    }
@@ -1973,7 +1978,7 @@ void PluginManager::Load()
          // These particular config edits were originally written to fix Bug 1914.
          if (regver <= "1.0") {
             // Nyquist prompt is a built-in that has moved to the tools menu.
-            if (effectSymbol == "Nyquist Prompt") {
+            if (effectSymbol == NYQUIST_PROMPT_ID) {
                registry.Write(KEY_EFFECTTYPE, "Tool");
             // Old version of SDE was in Analyze menu.  Now it is in Tools.
             // We don't want both the old and the new.
@@ -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);
@@ -324,4 +325,11 @@ private:
    friend class PluginRegistrationDialog;
 };
 
+// Defining these special names in the low-level PluginManager.h
+// is unfortunate
+// Internal name should be stable across versions
+#define NYQUIST_PROMPT_ID wxT("Nyquist Prompt")
+// User-visible name might change in later versions
+#define NYQUIST_PROMPT_NAME XO("Nyquist Prompt")
+
 #endif /* __AUDACITY_PLUGINMANAGER_H__ */

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/Effect.h

@@ -57,7 +57,6 @@ class WaveTrack;
  name into another alphabet.  */
 #define NYQUISTEFFECTS_FAMILY ( EffectFamilySymbol{ XO("Nyquist") } )
 
-#define NYQUIST_PROMPT_ID wxT("Nyquist Prompt")
 #define NYQUIST_WORKER_ID wxT("Nyquist Worker")
 
 // TODO:  Apr-06-2015

src/effects/EffectUI.cpp

@@ -1982,7 +1982,7 @@ wxDialog *EffectUI::DialogFactory( wxWindow &parent, EffectHostInterface *pHost,
             menuManager.mLastTool = ID;
             menuManager.mLastToolRegistration = MenuCreator::repeattypeplugin;
             menuManager.mRepeatToolFlags = EffectManager::kConfigured;
-            if (shortDesc == XO("Nyquist Prompt")) {
+            if (shortDesc == NYQUIST_PROMPT_NAME) {
                menuManager.mRepeatToolFlags = EffectManager::kRepeatNyquistPrompt;  //Nyquist Prompt is not configured
             }
             break;

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;
@@ -147,12 +147,12 @@ bool BuiltinEffectsModule::AutoRegisterPlugins(PluginManagerInterface & pm)
 {
    TranslatableString ignoredErrMsg;
    for (const auto &pair : mEffects)
+   {
+      const auto &path = pair.first;
+      if (!pm.IsPluginRegistered(path, &pair.second->name.Msgid()))
       {
          if ( pair.second->excluded )
             continue;
-      const auto &path = pair.first;
-      if (!pm.IsPluginRegistered(path))
-      {
          // No checking of error ?
          DiscoverPluginsAtPath(path, ignoredErrMsg,
             PluginManagerInterface::DefaultRegistrationCallback);

src/effects/nyquist/LoadNyquist.cpp

@@ -17,6 +17,7 @@
 #include "Nyquist.h"
 
 #include "../../FileNames.h"
+#include "../../PluginManager.h"
 
 // ============================================================================
 // List of effects that ship with Audacity.  These will be autoregistered.
@@ -183,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,
@@ -196,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

@@ -57,6 +57,7 @@ effects from this one class.
 #include "../../NoteTrack.h"
 #include "../../TimeTrack.h"
 #include "../../prefs/SpectrogramSettings.h"
+#include "../../PluginManager.h"
 #include "../../Project.h"
 #include "../../ProjectSettings.h"
 #include "../../ShuttleGetDefinition.h"
@@ -164,7 +165,7 @@ NyquistEffect::NyquistEffect(const wxString &fName)
 
    // Interactive Nyquist
    if (fName == NYQUIST_PROMPT_ID) {
-      mName = XO("Nyquist Prompt");
+      mName = NYQUIST_PROMPT_NAME;
       mType = EffectTypeTool;
       mIsTool = true;
       mPromptName = mName;
@@ -209,7 +210,7 @@ PluginPath NyquistEffect::GetPath()
 ComponentInterfaceSymbol NyquistEffect::GetSymbol()
 {
    if (mIsPrompt)
-      return XO("Nyquist Prompt");
+      return { NYQUIST_PROMPT_ID, NYQUIST_PROMPT_NAME };
 
    return mName;
 }
@@ -1793,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 &&
@@ -2051,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

src/menus/PluginMenus.cpp

@@ -316,7 +316,7 @@ MenuTable::BaseItemPtrs PopulateEffectsMenu(
          && (plug->GetSymbol() !=
                ComponentInterfaceSymbol("Nyquist Effects Prompt"))
          && (plug->GetSymbol() != ComponentInterfaceSymbol("Nyquist Tools Prompt"))
-         && (plug->GetSymbol() != ComponentInterfaceSymbol("Nyquist Prompt"))
+         && (plug->GetSymbol() != ComponentInterfaceSymbol(NYQUIST_PROMPT_ID))
 #endif
          )
          defplugs.push_back(plug);