Rewrites of TranslatableString and reimplementation of wxPLURAL...

... including move-construction of the base string, debug string formatting,
and contexts (not fully implemented)

include/audacity/Types.h

@@ -293,53 +293,79 @@ using CommandID = TaggedIdentifier< CommandIdTag, false >;
 using CommandIDs = std::vector<CommandID>;
 
 
-// Holds a msgid for the translation catalog and may also hold a context string
+// Holds a msgid for the translation catalog and may hold a closure that
-// and a formatter closure that captures formatting arguments
+// captures formatting arguments
 //
-// Two different wxString accessors -- one for the msgid itself, another for
+// Different string-valued accessors for the msgid itself, and for the
-// the user-visible translation.  The msgid should be used only in unusual cases
+// user-visible translation with substitution of captured format arguments.
-// and the translation more often
+// Also an accessor for format substitution into the English msgid, for debug-
+// only outputs.
+// The msgid should be used only in unusual cases and the translation more often
 //
 // Implicit conversions to and from wxString are intentionally disabled
 class TranslatableString : private wxString {
+   enum class Request;
+   template< size_t N > struct PluralTemp;
+
 public:
-   // A dual-purpose function
+   // A multi-purpose function, depending on the enum argument; the string
-   // Given the empty string, return a context string
+   // argument is unused in some cases
-   // Given the translation of the msgid into the current locale, substitute
+   // If there is no function, defaults are empty context string, no plurals,
-   // format arguments into it
+   // and no substitutions
-   using Formatter = std::function< wxString(const wxString &) >;
+   using Formatter = std::function< wxString(const wxString &, Request) >;
 
    // This special formatter causes msgids to be used verbatim, not looked up
-   // in any catalog
+   // in any catalog, so Translation() and Debug() return the same
    static const Formatter NullContextFormatter;
 
    TranslatableString() {}
 
    // Supply {} for the second argument to cause lookup of the msgid with
-   // empty context string
+   // empty context string (default context) rather than the null context
    explicit TranslatableString(
-      const wxString &str, Formatter formatter = NullContextFormatter)
+      wxString str, Formatter formatter = NullContextFormatter
-      : wxString{ str }, mFormatter{ std::move(formatter) } {}
+   )
-   // Supply {} for the second argument to cause lookup of the msgid with
+      : mFormatter{ std::move(formatter) }
-   // empty context string
+   {
-   explicit TranslatableString(
+      this->wxString::swap( str );
-      const wxChar *str, Formatter formatter = NullContextFormatter)
+   }
-      : wxString{ str }, mFormatter{ std::move(formatter) } {}
+
+   // copy and move
+   TranslatableString( const TranslatableString & ) = default;
+   TranslatableString &operator=( const TranslatableString & ) = default;
+   TranslatableString( TranslatableString && str )
+      : mFormatter( std::move( str.mFormatter ) )
+   {
+      this->wxString::swap( str );
+   }
+   TranslatableString &operator=( TranslatableString &&str )
+   {
+      mFormatter = std::move( str.mFormatter );
+      this->wxString::clear();
+      this->wxString::swap( str );
+      return *this;
+   }
 
    using wxString::empty;
 
    // MSGID is the English lookup key in the message catalog, not necessarily
    // for user's eyes if the locale is some other.
-   // The MSGID might not be all the information TranslatableString holds
+   // The MSGID might not be all the information TranslatableString holds.
-   // in future.
    // This is a deliberately ugly-looking function name.  Use with caution.
    Identifier MSGID() const { return Identifier{ *this }; }
 
-   wxString Translation() const;
+   wxString Translation() const { return DoFormat( false ); }
 
+   // Format as an English string for debugging logs and developers' eyes, not
+   // for end users
+   wxString Debug() const { return DoFormat( true ); }
+
+   // Warning: comparison of msgids only, which is not all of the information!
+   // This operator makes it easier to define a std::unordered_map on
+   // TranslatableStrings
    friend bool operator == (
       const TranslatableString &x, const TranslatableString &y)
-   { return x.MSGID() == y.MSGID(); }
+   { return (const wxString&)x == (const wxString&)y; }
 
    friend bool operator != (
       const TranslatableString &x, const TranslatableString &y)
@@ -348,23 +374,49 @@ public:
    // Returns true if context is NullContextFormatter
    bool IsVerbatim() const;
 
