Add new review recommendations.

Clean up extra code, remake static title, move settings to UpdateManager class.

src/AudacityApp.h

@@ -33,7 +33,6 @@ class Importer;
 class CommandHandler;
 class AppCommandEvent;
 class AudacityProject;
-class UpdateManager;
 
 class AudacityApp final : public wxApp {
  public:

src/prefs/ApplicationPrefs.cpp

@@ -16,6 +16,7 @@
 
 
 #include "ApplicationPrefs.h"
+#include "update/UpdateManager.h"
 
 #include <wx/defs.h>
 
@@ -26,9 +27,6 @@
 
 static ComponentInterfaceSymbol s_ComponentInterfaceSymbol{ XO("Application") };
 
-BoolSetting ApplicationPrefsSettings::DefaultUpdatesCheckingFlag{
-    L"/Update/DefaultUpdatesChecking", true };
-
 ApplicationPrefs::ApplicationPrefs(wxWindow * parent, wxWindowID winid)
 :  PrefsPanel(parent, winid, XO("Application"))
 {
@@ -74,7 +72,7 @@ void ApplicationPrefs::PopulateOrExchange(ShuttleGui & S)
    {
       S.TieCheckBox(
           XO("&Check for Updates...").Stripped(TranslatableString::Ellipses | TranslatableString::MenuCodes),
-          ApplicationPrefsSettings::DefaultUpdatesCheckingFlag);
+          UpdatesCheckingSettings::DefaultUpdatesCheckingFlag);
    }
    S.EndStatic();
    S.EndScroller();

src/prefs/ApplicationPrefs.h

@@ -18,10 +18,6 @@
 
 class ShuttleGui;
 
-namespace ApplicationPrefsSettings {
-    extern AUDACITY_DLL_API BoolSetting DefaultUpdatesCheckingFlag;
-}
-
 class ApplicationPrefs final : public PrefsPanel
 {
  public:

src/prefs/PrefsPanel.cpp

@@ -115,7 +115,7 @@ PrefsPanel::Factories
       PathStart,
       {
          {wxT(""),
-       wxT("Device,Playback,Recording,Quality,GUI,Tracks,ImportExport,Directories,Warnings,Effects,KeyConfig,Mouse,Application")
+       wxT("Device,Playback,Recording,Quality,GUI,Tracks,ImportExport,Directories,Warnings,Effects,KeyConfig,Mouse")
          },
          {wxT("/Tracks"), wxT("TracksBehaviors,Spectrum")},
       }

src/update/UpdateDataParser.cpp

@@ -2,7 +2,7 @@
  Audacity: A Digital Audio Editor
 
  @file UpdateDataParser.cpp
- @brief Declare a class that parse update server data format.
+ @brief Declare a class that parses update server data format.
 
  Anton Gerasimov
  **********************************************************************/
@@ -10,6 +10,7 @@
 #include "UpdateDataParser.h"
 
 #include "xml/XMLFileReader.h"
+#include "MemoryX.h"
 
 UpdateDataParser::UpdateDataParser()
 {}
@@ -21,11 +22,9 @@ bool UpdateDataParser::Parse(const VersionPatch::UpdateDataFormat& updateData, V
 {
     XMLFileReader xmlReader;
 
-    mVersionPatch = versionPatch;
-    auto ok = xmlReader.ParseString(this, updateData);
-    mVersionPatch = nullptr;
+    ValueRestorer<VersionPatch*> setter{ mVersionPatch, versionPatch };
 
-    return ok;
+    return xmlReader.ParseString(this, updateData);
 }
 
 wxArrayString UpdateDataParser::SplitChangelogSentences(const wxString& changelogContent)
@@ -40,11 +39,11 @@ wxArrayString UpdateDataParser::SplitChangelogSentences(const wxString& changelo
     while ((pos = s.find(delimiter)) != std::string::npos)
     {
         token = s.substr(0, pos + 1);
-        changelogSentenceList.Add(token);
+        changelogSentenceList.Add(wxString(token).Trim());
 
         s.erase(0, pos + delimiter.length());
     }
-    changelogSentenceList.Add(s);
+    changelogSentenceList.Add(wxString(s).Trim());
 
     return changelogSentenceList;
 }
@@ -107,6 +106,8 @@ void UpdateDataParser::HandleXMLEndTag(const wxChar* tag)
         mXmlParsingState == XmlParsedTags::kLinkTag)
         mXmlParsingState = XmlParsedTags::kNotUsedTag;
 
+    // If it is our working OS, using "kOsTag" for keeping ready for parse state for both tags:
+    // <Version> and <Link>, that ordered one after another.
     if (mXmlParsingState == XmlParsedTags::kVersionTag)
         mXmlParsingState = XmlParsedTags::kOsTag;
 }
@@ -116,23 +117,23 @@ void UpdateDataParser::HandleXMLContent(const wxString& content)
     if (mVersionPatch == nullptr)
         return;
 
-    wxString trimedContent(content);
+    wxString trimmedContent(content);
 
     switch (mXmlParsingState)
     {
     case XmlParsedTags::kDescriptionTag:
-        trimedContent.Trim(true).Trim(false);
-        mVersionPatch->changelog = SplitChangelogSentences(trimedContent);
+        trimmedContent.Trim(true).Trim(false);
+        mVersionPatch->changelog = SplitChangelogSentences(trimmedContent);
         break;
 
     case XmlParsedTags::kVersionTag:
-        trimedContent.Trim(true).Trim(false);
-        mVersionPatch->version = VersionId::ParseFromString(trimedContent);
+        trimmedContent.Trim(true).Trim(false);
+        mVersionPatch->version = VersionId::ParseFromString(trimmedContent);
         break;
 
     case XmlParsedTags::kLinkTag:
-        trimedContent.Trim(true).Trim(false);
-        mVersionPatch->download = trimedContent;
+        trimmedContent.Trim(true).Trim(false);
+        mVersionPatch->download = trimmedContent;
         break;
 
     default:

src/update/UpdateDataParser.h

@@ -2,7 +2,7 @@
  Audacity: A Digital Audio Editor
 
  @file UpdateDataParser.h
- @brief Declare a class that parse update server data format.
+ @brief Declare a class that parses update server data format.
 
  Anton Gerasimov
  **********************************************************************/
@@ -15,19 +15,19 @@
 #include <wx/arrstr.h>
 #include <map>
 
-/// A class that parse update server data format.
+/// A class that parses update server data format.
 class UpdateDataParser final : public XMLTagHandler
 {
 public:
     UpdateDataParser();
     ~UpdateDataParser();
 
-    /// <summary>
-    /// Parsing from update data format to VersionPatch fields.
-    /// </summary>
-    /// <param name="updateData">Input data.</param>
-    /// <param name="versionPatch">Parsed output data.</param>
-    /// <returns>True if success.</returns>
+    //! Parsing from update data format to VersionPatch fields.
+    /*!
+       @param updateData InputData.
+       @param versionPath Parsed output data.
+       @return True if success.
+    */
     bool Parse(const VersionPatch::UpdateDataFormat& updateData, VersionPatch* versionPatch);
 
 private:

src/update/UpdateManager.cpp

@@ -2,7 +2,7 @@
  Audacity: A Digital Audio Editor
 
  @file UpdateManager.cpp
- @brief Declare a class that managing of updates.
+ @brief Declare a class that handles managing of updates.
 
  Anton Gerasimov
  **********************************************************************/
@@ -15,12 +15,16 @@
 #include "Request.h"
 
 #include "widgets/ErrorDialog.h"
-#include "prefs/ApplicationPrefs.h"
 
 #include <wx/platinfo.h>
 #include <wx/utils.h>
 #include <wx/frame.h>
 
+#include <mutex>
+
+BoolSetting UpdatesCheckingSettings::DefaultUpdatesCheckingFlag{
+    L"/Update/DefaultUpdatesChecking", true };
+
 static const char* prefsUpdateScheduledTime = "/Update/UpdateScheduledTime";
 
 enum { ID_TIMER = wxID_HIGHEST + 1 };
@@ -30,7 +34,7 @@ BEGIN_EVENT_TABLE(UpdateManager, wxEvtHandler)
 END_EVENT_TABLE()
 
 UpdateManager::UpdateManager()
-    : mTrackingInterval(
+    : mUpdateCheckingInterval(
         std::chrono::milliseconds(std::chrono::hours(12)).count())
 {}
 
@@ -112,15 +116,15 @@ void UpdateManager::GetUpdates()
 
 void UpdateManager::OnTimer(wxTimerEvent& WXUNUSED(event))
 {
-    bool updatesCheckingEnabled = ApplicationPrefsSettings::DefaultUpdatesCheckingFlag.Read();
+    bool updatesCheckingEnabled = UpdatesCheckingSettings::DefaultUpdatesCheckingFlag.Read();
 
-    if (updatesCheckingEnabled && IsTimeToUpdatesChecking())
+    if (updatesCheckingEnabled && IsTimeForUpdatesChecking())
         GetUpdates();
 
-    mTimer.StartOnce(mTrackingInterval);
+    mTimer.StartOnce(mUpdateCheckingInterval);
 }
 
-bool UpdateManager::IsTimeToUpdatesChecking()
+bool UpdateManager::IsTimeForUpdatesChecking()
 {
     long long nextUpdatesCheckingTime = std::stoll(
         gPrefs->Read(prefsUpdateScheduledTime, "0").ToStdString());
@@ -136,7 +140,7 @@ bool UpdateManager::IsTimeToUpdatesChecking()
     // else this condition allow us to avoid from duplicating update notifications.
     if (nextUpdatesCheckingTime < currentTimeInMillisec)
     {
-        nextUpdatesCheckingTime = currentTimeInMillisec + mTrackingInterval;
+        nextUpdatesCheckingTime = currentTimeInMillisec + mUpdateCheckingInterval;
 
         gPrefs->Write(prefsUpdateScheduledTime,
             wxString(std::to_string(nextUpdatesCheckingTime)));

src/update/UpdateManager.h

@@ -2,7 +2,7 @@
  Audacity: A Digital Audio Editor
 
  @file UpdateManager.h
- @brief Declare a class that managing of updates.
+ @brief Declare a class that handles managing of updates.
 
  Anton Gerasimov
  **********************************************************************/
@@ -18,6 +18,10 @@
 #include <wx/event.h>
 #include <wx/timer.h>
 
+namespace UpdatesCheckingSettings {
+    extern AUDACITY_DLL_API BoolSetting DefaultUpdatesCheckingFlag;
+}
+
 /// A class that managing of updates.
 /**
     Opt-in request and show update dialog by the scheduled time.
@@ -42,12 +46,12 @@ private:
     VersionPatch mVersionPatch;
 
     wxTimer mTimer;
-    const int mTrackingInterval;
+    const int mUpdateCheckingInterval;
 
     void OnTimer(wxTimerEvent& event);
 
     /// Scheduling update time for avoiding multiplying update notifications.
-    bool IsTimeToUpdatesChecking();
+    bool IsTimeForUpdatesChecking();
 
 public:
     DECLARE_EVENT_TABLE()

src/update/UpdatePopupDialog.cpp

@@ -7,11 +7,11 @@
  Anton Gerasimov
  **********************************************************************/
 
-#include "update/UpdatePopupDialog.h"
+#include "UpdatePopupDialog.h"
+#include "UpdateManager.h"
 
 #include "ShuttleGui.h"
 #include "widgets/HelpSystem.h"
-#include "prefs/ApplicationPrefs.h"
 
 #include <wx/debug.h>
 #include <wx/sstream.h>
@@ -45,7 +45,7 @@ UpdatePopupDialog::UpdatePopupDialog (wxWindow* parent, const VersionPatch& vers
 
             S.Id (DontShowID).AddCheckBox (
                 XO ("Don't show this again at start up"),
-                !ApplicationPrefsSettings::DefaultUpdatesCheckingFlag.Read());
+                !UpdatesCheckingSettings::DefaultUpdatesCheckingFlag.Read());
 
             S.Prop(1).AddSpace(1, 0, 1);
 
@@ -80,7 +80,7 @@ void UpdatePopupDialog::OnSkip (wxCommandEvent&)
 
 void UpdatePopupDialog::OnDontShow (wxCommandEvent& event)
 {
-    ApplicationPrefsSettings::DefaultUpdatesCheckingFlag.Write(!event.IsChecked());
+    UpdatesCheckingSettings::DefaultUpdatesCheckingFlag.Write(!event.IsChecked());
 }
 
 HtmlWindow* UpdatePopupDialog::AddHtmlContent (wxWindow* parent)
@@ -88,13 +88,10 @@ HtmlWindow* UpdatePopupDialog::AddHtmlContent (wxWindow* parent)
     wxStringOutputStream o;
     wxTextOutputStream informationStr (o);
 
-    // i18n-hint Substitution of version number for %s.
-    static const auto title = XC("Audacity %s is available!", "update dialog")
-        .Format(mVersionPatch.version.GetString());
-
     informationStr
         << wxT("<html><body><h3>")
-        << title.Translation()
+        // i18n-hint Substitution of version number for %s.
+        << XC("Audacity %s is available!", "update dialog").Format(mVersionPatch.version.GetString()).Translation()
         << wxT("</h3><h5>")
         << XC("Changelog", "update dialog")
         << wxT("</h5><p>");

src/update/UpdatePopupDialog.h

@@ -12,20 +12,12 @@
 #include "widgets/wxPanelWrapper.h"
 #include "wx/string.h"
 
-#include "Project.h"
-
 #include "VersionPatch.h"
 
 class HtmlWindow;
 class wxWindow;
-class AudacityProject;
-class UpdateManager;
 
 /// Show dialog window with update information for the user.
-/**
-    Support user action behaviors as skip, download a new version,
-    and a checkbox that does not allow tracking version again.
-*/
 class UpdatePopupDialog final : public wxDialogWrapper
 {
     DECLARE_DYNAMIC_CLASS (AboutDialog)

src/update/VersionId.cpp

@@ -67,7 +67,5 @@ bool VersionId::operator< (const VersionId& other)
 
 bool VersionId::operator> (const VersionId& other)
 {
-    if (*this == other) return false;
-
-    return !(*this < other);
+    return !(*this < other) && (*this != other);
 }