Re: [Audacity-devel] [patch] language localization on menu "Repeat Last Effect"

[Ed M. <edg...@wa...>]

Works here on Win7.

--Ed


> -----Original Message-----
> From: Richard Ash [mailto:ri...@au...]
> Sent: Saturday, December 26, 2009 9:03 AM
> To: aud...@li...
> Subject: Re: [Audacity-devel] [patch] language localization on menu "Repeat Last
> Effect"
> 
> On Thu, 2009-12-24 at 20:25 -0800, Ed Musgrove wrote:
> > Patch attached
> > P3 bug changing language in Preferences This bug had a few unrelated
> > symptoms.
> >
> > Because this patch relies on a new string which is not translated yet
> > the steps to test are a tiny bit convoluted.
> 
> Or you could locally fix a translation to include the relevant string (even if you don't
> know the language, anything different from the English will do). In fact we can avoid
> the string change (mostly luck), see end of this email.
> 
> A number of thoughts on this patch:
> 
> -         wxString longDesc = f->GetEffectDescription();
> -         wxString shortDesc = f->GetEffectName();
> +         wxString longDescription = f->GetEffectDescription();
> Gratuitous change of variable name, making the patch seem much more complex that it
> actually is. I deleted all these lines and the patch still compiles (with a change where
> the new variable name was used in a line with another change). Please try to reduce
> your patches to the minimum changes that need to be made, they are much easier to
> review like that.
> 
> +      // to test "repeat last effect" translation fix
> +      // comment out next line:
> +      buildMenuLabel.Printf(_("Repeat "));
> +      // then uncomment the next line:
> +      //buildMenuLabel.Printf(_("Play One Second"));
> We aren't going to commit this into CVS, so it shouldn't be in the patch, especially not
> twice (which it was), the instructions in the email were sufficient, or you could have
> pasted this bit of the patch there, and attached just the patch which is actually suitable
> for comitting.
> 
> (minor) str.Printf("foo ") followed by .Append(str2) is equivalent to str.Printf("foo %s",
> str2) and avoids that weird string with a trailing space on it (which is very prone to get
> lost and cause a formatting issue). The %s also makes it clear to the translator that
> something will go there in the usage, adding an i18n-hint comment to the source to
> explain what that is would be good practise (in this case it's an effect name).
> 
> In fact the string "Repeat %s" is already in the .pot file and indeed translated in the
> French translation, so by making the third change above I can instantly see that the
> reduced patch attached works as intended using the current translation.
> 
> This is all in UI code (which is disabled during Audio I/O as well) so the performance
> impact of extra gettext hits should be negligable.
> 
> Richard