From 5fef82dccfe0eba8dd24587be318ef22be319734 Mon Sep 17 00:00:00 2001 From: Paul Licameli Date: Wed, 3 Apr 2019 11:27:15 -0400 Subject: [PATCH] Define Setting classes, bundling config path with settings value... ... the intention being, that no string literal for a path, or its default value, shall ever occur twice in the code, relying on long-distance coincidence of literal values. Instead, a named Setting object is constructed once, then read and written. For now, the Tie... functions in ShuttleGuiBase will take references to implicitly constructed temporary Setting objects. But all should later be made static objects, and the constructors made explicit. --- src/Prefs.cpp | 22 +++- src/Prefs.h | 172 +++++++++++++++++++++++++++++++- src/ShuttleGui.cpp | 16 +-- src/ShuttleGui.h | 31 ++---- src/commands/CommandManager.cpp | 6 ++ src/commands/CommandManager.h | 9 ++ src/commands/GetInfoCommand.cpp | 34 ++++--- src/prefs/WarningsPrefs.cpp | 1 + src/widgets/Warning.cpp | 1 - 9 files changed, 240 insertions(+), 52 deletions(-) diff --git a/src/Prefs.cpp b/src/Prefs.cpp index 1e50017f2..3978762ff 100755 --- a/src/Prefs.cpp +++ b/src/Prefs.cpp @@ -280,7 +280,7 @@ bool ChoiceSetting::Write( const wxString &value ) } EnumSettingBase::EnumSettingBase( - const wxString &key, + const SettingBase &key, EnumValueSymbols symbols, long defaultSymbol, @@ -398,3 +398,23 @@ void PreferenceInitializer::ReinitializeAll() for ( auto pInitializer : allInitializers() ) (*pInitializer)(); } + +wxConfigBase *SettingBase::GetConfig() const +{ + return gPrefs; +} + +bool SettingBase::Delete() +{ + auto config = GetConfig(); + return config && config->DeleteEntry( GetPath() ); +} + +bool BoolSetting::Toggle() +{ + bool value = Read(); + if ( Write( !value ) ) + return !value; + else + return value; +} diff --git a/src/Prefs.h b/src/Prefs.h index 1ffdb6cb7..6fa4cea41 100644 --- a/src/Prefs.h +++ b/src/Prefs.h @@ -36,6 +36,8 @@ // the second part (after the r) indicates the number of times the prefs have been reset within the same version #define AUDACITY_PREFS_VERSION_STRING "1.1.1r1" +#include + #include "../include/audacity/ComponentInterface.h" #include "MemoryX.h" // for wxArrayStringEx #include "widgets/FileConfig.h" @@ -55,6 +57,168 @@ extern int gMenusDirty; struct ByColumns_t{}; extern ByColumns_t ByColumns; +//! Base class for settings objects. It holds a configuration key path. +/* The constructors are non-explicit for convenience */ +class AUDACITY_DLL_API SettingBase +{ +public: + SettingBase( const char *path ) : mPath{ path } {} + SettingBase( const wxChar *path ) : mPath{ path } {} + SettingBase( const wxString &path ) : mPath{ path } {} + + wxConfigBase *GetConfig() const; + + const wxString &GetPath() const { return mPath; } + + //! Delete the key if present, and return true iff it was. + bool Delete(); + +protected: + SettingBase( const SettingBase& ) = default; + const RegistryPath mPath; +}; + +//! Class template adds an in-memory cache of a value to SettingBase. +template< typename T > +class CachingSettingBase : public SettingBase +{ +public: + explicit CachingSettingBase( const SettingBase &path ) + : SettingBase{ path } {} +protected: + CachingSettingBase( const CachingSettingBase & ) = default; + mutable T mCurrentValue{}; + mutable bool mValid{false}; +}; + +//! Class template adds default value, read, and write methods to CachingSetingBase +template< typename T > +class Setting : public CachingSettingBase< T > +{ +public: + using CachingSettingBase< T >::CachingSettingBase; + + using DefaultValueFunction = std::function< T() >; + + //! Usual overload supplies a default value + Setting( const SettingBase &path, const T &defaultValue ) + : CachingSettingBase< T >{ path } + , mDefaultValue{ defaultValue } + {} + + //! This overload causes recomputation of the default each time it is needed + Setting( const SettingBase &path, DefaultValueFunction function ) + : CachingSettingBase< T >{ path } + , mFunction{ function } + {} + + + const T& GetDefault() const + { + if ( mFunction ) + mDefaultValue = mFunction(); + return mDefaultValue; + } + + //! overload of Read returning a success flag + /*! The cache is unchanged if that is false */ + bool Read( T *pVar ) const + { + if ( pVar && this->mValid ) { + *pVar = this->mCurrentValue; + return true; + } + const auto config = this->GetConfig(); + if ( pVar && config ) { + if ((this->mValid = config->Read( this->mPath, &this->mCurrentValue ))) + *pVar = this->mCurrentValue; + return this->mValid; + } + return (this->mValid = false); + } + + //! overload of Read, always returning a value + /*! The value is the default stored in this in case the key is known to be absent from the config; + but it returns type T's default value if there was failure to read the config */ + T Read() const + { + return ReadWithDefault( GetDefault() ); + } + + //! new direct use is discouraged but it may be needed in legacy code + /*! Use the given default in case the preference is not defined, which may not be the + default-default stored in this object. */ + T ReadWithDefault( const T &defaultValue ) const + { + const auto config = this->GetConfig(); + return config + ? ( this->mValid = true, this->mCurrentValue = + config->ReadObject( this->mPath, defaultValue ) ) + : T{}; + } + + //! Write value to config and return true if successful + bool Write( const T &value ) + { + const auto config = this->GetConfig(); + if ( config ) { + this->mCurrentValue = value; + return DoWrite(); + } + return false; + } + + //! Reset to the default value + bool Reset() + { + return Write( GetDefault() ); + } + +protected: + //! Write cached value to config and return true if successful + /*! (But the config object is not flushed) */ + bool DoWrite( ) + { + const auto config = this->GetConfig(); + return this->mValid = + config ? config->Write( this->mPath, this->mCurrentValue ) : false; + } + + mutable T mDefaultValue{}; + const DefaultValueFunction mFunction; +}; + +//! This specialization of Setting for bool adds a Toggle method to negate the saved value +class BoolSetting final : public Setting< bool > +{ +public: + using Setting::Setting; + + //! Write the negation of the previous value, and then return the current value. + bool Toggle(); +}; + +//! Specialization of Setting for int +class IntSetting final : public Setting< int > +{ +public: + using Setting::Setting; +}; + +//! Specialization of Setting for double +class DoubleSetting final : public Setting< double > +{ +public: + using Setting::Setting; +}; + +//! Specialization of Setting for strings +class StringSetting final : public Setting< wxString > +{ +public: + using Setting::Setting; +}; + /// A table of EnumValueSymbol that you can access by "row" with /// operator [] but also allowing access to the "columns" of internal or /// translated strings, and also allowing convenient column-wise construction @@ -90,11 +254,11 @@ class AUDACITY_DLL_API ChoiceSetting { public: ChoiceSetting( - const wxString &key, + const SettingBase &key, EnumValueSymbols symbols, long defaultSymbol = -1 ) - : mKey{ key } + : mKey{ key.GetPath() } , mSymbols{ std::move( symbols ) } @@ -140,7 +304,7 @@ class AUDACITY_DLL_API EnumSettingBase : public ChoiceSetting { public: EnumSettingBase( - const wxString &key, + const SettingBase &key, EnumValueSymbols symbols, long defaultSymbol, @@ -175,7 +339,7 @@ class EnumSetting : public EnumSettingBase public: EnumSetting( - const wxString &key, + const SettingBase &key, EnumValueSymbols symbols, long defaultSymbol, diff --git a/src/ShuttleGui.cpp b/src/ShuttleGui.cpp index 55e683289..1d84710b0 100644 --- a/src/ShuttleGui.cpp +++ b/src/ShuttleGui.cpp @@ -1815,7 +1815,7 @@ bool ShuttleGuiBase::DoStep( int iStep ) /// between gui and stack variable and stack variable and shuttle. wxCheckBox * ShuttleGuiBase::TieCheckBox( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting) + const BoolSetting &Setting) { wxCheckBox * pCheck=NULL; @@ -1832,7 +1832,7 @@ wxCheckBox * ShuttleGuiBase::TieCheckBox( /// between gui and stack variable and stack variable and shuttle. wxCheckBox * ShuttleGuiBase::TieCheckBoxOnRight( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting) + const BoolSetting & Setting) { wxCheckBox * pCheck=NULL; @@ -1849,7 +1849,7 @@ wxCheckBox * ShuttleGuiBase::TieCheckBoxOnRight( /// between gui and stack variable and stack variable and shuttle. wxSlider * ShuttleGuiBase::TieSlider( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting & Setting, const int max, const int min) { @@ -1868,7 +1868,7 @@ wxSlider * ShuttleGuiBase::TieSlider( /// between gui and stack variable and stack variable and shuttle. wxSpinCtrl * ShuttleGuiBase::TieSpinCtrl( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int max, const int min) { @@ -1887,7 +1887,7 @@ wxSpinCtrl * ShuttleGuiBase::TieSpinCtrl( /// between gui and stack variable and stack variable and shuttle. wxTextCtrl * ShuttleGuiBase::TieTextBox( const TranslatableString & Prompt, - const SettingSpec< wxString > &Setting, + const StringSetting & Setting, const int nChars) { wxTextCtrl * pText=(wxTextCtrl*)NULL; @@ -1905,7 +1905,7 @@ wxTextCtrl * ShuttleGuiBase::TieTextBox( /// This one does it for double values... wxTextCtrl * ShuttleGuiBase::TieIntegerTextBox( const TranslatableString & Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int nChars) { wxTextCtrl * pText=(wxTextCtrl*)NULL; @@ -1923,7 +1923,7 @@ wxTextCtrl * ShuttleGuiBase::TieIntegerTextBox( /// This one does it for double values... wxTextCtrl * ShuttleGuiBase::TieNumericTextBox( const TranslatableString & Prompt, - const SettingSpec< double > &Setting, + const DoubleSetting & Setting, const int nChars) { wxTextCtrl * pText=(wxTextCtrl*)NULL; @@ -1984,7 +1984,7 @@ wxChoice *ShuttleGuiBase::TieChoice( /// if null, then use 0, 1, 2, ... wxChoice * ShuttleGuiBase::TieNumberAsChoice( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting & Setting, const TranslatableStrings & Choices, const std::vector * pInternalChoices, int iNoMatchSelector) diff --git a/src/ShuttleGui.h b/src/ShuttleGui.h index 11be1c669..dbca45efd 100644 --- a/src/ShuttleGui.h +++ b/src/ShuttleGui.h @@ -22,6 +22,7 @@ #include "MemoryX.h" #include // for wxLIST_FORMAT_LEFT +#include "Prefs.h" #include "WrappedType.h" class ChoiceSetting; @@ -103,20 +104,6 @@ using wxStaticBoxWrapper = wxStaticBox; using wxSliderWrapper = wxSlider; #endif -template< typename T > class SettingSpec { -public: - SettingSpec( const RegistryPath &path, const T &defaultValue = {} ) - : mPath{ path }, mDefaultValue{ defaultValue } - {} - - const RegistryPath &GetPath() const { return mPath; } - const T &GetDefault() const { return mDefaultValue; } - -private: - RegistryPath mPath; - T mDefaultValue; -}; - namespace DialogDefinition { struct Item { @@ -450,10 +437,10 @@ public: // so it doesn't need an argument that is writeable. virtual wxCheckBox * TieCheckBox( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting); + const BoolSetting &Setting); virtual wxCheckBox * TieCheckBoxOnRight( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting); + const BoolSetting &Setting); virtual wxChoice *TieChoice( const TranslatableString &Prompt, @@ -466,31 +453,31 @@ public: // emitting scripting information about Preferences. virtual wxChoice * TieNumberAsChoice( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const TranslatableStrings & Choices, const std::vector * pInternalChoices = nullptr, int iNoMatchSelector = 0 ); virtual wxTextCtrl * TieTextBox( const TranslatableString &Prompt, - const SettingSpec< wxString > &Setting, + const StringSetting &Setting, const int nChars); virtual wxTextCtrl * TieIntegerTextBox( const TranslatableString & Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int nChars); virtual wxTextCtrl * TieNumericTextBox( const TranslatableString & Prompt, - const SettingSpec< double > &Setting, + const DoubleSetting &Setting, const int nChars); virtual wxSlider * TieSlider( const TranslatableString & Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int max, const int min = 0); virtual wxSpinCtrl * TieSpinCtrl( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int max, const int min); //-- End of variants. diff --git a/src/commands/CommandManager.cpp b/src/commands/CommandManager.cpp index 18cd0041d..3e7031a47 100644 --- a/src/commands/CommandManager.cpp +++ b/src/commands/CommandManager.cpp @@ -586,6 +586,12 @@ auto CommandManager::Options::MakeCheckFn( return [=](AudacityProject&){ return gPrefs->ReadBool( key, defaultValue ); }; } +auto CommandManager::Options::MakeCheckFn( + const BoolSetting &setting ) -> CheckFn +{ + return MakeCheckFn( setting.GetPath(), setting.GetDefault() ); +} + /// /// Add a list of menu items to the current menu. When the user selects any /// one of these, the given functor will be called diff --git a/src/commands/CommandManager.h b/src/commands/CommandManager.h index c844eb6b3..afc98c60a 100644 --- a/src/commands/CommandManager.h +++ b/src/commands/CommandManager.h @@ -35,6 +35,8 @@ class wxMenu; class wxMenuBar; using CommandParameter = CommandID; +class BoolSetting; + struct MenuBarListEntry; struct SubMenuListEntry; struct CommandListEntry; @@ -139,6 +141,11 @@ class AUDACITY_DLL_API CommandManager final checker = MakeCheckFn( key, defaultValue ); return std::move(*this); } + // Take a BoolSetting + Options &&CheckTest ( const BoolSetting &setting ) && { + checker = MakeCheckFn( setting ); + return std::move(*this); + } const wxChar *accel{ wxT("") }; CheckFn checker; // default value means it's not a check item @@ -155,6 +162,8 @@ class AUDACITY_DLL_API CommandManager final private: static CheckFn MakeCheckFn( const wxString key, bool defaultValue ); + static CheckFn + MakeCheckFn( const BoolSetting &setting ); }; void AddItemList(const CommandID & name, diff --git a/src/commands/GetInfoCommand.cpp b/src/commands/GetInfoCommand.cpp index 080422a88..a99137dd9 100644 --- a/src/commands/GetInfoCommand.cpp +++ b/src/commands/GetInfoCommand.cpp @@ -191,6 +191,8 @@ bool GetInfoCommand::SendMenus(const CommandContext &context) return true; } +#include "../Prefs.h" + namespace { /**************************************************************************//** @@ -204,10 +206,10 @@ public: wxCheckBox * TieCheckBox( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting) override; + const BoolSetting &Setting) override; wxCheckBox * TieCheckBoxOnRight( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting) override; + const BoolSetting &Setting) override; wxChoice *TieChoice( const TranslatableString &Prompt, @@ -215,30 +217,30 @@ public: wxChoice * TieNumberAsChoice( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const TranslatableStrings & Choices, const std::vector * pInternalChoices, int iNoMatchSelector ) override; wxTextCtrl * TieTextBox( const TranslatableString &Prompt, - const SettingSpec< wxString > &Setting, + const StringSetting &Setting, const int nChars) override; wxTextCtrl * TieIntegerTextBox( const TranslatableString & Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int nChars) override; wxTextCtrl * TieNumericTextBox( const TranslatableString & Prompt, - const SettingSpec< double > &Setting, + const DoubleSetting &Setting, const int nChars) override; wxSlider * TieSlider( const TranslatableString & Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int max, const int min = 0) override; wxSpinCtrl * TieSpinCtrl( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int max, const int min) override; }; @@ -256,7 +258,7 @@ ShuttleGuiGetDefinition::~ShuttleGuiGetDefinition(void) wxCheckBox * ShuttleGuiGetDefinition::TieCheckBox( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting) + const BoolSetting &Setting) { StartStruct(); AddItem( Setting.GetPath(), "id" ); @@ -269,7 +271,7 @@ wxCheckBox * ShuttleGuiGetDefinition::TieCheckBox( wxCheckBox * ShuttleGuiGetDefinition::TieCheckBoxOnRight( const TranslatableString &Prompt, - const SettingSpec< bool > &Setting) + const BoolSetting &Setting) { StartStruct(); AddItem( Setting.GetPath(), "id" ); @@ -301,7 +303,7 @@ wxChoice * ShuttleGuiGetDefinition::TieChoice( wxChoice * ShuttleGuiGetDefinition::TieNumberAsChoice( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const TranslatableStrings & Choices, const std::vector * pInternalChoices, int iNoMatchSelector) { @@ -320,7 +322,7 @@ wxChoice * ShuttleGuiGetDefinition::TieNumberAsChoice( wxTextCtrl * ShuttleGuiGetDefinition::TieTextBox( const TranslatableString &Prompt, - const SettingSpec< wxString > &Setting, + const StringSetting &Setting, const int nChars) { StartStruct(); @@ -334,7 +336,7 @@ wxTextCtrl * ShuttleGuiGetDefinition::TieTextBox( wxTextCtrl * ShuttleGuiGetDefinition::TieIntegerTextBox( const TranslatableString & Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int nChars) { StartStruct(); @@ -348,7 +350,7 @@ wxTextCtrl * ShuttleGuiGetDefinition::TieIntegerTextBox( wxTextCtrl * ShuttleGuiGetDefinition::TieNumericTextBox( const TranslatableString & Prompt, - const SettingSpec< double > &Setting, + const DoubleSetting &Setting, const int nChars) { StartStruct(); @@ -362,7 +364,7 @@ wxTextCtrl * ShuttleGuiGetDefinition::TieNumericTextBox( wxSlider * ShuttleGuiGetDefinition::TieSlider( const TranslatableString & Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int max, const int min) { @@ -377,7 +379,7 @@ wxSlider * ShuttleGuiGetDefinition::TieSlider( wxSpinCtrl * ShuttleGuiGetDefinition::TieSpinCtrl( const TranslatableString &Prompt, - const SettingSpec< int > &Setting, + const IntSetting &Setting, const int max, const int min) { diff --git a/src/prefs/WarningsPrefs.cpp b/src/prefs/WarningsPrefs.cpp index 450af98ca..5dc25952d 100644 --- a/src/prefs/WarningsPrefs.cpp +++ b/src/prefs/WarningsPrefs.cpp @@ -22,6 +22,7 @@ #include +#include "../Prefs.h" #include "../ShuttleGui.h" //////////////////////////////////////////////////////////////////////////////// diff --git a/src/widgets/Warning.cpp b/src/widgets/Warning.cpp index 97171093c..deddb61da 100644 --- a/src/widgets/Warning.cpp +++ b/src/widgets/Warning.cpp @@ -19,7 +19,6 @@ the ability to not see similar warnings again for this session. #include "Warning.h" -#include "../Prefs.h" #include "../ShuttleGui.h" #include