Merge 3c5233f2a1 into c5d682c993

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

@@ -67,7 +67,7 @@ jobs:
           libzim \
           qt
 
-          wget https://github.com/mistydemeo/eb/releases/download/v4.4.3/eb-4.4.3.tar.bz2
+          wget ftp://ftp.sra.co.jp/pub/misc/eb/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,6 +3,11 @@
 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
@@ -77,31 +82,13 @@ 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)
-        # icu4c as transitive dependency of libzim may not be copied into app bundle, so we directly depends on it to assist macdeployqt
-        # 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 REQUIRED IMPORTED_TARGET icu-i18n icu-uc)
-        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?
+        # For some reason, icu4c as transitive dependency of libzim may not be copied into app bundle,
+        # so we directly depends on it to help macdeployqt or whatever
+        pkg_check_modules(BREW_ICU_FOR_LIBZIM_FORCE_LINK REQUIRED IMPORTED_TARGET icu-i18n icu-uc)
+        target_link_libraries(${GOLDENDICT} PUBLIC PkgConfig::BREW_ICU_FOR_LIBZIM_FORCE_LINK)
     endif ()
 endif ()
 

src/audio/audioplayerfactory.cc

@@ -3,39 +3,34 @@
 
 #include <QScopedPointer>
 #include <QObject>
-#include <utility>
 #include "audioplayerfactory.hh"
 #include "ffmpegaudioplayer.hh"
 #include "multimediaaudioplayer.hh"
 #include "externalaudioplayer.hh"
 #include "gddebug.hh"
 
-AudioPlayerFactory::AudioPlayerFactory( bool useInternalPlayer,
-                                        InternalPlayerBackend internalPlayerBackend,
-                                        QString audioPlaybackProgram ):
-  useInternalPlayer( useInternalPlayer ),
-  internalPlayerBackend( std::move( internalPlayerBackend ) ),
-  audioPlaybackProgram( std::move( audioPlaybackProgram ) )
+AudioPlayerFactory::AudioPlayerFactory( Config::Preferences const & p ):
+  useInternalPlayer( p.useInternalPlayer ),
+  internalPlayerBackend( p.internalPlayerBackend ),
+  audioPlaybackProgram( p.audioPlaybackProgram )
 {
   reset();
 }
 
-void AudioPlayerFactory::setPreferences( bool new_useInternalPlayer,
-                                         const InternalPlayerBackend & new_internalPlayerBackend,
-                                         const QString & new_audioPlaybackProgram )
+void AudioPlayerFactory::setPreferences( Config::Preferences const & p )
 {
-  if ( useInternalPlayer != new_useInternalPlayer ) {
-    useInternalPlayer     = new_useInternalPlayer;
-    internalPlayerBackend = new_internalPlayerBackend;
-    audioPlaybackProgram  = new_audioPlaybackProgram;
+  if ( p.useInternalPlayer != useInternalPlayer ) {
+    useInternalPlayer     = p.useInternalPlayer;
+    internalPlayerBackend = p.internalPlayerBackend;
+    audioPlaybackProgram  = p.audioPlaybackProgram;
     reset();
   }
-  else if ( useInternalPlayer && internalPlayerBackend != new_internalPlayerBackend ) {
-    internalPlayerBackend = new_internalPlayerBackend;
+  else if ( useInternalPlayer && p.internalPlayerBackend != internalPlayerBackend ) {
+    internalPlayerBackend = p.internalPlayerBackend;
     reset();
   }
-  else if ( !useInternalPlayer && new_audioPlaybackProgram != audioPlaybackProgram ) {
-    audioPlaybackProgram                       = new_audioPlaybackProgram;
+  else if ( !useInternalPlayer && p.audioPlaybackProgram != audioPlaybackProgram ) {
+    audioPlaybackProgram                       = p.audioPlaybackProgram;
     ExternalAudioPlayer * const externalPlayer = qobject_cast< ExternalAudioPlayer * >( playerPtr.data() );
     if ( externalPlayer ) {
       setAudioPlaybackProgram( *externalPlayer );
@@ -55,7 +50,7 @@ void AudioPlayerFactory::reset()
     // another object of the same type.
 
 #ifdef MAKE_FFMPEG_PLAYER
-    Q_ASSERT( InternalPlayerBackend::defaultBackend().isFfmpeg()
+    Q_ASSERT( Config::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 "internalplayerbackend.hh"
+#include "config.hh"
 
 class ExternalAudioPlayer;
 
@@ -13,12 +13,8 @@ class AudioPlayerFactory
   Q_DISABLE_COPY( AudioPlayerFactory )
 
 public:
-  explicit AudioPlayerFactory( bool useInternalPlayer,
-                               InternalPlayerBackend internalPlayerBackend,
-                               QString audioPlaybackProgram );
-  void setPreferences( bool new_useInternalPlayer,
-                       const InternalPlayerBackend & new_internalPlayerBackend,
-                       const QString & new_audioPlaybackProgram );
+  explicit AudioPlayerFactory( Config::Preferences const & );
+  void setPreferences( Config::Preferences const & );
   /// 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.
@@ -32,7 +28,7 @@ private:
   void setAudioPlaybackProgram( ExternalAudioPlayer & externalPlayer );
 
   bool useInternalPlayer;
-  InternalPlayerBackend internalPlayerBackend;
+  Config::InternalPlayerBackend internalPlayerBackend;
   QString audioPlaybackProgram;
   AudioPlayerPtr playerPtr;
 };

src/audio/internalplayerbackend.cc

@@ -1,51 +0,0 @@
-#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

@@ -1,61 +0,0 @@
-#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,6 +119,57 @@ 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,20 +3,19 @@
 
 #pragma once
 
-#include "audio/internalplayerbackend.hh"
-#include "ex.hh"
-#include <QDateTime>
-#include <QDomDocument>
-#include <QKeySequence>
-#include <QList>
-#include <QLocale>
-#include <QMetaType>
 #include <QObject>
-#include <QSet>
-#include <QSize>
+#include <QList>
 #include <QString>
-#include <QThread>
+#include <QSize>
+#include <QDateTime>
+#include <QKeySequence>
+#include <QSet>
+#include <QMetaType>
+#include "ex.hh"
+#include <QDomDocument>
+#include <QLocale>
 #include <optional>
+#include <QThread>
 
 /// Special group IDs
 enum GroupId : unsigned {
@@ -270,6 +269,66 @@ 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/ui/mainwindow.cc

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

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( InternalPlayerBackend::nameList() );
+  ui.internalPlayerBackend->addItems( Config::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( InternalPlayerBackend::defaultBackend().uiName() );
+      index = ui.internalPlayerBackend->findText( Config::InternalPlayerBackend::defaultBackend().uiName() );
     }
     Q_ASSERT( index >= 0 && "Logic error: the default backend must be present in the backend name list." );
     ui.internalPlayerBackend->setCurrentIndex( index );