Merge 3c5233f2a1 into bd13598cc2

This reverts commit 1272ea2d20a7908b9a69d6b551511916f97cc024.

.github/workflows/PR-check-cmake.yml

@@ -67,7 +67,7 @@ jobs:
           libzim \
           qt
 
-          wget ftp://ftp.sra.co.jp/pub/misc/eb/eb-4.4.3.tar.bz2
+          wget https://github.com/mistydemeo/eb/releases/download/v4.4.3/eb-4.4.3.tar.bz2
           tar xvjf eb-4.4.3.tar.bz2
           cd eb-4.4.3 && ./configure && make -j 8 && sudo make install && cd ..
 

cmake/Deps_Unix.cmake

@@ -3,11 +3,6 @@
 if (APPLE)
     # old & new homebrew's include paths
     target_include_directories(${GOLDENDICT} PRIVATE /usr/local/include /opt/homebrew/include)
-
-    # libzim depends on ICU, but the ICU from homebrew is "key-only", we need to manually prioritize it
-    # See `brew info icu4c` if this no longer works
-    # Note: Remove icu4c@75 if it fails again
-    set(ENV{PKG_CONFIG_PATH} "$ENV{PKG_CONFIG_PATH}:/usr/local/opt/icu4c@75/lib/pkgconfig:/opt/homebrew/opt/icu4c@75/lib/pkgconfig:/usr/local/opt/icu4c/lib/pkgconfig:/opt/homebrew/opt/icu4c/lib/pkgconfig")
 endif ()
 
 target_include_directories(${GOLDENDICT} PRIVATE
@@ -82,13 +77,31 @@ if (WITH_EPWING_SUPPORT)
 endif ()
 
 if (WITH_ZIM)
+    if (APPLE)
+        # ICU from homebrew is "key-only", we need to manually prioritize it -> see `brew info icu4c`
+        set(ENV{PKG_CONFIG_PATH} "$ENV{PKG_CONFIG_PATH}:/usr/local/opt/icu4c@76/lib/pkgconfig:/opt/homebrew/opt/icu4c@76/lib/pkgconfig:/usr/local/opt/icu4c/lib/pkgconfig:/opt/homebrew/opt/icu4c/lib/pkgconfig")
+    endif ()
+
     pkg_check_modules(ZIM REQUIRED IMPORTED_TARGET libzim)
     target_link_libraries(${GOLDENDICT} PRIVATE PkgConfig::ZIM)
+
     if (APPLE)
-        # For some reason, icu4c as transitive dependency of libzim may not be copied into app bundle,
+        # icu4c as transitive dependency of libzim may not be copied into app bundle, so we directly depends on it to assist macdeployqt
-        # so we directly depends on it to help macdeployqt or whatever
+        # Why such complexities: 1) System or XCode SDKS's icu exists 2) icu itself is depended by various stuffs and homebrew may need multiple versions of it
-        pkg_check_modules(BREW_ICU_FOR_LIBZIM_FORCE_LINK REQUIRED IMPORTED_TARGET icu-i18n icu-uc)
+        pkg_check_modules(BREW_ICU REQUIRED IMPORTED_TARGET icu-i18n icu-uc)
-        target_link_libraries(${GOLDENDICT} PUBLIC PkgConfig::BREW_ICU_FOR_LIBZIM_FORCE_LINK)
+        target_link_libraries(${GOLDENDICT} PUBLIC PkgConfig::BREW_ICU)
+
+        # Verify icu <-> zim matches
+        message("Zim include dirs -> ${ZIM_INCLUDE_DIRS}")
+        message("Homebrew icu include dirs-> ${BREW_ICU_INCLUDE_DIRS}")
+
+        list(GET BREW_ICU_INCLUDE_DIRS 0 ONE_OF_BREW_ICU_INCLUDE_DIR)
+        if (ONE_OF_BREW_ICU_INCLUDE_DIR IN_LIST ZIM_INCLUDE_DIRS)
+            message("ZIM OK!")
+        else ()
+            message(FATAL_ERROR "!!!! ZIM <-> icu error -> check `brew info libzim` and `brew list libzim`")
+        endif ()
+        # TODO: get rid of these 💩, how?
     endif ()
 endif ()
 

src/audio/audioplayerfactory.cc

@@ -3,34 +3,39 @@
 
 #include <QScopedPointer>
 #include <QObject>
+#include <utility>
 #include "audioplayerfactory.hh"
 #include "ffmpegaudioplayer.hh"
 #include "multimediaaudioplayer.hh"
 #include "externalaudioplayer.hh"
 #include "gddebug.hh"
 
-AudioPlayerFactory::AudioPlayerFactory( Config::Preferences const & p ):
+AudioPlayerFactory::AudioPlayerFactory( bool useInternalPlayer,
-  useInternalPlayer( p.useInternalPlayer ),
+                                        InternalPlayerBackend internalPlayerBackend,
-  internalPlayerBackend( p.internalPlayerBackend ),
+                                        QString audioPlaybackProgram ):
-  audioPlaybackProgram( p.audioPlaybackProgram )
+  useInternalPlayer( useInternalPlayer ),
+  internalPlayerBackend( std::move( internalPlayerBackend ) ),
+  audioPlaybackProgram( std::move( audioPlaybackProgram ) )
 {
   reset();
 }
 
-void AudioPlayerFactory::setPreferences( Config::Preferences const & p )
+void AudioPlayerFactory::setPreferences( bool new_useInternalPlayer,
+                                         const InternalPlayerBackend & new_internalPlayerBackend,
+                                         const QString & new_audioPlaybackProgram )
 {
-  if ( p.useInternalPlayer != useInternalPlayer ) {
+  if ( useInternalPlayer != new_useInternalPlayer ) {
-    useInternalPlayer     = p.useInternalPlayer;
+    useInternalPlayer     = new_useInternalPlayer;
-    internalPlayerBackend = p.internalPlayerBackend;
+    internalPlayerBackend = new_internalPlayerBackend;
-    audioPlaybackProgram  = p.audioPlaybackProgram;
+    audioPlaybackProgram  = new_audioPlaybackProgram;
     reset();
   }
-  else if ( useInternalPlayer && p.internalPlayerBackend != internalPlayerBackend ) {
+  else if ( useInternalPlayer && internalPlayerBackend != new_internalPlayerBackend ) {
-    internalPlayerBackend = p.internalPlayerBackend;
+    internalPlayerBackend = new_internalPlayerBackend;
     reset();
   }
-  else if ( !useInternalPlayer && p.audioPlaybackProgram != audioPlaybackProgram ) {
+  else if ( !useInternalPlayer && new_audioPlaybackProgram != audioPlaybackProgram ) {
-    audioPlaybackProgram                       = p.audioPlaybackProgram;
+    audioPlaybackProgram                       = new_audioPlaybackProgram;
     ExternalAudioPlayer * const externalPlayer = qobject_cast< ExternalAudioPlayer * >( playerPtr.data() );
     if ( externalPlayer ) {
       setAudioPlaybackProgram( *externalPlayer );
@@ -50,7 +55,7 @@ void AudioPlayerFactory::reset()
     // another object of the same type.
 
 #ifdef MAKE_FFMPEG_PLAYER
-    Q_ASSERT( Config::InternalPlayerBackend::defaultBackend().isFfmpeg()
+    Q_ASSERT( InternalPlayerBackend::defaultBackend().isFfmpeg()
               && "Adjust the code below after changing the default backend." );
 
     if ( !internalPlayerBackend.isQtmultimedia() ) {

src/audio/audioplayerfactory.hh

@@ -4,7 +4,7 @@
 #pragma once
 
 #include "audioplayerinterface.hh"
-#include "config.hh"
+#include "internalplayerbackend.hh"
 
 class ExternalAudioPlayer;
 
@@ -13,8 +13,12 @@ class AudioPlayerFactory
   Q_DISABLE_COPY( AudioPlayerFactory )
 
 public:
-  explicit AudioPlayerFactory( Config::Preferences const & );
+  explicit AudioPlayerFactory( bool useInternalPlayer,
-  void setPreferences( Config::Preferences const & );
+                               InternalPlayerBackend internalPlayerBackend,
+                               QString audioPlaybackProgram );
+  void setPreferences( bool new_useInternalPlayer,
+                       const InternalPlayerBackend & new_internalPlayerBackend,
+                       const QString & new_audioPlaybackProgram );
   /// The returned reference to a smart pointer is valid as long as this object
   /// exists. The pointer to the owned AudioPlayerInterface may change after the
   /// call to setPreferences(), but it is guaranteed to never be null.
@@ -28,7 +32,7 @@ private:
   void setAudioPlaybackProgram( ExternalAudioPlayer & externalPlayer );
 
   bool useInternalPlayer;
-  Config::InternalPlayerBackend internalPlayerBackend;
+  InternalPlayerBackend internalPlayerBackend;
   QString audioPlaybackProgram;
   AudioPlayerPtr playerPtr;
 };

src/audio/internalplayerbackend.cc

@@ -0,0 +1,51 @@
+#include "internalplayerbackend.hh"
+
+bool InternalPlayerBackend::anyAvailable()
+{
+#if defined( MAKE_FFMPEG_PLAYER ) || defined( MAKE_QTMULTIMEDIA_PLAYER )
+  return true;
+#else
+  return false;
+#endif
+}
+
+InternalPlayerBackend InternalPlayerBackend::defaultBackend()
+{
+#if defined( MAKE_FFMPEG_PLAYER )
+  return ffmpeg();
+#elif defined( MAKE_QTMULTIMEDIA_PLAYER )
+  return qtmultimedia();
+#else
+  return InternalPlayerBackend( QString() );
+#endif
+}
+
+QStringList InternalPlayerBackend::nameList()
+{
+  QStringList result;
+#ifdef MAKE_FFMPEG_PLAYER
+  result.push_back( ffmpeg().uiName() );
+#endif
+#ifdef MAKE_QTMULTIMEDIA_PLAYER
+  result.push_back( qtmultimedia().uiName() );
+#endif
+  return result;
+}
+
+bool InternalPlayerBackend::isFfmpeg() const
+{
+#ifdef MAKE_FFMPEG_PLAYER
+  return *this == ffmpeg();
+#else
+  return false;
+#endif
+}
+
+bool InternalPlayerBackend::isQtmultimedia() const
+{
+#ifdef MAKE_QTMULTIMEDIA_PLAYER
+  return *this == qtmultimedia();
+#else
+  return false;
+#endif
+}

src/audio/internalplayerbackend.hh

@@ -0,0 +1,61 @@
+#pragma once
+#include <QStringList>
+
+/// Overly engineered dummy/helper/wrapper "backend", which is not, to manage backends.
+class InternalPlayerBackend
+{
+public:
+  /// Returns true if at least one backend is available.
+  static bool anyAvailable();
+  /// Returns the default backend or null backend if none is available.
+  static InternalPlayerBackend defaultBackend();
+  /// Returns the name list of supported backends.
+  static QStringList nameList();
+
+  /// Returns true if built with FFmpeg player support and the name matches.
+  bool isFfmpeg() const;
+  /// Returns true if built with Qt Multimedia player support and the name matches.
+  bool isQtmultimedia() const;
+
+  QString const & uiName() const
+  {
+    return name;
+  }
+
+  void setUiName( QString const & name_ )
+  {
+    name = name_;
+  }
+
+  bool operator==( InternalPlayerBackend const & other ) const
+  {
+    return name == other.name;
+  }
+
+  bool operator!=( InternalPlayerBackend const & other ) const
+  {
+    return !operator==( other );
+  }
+
+private:
+#ifdef MAKE_FFMPEG_PLAYER
+  static InternalPlayerBackend ffmpeg()
+  {
+    return InternalPlayerBackend( "FFmpeg" );
+  }
+#endif
+
+#ifdef MAKE_QTMULTIMEDIA_PLAYER
+  static InternalPlayerBackend qtmultimedia()
+  {
+    return InternalPlayerBackend( "Qt Multimedia" );
+  }
+#endif
+
+  explicit InternalPlayerBackend( QString const & name_ ):
+    name( name_ )
+  {
+  }
+
+  QString name;
+};

src/config.cc

@@ -119,57 +119,6 @@ QKeySequence HotKey::toKeySequence() const
   ;
 }
 
-
-bool InternalPlayerBackend::anyAvailable()
-{
-#if defined( MAKE_FFMPEG_PLAYER ) || defined( MAKE_QTMULTIMEDIA_PLAYER )
-  return true;
-#else
-  return false;
-#endif
-}
-
-InternalPlayerBackend InternalPlayerBackend::defaultBackend()
-{
-#if defined( MAKE_FFMPEG_PLAYER )
-  return ffmpeg();
-#elif defined( MAKE_QTMULTIMEDIA_PLAYER )
-  return qtmultimedia();
-#else
-  return InternalPlayerBackend( QString() );
-#endif
-}
-
-QStringList InternalPlayerBackend::nameList()
-{
-  QStringList result;
-#ifdef MAKE_FFMPEG_PLAYER
-  result.push_back( ffmpeg().uiName() );
-#endif
-#ifdef MAKE_QTMULTIMEDIA_PLAYER
-  result.push_back( qtmultimedia().uiName() );
-#endif
-  return result;
-}
-
-bool InternalPlayerBackend::isFfmpeg() const
-{
-#ifdef MAKE_FFMPEG_PLAYER
-  return *this == ffmpeg();
-#else
-  return false;
-#endif
-}
-
-bool InternalPlayerBackend::isQtmultimedia() const
-{
-#ifdef MAKE_QTMULTIMEDIA_PLAYER
-  return *this == qtmultimedia();
-#else
-  return false;
-#endif
-}
-
 QString Preferences::sanitizeInputPhrase( QString const & inputWord ) const
 {
   QString result = inputWord;

src/config.hh

@@ -3,19 +3,20 @@
 
 #pragma once
 
-#include <QObject>
+#include "audio/internalplayerbackend.hh"
-#include <QList>
-#include <QString>
-#include <QSize>
-#include <QDateTime>
-#include <QKeySequence>
-#include <QSet>
-#include <QMetaType>
 #include "ex.hh"
+#include <QDateTime>
 #include <QDomDocument>
+#include <QKeySequence>
+#include <QList>
 #include <QLocale>
-#include <optional>
+#include <QMetaType>
+#include <QObject>
+#include <QSet>
+#include <QSize>
+#include <QString>
 #include <QThread>
+#include <optional>
 
 /// Special group IDs
 enum GroupId : unsigned {
@@ -269,66 +270,6 @@ struct CustomFonts
   }
 };
 
-/// This class encapsulates supported backend preprocessor logic,
-/// discourages duplicating backend names in code, which is error-prone.
-class InternalPlayerBackend
-{
-public:
-  /// Returns true if at least one backend is available.
-  static bool anyAvailable();
-  /// Returns the default backend or null backend if none is available.
-  static InternalPlayerBackend defaultBackend();
-  /// Returns the name list of supported backends.
-  static QStringList nameList();
-
-  /// Returns true if built with FFmpeg player support and the name matches.
-  bool isFfmpeg() const;
-  /// Returns true if built with Qt Multimedia player support and the name matches.
-  bool isQtmultimedia() const;
-
-  QString const & uiName() const
-  {
-    return name;
-  }
-
-  void setUiName( QString const & name_ )
-  {
-    name = name_;
-  }
-
-  bool operator==( InternalPlayerBackend const & other ) const
-  {
-    return name == other.name;
-  }
-
-  bool operator!=( InternalPlayerBackend const & other ) const
-  {
-    return !operator==( other );
-  }
-
-private:
-#ifdef MAKE_FFMPEG_PLAYER
-  static InternalPlayerBackend ffmpeg()
-  {
-    return InternalPlayerBackend( "FFmpeg" );
-  }
-#endif
-
-#ifdef MAKE_QTMULTIMEDIA_PLAYER
-  static InternalPlayerBackend qtmultimedia()
-  {
-    return InternalPlayerBackend( "Qt Multimedia" );
-  }
-#endif
-
-  explicit InternalPlayerBackend( QString const & name_ ):
-    name( name_ )
-  {
-  }
-
-  QString name;
-};
-
 /// Various user preferences
 struct Preferences
 {

src/dict/loaddictionaries.cc

@@ -224,7 +224,7 @@ void loadDictionaries( QWidget * parent,
 
   loadDicts.wait();
 
-  if ( loadDicts.getExceptionText().size() ) {
+  if ( !loadDicts.getExceptionText().empty() ) {
     QMessageBox::critical( parent,
                            QCoreApplication::translate( "LoadDictionaries", "Error loading dictionaries" ),
                            QString::fromUtf8( loadDicts.getExceptionText().c_str() ) );

src/ui/editdictionaries.cc

@@ -204,26 +204,11 @@ void EditDictionaries::acceptChangedSources( bool rebuildGroups )
   // Those hold pointers to dictionaries, we need to free them.
   groupInstances.clear();
 
-  groups.clear();
-  orderAndProps.clear();
-
   loadDictionaries( this, cfg, dictionaries, dictNetMgr );
 
-  Instances::updateNames( savedGroups, dictionaries );
-  Instances::updateNames( savedOrder, dictionaries );
-  Instances::updateNames( savedInactive, dictionaries );
-
   if ( rebuildGroups ) {
-    ui.tabs->removeTab( 1 );
+    orderAndProps->rebuild( savedOrder, savedInactive, dictionaries );
-    ui.tabs->removeTab( 1 );
+    groups->rebuild( dictionaries, savedGroups, orderAndProps->getCurrentDictionaryOrder() );
-
-    orderAndProps = new OrderAndProps( this, savedOrder, savedInactive, dictionaries );
-    groups        = new Groups( this, dictionaries, savedGroups, orderAndProps->getCurrentDictionaryOrder() );
-
-    ui.tabs->insertTab( 1, orderAndProps, QIcon( ":/icons/book.svg" ), tr( "&Dictionaries" ) );
-    ui.tabs->insertTab( 2, groups, QIcon( ":/icons/bookcase.svg" ), tr( "&Groups" ) );
-    connect( groups, &Groups::showDictionaryInfo, this, &EditDictionaries::showDictionaryInfo );
-    connect( orderAndProps, &OrderAndProps::showDictionaryHeadwords, this, &EditDictionaries::showDictionaryHeadwords );
   }
   setUpdatesEnabled( true );
 }

src/ui/groups.cc

@@ -64,6 +64,24 @@ Groups::Groups( QWidget * parent,
   countChanged();
 }
 
+void Groups::rebuild( vector< sptr< Dictionary::Class > > const & dicts_,
+                      Config::Groups const & groups_,
+                      Config::Group const & order )
+{
+  this->setUpdatesEnabled( false );
+  dicts  = dicts_;
+  groups = groups_;
+
+  ui.dictionaries->setAsSource();
+  ui.dictionaries->populate( Instances::Group( order, dicts, Config::Group() ).dictionaries, dicts );
+
+  // Populate groups' widget
+  ui.groups->populate( groups, dicts, ui.dictionaries->getCurrentDictionaries() );
+
+  countChanged();
+  this->setUpdatesEnabled( true );
+}
+
 void Groups::editGroup( unsigned id )
 {
   for ( int x = 0; x < groups.size(); ++x ) {

src/ui/groups.hh

@@ -18,7 +18,9 @@ public:
           std::vector< sptr< Dictionary::Class > > const &,
           Config::Groups const &,
           Config::Group const & order );
-
+  void rebuild( std::vector< sptr< Dictionary::Class > > const & dicts_,
+                Config::Groups const & groups_,
+                Config::Group const & order );
   /// Instructs the dialog to position itself on editing the given group.
   void editGroup( unsigned id );
 
@@ -31,7 +33,7 @@ public:
 
 private:
   Ui::Groups ui;
-  std::vector< sptr< Dictionary::Class > > const & dicts;
+  std::vector< sptr< Dictionary::Class > > dicts;
   Config::Groups groups;
 
   QToolButton * groupsListButton;

src/ui/mainwindow.cc

@@ -167,7 +167,8 @@ MainWindow::MainWindow( Config::Class & cfg_ ):
                  cfg.preferences.disallowContentFromOtherSites,
                  cfg.preferences.hideGoldenDictHeader ),
   dictNetMgr( this ),
-  audioPlayerFactory( cfg.preferences ),
+  audioPlayerFactory(
+    cfg.preferences.useInternalPlayer, cfg.preferences.internalPlayerBackend, cfg.preferences.audioPlaybackProgram ),
   wordFinder( this ),
   wordListSelChanged( false ),
   wasMaximized( false ),
@@ -2356,7 +2357,9 @@ void MainWindow::editPreferences()
 #endif
     }
 
-    audioPlayerFactory.setPreferences( cfg.preferences );
+    audioPlayerFactory.setPreferences( cfg.preferences.useInternalPlayer,
+                                       cfg.preferences.internalPlayerBackend,
+                                       cfg.preferences.audioPlaybackProgram );
 
     trayIconUpdateOrInit();
     applyProxySettings();

src/ui/orderandprops.cc

@@ -133,6 +133,25 @@ OrderAndProps::OrderAndProps( QWidget * parent,
   showDictNumbers();
 }
 
+void OrderAndProps::rebuild( Config::Group const & dictionaryOrder,
+                             Config::Group const & inactiveDictionaries,
+                             std::vector< sptr< Dictionary::Class > > const & allDictionaries )
+{
+  Instances::Group order( dictionaryOrder, allDictionaries, Config::Group() );
+  Instances::Group inactive( inactiveDictionaries, allDictionaries, Config::Group() );
+
+  Instances::complementDictionaryOrder( order, inactive, allDictionaries );
+
+  setUpdatesEnabled( false );
+  ui.dictionaryOrder->populate( order.dictionaries, allDictionaries );
+  ui.inactiveDictionaries->populate( inactive.dictionaries, allDictionaries );
+
+  disableDictionaryDescription();
+
+  showDictNumbers();
+  setUpdatesEnabled( true );
+}
+
 Config::Group OrderAndProps::getCurrentDictionaryOrder() const
 {
   Instances::Group g;

src/ui/orderandprops.hh

@@ -17,7 +17,9 @@ public:
                  Config::Group const & dictionaryOrder,
                  Config::Group const & inactiveDictionaries,
                  std::vector< sptr< Dictionary::Class > > const & allDictionaries );
-
+  void rebuild( Config::Group const & dictionaryOrder,
+                Config::Group const & inactiveDictionaries,
+                std::vector< sptr< Dictionary::Class > > const & allDictionaries );
   Config::Group getCurrentDictionaryOrder() const;
   Config::Group getCurrentInactiveDictionaries() const;
 

src/ui/preferences.cc

@@ -294,7 +294,7 @@ Preferences::Preferences( QWidget * parent, Config::Class & cfg_ ):
   ui.pronounceOnLoadMain->setChecked( p.pronounceOnLoadMain );
   ui.pronounceOnLoadPopup->setChecked( p.pronounceOnLoadPopup );
 
-  ui.internalPlayerBackend->addItems( Config::InternalPlayerBackend::nameList() );
+  ui.internalPlayerBackend->addItems( InternalPlayerBackend::nameList() );
 
   // Make sure that exactly one radio button in the group is checked and that
   // on_useExternalPlayer_toggled() is called.
@@ -306,7 +306,7 @@ Preferences::Preferences( QWidget * parent, Config::Class & cfg_ ):
 
     int index = ui.internalPlayerBackend->findText( p.internalPlayerBackend.uiName() );
     if ( index < 0 ) { // The specified backend is unavailable.
-      index = ui.internalPlayerBackend->findText( Config::InternalPlayerBackend::defaultBackend().uiName() );
+      index = ui.internalPlayerBackend->findText( InternalPlayerBackend::defaultBackend().uiName() );
     }
     Q_ASSERT( index >= 0 && "Logic error: the default backend must be present in the backend name list." );
     ui.internalPlayerBackend->setCurrentIndex( index );