refactor: torrent-complete-sound-command is a list (#1710)

* refactor: torrent-complete-sound-command is a list

gtk/conf.c

@@ -91,8 +91,6 @@ static void tr_prefs_init_defaults(tr_variant* d)
     tr_variantDictAddStr(d, TR_KEY_statusbar_stats, "total-ratio");
     tr_variantDictAddBool(d, TR_KEY_torrent_added_notification_enabled, true);
     tr_variantDictAddBool(d, TR_KEY_torrent_complete_notification_enabled, true);
-    tr_variantDictAddStr(d, TR_KEY_torrent_complete_sound_command,
-        "canberra-gtk-play -i complete-download -d 'transmission torrent downloaded'");
     tr_variantDictAddBool(d, TR_KEY_torrent_complete_sound_enabled, true);
     tr_variantDictAddBool(d, TR_KEY_show_options_window, TRUE);
     tr_variantDictAddBool(d, TR_KEY_main_window_is_maximized, FALSE);
@@ -108,6 +106,24 @@ static void tr_prefs_init_defaults(tr_variant* d)
     tr_variantDictAddBool(d, TR_KEY_compact_view, FALSE);
 }
 
+static void ensure_sound_cmd_is_a_list(tr_variant* dict)
+{
+    tr_quark key = TR_KEY_torrent_complete_sound_command;
+    tr_variant* list = NULL;
+    if (tr_variantDictFindList(dict, key, &list))
+    {
+        return;
+    }
+
+    tr_variantDictRemove(dict, key);
+    list = tr_variantDictAddList(dict, key, 5);
+    tr_variantListAddStr(list, "canberra-gtk-play");
+    tr_variantListAddStr(list, "-i");
+    tr_variantListAddStr(list, "complete-download");
+    tr_variantListAddStr(list, "-d");
+    tr_variantListAddStr(list, "transmission torrent downloaded");
+}
+
 static tr_variant* getPrefs(void)
 {
     static tr_variant settings;
@@ -118,6 +134,7 @@ static tr_variant* getPrefs(void)
         tr_variantInitDict(&settings, 0);
         tr_prefs_init_defaults(&settings);
         tr_sessionLoadSettings(&settings, gl_confdir, MY_CONFIG_NAME);
+        ensure_sound_cmd_is_a_list(&settings);
         loaded = TRUE;
     }
 
@@ -177,6 +194,31 @@ void gtr_pref_flag_set(tr_quark const key, gboolean value)
 ****
 ***/
 
