From 4dfac323c01386fb3e8955d3adf12f8288653fc5 Mon Sep 17 00:00:00 2001 From: James Crook Date: Sun, 4 Mar 2018 10:24:14 +0000 Subject: [PATCH 1/5] Make Macro Dialog optically stable. Expand and Shrink now happen 'in place'. Title changes to and from 'Apply Macro' <-> 'Edit Macros' --- src/BatchProcessDialog.cpp | 50 ++++++++++++++++++++++++++------------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/src/BatchProcessDialog.cpp b/src/BatchProcessDialog.cpp index af509b9fe..a958b8163 100644 --- a/src/BatchProcessDialog.cpp +++ b/src/BatchProcessDialog.cpp @@ -123,6 +123,14 @@ void ApplyMacroDialog::PopulateOrExchange(ShuttleGui &S) S.Id(ExpandID).AddButton(_("&Expand")); } S.EndHorizontalLay(); + S.StartHorizontalLay(wxALIGN_RIGHT, false); + { + S.AddSpace( 40 ); + S.AddPrompt( _("Apply Macro to:") ); + S.Id(ApplyToProjectID).AddButton(_("&Project")); + S.Id(ApplyToFilesID).AddButton(_("&Files...")); + } + S.EndHorizontalLay(); S.StartStatic(_("&Select Macro"), true); { @@ -132,14 +140,8 @@ void ApplyMacroDialog::PopulateOrExchange(ShuttleGui &S) mMacros->InsertColumn(0, _("Macro"), wxLIST_FORMAT_LEFT); } S.EndStatic(); - S.StartHorizontalLay(wxALIGN_RIGHT, false); { - S.SetBorder(10); - S.AddPrompt( _("Apply Macro to:") ); - S.Id(ApplyToProjectID).AddButton(_("&Project")); - S.Id(ApplyToFilesID).AddButton(_("&Files...")); - S.AddSpace( 40 ); S.AddStandardButtons( eCancelButton | eHelpButton); } S.EndHorizontalLay(); @@ -501,9 +503,9 @@ MacrosWindow::MacrosWindow(wxWindow * parent, bool bExpanded): ApplyMacroDialog(parent, true) { mbExpanded = bExpanded; - SetLabel(_("Macros")); // Provide visual label - SetName(_("Macros")); // Provide audible label - SetTitle(_("Macros")); + SetLabel(_("Edit Macros")); // Provide visual label + SetName(_("Edit Macros")); // Provide audible label + SetTitle(_("Edit Macros")); mChanged = false; mSelectedCommand = 0; @@ -563,10 +565,18 @@ void MacrosWindow::PopulateOrExchange(ShuttleGui & S) { S.Prop(0).StartHorizontalLay(wxALIGN_RIGHT, false); { - S.Id(ShrinkID).AddButton(_("&Shrink")); + S.Id(ShrinkID).AddButton(_("Shrin&k")); } S.EndHorizontalLay(); - S.StartStatic(_("&Macros"),1); + S.StartHorizontalLay(wxALIGN_RIGHT, false); + { + S.AddSpace( 40 ); + S.AddPrompt( _("Apply Macro to:") ); + S.Id(ApplyToProjectID).AddButton(_("&Project")); + S.Id(ApplyToFilesID).AddButton(_("&Files...")); + } + S.EndHorizontalLay(); + S.StartStatic(_("&Select Macros"),1); { S.SetStyle(wxSUNKEN_BORDER | wxLC_REPORT | wxLC_HRULES | wxLC_SINGLE_SEL | wxLC_EDIT_LABELS); @@ -587,7 +597,7 @@ void MacrosWindow::PopulateOrExchange(ShuttleGui & S) S.StartVerticalLay( 1 ); { - S.StartStatic(_("Ma&cro (Double-Click or press SPACE to edit)"), true); + S.StartStatic(_("&Macro (Double-Click or press SPACE to edit)"), true); { S.StartHorizontalLay(wxEXPAND,1); { @@ -620,10 +630,6 @@ void MacrosWindow::PopulateOrExchange(ShuttleGui & S) S.EndStatic(); S.StartHorizontalLay(wxALIGN_RIGHT, false); { - S.AddPrompt( _("Apply Macro to:") ); - S.Id(ApplyToProjectButtonID).AddButton(_("&Project"), wxALIGN_LEFT); - S.Id(ApplyToFilesButtonID).AddButton(_("&Files..."), wxALIGN_LEFT); - S.AddSpace( 40 ); S.AddStandardButtons( eOkButton | eCancelButton | eHelpButton); } S.EndHorizontalLay(); @@ -692,10 +698,22 @@ void MacrosWindow::UpdateDisplay( bool bExpanded ) mSelectedCommand = 0; SetMinSize( wxSize( 200,200 )); + // Get and set position for optical stability. + // Expanded and shrunk dialogs 'stay where they were'. + // That's OK , and what we want, even if we exapnd off-screen. + // We won't shrink to being off-screen, since the shrink button + // was clicked, so must have been on screen. + wxPoint p = GetPosition( ); if( mbExpanded ) Populate(); else ApplyMacroDialog::Populate(); + SetPosition( p ); + + wxString Title = mbExpanded ? _("Edit Macros") : _("Apply Macro"); + SetLabel( Title ); // Provide visual label + SetName( Title ); // Provide audible label + SetTitle( Title ); } void MacrosWindow::OnExpand(wxCommandEvent &WXUNUSED(event)) From 153da3a94df6c9823f1d7c964677fad255436418 Mon Sep 17 00:00:00 2001 From: James Crook Date: Sun, 4 Mar 2018 15:50:14 +0000 Subject: [PATCH 2/5] Fix Fade-Ends MacroCommands - Split parameters from command name. - Improve test for 'fixed Macros'. --- src/BatchCommands.cpp | 31 ++++++++++++++++++------------- src/BatchCommands.h | 1 + 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/BatchCommands.cpp b/src/BatchCommands.cpp index e1be1d88d..487eccb35 100644 --- a/src/BatchCommands.cpp +++ b/src/BatchCommands.cpp @@ -87,19 +87,13 @@ static const std::pair SpecialCommands[] = { }; // end CLEANSPEECH remnant -static const wxString MP3Conversion = wxT("MP3 Conversion"); -static const wxString FadeEnds = wxT("Fade Ends"); - MacroCommands::MacroCommands() { mMessage = ""; ResetMacro(); wxArrayString names = GetNames(); - - wxArrayString defaults; - defaults.Add( MP3Conversion ); - defaults.Add( FadeEnds ); + wxArrayString defaults = GetNamesOfDefaultMacros(); for( size_t i = 0;i; From 585a4c61703ce3dfaa0319f9f7aceff7b2becc54 Mon Sep 17 00:00:00 2001 From: James Crook Date: Sun, 4 Mar 2018 18:40:21 +0000 Subject: [PATCH 3/5] Prevent excessive macro reentry. User could have made a macro call itself. Or could have an 'exploding' macro chain with 'too much' in it. So limit it to a macro can call a macro, but not deeper. --- src/BatchCommands.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/BatchCommands.cpp b/src/BatchCommands.cpp index 487eccb35..a9b5ae004 100644 --- a/src/BatchCommands.cpp +++ b/src/BatchCommands.cpp @@ -751,16 +751,27 @@ bool MacroCommands::ApplyCommandInBatchMode(const wxString & command, const wxSt return ApplyCommand( command, params ); } +static int MacroReentryCount = 0; // ApplyMacro returns true on success, false otherwise. // Any error reporting to the user in setting up the chain // has already been done. bool MacroCommands::ApplyMacro(const wxString & filename) { + // Check for reentrant ApplyMacro commands. + // We'll allow 1 level of reentry, but not more. + // And we treat ignoring deeper levels as a success. + if( MacroReentryCount > 1 ) + return true; + + // Restore the reentry counter (to zero) when we exit. + auto cleanup1 = valueRestorer( MacroReentryCount); + MacroReentryCount++; + mFileName = filename; AudacityProject *proj = GetActiveProject(); bool res = false; - auto cleanup = finally( [&] { + auto cleanup2 = finally( [&] { if (!res) { if(proj) { // Macro failed or was cancelled; revert to the previous state @@ -801,7 +812,8 @@ bool MacroCommands::ApplyMacro(const wxString & filename) if (!proj) return false; - proj->PushState(longDesc, shortDesc); + if( MacroReentryCount <= 1 ) + proj->PushState(longDesc, shortDesc); return true; } From 70b1f69bbe77fa8d00b1384fbbd28b1ff8cdf819 Mon Sep 17 00:00:00 2001 From: James Crook Date: Sun, 4 Mar 2018 19:02:22 +0000 Subject: [PATCH 4/5] Use names not numbers in MacroIdOfName() Previously the ID for a macro was, e.g. Macro003. However this would not work if macros were added or deleted, since chains containing the old macro references would now refer to a different macro. So changed to using names. --- src/BatchProcessDialog.cpp | 21 +++++++++++++++++++++ src/BatchProcessDialog.h | 2 ++ src/Menus.cpp | 10 +++++++++- 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/BatchProcessDialog.cpp b/src/BatchProcessDialog.cpp index a958b8163..2b66d5371 100644 --- a/src/BatchProcessDialog.cpp +++ b/src/BatchProcessDialog.cpp @@ -192,6 +192,27 @@ void ApplyMacroDialog::OnApplyToProject(wxCommandEvent & WXUNUSED(event)) ApplyMacroToProject( item ); } +wxString ApplyMacroDialog::MacroIdOfName( const wxString & MacroName ) +{ + wxString Temp = MacroName; + Temp.Replace(" ",""); + Temp = wxString( "Macro_" ) + Temp; + return Temp; +} + + +// Does nothing if not found, rather than returning an error. +void ApplyMacroDialog::ApplyMacroToProject( const wxString & MacroID, bool bHasGui ) +{ + for( size_t i=0;iGetItemCount();i++){ + wxString name = mMacros->GetItemText(i); + if( MacroIdOfName( name ) == MacroID ){ + ApplyMacroToProject( i, bHasGui ); + return; + } + } +} + void ApplyMacroDialog::ApplyMacroToProject( int iMacro, bool bHasGui ) { wxString name = mMacros->GetItemText(iMacro); diff --git a/src/BatchProcessDialog.h b/src/BatchProcessDialog.h index 32568936d..29e4af050 100644 --- a/src/BatchProcessDialog.h +++ b/src/BatchProcessDialog.h @@ -56,7 +56,9 @@ class ApplyMacroDialog : public wxDialogWrapper { virtual wxString GetHelpPageName() {return "Tools_Menu#macros_compact_dialog";}; void PopulateMacros(); + static wxString MacroIdOfName( const wxString & MacroName ); void ApplyMacroToProject( int iMacro, bool bHasGui=true ); + void ApplyMacroToProject( const wxString & MacroID, bool bHasGui=true ); // These will be reused in the derived class... diff --git a/src/Menus.cpp b/src/Menus.cpp index a41be23f2..b592cc3a1 100644 --- a/src/Menus.cpp +++ b/src/Menus.cpp @@ -1713,7 +1713,8 @@ void AudacityProject::PopulateMacrosMenu( CommandManager* c, CommandFlag flags int i; for (i = 0; i < (int)names.GetCount(); i++) { - c->AddItem(wxString::Format("Macro%03i", i ), names[i], FN(OnApplyMacroDirectly), + wxString MacroID = ApplyMacroDialog::MacroIdOfName( names[i] ); + c->AddItem(MacroID, names[i], FN(OnApplyMacroDirectly), flags, flags); } @@ -6860,10 +6861,17 @@ void AudacityProject::OnApplyMacroDirectly(const CommandContext &context ) //wxLogDebug( "Macro was: %s", context.parameter); ApplyMacroDialog dlg(this); wxString Name = context.parameter; + +// We used numbers previously, but macros could get renumbered, making +// macros containing macros unpredictable. +#ifdef MACROS_BY_NUMBERS long item=0; // Take last three letters (of e.g. Macro007) and convert to a number. Name.Mid( Name.Length() - 3 ).ToLong( &item, 10 ); dlg.ApplyMacroToProject( item, false ); +#else + dlg.ApplyMacroToProject( Name, false ); +#endif ModifyUndoMenuItems(); } From 834c9df32a2fa01ad31a4f31129cb2679cb5f253 Mon Sep 17 00:00:00 2001 From: James Crook Date: Sun, 4 Mar 2018 19:16:30 +0000 Subject: [PATCH 5/5] Save Changes when Expanding/Shrinking Macro Dialog. Previously the selected macro was forgotten, as were any pending changes. --- src/BatchProcessDialog.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/BatchProcessDialog.cpp b/src/BatchProcessDialog.cpp index 2b66d5371..15482d63f 100644 --- a/src/BatchProcessDialog.cpp +++ b/src/BatchProcessDialog.cpp @@ -200,7 +200,7 @@ wxString ApplyMacroDialog::MacroIdOfName( const wxString & MacroName ) return Temp; } - +// Apply macro, given its ID. // Does nothing if not found, rather than returning an error. void ApplyMacroDialog::ApplyMacroToProject( const wxString & MacroID, bool bHasGui ) { @@ -213,6 +213,7 @@ void ApplyMacroDialog::ApplyMacroToProject( const wxString & MacroID, bool bHasG } } +// Apply macro, given its number in the list. void ApplyMacroDialog::ApplyMacroToProject( int iMacro, bool bHasGui ) { wxString name = mMacros->GetItemText(iMacro); @@ -711,6 +712,10 @@ void MacrosWindow::UpdateDisplay( bool bExpanded ) { if( bExpanded == mbExpanded ) return; + + if( !SaveChanges() ) + return; + mbExpanded = bExpanded; DestroyChildren(); SetSizer( nullptr );