From 24e3059d92dfe5fe2098cc79594668a12b7d1add Mon Sep 17 00:00:00 2001 From: Jan Lindemann Date: Mon, 17 Aug 2026 09:56:03 +0200 Subject: [PATCH 1/2] lib.App.add_cmd_to_parser -> .make_sub_parser() Code beautification: add_cmd_to_parser() isn't very telling about its return type and the fact that it creates an object, hence the name change. Also, annotate its argument with a private argparse type to avoid a cast. My concern that argparse will break the private type at some point in the future is outweighed by the gained clode clarity in this function. Signed-off-by: Jan Lindemann --- src/python/jw/pkg/lib/App.py | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/python/jw/pkg/lib/App.py b/src/python/jw/pkg/lib/App.py index a778f73c..704d5394 100644 --- a/src/python/jw/pkg/lib/App.py +++ b/src/python/jw/pkg/lib/App.py @@ -6,7 +6,9 @@ import os import sys import warnings -from argparse import ArgumentDefaultsHelpFormatter, ArgumentParser, Namespace +from argparse import ( + ArgumentDefaultsHelpFormatter, ArgumentParser, Namespace, _SubParsersAction +) from typing import TYPE_CHECKING, Any, NamedTuple, cast, override from .AsyncRunner import AsyncRunner @@ -151,21 +153,20 @@ class App: # export def _build_parser(self, argv: list[str] | None = None) -> None: - def add_cmd_to_parser(cmd: AbstractCmd, parsers: Any) -> ArgumentParser: - parser = cast( - 'ArgumentParser', - parsers.add_parser( - cmd.name, - help = cmd.help, - description = cmd.description, - aliases = cmd.aliases, - formatter_class = ArgumentDefaultsHelpFormatter, - ) + def make_sub_parser( + cmd: AbstractCmd, parsers: _SubParsersAction[ArgumentParser] + ) -> ArgumentParser: + ret = parsers.add_parser( + cmd.name, + help = cmd.help, + description = cmd.description, + aliases = cmd.aliases, + formatter_class = ArgumentDefaultsHelpFormatter, ) - parser.set_defaults(func = cmd.run) - cmd.add_arguments(parser) - cmd.set_parser(parser) - return parser + ret.set_defaults(func = cmd.run) + cmd.add_arguments(ret) + cmd.set_parser(ret) + return ret def add_cmds_to_parser( parent: AbstractCmd | App, @@ -187,7 +188,7 @@ class App: # export cmd.set_parent(parent) if cmd.name in scs: log(WARNING, f'Duplicate subcommand name: {cmd.name}') - scs[cmd.name] = _SubCommand(cmd, add_cmd_to_parser(cmd, subparsers)) + scs[cmd.name] = _SubCommand(cmd, make_sub_parser(cmd, subparsers)) for alias in cmd.aliases: if alias != cmd.name and alias in scs: log( -- 2.55.0 From 6b4bcdfaf8f862283d5e9a37f04f0132f8996719 Mon Sep 17 00:00:00 2001 From: Jan Lindemann Date: Sun, 16 Aug 2026 17:34:38 +0200 Subject: [PATCH 2/2] lib.App: Add a root command slot The application's top-level behavior is defined by overriding App._add_arguments() and App._run(). The lightweight run-and-options unit, Cmd, can already be mounted at any node of the command tree, but the root is reserved for the application itself. An application that wants to host a plain command at the top level therefore has to subclass App and carry its full lifecycle implementation. Add a root parameter to App.__init__(). When it is given a command class, App instantiates it and uses it as the top level: the command's options are registered on the top-level parser, it becomes the parent of the top-level subcommands, and App._run() delegates the run to it. The command's children are wired as the top-level subcommands, so the same Cmd can now occupy the root node. When root is not given, the previous auto-discovery behavior is preserved unchanged. Keep the top-level subcommand heading as plain "Available subcommands" whether it is hosted by the application or by a root command, while nested command levels continue to qualify the heading with the parent name. Add a unit test that mounts a root command hosting a child and checks option registration, dispatch, setup and teardown, and resolution of the application through the parent chain. Assisted-by: unsloth/Qwen3.8-27B-GGUF:Q4_K_M with pi.dev v0.84.2 Signed-off-by: Jan Lindemann --- src/python/jw/pkg/lib/App.py | 72 +++++++++++++--- test/unit/python/jw/pkg/lib/App/Makefile | 7 ++ test/unit/python/jw/pkg/lib/App/test.py | 103 +++++++++++++++++++++++ 3 files changed, 171 insertions(+), 11 deletions(-) create mode 100644 test/unit/python/jw/pkg/lib/App/Makefile create mode 100644 test/unit/python/jw/pkg/lib/App/test.py diff --git a/src/python/jw/pkg/lib/App.py b/src/python/jw/pkg/lib/App.py index 704d5394..9dda8028 100644 --- a/src/python/jw/pkg/lib/App.py +++ b/src/python/jw/pkg/lib/App.py @@ -112,6 +112,7 @@ class App: # export name_filter: str = '^Cmd.*', modules: list[str] | None = None, eloop: asyncio.AbstractEventLoop | None = None, + root: type[AbstractCmd] | None = None, ) -> None: self.__args: Namespace | None = None @@ -143,12 +144,22 @@ class App: # export self.__eloop = eloop self.__own_eloop = False - cmd_classes: LoadTypes[AbstractCmd] = LoadTypes( - modules if modules else ['__main__'], - type_name_filter = name_filter, - type_filter = [AbstractCmd], - ) - self.__cmds: list[AbstractCmd] = [cmd_class(self) for cmd_class in cmd_classes] + if root is not None: + # -- The application's top-level behavior is delegated to a root + # command. The root hosts the top-level options and its + # subcommands become the top-level subcommands. It is the same + # kind of command that can be mounted at any other node in the + # tree. + self.__root: AbstractCmd | None = root(self) + self.__cmds: Collection[AbstractCmd] = list(self.__root.children) + else: + self.__root = None + cmd_classes: LoadTypes[AbstractCmd] = LoadTypes( + modules if modules else ['__main__'], + type_name_filter = name_filter, + type_filter = [AbstractCmd], + ) + self.__cmds = [cmd_class(self) for cmd_class in cmd_classes] self._build_parser() def _build_parser(self, argv: list[str] | None = None) -> None: @@ -172,13 +183,17 @@ class App: # export parent: AbstractCmd | App, parser: ArgumentParser, cmds: Collection[AbstractCmd], - all: bool = False + all: bool, + top_level: bool, ) -> None: if not cmds: return title = 'Available subcommands' - if isinstance(parent, AbstractCmd): + # -- Only nested command levels qualify the title with the parent + # name; the top level (hosted by the application itself or by a + # root command) keeps the plain title. + if not top_level and isinstance(parent, AbstractCmd): title += ' of ' + parent.name subparsers = parser.add_subparsers( title = title, metavar = '', dest = 'command' @@ -203,7 +218,11 @@ class App: # export if id(sc) not in seen: seen.add(id(sc)) add_cmds_to_parser( - sc.cmd, sc.parser, sc.cmd.children, all = all + sc.cmd, + sc.parser, + sc.cmd.children, + all = all, + top_level = False, ) return # -- Re-parse the command line to find the invoked subcommand. @@ -214,7 +233,13 @@ class App: # export cmd_name = getattr(args, 'command', None) if cmd_name in scs: sc = scs[cmd_name] - add_cmds_to_parser(sc.cmd, sc.parser, sc.cmd.children, all = all) + add_cmds_to_parser( + sc.cmd, + sc.parser, + sc.cmd.children, + all = all, + top_level = False, + ) cmdline = sys.argv if argv is None else argv if argv is None: @@ -229,6 +254,12 @@ class App: # export add_help = False, ) self._add_arguments(self.__parser) + if self.__root is not None: + # -- The root command hosts the top-level behavior: register its + # options on the top-level parser and make it the parent of the + # top-level subcommands. + self.__root.set_parser(self.__parser) + self.__root.add_arguments(self.__parser) if not add_all_parsers: # Parse known args and configure logging, but only if we're not on @@ -241,7 +272,22 @@ class App: # export set_log_level(args.log_level) log(DEBUG, f'-------------- Running: >{pretty_cmd(cmdline)}<') - add_cmds_to_parser(self, self.__parser, self.__cmds, all = add_all_parsers) + if self.__root is None: + add_cmds_to_parser( + self, + self.__parser, + self.__cmds, + all = add_all_parsers, + top_level = False, + ) + else: + add_cmds_to_parser( + self.__root, + self.__parser, + self.__root.children, + all = add_all_parsers, + top_level = True, + ) # -- Add help only now, wouldn't want to have parse_known_args() exit # on --help with subcommands missing @@ -343,6 +389,10 @@ class App: # export # want to do something else, for instance if you don't have sub-commands, # or if want to do anything before and / or after the subcommands. async def _run(self, args: Namespace) -> None | int: + if self.__root is not None: + # -- Delegate the top-level behavior (options, setup, dispatch, + # teardown) to the root command. + return cast('None | int', await self.__root.run(args)) if not hasattr(args, 'func'): self.__parser.print_help() return None diff --git a/test/unit/python/jw/pkg/lib/App/Makefile b/test/unit/python/jw/pkg/lib/App/Makefile new file mode 100644 index 00000000..de693650 --- /dev/null +++ b/test/unit/python/jw/pkg/lib/App/Makefile @@ -0,0 +1,7 @@ +TOPDIR = ../../../../../../.. + +include $(TOPDIR)/make/proj.mk +include $(JWBDIR)/make/py-run.mk + +all: +test: run diff --git a/test/unit/python/jw/pkg/lib/App/test.py b/test/unit/python/jw/pkg/lib/App/test.py new file mode 100644 index 00000000..2b899d4f --- /dev/null +++ b/test/unit/python/jw/pkg/lib/App/test.py @@ -0,0 +1,103 @@ +import asyncio + +import sys + +from jw.pkg.lib.App import App +from jw.pkg.lib.Cmd import Cmd + +# -- A minimal command tree: a root command hosting one child command. The +# root mimics the "run + opts" unit the App can now mount at the top level: +# it adds options, sets up, dispatches to the selected child (or runs its own +# default), and tears down. + +class ChildCmd(Cmd): + + def __init__(self, parent): + super().__init__(parent, 'child', 'A child command') + self.ran = False + self.got_opt = None + + def add_arguments(self, parser): + super().add_arguments(parser) + parser.add_argument('--child-opt', default = 'child-default') + + async def _run(self, args): + self.ran = True + self.got_opt = args.child_opt + +class RootCmd(Cmd): + + made = [] + + def __init__(self, parent): + super().__init__(parent, 'root', 'Root command') + self.child = ChildCmd(self) + self.add_subcommands(self.child) + self.setup = False + self.teardown = False + self.default = False + RootCmd.made.append(self) + + def add_arguments(self, parser): + super().add_arguments(parser) + parser.add_argument('--root-opt', default = 'root-default') + + async def _run(self, args): + self.setup = True + try: + if hasattr(args, 'func'): + await args.func(args) + else: + self.default = True + finally: + self.teardown = True + +# -- App.__init__ builds the parser from sys.argv, so point it at a clean +# command line while constructing the app. + +saved_argv = sys.argv +sys.argv = ['jw-pkg-test'] +try: + app = App(description = 'Root slot test', root = RootCmd) +finally: + sys.argv = saved_argv + +root = RootCmd.made[0] +child = root.child + +# -- Parent chain: the child is parented to the root, the root to the app, +# and both resolve their application through the chain. +assert child.parent is root, 'child.parent should be the root' +assert root.parent is app, 'root.parent should be the app' +assert child.app is app, 'child.app should resolve to the app' +assert root.app is app, 'root.app should resolve to the app' + +# -- The root's options are registered on the top-level parser, and the child +# is wired as a top-level subcommand. +args = app.parser.parse_args(['--root-opt', 'RV', 'child', '--child-opt', 'CV']) +assert args.root_opt == 'RV', args +assert args.child_opt == 'CV', args +assert hasattr(args, 'func'), 'selecting a subcommand must set the dispatch target' + +# -- Dispatching with a selected child runs the child, wrapped by the root's +# setup and teardown. +asyncio.run(app._run(args)) +assert child.ran is True, 'the selected child must run' +assert child.got_opt == 'CV', child.got_opt +assert root.setup is True, 'root setup must run' +assert root.teardown is True, 'root teardown must run' +assert root.default is False, 'default must not run when a child is selected' + +# -- Dispatching with no subcommand runs the root's default, still wrapped by +# setup and teardown, and does not run the child. +root.setup = root.teardown = root.default = False +child.ran = False +args = app.parser.parse_args(['--root-opt', 'RV2']) +assert not hasattr(args, 'func'), 'no subcommand must not set a dispatch target' +asyncio.run(app._run(args)) +assert root.setup is True, 'root setup must run' +assert root.teardown is True, 'root teardown must run' +assert root.default is True, 'root default must run when no child is selected' +assert child.ran is False, 'the child must not run when no child is selected' + +print('All App root-slot tests passed') -- 2.55.0