Re: [Audacity-devel] [patch] Nyquist language localization

[Richard A. <ri...@au...>]

On Mon, 2009-12-28 at 21:30 -0800, Ed Musgrove wrote:
> Patch attached
> P3 bug changing language in Preferences , some elements don't change
> The problems listed under this bug are due to multiple different issues. 
> 
> Bit more complicated but the cause is similar to other recent fixes to other
> symptoms. A string was cached and re-used even after it was stale due to
> language change. The cached string variable was also re-used in other code
> so I split that up.

I think this can be done using the mInteractive variable, i.e. with one
fewer boolean variable floating around. This is already used to control
the display of the Nyquist Prompt dialogue (line 399), so would be a the
logical variable to control the rest of the functionality that depends
on this. This makes mUseFName redundant, and so cleans the patch up
quite a bit.


> There is one potential problem I do not know how to test--see end of
> message.
> 

> What I am trying to exercise is the code in Nyquist.cpp around line 250 but
> I have no idea how to do so:
> 
> void EffectNyquist::Parse(wxString line)
> {
> [...]
>    if (len >= 2 && tokens[0] == wxT("name")) {
> #ifdef DEBUG
>       if (!mUseFName) wxMessageBox(wxT("Should never see this tell\nEd
> Musgrove\nNyquist.cpp language localization is broken at line 250."));
> #endif
>       mName = UnQuote(tokens[1]);
>       return;
>    } 

This code is executed every time a plug-in is loaded from file and the 
;name "Name of Plugin"
line is parsed to set the plug-in name.

There is no reason why this line couldn't be typed at the Nyquist
Prompt, so I don't think you need to do error checking here - if the
user-supplied Nyquist code changes the name of the effect then we should
honour this and set the mName variable.

> --- Nyquist.cpp	12 Dec 2009 23:47:38 -0000	1.87
> +++ Nyquist.cpp	29 Dec 2009 04:55:10 -0000
> @@ -464,10 +467,17 @@
>           ctrl->ticks = (int)(ctrl->high - ctrl->low);
>        }
>     }
> -
> -   NyquistDialog dlog(mParent, -1, mName, mInfo, &mControls);
> -   dlog.CentreOnParent();
> -   int result = dlog.ShowModal();
> +   int result;
> +   if (mUseFName) {
> +      NyquistDialog dlog(mParent, -1, mName, mInfo, &mControls);
> +      dlog.CentreOnParent();
> +      result = dlog.ShowModal();
> +   }
> +   else {
> +      NyquistDialog dlog(mParent, -1, _("Nyquist Prompt..."), mInfo, &mControls);
> +      dlog.CentreOnParent();
> +      result = dlog.ShowModal();
> +   }
>  
This is more invasive than it needs to be. At a minimum
-   NyquistDialog dlog(mParent, -1, mName, mInfo, &mControls);
-   dlog.CentreOnParent();
-   int result = dlog.ShowModal();
+   int result;
+   if (mUseFName) {
+      NyquistDialog dlog(mParent, -1, mName, mInfo, &mControls);
+   }
+   else {
+      NyquistDialog dlog(mParent, -1, _("Nyquist Prompt..."), mInfo, &mControls);
+   }
+   dlog.CentreOnParent();
+   result = dlog.ShowModal();
(which avoids the straight code duplication), but a temporary variable
is probably more readable:
-   NyquistDialog dlog(mParent, -1, mName, mInfo, &mControls);
-   dlog.CentreOnParent();
-   int result = dlog.ShowModal();
+   int result;
+   wxString lName;
+   if (mUseFName)
+      lName = mName;
+   else 
+      lname = _("Nyquist Prompt...");
+
+   NyquistDialog dlog(mParent, -1, lName, mInfo, &mControls);
+   dlog.CentreOnParent();
+   result = dlog.ShowModal();

which reduces the conditional to the one line and one variable which is
actually different in the two cases.

Richard