From cf948ece5285406fcee0f116a7fd7f6807c57feb Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 14 May 2021 13:02:51 -0400 Subject: [PATCH 1/2] Eliminate duplications of string literal "Nyquist Prompt" --- src/PluginManager.cpp | 2 +- src/PluginManager.h | 7 +++++++ src/effects/Effect.h | 1 - src/effects/EffectUI.cpp | 2 +- src/effects/nyquist/LoadNyquist.cpp | 1 + src/effects/nyquist/Nyquist.cpp | 5 +++-- src/menus/PluginMenus.cpp | 2 +- 7 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/PluginManager.cpp b/src/PluginManager.cpp index 18c20aac6..74f493748 100644 --- a/src/PluginManager.cpp +++ b/src/PluginManager.cpp @@ -1973,7 +1973,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. diff --git a/src/PluginManager.h b/src/PluginManager.h index 12a23d432..b9f87dc67 100644 --- a/src/PluginManager.h +++ b/src/PluginManager.h @@ -324,4 +324,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__ */ diff --git a/src/effects/Effect.h b/src/effects/Effect.h index 8eeb09a8d..4ac97f979 100644 --- a/src/effects/Effect.h +++ b/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 diff --git a/src/effects/EffectUI.cpp b/src/effects/EffectUI.cpp index 2d2651f8d..6f380f15b 100644 --- a/src/effects/EffectUI.cpp +++ b/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; diff --git a/src/effects/nyquist/LoadNyquist.cpp b/src/effects/nyquist/LoadNyquist.cpp index 0a10e7f2c..8634f3626 100644 --- a/src/effects/nyquist/LoadNyquist.cpp +++ b/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. diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index 46ab203a5..f670e405c 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/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; } diff --git a/src/menus/PluginMenus.cpp b/src/menus/PluginMenus.cpp index eb455834a..8eaeba3da 100644 --- a/src/menus/PluginMenus.cpp +++ b/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); From d7f643768c1a420aa1b07b64295ecf17b1ea2106 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Fri, 14 May 2021 13:10:48 -0400 Subject: [PATCH 2/2] 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 | 10 +++++++++- src/PluginManager.cpp | 15 +++++++++++++-- src/PluginManager.h | 3 ++- src/commands/LoadCommands.cpp | 9 +++++---- src/effects/LoadEffects.cpp | 12 ++++++------ src/effects/nyquist/LoadNyquist.cpp | 16 +++++++++++++++- src/effects/nyquist/Nyquist.cpp | 4 ++++ 7 files changed, 54 insertions(+), 15 deletions(-) diff --git a/include/audacity/PluginInterface.h b/include/audacity/PluginInterface.h index c937c3b00..8bdce98da 100644 --- a/include/audacity/PluginInterface.h +++ b/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; diff --git a/src/PluginManager.cpp b/src/PluginManager.cpp index 74f493748..7aeac0ff9 100644 --- a/src/PluginManager.cpp +++ b/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 diff --git a/src/PluginManager.h b/src/PluginManager.h index b9f87dc67..26a97590d 100644 --- a/src/PluginManager.h +++ b/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); diff --git a/src/commands/LoadCommands.cpp b/src/commands/LoadCommands.cpp index 544245473..d7f787eee 100644 --- a/src/commands/LoadCommands.cpp +++ b/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. diff --git a/src/effects/LoadEffects.cpp b/src/effects/LoadEffects.cpp index b9dfdd037..dbbee5d36 100644 --- a/src/effects/LoadEffects.cpp +++ b/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); diff --git a/src/effects/nyquist/LoadNyquist.cpp b/src/effects/nyquist/LoadNyquist.cpp index 8634f3626..ef11b5375 100644 --- a/src/effects/nyquist/LoadNyquist.cpp +++ b/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 ? diff --git a/src/effects/nyquist/Nyquist.cpp b/src/effects/nyquist/Nyquist.cpp index f670e405c..dfefef716 100644 --- a/src/effects/nyquist/Nyquist.cpp +++ b/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