Undoing and redoing of Tags...

Use reference counting (in std::shared_ptr) to avoid replicating Tags with each
Undo state.

src/Menus.cpp

@@ -5582,8 +5582,24 @@ void AudacityProject::OnImportRaw()
 
 void AudacityProject::OnEditMetadata()
 {
-   if (mTags->ShowEditDialog(this, _("Edit Metadata Tags"), true))
-      PushState(_("Edit Metadata Tags"), _("Metadata Tags"));
+   (void)DoEditMetadata(_("Edit Metadata Tags"), _("Metadata Tags"), true);
+}
+
+bool AudacityProject::DoEditMetadata
+(const wxString &title, const wxString &shortUndoDescription, bool force)
+{
+   // Back up my tags
+   auto newTags = mTags->Duplicate();
+
+   if (newTags->ShowEditDialog(this, title, force)) {
+      // Commit the change to project state only now.
+      mTags = newTags;
+      PushState(title, shortUndoDescription);
+
+      return true;
+   }
+
+   return false;
 }
 
 void AudacityProject::HandleMixAndRender(bool toNewTrack)

src/Menus.h

@@ -320,6 +320,7 @@ void OnImportMIDI();
 void OnImportRaw();
 
 void OnEditMetadata();
+bool DoEditMetadata(const wxString &title, const wxString &shortUndoDescription, bool force);
 
 void OnMixAndRender();
 void OnMixAndRenderToNewTrack();

src/Project.cpp

