Repository: repobee/repobee

Issue:
Allow extension commands to add new categories
An extension command should be able to add a new category. It should also be possible for several extension commands to attach to the same category. We'll need this for example for `repobee plugin install` and `repobee plugin uninstall`.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/repobee/repobee/pull/516?src=pr&el=h1) Report
> Merging [#516](https://codecov.io/gh/repobee/repobee/pull/516?src=pr&el=desc) into [master](https://codecov.io/gh/repobee/repobee/commit/88c22d8b2b2e5cd98294aa504c74d025ea6879ae&el=desc) will **decrease** coverage by `3.51%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/repobee/repobee/pull/516/graphs/tree.svg?width=650&height=150&src=pr&token=5vOKgkphZe)](https://codecov.io/gh/repobee/repobee/pull/516?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #516      +/-   ##
==========================================
- Coverage   91.82%   88.31%   -3.52%     
==========================================
  Files          43       43              
  Lines        2509     2515       +6     
  Branches      410      413       +3     
==========================================
- Hits         2304     2221      -83     
- Misses        162      248      +86     
- Partials       43       46       +3     
```


| [Impacted Files](https://codecov.io/gh/repobee/repobee/pull/516?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/repobee\_plug/\_pluginmeta.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL3JlcG9iZWVfcGx1Zy9fcGx1Z2lubWV0YS5weQ==) | `93.18% <ø> (-0.16%)` | :arrow_down: |
| [src/\_repobee/cli/mainparser.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NsaS9tYWlucGFyc2VyLnB5) | `95.04% <100.00%> (+0.25%)` | :arrow_up: |
| [src/\_repobee/main.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL21haW4ucHk=) | `89.47% <100.00%> (+3.57%)` | :arrow_up: |
| [src/repobee\_plug/cli.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL3JlcG9iZWVfcGx1Zy9jbGkucHk=) | `95.03% <100.00%> (+0.37%)` | :arrow_up: |
| [src/\_repobee/ext/gitlab.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2V4dC9naXRsYWIucHk=) | `63.86% <0.00%> (-28.56%)` | :arrow_down: |
| [src/\_repobee/cli/parsing.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NsaS9wYXJzaW5nLnB5) | `87.57% <0.00%> (-4.35%)` | :arrow_down: |
| [src/\_repobee/git.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2dpdC5weQ==) | `96.62% <0.00%> (-3.38%)` | :arrow_down: |
| [src/\_repobee/command/repos.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2NvbW1hbmQvcmVwb3MucHk=) | `91.80% <0.00%> (-3.28%)` | :arrow_down: |
| [src/\_repobee/ext/query.py](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree#diff-c3JjL19yZXBvYmVlL2V4dC9xdWVyeS5weQ==) | `48.48% <0.00%> (-1.52%)` | :arrow_down: |
| ... and [6 more](https://codecov.io/gh/repobee/repobee/pull/516/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/repobee/repobee/pull/516?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/repobee/repobee/pull/516?src=pr&el=footer). Last update [88c22d8...a3e8b97](https://codecov.io/gh/repobee/repobee/pull/516?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/src/_repobee/cli/mainparser.py b/src/_repobee/cli/mainparser.py
index 7e2c255..16d7f61 100644
--- a/src/_repobee/cli/mainparser.py
+++ b/src/_repobee/cli/mainparser.py
@@ -635,7 +635,7 @@ def _add_extension_parsers(
             parents.append(repo_name_parser)
 
         def _add_ext_parser(
-            parents: List[argparse.ArgumentParser],
+            name=None, parents: List[argparse.ArgumentParser] = None
         ) -> argparse.ArgumentParser:
             """Add a new parser either to the extension command's category (if
             specified), or to the top level subparsers (if category is not
@@ -644,24 +644,48 @@ def _add_extension_parsers(
             return (
                 parsers_mapping.get(cmd.category) or subparsers
             ).add_parser(
-                cmd.name,
+                name or cmd.name,
                 help=cmd.help,
                 description=cmd.description,
                 parents=parents,
                 formatter_class=_OrderedFormatter,
             )
 
-        if cmd.name in plug.cli.CoreCommand:
+        category = (
+            cmd.name.category
+            if isinstance(cmd.name, plug.cli.Action)
+            else cmd.category
+        )
+        if category and category not in parsers_mapping:
+            # new category
+            category_cmd = subparsers.add_parser(
+                category.name,
+                help=category.help,
+                description=category.description,
+            )
+            category_parsers = category_cmd.add_subparsers(dest=ACTION)
+            category_parsers.required = True
+            parsers_mapping[category] = category_parsers
+
+        if cmd.name in parsers_mapping:
             ext_parser = parsers_mapping[cmd.name]
             cmd.parser(
                 config=parsed_config,
                 show_all_opts=show_all_opts,
                 parser=ext_parser,
             )
+        elif isinstance(cmd.name, plug.cli.Action):
+            action = cmd.name
+            ext_parser = _add_ext_parser(parents=parents, name=action.name)
+            cmd.parser(
+                config=parsed_config,
+                show_all_opts=show_all_opts,
+                parser=ext_parser,
+            )
         elif callable(cmd.parser):
             action = cmd.category.get(cmd.name) if cmd.category else None
             ext_parser = parsers_mapping.get(action) or _add_ext_parser(
-                parents
+                parents=parents
             )
             cmd.parser(
                 config=parsed_config,
@@ -670,7 +694,7 @@ def _add_extension_parsers(
             )
         else:
             parents.append(cmd.parser)
-            ext_parser = _add_ext_parser(parents)
+            ext_parser = _add_ext_parser(parents=parents)
 
         try:
             _add_traceback_arg(ext_parser)
diff --git a/src/_repobee/main.py b/src/_repobee/main.py
index d423e27..431acda 100644
--- a/src/_repobee/main.py
+++ b/src/_repobee/main.py
@@ -102,7 +102,7 @@ def run(
         parsed_args, api, ext_commands = _parse_args(
             cmd, config_file, show_all_opts
         )
-        _repobee.cli.dispatch.dispatch_command(
+        return _repobee.cli.dispatch.dispatch_command(
             parsed_args, api, config_file, ext_commands
         )
     finally:
diff --git a/src/repobee_plug/_pluginmeta.py b/src/repobee_plug/_pluginmeta.py
index a74cf78..c686161 100644
--- a/src/repobee_plug/_pluginmeta.py
+++ b/src/repobee_plug/_pluginmeta.py
@@ -160,7 +160,7 @@ def _generate_command_func(attrdict: Mapping[str, Any]) -> Callable:
 
         return _containers.ExtensionCommand(
             parser=functools.partial(_attach_options, plugin=self),
-            name=settings.action_name
+            name=settings.action
             or self.__class__.__name__.lower().replace("_", "-"),
             help=settings.help,
             description=settings.description,
diff --git a/src/repobee_plug/cli.py b/src/repobee_plug/cli.py
index eb3e7d2..4c2180c 100644
--- a/src/repobee_plug/cli.py
+++ b/src/repobee_plug/cli.py
@@ -3,7 +3,17 @@ import abc
 import collections
 import enum
 import itertools
-from typing import List, Tuple, Optional, Set, Mapping, Iterable, Any, Callable
+from typing import (
+    List,
+    Tuple,
+    Optional,
+    Set,
+    Mapping,
+    Iterable,
+    Any,
+    Callable,
+    Union,
+)
 
 from repobee_plug import _containers
 
@@ -49,7 +59,7 @@ _Option.__new__.__defaults__ = (None,) * len(_Option._fields)
 _CommandSettings = collections.namedtuple(
     "_CommandSettings",
     [
-        "action_name",
+        "action",
         "category",
         "help",
         "description",
@@ -66,7 +76,7 @@ _CommandExtensionSettings = collections.namedtuple(
 
 
 def command_settings(
-    action_name: Optional[str] = None,
+    action: Optional[Union[str, "Action"]] = None,
     category: Optional["CoreCommand"] = None,
     help: str = "",
     description: str = "",
@@ -100,10 +110,13 @@ def command_settings(
         Hello, world!
 
     Args:
-        action_name: The name of the action that the command will be
-            available under. Defaults to the name of the plugin class.
+        action: The name of this command, or a :py:class:`Action` object that
+            defines both category and action for the command. Defaults to the
+            name of the plugin class.
         category: The category to place this command in. If not specified,
-            then the command will be top-level (i.e. uncategorized).
+            then the command will be top-level (i.e. uncategorized). If
+            ``action`` is an :py:class:`Action` (as opposed to a ``str``),
+            then this argument is not allowed.
         help: A help section for the command. This appears when listing the
             help section of the command's category.
         description: A help section for the command. This appears when
@@ -114,9 +127,19 @@ def command_settings(
         config_section_name: The name of the configuration section the
             command should look for configurable options in. Defaults
             to the name of the plugin the command is defined in.
+    Returns:
+        A settings object used internally by RepoBee.
     """
+    if isinstance(action, Action):
+        if category:
+            raise TypeError(
+                "argument 'category' not allowed when argument 'action' is an "
+                "Action object"
+            )
+        category = action.category
+
     return _CommandSettings(
-        action_name=action_name,
+        action=action,
         category=category,
         help=help,
         description=description,
@@ -126,6 +149,26 @@ def command_settings(
     )
 
 
+def category(
+    name: str, action_names: List[str], help: str = "", description: str = ""
+) -> "Category":
+    """Create a category for CLI actions.
+
+    Args:
+        name: Name of the category.
+        action_names: The actions of this category.
+    Returns:
+        A CLI category.
+    """
+    action_names = set(action_names)
+    return Category(
+        name=name,
+        action_names=action_names,
+        help=help,
+        description=description,
+    )
+
+
 def command_extension_settings(
     actions: List["Action"], config_section_name: Optional[str] = None
 ) -> _CommandExtensionSettings:
@@ -375,29 +418,37 @@ class Category(ImmutableMixin, abc.ABC):
 
     For example, the command ``repobee issues list`` has category ``issues``
     and action ``list``. Actions are unique only within their category.
-
-    Attributes:
-        name: Name of this category.
-        actions: A tuple of names of actions applicable to this category.
     """
 
+    help: str = ""
+    description: str = ""
     name: str
     actions: Tuple["Action"]
     action_names: Set[str]
     _action_table: Mapping[str, "Action"]
 
-    def __init__(self):
+    def __init__(
+        self,
+        name: Optional[str] = None,
+        action_names: Optional[Set[str]] = None,
+        help: Optional[str] = None,
+        description: Optional[str] = None,
+    ):
         # determine the name of this category based on the runtime type of the
         # inheriting class
-        name = self.__class__.__name__.lower().strip("_")
+        name = name or self.__class__.__name__.lower().strip("_")
         # determine the action names based on type annotations in the
         # inheriting class
-        action_names = {
+        action_names = (action_names or set()) | {
             name
             for name, tpe in self.__annotations__.items()
-            if issubclass(tpe, Action)
+            if isinstance(tpe, type) and issubclass(tpe, Action)
         }
 
+        object.__setattr__(self, "help", help or self.help)
+        object.__setattr__(
+            self, "description", description or self.description
+        )
         object.__setattr__(self, "name", name)
         object.__setattr__(self, "action_names", set(action_names))
         # This is just to reserve the name 'actions'
@@ -410,7 +461,7 @@ class Category(ImmutableMixin, abc.ABC):
         actions = []
         for action_name in action_names:
             action = Action(action_name.replace("_", "-"), self)
-            object.__setattr__(self, action_name, action)
+            object.__setattr__(self, action_name.replace("-", "_"), action)
             actions.append(action)
 
         object.__setattr__(self, "actions", tuple(actions))


Test patch:
diff --git a/tests/unit_tests/repobee_plug/test_pluginmeta.py b/tests/unit_tests/repobee_plug/test_pluginmeta.py
index b997f00..e2e987a 100644
--- a/tests/unit_tests/repobee_plug/test_pluginmeta.py
+++ b/tests/unit_tests/repobee_plug/test_pluginmeta.py
@@ -159,7 +159,7 @@ class TestDeclarativeExtensionCommand:
         class ExtCommand(plug.Plugin, plug.cli.Command):
             __settings__ = plug.cli.command_settings(
                 category=expected_category,
-                action_name=expected_name,
+                action=expected_name,
                 help=expected_help,
                 description=expected_description,
                 base_parsers=expected_base_parsers,
@@ -353,6 +353,102 @@ class TestDeclarativeExtensionCommand:
 
         assert parsed_args.old == old
 
+    def test_create_new_category(self):
+        """Test that command can be added to a new category."""
+
+        class Greetings(plug.cli.Category):
+            hello: plug.cli.Action
+
+        class Hello(plug.Plugin, plug.cli.Command):
+            __settings__ = plug.cli.command_settings(action=Greetings().hello)
+            name = plug.cli.positional()
+            age = plug.cli.positional(converter=int)
+
+            def command(self, args, api):
+                return plug.Result(
+                    name=self.plugin_name,
+                    msg="Nice!",
+                    status=plug.Status.SUCCESS,
+                    data={"name": args.name, "age": args.age},
+                )
+
+        name = "Bob"
+        age = 24
+        results_mapping = repobee.run(
+            f"greetings hello {name} {age}".split(), plugins=[Hello]
+        )
+        _, results = list(results_mapping.items())[0]
+        result, *_ = results
+
+        assert result.data["name"] == name
+        assert result.data["age"] == age
+
+    def test_add_two_actions_to_new_category(self):
+        """Test that it's possible to add multiple actions to a custom
+        category.
+        """
+
+        class Greetings(plug.cli.Category):
+            hello: plug.cli.Action
+            bye: plug.cli.Action
+
+        category = Greetings()
+
+        class Hello(plug.Plugin, plug.cli.Command):
+            __settings__ = plug.cli.command_settings(action=category.hello)
+            name = plug.cli.positional()
+
+            def command(self, args, api):
+                return plug.Result(
+                    name=self.plugin_name,
+                    msg=f"Hello {args.name}",
+                    status=plug.Status.SUCCESS,
+                )
+
+        class Bye(plug.Plugin, plug.cli.Command):
+            __settings__ = plug.cli.command_settings(action=category.bye)
+            name = plug.cli.positional()
+
+            def command(self, args, api):
+                return plug.Result(
+                    name=self.plugin_name,
+                    msg=f"Bye {args.name}",
+                    status=plug.Status.SUCCESS,
+                )
+
+        name = "Alice"
+        hello_results = repobee.run(
+            f"greetings hello {name}".split(), plugins=[Hello, Bye]
+        )
+        bye_results = repobee.run(
+            f"greetings bye {name}".split(), plugins=[Hello, Bye]
+        )
+
+        assert hello_results[category.hello][0].msg == f"Hello {name}"
+        assert bye_results[category.bye][0].msg == f"Bye {name}"
+
+    def test_raises_when_both_action_and_category_given(self):
+        """It is not allowed to give an Action object to the action argument,
+        and at the same time give a Category, as the Action object defines
+        both.
+        """
+        category = plug.cli.category("cat", action_names=["greetings"])
+
+        with pytest.raises(TypeError) as exc_info:
+
+            class Greetings(plug.Plugin, plug.cli.Command):
+                __settings__ = plug.cli.command_settings(
+                    action=category.greetings, category=category
+                )
+
+                def command(self, args, api):
+                    pass
+
+        assert (
+            "argument 'category' not allowed when argument "
+            "'action' is an Action object"
+        ) in str(exc_info.value)
+
 
 class TestDeclarativeCommandExtension:
     """Test creating command extensions to existing commands."""