diff --git a/changelog.d/+group-help-fix.fixed.md b/changelog.d/+group-help-fix.fixed.md new file mode 100644 index 0000000..ddc90e4 --- /dev/null +++ b/changelog.d/+group-help-fix.fixed.md @@ -0,0 +1 @@ +Fixed group bare invocation showing "Unknown command" instead of group help, and help output now lists subcommands by name instead of raw argparse internals. diff --git a/src/milo/commands.py b/src/milo/commands.py index 4201868..ee48d33 100644 --- a/src/milo/commands.py +++ b/src/milo/commands.py @@ -9,6 +9,7 @@ import os import sys from collections.abc import Callable +from dataclasses import dataclass from typing import TYPE_CHECKING, Any, NoReturn from milo._command_defs import ( @@ -41,6 +42,39 @@ ] +# --------------------------------------------------------------------------- +# Resolve result types — discriminated union for command resolution +# --------------------------------------------------------------------------- + + +@dataclass(frozen=True, slots=True) +class ResolvedCommand: + """A command was found and should be dispatched.""" + + command: CommandDef | LazyCommandDef + fmt: str + + +@dataclass(frozen=True, slots=True) +class ResolvedGroup: + """A group was invoked without a subcommand — show its help.""" + + group: Group + fmt: str + prog: str = "" + + +@dataclass(frozen=True, slots=True) +class ResolvedNothing: + """No command or group matched — may offer did-you-mean.""" + + attempted: str | None + fmt: str + + +ResolveResult = ResolvedCommand | ResolvedGroup | ResolvedNothing + + class _MiloArgumentParser(argparse.ArgumentParser): """ArgumentParser subclass that provides did-you-mean suggestions.""" @@ -620,7 +654,7 @@ def _add_commands_to_subparsers( "--format", choices=["plain", "json", "table"], default="plain", - help="Output format (default: plain)", + help="Output format", ) def _add_groups_to_subparsers( @@ -710,6 +744,7 @@ def _add_arguments_from_schema( def run(self, argv: list[str] | None = None) -> Any: """Parse args and dispatch to the appropriate command.""" parser = self.build_parser() + self._parser = parser args = parser.parse_args(argv) # --completions mode @@ -747,20 +782,26 @@ def run(self, argv: list[str] | None = None) -> Any: ctx = self._build_context(args) # Resolve command from args (may be nested in groups) - found, fmt = self._resolve_command_from_args(args) - if not found: - # Did-you-mean suggestion for typos - cmd_name = getattr(args, "_command", None) - if cmd_name: - suggestion = self.suggest_command(cmd_name) + result = self._resolve_command_from_args(args) + + if isinstance(result, ResolvedGroup): + result.group.format_help(result.prog) + return None + + if isinstance(result, ResolvedNothing): + if result.attempted: + suggestion = self.suggest_command(result.attempted) if suggestion: sys.stderr.write( - f"Unknown command: {cmd_name!r}. Did you mean {suggestion!r}?\n" + f"Unknown command: {result.attempted!r}. Did you mean {suggestion!r}?\n" ) return None - parser.print_help() + self._format_root_help() return None + found = result.command + fmt = result.fmt + # Resolve lazy commands cmd = found.resolve() if isinstance(found, LazyCommandDef) else found @@ -914,55 +955,94 @@ def _build_context(self, args: argparse.Namespace) -> Context: globals=user_globals, ) - def _resolve_command_from_args( - self, args: argparse.Namespace - ) -> tuple[CommandDef | LazyCommandDef | None, str]: - """Walk the parsed args to find the leaf command.""" + def _format_root_help(self) -> None: + """Render root help from command/group registries and the actual parser.""" + from milo.help import HelpState + from milo.templates import get_env + + commands = tuple( + [ + {"name": cmd.name, "help": getattr(cmd, "description", "")} + for cmd in self._commands.values() + if not getattr(cmd, "hidden", False) + ] + + [ + {"name": g.name, "help": g.description} + for g in self._groups.values() + if not g.hidden + ] + ) + + # Derive options from the actual parser so new flags are never missed + options: list[dict[str, Any]] = [] + for action in self._parser._actions: + if isinstance(action, argparse._SubParsersAction): + continue + flags = ", ".join(action.option_strings) if action.option_strings else "" + if not flags: + continue + entry: dict[str, Any] = {"flags": flags, "help": action.help or ""} + if action.metavar: + entry["metavar"] = action.metavar + options.append(entry) + + state = HelpState( + prog=self.name, + description=self.description, + commands=commands, + options=tuple(options), + ) + env = get_env() + template = env.get_template("help.kida") + sys.stdout.write(template.render(state=state) + "\n") + sys.stdout.flush() + + def _resolve_command_from_args(self, args: argparse.Namespace) -> ResolveResult: + """Walk the parsed args to find the leaf command, group, or nothing.""" fmt = getattr(args, "format", "plain") - # Check top-level command cmd_name = getattr(args, "_command", None) if not cmd_name: - return None, fmt + return ResolvedNothing(attempted=None, fmt=fmt) - # Is it a direct command? + # Direct command? cmd = self.get_command(cmd_name) if cmd: - return cmd, fmt + return ResolvedCommand(command=cmd, fmt=fmt) - # Is it a group? Walk into it. + # Group? Walk into it. group = self._groups.get(cmd_name) if group is None: resolved = self._group_alias_map.get(cmd_name) if resolved: group = self._groups.get(resolved) if group is None: - return None, fmt + return ResolvedNothing(attempted=cmd_name, fmt=fmt) - return self._resolve_group_command(group, args, fmt) + return self._resolve_group_command(group, args, fmt, prog=self.name) def _resolve_group_command( self, group: Group, args: argparse.Namespace, fmt: str, - ) -> tuple[CommandDef | None, str]: + prog: str = "", + ) -> ResolveResult: """Recursively resolve a command within a group from parsed args.""" + group_prog = f"{prog} {group.name}".strip() sub_name = getattr(args, f"_command_{group.name}", None) if not sub_name: - return None, fmt + return ResolvedGroup(group=group, fmt=fmt, prog=group_prog) - # Check if it's a command in this group cmd = group.get_command(sub_name) if cmd: - return cmd, fmt + return ResolvedCommand(command=cmd, fmt=fmt) - # Check if it's a nested sub-group sub_group = group.get_group(sub_name) if sub_group: - return self._resolve_group_command(sub_group, args, fmt) + return self._resolve_group_command(sub_group, args, fmt, prog=group_prog) - return None, fmt + return ResolvedNothing(attempted=sub_name, fmt=fmt) def call(self, command_name: str, **kwargs: Any) -> Any: """Programmatically call a command by name or dotted path. diff --git a/src/milo/groups.py b/src/milo/groups.py index 5558ae5..6aad7f3 100644 --- a/src/milo/groups.py +++ b/src/milo/groups.py @@ -181,6 +181,38 @@ def to_def(self) -> GroupDef: hidden=self.hidden, ) + def format_help(self, prog_prefix: str = "") -> str: + """Render help from this group's command/group registries. + + Writes rendered help to stdout and returns the output string. + """ + import sys + + from milo.help import HelpState + from milo.templates import get_env + + prog = f"{prog_prefix} {self.name}".strip() if prog_prefix else self.name + commands = tuple( + [ + {"name": cmd.name, "help": getattr(cmd, "description", "")} + for cmd in self._commands.values() + if not getattr(cmd, "hidden", False) + ] + + [ + {"name": g.name, "help": g.description} + for g in self._groups.values() + if not g.hidden + ] + ) + + state = HelpState(prog=prog, description=self.description, commands=commands) + env = get_env() + template = env.get_template("help.kida") + output = template.render(state=state) + sys.stdout.write(output + "\n") + sys.stdout.flush() + return output + def walk_commands(self, prefix: str = ""): """Yield (dotted_path, CommandDef) for all commands in this tree.""" path_prefix = f"{prefix}{self.name}." if prefix else f"{self.name}." diff --git a/src/milo/help.py b/src/milo/help.py index 6abc7e5..5cd114b 100644 --- a/src/milo/help.py +++ b/src/milo/help.py @@ -17,6 +17,8 @@ class HelpState: usage: str = "" groups: tuple[dict[str, Any], ...] = () examples: tuple[dict[str, Any], ...] = () + commands: tuple[dict[str, Any], ...] = () + options: tuple[dict[str, Any], ...] = () class HelpRenderer(argparse.HelpFormatter): diff --git a/src/milo/templates/help.kida b/src/milo/templates/help.kida index c3d40c1..1785f5d 100644 --- a/src/milo/templates/help.kida +++ b/src/milo/templates/help.kida @@ -1,13 +1,30 @@ {{ state.prog | bold }}{% if state.description %} {{ "-" | dim }} {{ state.description }}{% endif %} +{%- if state.commands %} + +{{ "commands" | yellow | bold }}: +{%- for cmd in state.commands %} + {{ cmd.name | cyan }} {{ cmd.help }} +{%- endfor %} +{%- endif %} +{%- for group in state.groups %} -{% for group in state.groups %} {{ group.title | yellow | bold }}: -{% for action in group.actions %} {% if action.option_strings %}{{ action.option_strings | join(", ") | green }}{% else %}{{ action.dest | cyan }}{% endif %}{% if action.metavar %} {{ action.metavar | dim }}{% endif %} {{ action.help }}{% if action.default is not none and action.default != "==SUPPRESS==" %} {{ ("(default: " ~ action.default ~ ")") | dim }}{% endif %} -{% endfor %} -{% endfor %} -{% if state.examples %} +{%- for action in group.actions %} + {% if action.option_strings %}{{ action.option_strings | join(", ") | green }}{% if action.metavar %} {{ action.metavar | dim }}{% endif %} {{ action.help }}{% if action.default is not none and action.default != "==SUPPRESS==" and action.default is not false and action.default != 0 and action.default != "" %} {{ ("(default: " ~ action.default ~ ")") | dim }}{% endif %}{% else %}{{ action.dest | cyan }}{% if action.metavar %} {{ action.metavar | dim }}{% endif %} {{ action.help }}{% if action.default is not none and action.default != "==SUPPRESS==" and action.default is not false and action.default != 0 and action.default != "" %} {{ ("(default: " ~ action.default ~ ")") | dim }}{% endif %}{% endif %} +{%- endfor %} +{%- endfor %} +{%- if state.options %} + +{{ "options" | yellow | bold }}: +{%- for opt in state.options %} + {{ opt.flags | green }}{% if opt.metavar %} {{ opt.metavar | dim }}{% endif %} {{ opt.help }}{% if opt.default %} {{ ("(default: " ~ opt.default ~ ")") | dim }}{% endif %} +{%- endfor %} +{%- endif %} +{%- if state.examples %} + {{ "examples" | yellow | bold }}: -{% for ex in state.examples %} {{ ("$ " ~ ex.command) | green }}{% if ex.description %} +{%- for ex in state.examples %} + {{ ("$ " ~ ex.command) | green }}{% if ex.description %} {{ ex.description | dim }}{% endif %} -{% endfor %} -{% endif %} +{%- endfor %} +{%- endif %} diff --git a/tests/test_groups.py b/tests/test_groups.py index 9d9dadc..176e978 100644 --- a/tests/test_groups.py +++ b/tests/test_groups.py @@ -327,3 +327,142 @@ def test_top_level_commands_still_shown(self): txt = generate_llms_txt(cli) assert "## Commands" in txt assert "**init**" in txt + + +# --------------------------------------------------------------------------- +# ResolveResult types and dispatch +# --------------------------------------------------------------------------- + + +class TestResolveResult: + """Verify _resolve_command_from_args returns correct discriminated types.""" + + def test_resolves_command(self): + from milo.commands import ResolvedCommand + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["init", "--name", "x"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedCommand) + assert result.command.name == "init" + + def test_resolves_group(self): + from milo.commands import ResolvedGroup + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["site"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedGroup) + assert result.group.name == "site" + + def test_resolves_nested_group(self): + from milo.commands import ResolvedGroup + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["site", "config"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedGroup) + assert result.group.name == "config" + + def test_resolves_nested_command(self): + from milo.commands import ResolvedCommand + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args(["site", "build"]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedCommand) + assert result.command.name == "build" + + def test_resolves_nothing_no_args(self): + from milo.commands import ResolvedNothing + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args([]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedNothing) + assert result.attempted is None + + def test_resolves_nothing_preserves_fmt(self): + from milo.commands import ResolvedNothing + + cli = _make_cli_with_groups() + parser = cli.build_parser() + args = parser.parse_args([]) + result = cli._resolve_command_from_args(args) + assert isinstance(result, ResolvedNothing) + assert result.fmt == "plain" + + +# --------------------------------------------------------------------------- +# Group bare invocation and help rendering +# --------------------------------------------------------------------------- + + +class TestGroupBareInvocation: + def test_group_bare_shows_help_not_error(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site"]) + assert result.exit_code == 0 + assert result.stderr == "" + assert "build" in result.output + assert "serve" in result.output + + def test_nested_group_bare_shows_help(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site", "config"]) + assert result.exit_code == 0 + assert result.stderr == "" + assert "show" in result.output + assert "validate" in result.output + + def test_unknown_command_still_suggests(self): + cli = _make_cli_with_groups() + result = cli.invoke(["siet"]) + assert result.exit_code == 2 + assert "Did you mean" in result.stderr + + def test_root_no_args_shows_help(self): + cli = _make_cli_with_groups() + result = cli.invoke([]) + assert result.exit_code == 0 + assert "bengal" in result.output + + +class TestHelpSubcommandListing: + def test_root_help_lists_commands_by_name(self): + cli = _make_cli_with_groups() + result = cli.invoke([]) + assert "init" in result.output + assert "site" in result.output + assert "_command" not in result.output + + def test_group_help_lists_subcommands_by_name(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site"]) + assert "build" in result.output + assert "serve" in result.output + assert "_command_site" not in result.output + + def test_nested_group_help_lists_subcommands(self): + cli = _make_cli_with_groups() + result = cli.invoke(["site", "config"]) + assert "show" in result.output + assert "validate" in result.output + assert "_command_config" not in result.output + + def test_root_help_shows_options(self): + cli = _make_cli_with_groups() + result = cli.invoke([]) + assert "--verbose" in result.output + assert "--help" in result.output + + def test_group_help_no_argparse_internals(self): + """Group help should use registry data, not argparse positional args.""" + cli = _make_cli_with_groups() + result = cli.invoke(["site"]) + assert "positional arguments" not in result.output