[BUGFIX] Fix global options not working after sub (#500)

* [BACKEND] Error if arg after positional arg

* working

* dry-run default

* remove unused code

docs/usage.rst

@@ -69,6 +69,12 @@ View Options
 -----------------
 .. code-block::
 
-   ytdl-sub view [URL]
+   ytdl-sub view [-sc] [URL]
+
+.. code-block:: text
+
+  -sc, --split-chapters
+                        View source variables after splitting by chapters
+
 
 Preview the source variables for a given URL. Helps when creating new configs.

src/ytdl_sub/cli/download_args_parser.py

@@ -7,7 +7,7 @@
 
 from mergedeep import mergedeep
 
-from ytdl_sub.cli.main_args_parser import MainArgs
+from ytdl_sub.cli.main_args_parser import MainArguments
 from ytdl_sub.config.config_validator import ConfigOptions
 from ytdl_sub.utils.exceptions import InvalidDlArguments
 
@@ -41,7 +41,7 @@ def __init__(self, extra_arguments: List[str], config_options: ConfigOptions):
         self._config_options = config_options
 
         for arg in extra_arguments:
-            if arg in MainArgs.all():
+            if arg in MainArguments.all_arguments():
                 raise InvalidDlArguments(
                     f"'{arg}' is a ytdl-sub argument and must placed behind 'dl'"
                 )

src/ytdl_sub/cli/main.py

@@ -35,6 +35,10 @@ def _download_subscriptions_from_yaml_files(
     Returns
     -------
     List of (subscription, transaction_log)
+
+    Raises
+    ------
+    Validation exception if main arg is specified as a subscription path
     """
     subscription_paths: List[str] = args.subscription_paths
     subscriptions: List[Subscription] = []

src/ytdl_sub/cli/main_args_parser.py

@@ -1,60 +1,111 @@
 import argparse
-from enum import Enum
+import dataclasses
 from typing import List
 
 from ytdl_sub import __local_version__
 from ytdl_sub.utils.logger import LoggerLevels
 
 
-class MainArgs(Enum):
-    CONFIG = "--config"
-    DRY_RUN = "--dry-run"
-    LOG_LEVEL = "--log-level"
+@dataclasses.dataclass
+class CLIArgument:
+    short: str
+    long: str
+    is_positional: bool = False
+
+
+class MainArguments:
+    CONFIG = CLIArgument(
+        short="-c",
+        long="--config",
+    )
+    DRY_RUN = CLIArgument(
+        short="-d",
+        long="--dry-run",
+        is_positional=True,
+    )
+    LOG_LEVEL = CLIArgument(
+        short="-l",
+        long="--log-level",
+        is_positional=True,
+    )
 
     @classmethod
-    def all(cls) -> List[str]:
+    def all(cls) -> List[CLIArgument]:
         """
         Returns
         -------
-        List of all args used in main CLI
+        List of MainArgument classes
+        """
+        return [cls.CONFIG, cls.DRY_RUN, cls.LOG_LEVEL]
+
+    @classmethod
+    def all_arguments(cls) -> List[str]:
         """
-        return list(map(lambda arg: arg.value, cls))
+        Returns
+        -------
+        List of all string args that can be used in the CLI
+        """
+        all_args = []
+        for arg in cls.all():
+            all_args.extend([arg.short, arg.long])
+        return all_args
+
+
+###################################################################################################
+# SHARED OPTIONS
+def _add_shared_arguments(arg_parser: argparse.ArgumentParser, suppress_defaults: bool) -> None:
+    """
+    Add shared arguments to sub parsers. Needed to be able to specify args after positional args.
+    i.e. support both ``ytdl-sub --dry-run sub`` and ``ytdl-sub sub --dry-run``
+
+    Parameters
+    ----------
+    arg_parser
+        The parser to add shared args to
+    suppress_defaults
+        bool. Suppress sub parser defaults so they do not override the defaults in the parent parser
+    """
+    arg_parser.add_argument(
+        MainArguments.CONFIG.short,
+        MainArguments.CONFIG.long,
+        metavar="CONFIGPATH",
+        type=str,
+        help="path to the config yaml, uses config.yaml if not provided",
+        default=argparse.SUPPRESS if suppress_defaults else "config.yaml",
+    )
+    arg_parser.add_argument(
+        MainArguments.DRY_RUN.short,
+        MainArguments.DRY_RUN.long,
+        action="store_true",
+        help="preview what a download would output, "
+        "does not perform any video downloads or writes to output directories",
+        default=argparse.SUPPRESS if suppress_defaults else False,
+    )
+    arg_parser.add_argument(
+        MainArguments.LOG_LEVEL.short,
+        MainArguments.LOG_LEVEL.long,
+        metavar="|".join(LoggerLevels.names()),
+        type=str,
+        help="level of logs to print to console, defaults to info",
+        default=argparse.SUPPRESS if suppress_defaults else LoggerLevels.INFO.name,
+        choices=LoggerLevels.names(),
+        dest="ytdl_sub_log_level",
+    )
 
 
 ###################################################################################################
 # GLOBAL PARSER
 parser = argparse.ArgumentParser(
-    description="ytdl-sub: Automate download and adding metadata with YoutubeDL"
+    description="ytdl-sub: Automate download and adding metadata with YoutubeDL",
 )
 parser.add_argument("--version", action="version", version="%(prog)s " + __local_version__)
-parser.add_argument(
-    "-c",
-    MainArgs.CONFIG.value,
-    metavar="CONFIGPATH",
-    type=str,
-    help="path to the config yaml, uses config.yaml if not provided",
-    default="config.yaml",
-)
-parser.add_argument(
-    MainArgs.DRY_RUN.value,
-    action="store_true",
-    help="preview what a download would output, "
-    "does not perform any video downloads or writes to output directories",
-)
-parser.add_argument(
-    MainArgs.LOG_LEVEL.value,
-    metavar="|".join(LoggerLevels.names()),
-    type=str,
-    help="level of logs to print to console, defaults to info",
-    default=LoggerLevels.INFO.name,
-    choices=LoggerLevels.names(),
-    dest="ytdl_sub_log_level",
-)
+_add_shared_arguments(parser, suppress_defaults=False)
 
 subparsers = parser.add_subparsers(dest="subparser")
 ###################################################################################################
 # SUBSCRIPTION PARSER
 subscription_parser = subparsers.add_parser("sub")
+_add_shared_arguments(subscription_parser, suppress_defaults=True)
 subscription_parser.add_argument(
     "subscription_paths",
     metavar="SUBPATH",
@@ -69,23 +120,18 @@ def all(cls) -> List[str]:
 # VIEW PARSER
 
 
-class ViewArgs(Enum):
-    SPLIT_CHAPTERS = "--split-chapters"
-
-    @classmethod
-    def all(cls) -> List[str]:
-        """
-        Returns
-        -------
-        List of all args used in main CLI
-        """
-        return list(map(lambda arg: arg.value, cls))
+class ViewArguments:
+    SPLIT_CHAPTERS = CLIArgument(
+        short="-sc",
+        long="--split-chapters",
+    )
 
 
 view_parser = subparsers.add_parser("view")
+_add_shared_arguments(view_parser, suppress_defaults=True)
 view_parser.add_argument(
-    "-sc",
-    ViewArgs.SPLIT_CHAPTERS.value,
+    ViewArguments.SPLIT_CHAPTERS.short,
+    ViewArguments.SPLIT_CHAPTERS.long,
     action="store_true",
     help="View source variables after splitting by chapters",
 )

tests/expected_transaction_log.py

@@ -1,4 +1,5 @@
 import os
+from pathlib import Path
 from typing import List
 
 from resources import REGENERATE_FIXTURES
@@ -10,7 +11,7 @@
 
 
 def assert_transaction_log_matches(
-    output_directory: str,
+    output_directory: Path,
     transaction_log: FileHandlerTransactionLog,
     transaction_log_summary_file_name: str,
     regenerate_transaction_log: bool = REGENERATE_FIXTURES,

tests/unit/cli/test_download_args_parser.py

@@ -7,7 +7,7 @@
 import pytest
 
 from ytdl_sub.cli.download_args_parser import DownloadArgsParser
-from ytdl_sub.cli.main_args_parser import MainArgs
+from ytdl_sub.cli.main_args_parser import MainArguments
 from ytdl_sub.cli.main_args_parser import parser
 from ytdl_sub.config.config_validator import ConfigOptions
 from ytdl_sub.utils.exceptions import InvalidDlArguments
@@ -151,7 +151,7 @@ def test_error_two_different_types(self, config_options_generator):
                 extra_arguments=extra_args, config_options=config_options
             ).to_subscription_dict()
 
-    @pytest.mark.parametrize("main_argument", MainArgs.all())
+    @pytest.mark.parametrize("main_argument", MainArguments.all_arguments())
     def test_error_uses_main_args(self, main_argument, config_options_generator):
         config_options = config_options_generator()
         extra_args = _get_extra_arguments(cmd_string=f"dl {main_argument}")

tests/unit/cli/test_main.py

@@ -0,0 +1,21 @@
+import sys
+from unittest.mock import patch
+
+import pytest
+
+from ytdl_sub.cli.main import main
+from ytdl_sub.utils.exceptions import ValidationException
+
+
+def test_args_after_sub_work():
+    with patch.object(
+        sys,
+        "argv",
+        ["ytdl-sub", "-c", "examples/tv_show_config.yaml", "sub", "--log-level", "debug"],
+    ), patch("ytdl_sub.cli.main._download_subscriptions_from_yaml_files") as mock_sub:
+        main()
+
+        assert mock_sub.call_count == 1
+        assert mock_sub.call_args.kwargs["args"].config == "examples/tv_show_config.yaml"
+        assert mock_sub.call_args.kwargs["args"].subscription_paths == ["subscriptions.yaml"]
+        assert mock_sub.call_args.kwargs["args"].ytdl_sub_log_level == "debug"