+char** gtr_pref_strv_get(tr_quark const key)
+{
+    char** ret = NULL;
+
+    tr_variant* list = NULL;
+    if (tr_variantDictFindList(getPrefs(), key, &list))
+    {
+        size_t out = 0;
+        size_t const n = tr_variantListSize(list);
+        ret = g_new0(char*, n + 1);
+
+        for (size_t i = 0; i < n; ++i)
+        {
+            char const* str = NULL;
+            size_t len = 0;
+            if (tr_variantGetStr(tr_variantListChild(list, i), &str, &len))
+            {
+                ret[out++] = g_strndup(str, len);
+            }
+        }
+    }
+
+    return ret;
+}
+
 char const* gtr_pref_string_get(tr_quark const key)
 {
     char const* str;

gtk/conf.h

@@ -37,6 +37,8 @@ void gtr_pref_double_set(tr_quark const key, double value);
 gboolean gtr_pref_flag_get(tr_quark const key);
 void gtr_pref_flag_set(tr_quark const key, gboolean value);
 
+char** gtr_pref_strv_get(tr_quark const key);
+
 char const* gtr_pref_string_get(tr_quark const key);
 void gtr_pref_string_set(tr_quark const key, char const* value);
 

gtk/notify.c

@@ -177,11 +177,18 @@ static void notify_callback(GObject* source, GAsyncResult* res, gpointer user_da
 
 void gtr_notify_torrent_completed(TrCore* core, int torrent_id)
 {
-    char const* cmd = gtr_pref_string_get(TR_KEY_torrent_complete_sound_command);
-
     if (gtr_pref_flag_get(TR_KEY_torrent_complete_sound_enabled))
     {
-        g_spawn_command_line_async(cmd, NULL);
+        char** argv = gtr_pref_strv_get(TR_KEY_torrent_complete_sound_command);
+        g_spawn_async(NULL /*cwd*/,
+            argv,
+            NULL /*envp*/,
+            G_SPAWN_SEARCH_PATH,
+            NULL /*GSpawnChildSetupFunc*/,
+            NULL /*user_data*/,
+            NULL /*child_pid*/,
+            NULL);
+        g_strfreev(argv);
     }
 
     if (!gtr_pref_flag_get(TR_KEY_torrent_complete_notification_enabled))

gtk/tr-prefs.h

@@ -12,38 +12,6 @@
 
 GtkWidget* gtr_prefs_dialog_new(GtkWindow* parent, GObject* core);
 
-/* if you add a key here,  you /must/ add its
- * default in tr_prefs_init_defaults(void) */
-
-#define PREF_KEY_BLOCKLIST_UPDATES_ENABLED "blocklist-updates-enabled"
-#define PREF_KEY_COMPACT_VIEW "compact-view"
-#define PREF_KEY_DIR_WATCH_ENABLED "watch-dir-enabled"
-#define PREF_KEY_DIR_WATCH "watch-dir"
-#define PREF_KEY_FILTERBAR "show-filterbar"
-#define PREF_KEY_INHIBIT_HIBERNATION "inhibit-desktop-hibernation"
-#define PREF_KEY_MAIN_WINDOW_HEIGHT "main-window-height"
-#define PREF_KEY_MAIN_WINDOW_IS_MAXIMIZED "main-window-is-maximized"
-#define PREF_KEY_MAIN_WINDOW_WIDTH "main-window-width"
-#define PREF_KEY_MAIN_WINDOW_X "main-window-x"
-#define PREF_KEY_MAIN_WINDOW_Y "main-window-y"
-#define PREF_KEY_OPEN_DIALOG_FOLDER "open-dialog-dir"
-#define PREF_KEY_OPTIONS_PROMPT "show-options-window"
-#define PREF_KEY_SHOW_BACKUP_TRACKERS "show-backup-trackers"
-#define PREF_KEY_SHOW_MORE_PEER_INFO "show-extra-peer-details"
-#define PREF_KEY_SHOW_MORE_TRACKER_INFO "show-tracker-scrapes"
-#define PREF_KEY_SHOW_TRAY_ICON "show-notification-area-icon"
-#define PREF_KEY_SORT_MODE "sort-mode"
-#define PREF_KEY_SORT_REVERSED "sort-reversed"
-#define PREF_KEY_STATUSBAR "show-statusbar"
-#define PREF_KEY_STATUSBAR_STATS "statusbar-stats"
-#define PREF_KEY_TOOLBAR "show-toolbar"
-#define PREF_KEY_TORRENT_ADDED_NOTIFICATION_ENABLED "torrent-added-notification-enabled"
-#define PREF_KEY_TORRENT_COMPLETE_NOTIFICATION_ENABLED "torrent-complete-notification-enabled"
-#define PREF_KEY_TORRENT_COMPLETE_SOUND_COMMAND "torrent-complete-sound-command"
-#define PREF_KEY_TORRENT_COMPLETE_SOUND_ENABLED "torrent-complete-sound-enabled"
-#define PREF_KEY_TRASH_CAN_ENABLED "trash-can-enabled"
-#define PREF_KEY_USER_HAS_GIVEN_INFORMED_CONSENT "user-has-given-informed-consent"
-
 enum
 {
     MAIN_WINDOW_REFRESH_INTERVAL_SECONDS = 2,

libtransmission/variant.c

@@ -1067,6 +1067,13 @@ void tr_variantMergeDicts(tr_variant* target, tr_variant const* source)
 
         if (tr_variantDictChild((tr_variant*)source, i, &key, &val))
         {
+            // if types differ, ensure that target will overwrite source
+            tr_variant* const target_child = tr_variantDictFind(target, key);
+            if (target_child && !tr_variantIsType(target_child, val->type))
+            {
+                tr_variantDictRemove(target, key);
+            }
+
             if (tr_variantIsBool(val))
             {
                 bool boolVal = false;

qt/.clang-tidy

@@ -56,9 +56,6 @@ Checks: >
   -readability-redundant-access-specifiers,
   -readability-static-accessed-through-instance
 
-WarningsAsErrors: >
-  *
-
 CheckOptions:
   - { key: readability-identifier-naming.ClassCase,                value: CamelCase  }
   - { key: readability-identifier-naming.ClassMethodCase,          value: camelBack  }

qt/Application.cc

@@ -430,7 +430,9 @@ void Application::onTorrentsCompleted(torrent_ids_t const& ids) const
 #if defined(Q_OS_WIN) || defined(Q_OS_MAC)
         beep();
 #else
-        QProcess::execute(prefs_->getString(Prefs::COMPLETE_SOUND_COMMAND));
+        auto args = prefs_->get<QStringList>(Prefs::COMPLETE_SOUND_COMMAND);
+        auto const command = args.takeFirst();
+        QProcess::execute(command, args);
 #endif
     }
 }

qt/Prefs.cc

@@ -6,6 +6,7 @@
  *
  */
 
+#include <array>
 #include <cassert>
 #include <cstdlib>
 #include <string_view>
@@ -30,6 +31,28 @@ using ::trqt::variant_helpers::getValue;
 ****
 ***/
 
+namespace
+{
+
+void ensureSoundCommandIsAList(tr_variant* dict)
+{
+    tr_quark key = TR_KEY_torrent_complete_sound_command;
+    tr_variant* list = nullptr;
+    if (tr_variantDictFindList(dict, key, &list))
+    {
+        return;
+    }
+
+    tr_variantDictRemove(dict, key);
+    dictAdd(dict, key, std::array<std::string_view, 5>{
+            "canberra-gtk-play",
+            "-i", "complete-download",
+            "-d", "transmission torrent downloaded"
+        });
+}
+
+} // anonymous namespace
+
 std::array<Prefs::PrefItem, Prefs::PREFS_COUNT> const Prefs::Items
 {
     /* gui settings */
@@ -68,7 +91,7 @@ std::array<Prefs::PrefItem, Prefs::PREFS_COUNT> const Prefs::Items
     { SESSION_REMOTE_AUTH, TR_KEY_remote_session_requres_authentication, QVariant::Bool },
     { SESSION_REMOTE_USERNAME, TR_KEY_remote_session_username, QVariant::String },
     { SESSION_REMOTE_PASSWORD, TR_KEY_remote_session_password, QVariant::String },
-    { COMPLETE_SOUND_COMMAND, TR_KEY_torrent_complete_sound_command, QVariant::String },
+    { COMPLETE_SOUND_COMMAND, TR_KEY_torrent_complete_sound_command, QVariant::StringList },
     { COMPLETE_SOUND_ENABLED, TR_KEY_torrent_complete_sound_enabled, QVariant::Bool },
     { USER_HAS_GIVEN_INFORMED_CONSENT, TR_KEY_user_has_given_informed_consent, QVariant::Bool },
 
@@ -190,6 +213,7 @@ Prefs::Prefs(QString config_dir) :
     tr_variantInitDict(&top, 0);
     initDefaults(&top);
     tr_sessionLoadSettings(&top, config_dir_.toUtf8().constData(), nullptr);
+    ensureSoundCommandIsAList(&top);
 
     for (int i = 0; i < PREFS_COUNT; ++i)
     {
@@ -245,6 +269,16 @@ Prefs::Prefs(QString config_dir) :
             }
             break;
 
+        case QVariant::StringList:
+            {
+                auto const value = getValue<QStringList>(b);
+                if (value)
+                {
+                    values_[i].setValue(*value);
+                }
+            }
+            break;
+
         case QVariant::Bool:
             {
                 auto const value = getValue<bool>(b);
@@ -332,6 +366,10 @@ Prefs::~Prefs()
             dictAdd(&current_settings, key, val.toString());
             break;
 
+        case QVariant::StringList:
+            dictAdd(&current_settings, key, val.toStringList());
+            break;
+
         case QVariant::Bool:
             dictAdd(&current_settings, key, val.toBool());
             break;
@@ -378,8 +416,6 @@ void Prefs::initDefaults(tr_variant* d) const
     auto constexpr SessionPassword = std::string_view {};
     auto constexpr SessionUsername = std::string_view {};
     auto constexpr SortMode = std::string_view { "sort-by-name" };
-    auto constexpr SoundCommand =
-        std::string_view { "canberra-gtk-play -i complete-download -d 'transmission torrent downloaded'" };
     auto constexpr StatsMode = std::string_view { "total-ratio" };
     auto constexpr WindowLayout = std::string_view { "menu,toolbar,filter,list,statusbar" };
 
@@ -404,7 +440,6 @@ void Prefs::initDefaults(tr_variant* d) const
     dictAdd(d, TR_KEY_sort_reversed, false);
     dictAdd(d, TR_KEY_torrent_added_notification_enabled, true);
     dictAdd(d, TR_KEY_torrent_complete_notification_enabled, true);
-    dictAdd(d, TR_KEY_torrent_complete_sound_command, SoundCommand);
     dictAdd(d, TR_KEY_torrent_complete_sound_enabled, true);
     dictAdd(d, TR_KEY_user_has_given_informed_consent, false);
     dictAdd(d, TR_KEY_watch_dir_enabled, false);

qt/VariantHelpers.h

@@ -103,6 +103,37 @@ auto getValue(tr_variant const* variant)
     return ret;
 }
 
+template<typename C, typename T = typename C::value_type,
+    typename std::enable_if<
+    std::is_same_v<C, QStringList>||
+    std::is_same_v<C, QList<T>>||
+    std::is_same_v<C, std::vector<T>>
+    >::type* = nullptr
+    >
+auto getValue(tr_variant const* variant)
+{
+    std::optional<C> ret;
+
+    if (tr_variantIsList(variant))
+    {
+        auto list = C {};
+
+        for (size_t i = 0, n = tr_variantListSize(variant); i < n; ++i)
+        {
+            tr_variant* const child = tr_variantListChild(const_cast<tr_variant*>(variant), i);
+            auto const value = getValue<T>(child);
+            if (value)
+            {
+                list.push_back(*value);
+            }
+        }
+
+        ret = list;
+    }
+
+    return ret;
+}
+
 template<typename T>
 bool change(T& setme, T const& value)
 {