From 36f576ca93a5d5d1f1f646c908f2fcd12573ebb1 Mon Sep 17 00:00:00 2001
From: Kieran Eglin <kieran.eglin@gmail.com>
Date: Wed, 17 Apr 2024 12:23:18 -0700
Subject: [PATCH] Refactored windows_filenames to be a global flag; added tests

---
 lib/pinchflat/downloading/download_option_builder.ex |  1 -
 lib/pinchflat/yt_dlp/command_runner.ex               | 12 ++++++++----
 lib/pinchflat/yt_dlp/media_collection.ex             |  1 -
 .../downloading/download_option_builder_test.exs     |  1 -
 test/pinchflat/yt_dlp/command_runner_test.exs        |  8 ++++++++
 5 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/lib/pinchflat/downloading/download_option_builder.ex b/lib/pinchflat/downloading/download_option_builder.ex
index 5dc1bb2..5780294 100644
--- a/lib/pinchflat/downloading/download_option_builder.ex
+++ b/lib/pinchflat/downloading/download_option_builder.ex
@@ -44,7 +44,6 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilder do
   defp default_options do
     [
       :no_progress,
-      :windows_filenames,
       # Add force-overwrites to make sure redownloading works
       :force_overwrites,
       # This makes the date metadata conform to what jellyfin expects
diff --git a/lib/pinchflat/yt_dlp/command_runner.ex b/lib/pinchflat/yt_dlp/command_runner.ex
index 572c3ac..d16cbea 100644
--- a/lib/pinchflat/yt_dlp/command_runner.ex
+++ b/lib/pinchflat/yt_dlp/command_runner.ex
@@ -30,10 +30,9 @@ defmodule Pinchflat.YtDlp.CommandRunner do
 
     output_filepath = generate_output_filepath(addl_opts)
     print_to_file_opts = [{:print_to_file, output_template}, output_filepath]
-    external_file_opts = build_external_file_options()
+    user_configured_opts = cookie_file_options() ++ global_options()
     # These must stay in exactly this order, hence why I'm giving it its own variable.
-    all_opts = command_opts ++ print_to_file_opts ++ external_file_opts
-
+    all_opts = command_opts ++ print_to_file_opts ++ user_configured_opts
     formatted_command_opts = [url] ++ CliUtils.parse_options(all_opts)
     Logger.info("[yt-dlp] called with: #{Enum.join(formatted_command_opts, " ")}")
 
@@ -58,6 +57,7 @@ defmodule Pinchflat.YtDlp.CommandRunner do
   def version do
     command = backend_executable()
 
+    # TODO: fix to use CliUtils.wrap_cmd (and look at apprise too)
     case System.cmd(command, ["--version"]) do
       {output, 0} ->
         {:ok, String.trim(output)}
@@ -74,7 +74,11 @@ defmodule Pinchflat.YtDlp.CommandRunner do
     end
   end
 
-  defp build_external_file_options do
+  defp global_options do
+    [:windows_filenames]
+  end
+
+  defp cookie_file_options do
     base_dir = Application.get_env(:pinchflat, :extras_directory)
     filename_options_map = %{cookies: "cookies.txt"}
 
diff --git a/lib/pinchflat/yt_dlp/media_collection.ex b/lib/pinchflat/yt_dlp/media_collection.ex
index 2644362..7ef28dd 100644
--- a/lib/pinchflat/yt_dlp/media_collection.ex
+++ b/lib/pinchflat/yt_dlp/media_collection.ex
@@ -69,7 +69,6 @@ defmodule Pinchflat.YtDlp.MediaCollection do
     # the first video has not released yet (ie: is a premier). We don't care about
     # available formats since we're just getting the source details
     default_opts = [
-      :windows_filenames,
       :simulate,
       :skip_download,
       :ignore_no_formats_error,
diff --git a/test/pinchflat/downloading/download_option_builder_test.exs b/test/pinchflat/downloading/download_option_builder_test.exs
index 8ce35a2..8008b7f 100644
--- a/test/pinchflat/downloading/download_option_builder_test.exs
+++ b/test/pinchflat/downloading/download_option_builder_test.exs
@@ -53,7 +53,6 @@ defmodule Pinchflat.Downloading.DownloadOptionBuilderTest do
       assert {:ok, res} = DownloadOptionBuilder.build(media_item)
 
       assert :no_progress in res
-      assert :windows_filenames in res
       assert :force_overwrites in res
       assert {:parse_metadata, "%(upload_date>%Y-%m-%d)s:(?P<meta_date>.+)"} in res
     end
diff --git a/test/pinchflat/yt_dlp/command_runner_test.exs b/test/pinchflat/yt_dlp/command_runner_test.exs
index 17cf61c..f4c9309 100644
--- a/test/pinchflat/yt_dlp/command_runner_test.exs
+++ b/test/pinchflat/yt_dlp/command_runner_test.exs
@@ -78,6 +78,14 @@ defmodule Pinchflat.YtDlp.CommandRunnerTest do
     end
   end
 
+  describe "run/4 when testing global options" do
+    test "creates windows-safe filenames" do
+      assert {:ok, output} = Runner.run(@media_url, [], "")
+
+      assert String.contains?(output, "--windows-filenames")
+    end
+  end
+
   describe "version/0" do
     test "adds the version arg" do
       assert {:ok, output} = Runner.version()