-   // Capture variadic format arguments (by copy).  The substitution is
+   // Capture variadic format arguments (by copy) when there is no plural.
-   // computed later in a call to Translate() after msgid is looked up in the
+   // The substitution is computed later in a call to Translate() after msgid is
-   // translation catalog.
+   // looked up in the translation catalog.
    // Any format arguments that are also of type TranslatableString will be
-   // translated too at substitution time
+   // translated too at substitution time, for non-debug formatting
-   template <typename... Args>
+   template< typename... Args >
-   TranslatableString&& Format( Args&&... args ) &&
+   TranslatableString &&Format( Args &&...args ) &&
    {
-      wxString context;
+      auto prevFormatter = mFormatter;
-      if ( this->mFormatter )
+      this->mFormatter = [prevFormatter, args...]
-         context = this->mFormatter({});
+      (const wxString &str, Request request) -> wxString {
-      this->mFormatter = [context, args...](const wxString &str){
+         switch ( request ) {
-         if (str.empty())
+            case Request::Context:
-            return context;
+               return DoGetContext( prevFormatter );
-         else
+            case Request::Format:
-            return wxString::Format(
+            case Request::DebugFormat:
-               str, TranslatableString::TranslateArgument(args)... );
+            default: {
+               bool debug = request == Request::DebugFormat;
+               return wxString::Format(
+                  DoFormat( prevFormatter, str, debug ),
+                  TranslatableString::TranslateArgument( args, debug )...
+               );
+            }
+         }
+      };
+      return std::move( *this );
+   }
+
+   // Choose a non-default and non-null disambiguating context for lookups
+   // (but this is not fully implemented)
+   // This is meant to be the first of chain-call modifications of the
+   // TranslatableString object; it will destroy any previously captured
+   // information
+   TranslatableString &&Context( const wxString &context ) &&
+   {
+      this->mFormatter = [context]
+      (const wxString &str, Request request) -> wxString {
+         switch ( request ) {
+            case Request::Context:
+               return context;
+            default:
+               return str;
+         }
       };
       return std::move( *this );
    }
@@ -372,15 +424,84 @@ public:
    // Append another translatable string; lookup of msgids for
    // this and for the argument are both delayed until Translate() is invoked
    // on this, and then the formatter concatenates the translations
-   TranslatableString &operator +=( const TranslatableString &arg );
+   TranslatableString &&Join(
+      const TranslatableString &arg, const wxString &separator = {} ) &&;
+   TranslatableString &operator +=( const TranslatableString &arg )
+   {
+      std::move(*this).Join( arg );
+      return *this;
+   }
+
+   // Implements the wxPLURAL macro, which specifies a second msgid, a list
+   // of format arguments, and which of those format arguments selects among
+   // messages; the translated strings to select among, depending on language,
+   // might actually be more or fewer than two.  See Internat.h.
+   template< size_t N >
+   PluralTemp< N > Plural( const wxString &pluralStr ) &&
+   {
+     return PluralTemp< N >{ *this, pluralStr };
+   }
 
-   friend std::hash< TranslatableString >;
 private:
+   enum class Request {
+      Context,     // return a disambiguating context string
+      Format,      // Given the msgid, format the string for end users
+      DebugFormat, // Given the msgid, format the string for developers
+   };
 
-   template< typename T > static const T &TranslateArgument( const T &arg )
+   static const wxChar *const NullContextName;
+   friend std::hash< TranslatableString >;
+
+   static wxString DoGetContext( const Formatter &formatter );
+   static wxString DoFormat(
+      const Formatter &formatter, const wxString &format, bool debug );
+   wxString DoFormat( bool debug ) const
+   {  return DoFormat( mFormatter, *this, debug ); }
+
+   static wxString DoChooseFormat(
+      const Formatter &formatter,
+      const wxString &singular, const wxString &plural, unsigned nn, bool debug );
+
+   template< typename T > static const T &TranslateArgument( const T &arg, bool )
    { return arg; }
-   static wxString TranslateArgument ( const TranslatableString &arg )
+   static wxString TranslateArgument( const TranslatableString &arg, bool debug )
-   { return arg.Translation(); }
+   { return arg.DoFormat( debug ); }
+
+   template< size_t N > struct PluralTemp{
+      TranslatableString &ts;
+      const wxString &pluralStr;
+      template< typename... Args >
+         TranslatableString &&operator()( Args&&... args )
+      {
+         // Pick from the pack the argument that specifies number
+         auto selector =
+            std::template get< N >( std::forward_as_tuple( args... ) );
+         // We need an unsigned value.  Guard against negative values.
+         auto nn = static_cast<unsigned>(
+            std::max<unsigned long long>( 0, selector )
+         );
+         auto plural = this->pluralStr;
+         auto prevFormatter = this->ts.mFormatter;
+         this->ts.mFormatter = [prevFormatter, plural, nn, args...]
+         (const wxString &str, Request request) -> wxString {
+            switch ( request ) {
+               case Request::Context:
+                  return DoGetContext( prevFormatter );
+               case Request::Format:
+               case Request::DebugFormat:
+               default:
+               {
+                  bool debug = request == Request::DebugFormat;
+                  return wxString::Format(
+                     DoChooseFormat( prevFormatter, str, plural, nn, debug ),
+                     TranslatableString::TranslateArgument( args, debug )...
+                  );
+               }
+            }
+         };
+         return std::move(ts);
+      }
+   };
 
    Formatter mFormatter;
 };
