Thanks for the comments Vaughan,
On 15 May 2013 02:23, Vaughan Johnson <vaughan@...> wrote:
> Overall, I think this could turn into a bigger effort than has been
> considered, possibly be several months, and I don't want it to delay the
> next release. I don't think it would be tragic to release with only some
> effects having Help, but it needs to be considered whether that's a
> gating factor.
I've no intention of holding up 2.0.4 for the Help buttons - so far
this is just me poking around in the code to see what is there.
Perhaps I'll have something concrete in time for 2.0.5 :=)
However, I would like Nyquist Generate Prompt to be considered. It
seems to be a simple extension of the Nyquist Prompt, and it would
provide an easier way into learning how to use Nyquist, as well as
providing a very useful tool for people that use Audacity creatively.
>
>
> On 5/14/2013 4:26 PM, Martyn Shaw wrote:
>> ...
>> On 14/05/2013 17:38, Steve the Fiddle wrote:
>>> On 14 May 2013 00:51, Martyn Shaw <martynshaw99@...> wrote:
>>>> On 13/05/2013 12:02, Steve the Fiddle wrote:
>>>>> On 13 May 2013 00:41, Martyn Shaw <martynshaw99@...> wrote:
>>>>...
>>>>>> Making all our effects having an optional 'help' link to an
>>>>>> appropriate wiki page would be a great help to users, I think. That's
>>>>>> well ot in this thread though. But since I talking about it, is there
>>>>>> a general way to put a 'help' button into our CreateStdButtonSizer and
>>>>>> let effects give the location? Or another way?
>>>>...
>>>> eHelpButton appears to be there, but very little used. Do you fancy
>>>> investigating it? At a brief glance it looks not fully implemented.
>>>>...
>>> I've made some progress on effect Help buttons (patch attached).
>>> Currently this has only been tested on Linux.
>>
>> That works here, but duplicates code for no good reason that I can see.
>
> Indeed. Not object-oriented. Common behaviors should be inherited, not
> duplicated in descendants. If you did this in all effects, it would be
> thousands of lines of near-duplicate code, which is not just bloat, but
> turns into a maintenance nightmare if some of that code needs to change,
> because you'd have to change all the duplicates.
>
> There was something earlier in this thread about ErrorDialog. This is
> the user asking for help, not having made an error.
I thought that was a bit strange, but that is where ShowHelpDialog is
currently located.
Would it perhaps be better to move ShowHelpDialog and the common
EffectHelp together in a new HelpDialog class that can handle both?
>
> Afaik, all the built-in effect dialogs descend from EffectDialog, so
> that's the natural place to put code that all the descendants will want
> to use.
>
> I suggest an OnHelp() method on EffectDialog, with the common code, and
> a new wxString member vars, that specify the links to use. Then each
> descendant sets that link value for its manual page, and the code in the
> base class doesn't have to be duplicated in all the descendants, it just
> refers to that member var.
>
> I don't know why NyquistInputDialog does not descend from EffectDialog
> -- probably whoever wrote it just didn't think about that, and it might
> be a good change.
>
>
> Frankly, I'm not very motivated by this effort, not high priority to me.
> So, if the above is clear, Steve, I recommend you do it, for the
> experience. (And maybe do a little reading on overview of
> object-oriented principles.)
>
> If it's all Greek to you, I could do the infrastructure and a sample
> effect to use as a model, after which it would be pretty mechanical to
> add new Help buttons.
Thanks for the tips - I can work with that.
Steve
>
>
>>
>>> Perhaps someone could offer some guidance as to how to progress the
>>> code from here?
>>
>> How about calling ShowHelpDialog thus:
>>
>> void NyquistInputDialog::OnHelp(wxCommandEvent & /* event */)
>> {
>> AudacityProject * pProj = GetActiveProject();
>
> It's an ugly hack that this line appears so many places. It was a bad
> idea in the first place, and I'm dismayed that it has proliferated.
> Please do not do it unless there's no other reasonable way to determine
> the project.
>
> EffectDialog gets passed wxWindow * parent. I believe that in all cases,
> it's actually the AudacityProject*, which btw descends from wxWindow.
>
> So if it were me, I'd change it to pass the AudacityProject*, store it
> on a member var on EffectDialog, and still pass that pointer as the
> parent to the wxDialog constructor.
>
> (I think most places that use GetActiveProject() could be fixed similarly.)
>
>
>
>> wxString localPage;
>> wxString remotePage;
>
> These correspond to the 2 new member vars I'm suggesting for EffectDialog.
>
>
>>
>> wxString title = GetTitle();
>> if(title == wxT("Nyquist Prompt...")) {
>
> So this, if it were put on EffectDialog::OnHelp() I'm suggesting, would
> require EffectDialog to be modified every time you add a Help button.
> The strings are specific to each descendant effect, so they should be
> specified in there. That way you don't even need to check the title.
>
>
> And btw, as you were putting this in the NyquistInputDialog, there's no
> need for a conditional anyway -- you *know* it's the Nyquist dialog and
> you don't need to check the title. Object-oriented!
>
> Also, string constants are relatively bulky, so bad to duplicate. And a
> maintenance burden -- if one changes, you've got to make sure you change
> the others. So duplicating the "Nyquist Prompt..." in the conditional is
> less desirable than just making it a string constant member var. But as
> I say, there's zero reason to check the title.
>
>
>> localPage = FileNames::HtmlHelpDir() +
>> wxT("man/nyquist_prompt.html");
>> remotePage =
>> wxT("http://manual.audacityteam.org/o/man/nyquist_prompt.html");
>> }
>> else { // really this should be a different page
>> localPage = FileNames::HtmlHelpDir() +
>> wxT("man/nyquist_prompt.html");
>> remotePage =
>> wxT("http://manual.audacityteam.org/o/man/nyquist_prompt.html");
>> }
>
> Looks like the code in these 2 clauses is identical. Why? I agree both
> should be set and calling ShowHelpDialog() is more appropriate than
> OpenInDefaultBrowser().
>
>
>> ShowHelpDialog(pProj, localPage, remotePage);
>> }
>>
>> with appropriate .h files included?
>>
>> Any why the 'stf' comments? I've done that in the past too but it is
>> generally not useful.
>
> Yes, it's considered bad practice. Plus we can always use SVN blame.
>
> I do it, however, infrequently, to mark ("//v") things that need further
> attention and probably removal.
>
>
> - V
>
> ------------------------------------------------------------------------------
> AlienVault Unified Security Management (USM) platform delivers complete
> security visibility with the essential security capabilities. Easily and
> efficiently configure, manage, and operate all of your security controls
> from a single console and one unified framework. Download a free trial.
> http://p.sf.net/sfu/alienvault_d2d
> _______________________________________________
> audacity-devel mailing list
> audacity-devel@...
> https://lists.sourceforge.net/lists/listinfo/audacity-devel
|
