cleanup: remove unnecessary variables in ArticleMaker

4 variables are removed because they are always read-only and always the same as global config. There is no reason to keep or update a separate copy.

.clang-format

@@ -52,6 +52,7 @@ IndentAccessModifiers: false
 IndentPPDirectives: BeforeHash
 MaxEmptyLinesToKeep: 2
 NamespaceIndentation: Inner
+PackConstructorInitializers: Never
 PointerAlignment: Middle
 ReflowComments: false
 SortIncludes: false

article_maker.cc

@@ -24,24 +24,13 @@ using std::list;
 
 ArticleMaker::ArticleMaker( vector< sptr< Dictionary::Class > > const & dictionaries_,
                             vector< Instances::Group > const & groups_,
-                            const Config::Preferences & cfg_,
-                            QString const & displayStyle_,
-                            QString const & addonStyle_):
+                            const Config::Preferences & cfg_ ):
   dictionaries( dictionaries_ ),
   groups( groups_ ),
-  cfg(cfg_),
-  displayStyle( displayStyle_ ),
-  addonStyle( addonStyle_ ),
-  collapseBigArticles( true )
-, articleLimitSize( 500 )
+  cfg( cfg_ )
 {
 }
 
-void ArticleMaker::setDisplayStyle( QString const & st, QString const & adst )
-{
-  displayStyle = st;
-  addonStyle = adst;
-}
 
 std::string ArticleMaker::makeHtmlHeader( QString const & word,
                                           QString const & icon,
@@ -87,19 +76,19 @@ std::string ArticleMaker::makeHtmlHeader( QString const & word,
   {
     result += R"(<link href="qrc:///article-style.css"  media="all" rel="stylesheet" type="text/css">)";
 
-    if ( displayStyle.size() )
+    if ( cfg.displayStyle.size() )
     {
       // Load an additional stylesheet
-      QString displayStyleCssFile = QString("qrc:///article-style-st-%1.css").arg(displayStyle);
+      QString displayStyleCssFile = QString("qrc:///article-style-st-%1.css").arg(cfg.displayStyle);
       result += "<link href=\"" + displayStyleCssFile.toStdString() +
                 R"("  media="all" rel="stylesheet" type="text/css">)";
     }
 
     result += readCssFile(Config::getUserCssFileName() ,"all");
 
-    if( !addonStyle.isEmpty() )
+    if( !cfg.addonStyle.isEmpty() )
     {
-      QString name = Config::getStylesDir() + addonStyle
+      QString name = Config::getStylesDir() + cfg.addonStyle
                      + QDir::separator() + "article-style.css";
 
       result += readCssFile(name ,"all");
@@ -122,9 +111,9 @@ std::string ArticleMaker::makeHtmlHeader( QString const & word,
 
     result += readCssFile(Config::getUserCssPrintFileName() ,"print");
 
-    if( !addonStyle.isEmpty() )
+    if( !cfg.addonStyle.isEmpty() )
     {
-      QString name = Config::getStylesDir() + addonStyle
+      QString name = Config::getStylesDir() + cfg.addonStyle
                      + QDir::separator() + "article-style-print.css";
       result += readCssFile(name ,"print");
     }
@@ -369,13 +358,13 @@ sptr< Dictionary::DataRequest > ArticleMaker::makeDefinitionFor(
 
     return  std::make_shared<ArticleRequest>( phrase, activeGroup ? activeGroup->name : "",
                                contexts, unmutedDicts, header,
-                               collapseBigArticles ? articleLimitSize : -1,
+                               cfg.collapseBigArticles ? cfg.articleSizeLimit : -1,
                                cfg.alwaysExpandOptionalParts, ignoreDiacritics );
   }
   else
     return std::make_shared<ArticleRequest>( phrase, activeGroup ? activeGroup->name : "",
                                contexts, activeDicts, header,
-                               collapseBigArticles ? articleLimitSize : -1,
+                               cfg.collapseBigArticles ? cfg.articleSizeLimit : -1,
                                cfg.alwaysExpandOptionalParts, ignoreDiacritics );
 }
 
@@ -423,12 +412,6 @@ sptr< Dictionary::DataRequest > ArticleMaker::makePicturePage( string const & ur
   return r;
 }
 
-void ArticleMaker::setCollapseParameters( bool autoCollapse, int articleSize )
-{
-  collapseBigArticles = autoCollapse;
-  articleLimitSize = articleSize;
-}
-
 
 bool ArticleMaker::adjustFilePath( QString & fileName )
 {

article_maker.hh

@@ -22,26 +22,15 @@ class ArticleMaker: public QObject
   std::vector< Instances::Group > const & groups;
   const Config::Preferences & cfg;
 
-  QString displayStyle, addonStyle;
-
-  bool collapseBigArticles;
-  int articleLimitSize;
-
 public:
 
   /// On construction, a reference to all dictionaries and a reference all
   /// groups' instances are to be passed. Those references are kept stored as
   /// references, and as such, any changes to them would reflect on the results
   /// of the inquiries, although those changes are perfectly legal.
-  ArticleMaker( std::vector< sptr< Dictionary::Class > > const & dictionaries,
-                std::vector< Instances::Group > const & groups,
-                const Config::Preferences & cfg,
-                QString const & displayStyle,
-                QString const & addonStyle);
-
-  /// Sets the display style to use for any new requests. This affects the
-  /// choice of the stylesheet file.
-  void setDisplayStyle( QString const &, QString const & addonStyle );
+ ArticleMaker( std::vector< sptr< Dictionary::Class > > const & dictionaries,
+               std::vector< Instances::Group > const & groups,
+               const Config::Preferences & cfg );
 
   /// Looks up the given phrase within the given group, and creates a full html
   /// page text containing its definition.
@@ -74,9 +63,6 @@ public:
   /// Return true if path successfully adjusted
   static bool adjustFilePath( QString & fileName );
 
-  /// Set collapse articles parameters
-  void setCollapseParameters( bool autoCollapse, int articleSize );
-
 private:
   std::string readCssFile(QString const& fileName, std::string type) const;
   /// Makes everything up to and including the opening body tag.

mainwindow.cc

@@ -129,8 +129,7 @@ MainWindow::MainWindow( Config::Class & cfg_ ):
   cfg( cfg_ ),
   history( History::Load(), cfg_.preferences.maxStringsInHistory, cfg_.maxHeadwordSize ),
   dictionaryBar( this, configEvents, cfg.editDictionaryCommandLine, cfg.preferences.maxDictionaryRefsInContextMenu ),
-  articleMaker( dictionaries, groupInstances, cfg.preferences , cfg.preferences.displayStyle,
-                cfg.preferences.addonStyle ),
+  articleMaker( dictionaries, groupInstances, cfg.preferences ),
   articleNetMgr( this, dictionaries, articleMaker,
                  cfg.preferences.disallowContentFromOtherSites, cfg.preferences.hideGoldenDictHeader ),
   dictNetMgr( this ),
@@ -187,8 +186,6 @@ MainWindow::MainWindow( Config::Class & cfg_ ):
 
   ui.setupUi( this );
 
-  articleMaker.setCollapseParameters( cfg.preferences.collapseBigArticles, cfg.preferences.articleSizeLimit );
-
   // Set own gesture recognizers
 #ifndef Q_OS_MAC
   Gestures::registerRecognizers();
@@ -2201,8 +2198,6 @@ void MainWindow::editPreferences()
     p.fts.ignoreWordsOrder = cfg.preferences.fts.ignoreWordsOrder;
     p.fts.ignoreDiacritics = cfg.preferences.fts.ignoreDiacritics;
 
-    bool needReload = false;
-
     // See if we need to reapply Qt stylesheets
     if( cfg.preferences.displayStyle != p.displayStyle ||
       cfg.preferences.darkMode != p.darkMode )
@@ -2210,42 +2205,10 @@ void MainWindow::editPreferences()
       applyQtStyleSheet( p.addonStyle, p.displayStyle, p.darkMode );
     }
 
-    // see if we need to reapply articleview style
-    if( cfg.preferences.displayStyle != p.displayStyle ||
-      cfg.preferences.addonStyle != p.addonStyle ||
-      cfg.preferences.darkReaderMode != p.darkReaderMode )
-    {
-      articleMaker.setDisplayStyle( p.displayStyle, p.addonStyle );
-      needReload = true;
-    }
-
-    if( cfg.preferences.collapseBigArticles != p.collapseBigArticles
-        || cfg.preferences.articleSizeLimit != p.articleSizeLimit )
-    {
-      articleMaker.setCollapseParameters( p.collapseBigArticles, p.articleSizeLimit );
-    }
-
-    // See if we need to reapply expand optional parts mode
-    if( cfg.preferences.alwaysExpandOptionalParts != p.alwaysExpandOptionalParts )
-    {
-      needReload = true;
-    }
-
     // See if we need to change help language
     if( cfg.preferences.helpLanguage != p.helpLanguage )
       closeGDHelp();
 
-    for( int x = 0; x < ui.tabWidget->count(); ++x )
-    {
-      ArticleView & view =
-        dynamic_cast< ArticleView & >( *( ui.tabWidget->widget( x ) ) );
-
-      view.setSelectionBySingleClick( p.selectWordBySingleClick );
-
-      if( needReload )
-        view.reload();
-    }
-
     if( cfg.preferences.historyStoreInterval != p.historyStoreInterval )
       history.setSaveInterval( p.historyStoreInterval );
 
@@ -2254,8 +2217,33 @@ void MainWindow::editPreferences()
 
     if( cfg.preferences.maxNetworkCacheSize != p.maxNetworkCacheSize )
       setupNetworkCache( p.maxNetworkCacheSize );
+
+    bool needReload =
+      ( cfg.preferences.displayStyle != p.displayStyle
+        || cfg.preferences.addonStyle != p.addonStyle
+        || cfg.preferences.darkReaderMode != p.darkReaderMode
+        || cfg.preferences.collapseBigArticles != p.collapseBigArticles
+        || cfg.preferences.articleSizeLimit != p.articleSizeLimit
+        || cfg.preferences.alwaysExpandOptionalParts != p.alwaysExpandOptionalParts // DSL format's special feature
+      );
+
+    // This line must be here because the components below require cfg's value to reconfigure
+    // After this point, p must not be accessed.
     cfg.preferences = p;
 
+    // Loop through all tabs and reload pages due to ArticleMaker's change.
+    for( int x = 0; x < ui.tabWidget->count(); ++x )
+    {
+      ArticleView & view =
+        dynamic_cast< ArticleView & >( *( ui.tabWidget->widget( x ) ) );
+
+      view.setSelectionBySingleClick( p.selectWordBySingleClick );
+
+      if( needReload ) {
+        view.reload();
+      }
+    }
+
     audioPlayerFactory.setPreferences( cfg.preferences );
 
     updateTrayIcon();