@@ -394,6 +515,7 @@ inline TranslatableString operator +(
 using TranslatableStrings = std::vector<TranslatableString>;
 
 // For using std::unordered_map on TranslatableString
+// Note:  hashing on msgids only, which is not all of the information
 namespace std
 {
    template<> struct hash< TranslatableString > {

src/Internat.cpp

@@ -291,51 +291,80 @@ std::vector< Identifier > Identifier::split( wxChar separator ) const
    return { strings.begin(), strings.end() };
 }
 
-static const wxChar *const NullContextName = wxT("*");
+const wxChar *const TranslatableString::NullContextName = wxT("*");
 
 const TranslatableString::Formatter
 TranslatableString::NullContextFormatter {
-   [](const wxString & str) -> wxString {
+   [](const wxString & str, TranslatableString::Request request) -> wxString {
-      if (str.empty())
+      switch ( request ) {
-         return NullContextName;
+         case Request::Context:
-      else
+            return NullContextName;
-         return str;
+         case Request::Format:
+         case Request::DebugFormat:
+         default:
+            return str;
+      }
    }
 };
 
 bool TranslatableString::IsVerbatim() const
 {
-   return mFormatter && mFormatter({}) == NullContextName;
+   return DoGetContext( mFormatter ) == NullContextName;
 }
 
-wxString TranslatableString::Translation() const
+wxString TranslatableString::DoGetContext( const Formatter &formatter )
 {
-   wxString context;
+   return formatter ? formatter( {}, Request::Context ) : wxString{};
-   if ( mFormatter )
-      context = mFormatter({});
-
-   wxString result = (context == NullContextName)
-      ? *this
-      : wxGetTranslation( *this
-         // , wxString{}, context
-      );
-
-   if ( mFormatter )
-      result = mFormatter( result );
-
-   return result;
 }
 
-TranslatableString &TranslatableString::operator +=(
+wxString TranslatableString::DoFormat(
-   const TranslatableString &arg )
+   const Formatter &formatter, const wxString &format, bool debug )
+{
+   return formatter
+      ? formatter( format, debug ? Request::DebugFormat : Request::Format )
+      : // come here for most translatable strings, which have no formatting
+         ( debug ? format : wxGetTranslation( format ) );
+}
+
+wxString TranslatableString::DoChooseFormat(
+   const Formatter &formatter,
+   const wxString &singular, const wxString &plural, unsigned nn, bool debug )
+{
+   // come here for translatable strings that choose among forms by number;
+   // if not debugging, then two keys are passed to an overload of
+   // wxGetTranslation, and also a number.
+   // Some languages might choose among more or fewer than two forms
+   // (e.g. Arabic has duals and Russian has complicated declension rules)
+   wxString context;
+   return ( debug || NullContextName == (context = DoGetContext(formatter)) )
+      ? ( nn == 1 ? singular : plural )
+      : wxGetTranslation(
+            singular, plural, nn
+            // , wxString{}
+            // , context
+         );
+}
+
+TranslatableString &&TranslatableString::Join(
+   const TranslatableString &arg, const wxString &separator ) &&
 {
    auto prevFormatter = mFormatter;
-   mFormatter = [prevFormatter, arg](const wxString &str){
+   mFormatter =
-      if (str.empty())
+   [prevFormatter, arg, separator](const wxString &str, Request request)
-         return prevFormatter ? prevFormatter({}) : wxString{};
+      -> wxString {
-      else
+      switch ( request ) {
-         return (prevFormatter ? prevFormatter(str) : str)
+         case Request::Context:
-            + arg.Translation();
+            return DoGetContext( prevFormatter );
+         case Request::Format:
+         case Request::DebugFormat:
+         default: {
+            bool debug = request == Request::DebugFormat;
+            return
+               DoFormat( prevFormatter, str, debug )
+                  + separator
+                  + arg.DoFormat( debug );
+         }
+      }
    };
-   return *this;
+   return std::move( *this );
 }

src/Internat.h

@@ -78,7 +78,12 @@ extern AUDACITY_DLL_API const wxString& GetCustomSubstitution(const wxString& st
 //
 // Your i18n-comment should therefore say something like,
 // "In the string after this one, ..."
-#define wxPLURAL(sing, plur, n)  wxGetTranslation( wxT(sing), wxT(plur), n)
+//
+// The macro call is then followed by a sequence of format arguments in
+// parentheses.  The third argument of the macro call is the zero-based index
+// of the format argument that selects singular or plural
+#define wxPLURAL(sing, plur, n) \
+   TranslatableString{ wxT(sing), {} }.Plural<(n)>( wxT(plur) )
 
 #endif
 

src/PluginManager.cpp

@@ -1899,7 +1899,11 @@ bool PluginManager::DropFile(const wxString &fileName)
 
             // Ask whether to enable the plug-ins
             if (auto nIds = ids.size()) {
-               auto message = wxPLURAL( "Enable this plug-in?", "Enable these plug-ins?", nIds );
+               auto message = wxPLURAL(
+                  "Enable this plug-in?",
+                  "Enable these plug-ins?",
+                  0
+               )( nIds ).Translation();
                message += wxT("\n");
                for (const auto &name : names)
                   message += name + wxT("\n");

src/ProjectManager.cpp

@@ -1041,10 +1041,9 @@ wxString ProjectManager::GetHoursMinsString(int iMinutes)
    int iHours = iMinutes / 60;
    int iMins = iMinutes % 60;
 
-   auto sHours =
+   auto sHours = wxPLURAL( "%d hour", "%d hours", 0 )( iHours ).Translation();
-      wxString::Format( wxPLURAL("%d hour", "%d hours", iHours), iHours );
+
-   auto sMins =
+   auto sMins = wxPLURAL( "%d minute", "%d minutes", 0 )( iMins ).Translation();
-      wxString::Format( wxPLURAL("%d minute", "%d minutes", iMins), iMins );
 
    /* i18n-hint: A time in hours and minutes. Only translate the "and". */
    sFormatted.Printf( _("%s and %s."), sHours, sMins);

src/menus/ClipMenus.cpp

@@ -327,17 +327,16 @@ wxString ClipBoundaryMessage(const std::vector<FoundClipBoundary>& results)
                clips.
              */
          _("dummyStringClipBoundaryMessage");
-         auto format = wxPLURAL(
+         auto str = wxPLURAL(
             "%s %d of %d clip %s",
             "%s %d of %d clips %s",
-            nClips
+            2
-         );
+         )(
-         str = wxString::Format(format,
             result.clipStart1 ? _("start") : _("end"),
             result.index1 + 1,
             nClips,
             longName
-         );
+         ).Translation();
       }
       else {
             /* i18n-hint: in the string after this one,
@@ -350,19 +349,18 @@ wxString ClipBoundaryMessage(const std::vector<FoundClipBoundary>& results)
                clips.
              */
          _("dummyStringClipBoundaryMessageLong");
-         auto format = wxPLURAL(
+         auto str = wxPLURAL(
             "%s %d and %s %d of %d clip %s",
             "%s %d and %s %d of %d clips %s",
-            nClips
+            4
-         );
+         )(
-         str = wxString::Format(format,
             result.clipStart1 ? _("start") : _("end"),
             result.index1 + 1,
             result.clipStart2 ? _("start") : _("end"),
             result.index2 + 1,
             nClips,
             longName
-         );
+         ).Translation();
       }
 
       if (message.empty())
@@ -587,13 +585,15 @@ void DoSelectClip(AudacityProject &project, bool next)
                last number counts the clips,
                string names a track */
          _("dummyStringOnSelectClip");
-         auto format = wxPLURAL(
+         auto str = wxPLURAL(
             "%d of %d clip %s",
             "%d of %d clips %s",
-            nClips
+            1
-         );
+         )(
-         auto str =
+            result.index + 1,
-            wxString::Format( format, result.index + 1, nClips, longName );
+            nClips,
+            longName
+         ).Translation();
 
          if (message.empty())
             message = str;