Repository: theacodes/nox
Pull request #306 (2020-03-29 09:07:23)

Force adding group for command-line arguments
Implements #303 


Patch:
diff --git a/nox/_option_set.py b/nox/_option_set.py
index 79ae39b..0c91e9c 100644
--- a/nox/_option_set.py
+++ b/nox/_option_set.py
@@ -20,12 +20,27 @@ and surfaced in documentation."""
 import argparse
 import collections
 import functools
-from argparse import ArgumentError, ArgumentParser, Namespace, _ArgumentGroup
-from typing import Any, Callable, Dict, List, Optional, Tuple, Union
+from argparse import ArgumentError, ArgumentParser, Namespace
+from typing import Any, Callable, List, Optional, Tuple, Union
 
 import argcomplete
 
 
+class OptionGroup:
+    """A single group for command-line options.
+
+    Args:
+        name (str): The name used to refer to the group.
+        args: Passed through to``ArgumentParser.add_argument_group``.
+        kwargs: Passed through to``ArgumentParser.add_argument_group``.
+    """
+
+    def __init__(self, name: str, *args: Any, **kwargs: Any) -> None:
+        self.name = name
+        self.args = args
+        self.kwargs = kwargs
+
+
 class Option:
     """A single option that can be specified via command-line or configuration
     file.
@@ -35,8 +50,8 @@ class Option:
             object.
         flags (Sequence[str]): The list of flags used by argparse. Effectively
             the ``*args`` for ``ArgumentParser.add_argument``.
+        group (OptionGroup): The argument group this option belongs to.
         help (str): The help string pass to argparse.
-        group (str): The argument group this option belongs to, if any.
         noxfile (bool): Whether or not this option can be set in the
             configuration file.
         merge_func (Callable[[Namespace, Namespace], Any]): A function that
@@ -61,8 +76,8 @@ class Option:
         self,
         name: str,
         *flags: str,
+        group: OptionGroup,
         help: Optional[str] = None,
-        group: Optional[str] = None,
         noxfile: bool = False,
         merge_func: Optional[Callable[[Namespace, Namespace], Any]] = None,
         finalizer_func: Optional[Callable[[Any, Namespace], Any]] = None,
@@ -73,8 +88,8 @@ class Option:
     ) -> None:
         self.name = name
         self.flags = flags
-        self.help = help
         self.group = group
+        self.help = help
         self.noxfile = noxfile
         self.merge_func = merge_func
         self.finalizer_func = finalizer_func
@@ -182,7 +197,7 @@ class OptionSet:
         )  # type: collections.OrderedDict[str, Option]
         self.groups = (
             collections.OrderedDict()
-        )  # type: collections.OrderedDict[str, Tuple[Tuple[Any, ...], Dict[str, Any]]]
+        )  # type: collections.OrderedDict[str, OptionGroup]
 
     def add_options(self, *args: Option) -> None:
         """Adds a sequence of Options to the OptionSet.
@@ -193,23 +208,14 @@ class OptionSet:
         for option in args:
             self.options[option.name] = option
 
-    def add_group(self, name: str, *args: Any, **kwargs: Any) -> None:
-        """Adds a new argument group.
+    def add_groups(self, *args: OptionGroup) -> None:
+        """Adds a sequence of OptionGroups to the OptionSet.
 
-        When :func:`parser` is invoked, the OptionSet will turn all distinct
-        argument groups into separate sections in the ``--help`` output using
-        ``ArgumentParser.add_argument_group``.
+        Args:
+            args (Sequence[OptionGroup])
         """