@@ -1026,6 +1026,9 @@ AudacityProject::AudacityProject(wxWindow * parent, wxWindowID id,
    // MM: Give track panel the focus to ensure keyboard commands work
    mTrackPanel->SetFocus();
 
+   // Create tags object
+   mTags = std::make_shared<Tags>();
+
    InitialState();
    FixScrollbars();
    mRuler->SetLeftOffset(mTrackPanel->GetLeftOffset());  // bevel on AdornedRuler
@@ -1051,9 +1054,6 @@ AudacityProject::AudacityProject(wxWindow * parent, wxWindowID id,
 #endif
    mIconized = false;
 
-   // Create tags object
-   mTags = new Tags();
-
    mTrackFactory = new TrackFactory(mDirManager, &mViewInfo);
 
    int widths[] = {0, GetControlToolBar()->WidthForStatusBar(mStatusBar), -1, 150};
@@ -1250,9 +1250,9 @@ bool AudacityProject::IsAudioActive() const
       gAudioIO->IsStreamActive(GetAudioIOToken());
 }
 
-Tags *AudacityProject::GetTags()
+const Tags *AudacityProject::GetTags()
 {
-   return mTags;
+   return mTags.get();
 }
 
 wxString AudacityProject::GetName()
@@ -2320,8 +2320,7 @@ void AudacityProject::OnCloseWindow(wxCloseEvent & event)
    delete mTrackFactory;
    mTrackFactory = NULL;
 
-   delete mTags;
-   mTags = NULL;
+   mTags.reset();
 
    delete mImportXMLTagHandler;
    mImportXMLTagHandler = NULL;
@@ -3182,7 +3181,7 @@ bool AudacityProject::HandleXMLTag(const wxChar *tag, const wxChar **attrs)
 XMLTagHandler *AudacityProject::HandleXMLChild(const wxChar *tag)
 {
    if (!wxStrcmp(tag, wxT("tags"))) {
-      return mTags;
+      return mTags.get();
    }
 
    if (!wxStrcmp(tag, wxT("wavetrack"))) {
@@ -3793,12 +3792,40 @@ bool AudacityProject::Import(const wxString &fileName, WaveTrackArray* pTrackArr
 {
    Track **newTracks;
    int numTracks;
-   wxString errorMessage=wxT("");
+   wxString errorMessage = wxEmptyString;
+
+   // Backup Tags, before the import.  Be prepared to roll back changes.
+   struct TempTags {
+      TempTags(std::shared_ptr<Tags> & pTags_)
+         : pTags(pTags_)
+      {
+         oldTags = pTags;
+         if (oldTags)
+            pTags = oldTags->Duplicate();
+      }
+
+      ~TempTags()
+      {
+         if (oldTags) {
+            // roll back
+            pTags = oldTags;
+         }
+      }
+
+      void Commit()
+      {
+         oldTags.reset();
+      }
+
+      std::shared_ptr<Tags> & pTags;
+      std::shared_ptr<Tags> oldTags;
+   };
+   TempTags tempTags(mTags);
 
    numTracks = Importer::Get().Import(fileName,
                                             mTrackFactory,
                                             &newTracks,
-                                            mTags,
+                                            mTags.get(),
                                             errorMessage);
 
    if (!errorMessage.IsEmpty()) {
@@ -3814,9 +3841,17 @@ bool AudacityProject::Import(const wxString &fileName, WaveTrackArray* pTrackArr
 
    wxGetApp().AddFileToHistory(fileName);
 
+   // no more errors
+   tempTags.Commit();
+
    // for LOF ("list of files") files, do not import the file as if it
    // were an audio file itself
    if (fileName.AfterLast('.').IsSameAs(wxT("lof"), false)) {
+      // PRL: don't redundantly do the steps below, because we already
+      // did it in case of LOF, because of some weird recursion back to this
+      // same function.  I think this should be untangled.
+
+      // So Undo history push is not bypassed, despite appearances.
       return false;
    }
 
@@ -3830,6 +3865,7 @@ bool AudacityProject::Import(const wxString &fileName, WaveTrackArray* pTrackArr
       }
    }
 
+   // PRL: Undo history is incremented inside this:
    AddImportedTracks(fileName, newTracks, numTracks);
 
    int mode = gPrefs->Read(wxT("/AudioFiles/NormalizeOnLoad"), 0L);
@@ -3984,7 +4020,7 @@ void AudacityProject::InitialState()
 
    GetUndoManager()->ClearStates();
 
-   GetUndoManager()->PushState(mTracks, mViewInfo.selectedRegion,
+   GetUndoManager()->PushState(mTracks, mViewInfo.selectedRegion, mTags,
                           _("Created new project"), wxT(""));
 
    GetUndoManager()->StateSaved();
@@ -4008,7 +4044,7 @@ void AudacityProject::PushState(const wxString &desc,
                                 const wxString &shortDesc,
                                 UndoPush flags )
 {
-   GetUndoManager()->PushState(mTracks, mViewInfo.selectedRegion,
+   GetUndoManager()->PushState(mTracks, mViewInfo.selectedRegion, mTags,
                           desc, shortDesc, flags);
 
    mDirty = true;
@@ -4044,7 +4080,7 @@ void AudacityProject::RollbackState()
 
 void AudacityProject::ModifyState(bool bWantsAutoSave)
 {
-   GetUndoManager()->ModifyState(mTracks, mViewInfo.selectedRegion);
+   GetUndoManager()->ModifyState(mTracks, mViewInfo.selectedRegion, mTags);
    if (bWantsAutoSave)
       AutoSave();
 }
@@ -4054,6 +4090,9 @@ void AudacityProject::ModifyState(bool bWantsAutoSave)
 //    Need to keep it and its tracks "t" available for Undo/Redo/SetStateTo.
 void AudacityProject::PopState(const UndoState &state)
 {
+   // Restore tags
+   mTags = state.tags;
+
    TrackList *const tracks = state.tracks.get();
 
    mTracks->Clear(true);

src/Project.h

@@ -185,7 +185,7 @@ class AUDACITY_DLL_API AudacityProject:  public wxFrame,
    DirManager *GetDirManager();
    TrackFactory *GetTrackFactory();
    AdornedRulerPanel *GetRulerPanel();
-   Tags *GetTags();
+   const Tags *GetTags();
    int GetAudioIOToken() const;
    bool IsAudioActive() const;
    void SetAudioIOToken(int token);
@@ -526,7 +526,10 @@ public:
    wxMenu *mRecentFilesMenu;
 
    // Tags (artist name, song properties, MP3 ID3 info, etc.)
-   Tags *mTags;
+   // The structure may be shared with undo history entries
+   // To keep undo working correctly, always replace this with a new duplicate
+   // BEFORE doing any editing of it!
+   std::shared_ptr<Tags> mTags;
 
    // List of tracks and display info
    TrackList *mTracks;

src/Tags.cpp

@@ -233,6 +233,11 @@ Tags::~Tags()
 {
 }
 
+std::shared_ptr<Tags> Tags::Duplicate() const
+{
+   return std::make_shared<Tags>(*this);
+}
+
 Tags & Tags::operator=(const Tags & src)
 {
    mEditTitle = src.mEditTitle;

src/Tags.h

@@ -77,6 +77,8 @@ class AUDACITY_DLL_API Tags: public XMLTagHandler {
    Tags();  // constructor
    virtual ~Tags();
 
+   std::shared_ptr<Tags> Duplicate() const;
+
    Tags & operator= (const Tags & src );
 
    bool ShowEditDialog(wxWindow *parent, const wxString &title, bool force = false);

src/UndoManager.cpp

@@ -31,6 +31,7 @@ UndoManager
 #include "WaveTrack.h"          // temp
 #include "NoteTrack.h"  // for Sonify* function declarations
 #include "Diags.h"
+#include "Tags.h"
 
 #include "UndoManager.h"
 
@@ -41,8 +42,9 @@ struct UndoStackElem {
    UndoStackElem(std::unique_ptr<TrackList> &&tracks_,
       const wxString &description_,
       const wxString &shortDescription_,
-      const SelectedRegion &selectedRegion_)
-      : state(std::move(tracks_), selectedRegion_)
+      const SelectedRegion &selectedRegion_,
+      const std::shared_ptr<Tags> &tags_)
+      : state(std::move(tracks_), tags_, selectedRegion_)
       , description(description_)
       , shortDescription(shortDescription_)
    {
@@ -196,7 +198,8 @@ bool UndoManager::RedoAvailable()
 }
 
 void UndoManager::ModifyState(const TrackList * l,
-                              const SelectedRegion &selectedRegion)
+                              const SelectedRegion &selectedRegion,
+                              const std::shared_ptr<Tags> &tags)
 {
    if (current == wxNOT_FOUND) {
       return;
@@ -217,12 +220,15 @@ void UndoManager::ModifyState(const TrackList * l,
 
    // Replace
    stack[current]->state.tracks = std::move(tracksCopy);
+   stack[current]->state.tags = tags;
+
    stack[current]->state.selectedRegion = selectedRegion;
    SonifyEndModifyState();
 }
 
 void UndoManager::PushState(const TrackList * l,
                             const SelectedRegion &selectedRegion,
+                            const std::shared_ptr<Tags> &tags,
                             const wxString &longDescription,
                             const wxString &shortDescription,
                             UndoPush flags)
@@ -233,7 +239,7 @@ void UndoManager::PushState(const TrackList * l,
    if (((flags & UndoPush::CONSOLIDATE) != UndoPush::MINIMAL) && lastAction == longDescription &&
        consolidationCount < 2) {
       consolidationCount++;
-      ModifyState(l, selectedRegion);
+      ModifyState(l, selectedRegion, tags);
       // MB: If the "saved" state was modified by ModifyState, reset
       //  it so that UnsavedChanges returns true.
       if (current == saved) {
@@ -257,10 +263,14 @@ void UndoManager::PushState(const TrackList * l,
       t = iter.Next();
    }
 
+   // Assume tags was duplicted before any changes.
+   // Just save a new shared_ptr to it.
    stack.emplace_back(
       std::make_unique<UndoStackElem>
-         (std::move(tracksCopy), longDescription, shortDescription, selectedRegion)
+         (std::move(tracksCopy),
+            longDescription, shortDescription, selectedRegion, tags)
    );
+
    current++;
 
    if (saved >= current) {

src/UndoManager.h

@@ -54,16 +54,20 @@
 #include "ondemand/ODTaskThread.h"
 #include "SelectedRegion.h"
 
+class Tags;
 class Track;
 class TrackList;
 
 struct UndoStackElem;
 struct UndoState {
-   UndoState(std::unique_ptr<TrackList> &&tracks_, const SelectedRegion &selectedRegion_)
-      : tracks(std::move(tracks_)), selectedRegion(selectedRegion_)
+   UndoState(std::unique_ptr<TrackList> &&tracks_,
+      const std::shared_ptr<Tags> &tags_,
+      const SelectedRegion &selectedRegion_)
+      : tracks(std::move(tracks_)), tags(tags_), selectedRegion(selectedRegion_)
    {}
 
    std::unique_ptr<TrackList> tracks;
+   std::shared_ptr<Tags> tags;
    SelectedRegion selectedRegion; // by value
 };
 
@@ -91,10 +95,11 @@ class AUDACITY_DLL_API UndoManager {
 
    void PushState(const TrackList * l,
                   const SelectedRegion &selectedRegion,
+                  const std::shared_ptr<Tags> &tags,
                   const wxString &longDescription, const wxString &shortDescription,
                   UndoPush flags = UndoPush::AUTOSAVE);
    void ModifyState(const TrackList * l,
-                    const SelectedRegion &selectedRegion);
+                    const SelectedRegion &selectedRegion, const std::shared_ptr<Tags> &tags);
    void ClearStates();
    void RemoveStates(int num);  // removes the 'num' oldest states
    void RemoveStateAt(int n);   // removes the n'th state (1 is oldest)

src/export/Export.cpp

@@ -364,7 +364,7 @@ bool Exporter::Process(AudacityProject *project, bool selectedOnly, double t0, d
 
    // Let user edit MetaData
    if (mPlugins[mFormat]->GetCanMetaData(mSubFormat)) {
-      if (!(project->GetTags()->ShowEditDialog(project, _("Edit Metadata Tags"), mProject->GetShowId3Dialog()))) {
+      if (!(project->DoEditMetadata(_("Edit Metadata Tags for Export"), _("Exported Tags"), mProject->GetShowId3Dialog()))) {
          return false;
       }
    }

src/export/ExportFFmpeg.cpp

@@ -312,7 +312,8 @@ bool ExportFFmpeg::Init(const char *shortname, AudacityProject *project, const T
    if (!InitCodecs(project))
       return false;
 
-   if (metadata == NULL) metadata = project->GetTags();
+   if (metadata == NULL)
+      metadata = project->GetTags();
 
    // Add metadata BEFORE writing the header.
    // At the moment that works with ffmpeg-git and ffmpeg-0.5 for MP4.