-        self.groups[name] = (args, kwargs)
-
-    def _add_to_parser(
-        self, parser: Union[_ArgumentGroup, ArgumentParser], option: Option
-    ) -> None:
-        argument = parser.add_argument(
-            *option.flags, help=option.help, default=option.default, **option.kwargs
-        )
-        if getattr(option, "completer"):
-            setattr(argument, "completer", option.completer)
+        for option_group in args:
+            self.groups[option_group.name] = option_group
 
     def parser(self) -> ArgumentParser:
         """Returns an ``ArgumentParser`` for this option set.
@@ -220,18 +226,19 @@ class OptionSet:
         parser = argparse.ArgumentParser(*self.parser_args, **self.parser_kwargs)
 
         groups = {
-            name: parser.add_argument_group(*args, **kwargs)
-            for name, (args, kwargs) in self.groups.items()
+            name: parser.add_argument_group(*option_group.args, **option_group.kwargs)
+            for name, option_group in self.groups.items()
         }
 
         for option in self.options.values():
             if option.hidden:
                 continue
 
-            if option.group is not None:
-                self._add_to_parser(groups[option.group], option)
-            else:
-                self._add_to_parser(parser, option)
+            argument = groups[option.group.name].add_argument(
+                *option.flags, help=option.help, default=option.default, **option.kwargs
+            )
+            if getattr(option, "completer"):
+                setattr(argument, "completer", option.completer)
 
         return parser
 
diff --git a/nox/_options.py b/nox/_options.py
index 168890a..782a512 100644
--- a/nox/_options.py
+++ b/nox/_options.py
@@ -27,15 +27,17 @@ options = _option_set.OptionSet(
     description="Nox is a Python automation toolkit.", add_help=False
 )
 
-options.add_group(
-    "primary",
-    "Primary arguments",
-    "These are the most common arguments used when invoking Nox.",
-)
-options.add_group(
-    "secondary",
-    "Additional arguments & flags",
-    "These arguments are used to control Nox's behavior or control advanced features.",
+options.add_groups(
+    _option_set.OptionGroup(
+        "primary",
+        "Primary arguments",
+        "These are the most common arguments used when invoking Nox.",
+    ),
+    _option_set.OptionGroup(
+        "secondary",
+        "Additional arguments & flags",
+        "These arguments are used to control Nox's behavior or control advanced features.",
+    ),
 )
 
 
@@ -148,14 +150,14 @@ options.add_options(
         "help",
         "-h",
         "--help",
-        group="primary",
+        group=options.groups["primary"],
         action="store_true",
         help="Show this help message and exit.",
     ),
     _option_set.Option(
         "version",
         "--version",
-        group="primary",
+        group=options.groups["primary"],
         action="store_true",
         help="Show the Nox version and exit.",
     ),
@@ -164,7 +166,7 @@ options.add_options(
         "-l",
         "--list-sessions",
         "--list",
-        group="primary",
+        group=options.groups["primary"],
         action="store_true",
         help="List all available sessions and exit.",
     ),
@@ -174,7 +176,7 @@ options.add_options(
         "-e",
         "--sessions",
         "--session",
-        group="primary",
+        group=options.groups["primary"],
         noxfile=True,
         merge_func=functools.partial(_session_filters_merge_func, "sessions"),
         nargs="*",
@@ -187,7 +189,7 @@ options.add_options(
         "-p",
         "--pythons",
         "--python",
-        group="primary",
+        group=options.groups["primary"],
         noxfile=True,
         merge_func=functools.partial(_session_filters_merge_func, "pythons"),
         nargs="*",
@@ -197,6 +199,7 @@ options.add_options(
         "keywords",
         "-k",
         "--keywords",
+        group=options.groups["primary"],
         noxfile=True,
         merge_func=functools.partial(_session_filters_merge_func, "keywords"),
         help="Only run sessions that match the given expression.",
@@ -204,7 +207,7 @@ options.add_options(
     _option_set.Option(
         "posargs",
         "posargs",
-        group="primary",
+        group=options.groups["primary"],
         nargs=argparse.REMAINDER,
         help="Arguments following ``--`` that are passed through to the session(s).",
         finalizer_func=_posargs_finalizer,
@@ -213,7 +216,7 @@ options.add_options(
         "verbose",
         "-v",
         "--verbose",
-        group="secondary",
+        group=options.groups["secondary"],
         action="store_true",
         help="Logs the output of all commands run including commands marked silent.",
         noxfile=True,
@@ -222,14 +225,14 @@ options.add_options(
         "reuse_existing_virtualenvs",
         ("-r", "--reuse-existing-virtualenvs"),
         ("--no-reuse-existing-virtualenvs",),
-        group="secondary",
+        group=options.groups["secondary"],
         help="Re-use existing virtualenvs instead of recreating them.",
     ),
     _option_set.Option(
         "noxfile",
         "-f",
         "--noxfile",
-        group="secondary",
+        group=options.groups["secondary"],
         default="noxfile.py",
         help="Location of the Python file containing nox sessions.",
     ),
@@ -238,48 +241,48 @@ options.add_options(
         "--envdir",
         noxfile=True,
         merge_func=_envdir_merge_func,
-        group="secondary",
+        group=options.groups["secondary"],
         help="Directory where nox will store virtualenvs, this is ``.nox`` by default.",
     ),
     *_option_set.make_flag_pair(
         "stop_on_first_error",
         ("-x", "--stop-on-first-error"),
         ("--no-stop-on-first-error",),
-        group="secondary",
+        group=options.groups["secondary"],
         help="Stop after the first error.",
     ),
     *_option_set.make_flag_pair(
         "error_on_missing_interpreters",
         ("--error-on-missing-interpreters",),
         ("--no-error-on-missing-interpreters",),
-        group="secondary",
+        group=options.groups["secondary"],
         help="Error instead of skipping sessions if an interpreter can not be located.",
     ),
     *_option_set.make_flag_pair(
         "error_on_external_run",
         ("--error-on-external-run",),
         ("--no-error-on-external-run",),
-        group="secondary",
+        group=options.groups["secondary"],
         help="Error if run() is used to execute a program that isn't installed in a session's virtualenv.",
     ),
     _option_set.Option(
         "install_only",
         "--install-only",
-        group="secondary",
+        group=options.groups["secondary"],
         action="store_true",
         help="Skip session.run invocations in the Noxfile.",
     ),
     _option_set.Option(
         "report",
         "--report",
-        group="secondary",
+        group=options.groups["secondary"],
         noxfile=True,
         help="Output a report of all sessions to the given filename.",
     ),
     _option_set.Option(
         "non_interactive",
         "--non-interactive",
-        group="secondary",
+        group=options.groups["secondary"],
         action="store_true",
         help="Force session.interactive to always be False, even in interactive sessions.",
     ),
@@ -287,7 +290,7 @@ options.add_options(
         "nocolor",
         "--nocolor",
         "--no-color",
-        group="secondary",
+        group=options.groups["secondary"],
         default=lambda: "NO_COLOR" in os.environ,
         action="store_true",
         help="Disable all color output.",
@@ -296,13 +299,17 @@ options.add_options(
         "forcecolor",
         "--forcecolor",
         "--force-color",
-        group="secondary",
+        group=options.groups["secondary"],
         default=False,
         action="store_true",
         help="Force color output, even if stdout is not an interactive terminal.",
     ),
     _option_set.Option(
-        "color", "--color", hidden=True, finalizer_func=_color_finalizer
+        "color",
+        "--color",
+        group=options.groups["secondary"],
+        hidden=True,
+        finalizer_func=_color_finalizer,
     ),
 )
 


Test patch:
diff --git a/tests/test__option_set.py b/tests/test__option_set.py
index 3bcf4f2..3ad8b59 100644
--- a/tests/test__option_set.py
+++ b/tests/test__option_set.py
@@ -22,7 +22,12 @@ from nox import _option_set, _options
 class TestOptionSet:
     def test_namespace(self):
         optionset = _option_set.OptionSet()
-        optionset.add_options(_option_set.Option("option_a", default="meep"))
+        optionset.add_groups(_option_set.OptionGroup("group_a"))
+        optionset.add_options(
+            _option_set.Option(
+                "option_a", group=optionset.groups["group_a"], default="meep"
+            )
+        )
 
         namespace = optionset.namespace()
 
@@ -32,7 +37,12 @@ class TestOptionSet:
 
     def test_namespace_values(self):
         optionset = _option_set.OptionSet()
-        optionset.add_options(_option_set.Option("option_a", default="meep"))
+        optionset.add_groups(_option_set.OptionGroup("group_a"))
+        optionset.add_options(
+            _option_set.Option(
+                "option_a", group=optionset.groups["group_a"], default="meep"
+            )
+        )
 
         namespace = optionset.namespace(option_a="moop")