Skip to content

Commit b8e0833

Browse files
chore: minor improvement and more clean up
1 parent 05c602e commit b8e0833

6 files changed

Lines changed: 347 additions & 112 deletions

File tree

cmd2/annotated.py

Lines changed: 44 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,7 @@ def do_paint(
3939
4040
- ``str`` -- default string argument
4141
- ``int``, ``float`` -- sets ``type=`` for argparse
42-
- ``bool`` with default ``False`` -- ``--flag`` with ``store_true``
43-
- ``bool`` with default ``True`` -- ``--no-flag`` with ``store_false``
42+
- ``bool`` with default -- ``--flag / --no-flag`` via ``BooleanOptionalAction``
4443
- positional ``bool`` -- parsed from ``true/false``, ``yes/no``, ``on/off``, ``1/0``
4544
- ``pathlib.Path`` -- sets ``type=Path``
4645
- ``enum.Enum`` subclass -- ``type=converter``, ``choices`` from member values
@@ -278,20 +277,15 @@ def _resolve_bool(
278277
_args: tuple[Any, ...],
279278
*,
280279
is_positional: bool,
281-
has_default: bool,
282-
default: Any,
283280
metadata: ArgMetadata,
281+
**_ctx: Any,
284282
) -> dict[str, Any]:
285283
"""Resolve bool -- flag or positional depending on context."""
286284
if not is_positional:
287285
action_str = getattr(metadata, 'action', None) if metadata else None
288286
if action_str:
289-
action = action_str
290-
elif has_default and default is True:
291-
action = 'store_false'
292-
else:
293-
action = 'store_true'
294-
return {'action': action, 'is_bool_flag': True}
287+
return {'action': action_str, 'is_bool_flag': True}
288+
return {'action': argparse.BooleanOptionalAction, 'is_bool_flag': True}
295289
return {'type': _parse_bool}
296290

297291

@@ -307,8 +301,18 @@ def _make_collection_resolver(collection_type: type) -> Callable[..., dict[str,
307301
"""Create a resolver for single-arg collections (list[T], set[T])."""
308302

309303
def _resolve(_tp: Any, args: tuple[Any, ...], *, has_default: bool = False, **_ctx: Any) -> dict[str, Any]:
304+
if len(args) == 0:
305+
# Bare list/tuple without type args -- treat as list[str]/set[str]
306+
nargs = '*' if has_default else '+'
307+
return {
308+
'is_collection': True,
309+
'nargs': nargs,
310+
'base_type': str,
311+
'action': _CollectionCastingAction,
312+
'container_factory': collection_type,
313+
}
310314
if len(args) != 1:
311-
return {}
315+
return {} # pragma: no cover
312316
element_type, inner = _resolve_element(args[0])
313317
nargs = '*' if has_default else '+'
314318
return {
@@ -327,12 +331,17 @@ def _resolve_tuple(_tp: Any, args: tuple[Any, ...], *, has_default: bool = False
327331
"""Resolve tuple[T, ...] and tuple[T1, T2, ...]."""
328332
cast_kwargs = {'action': _CollectionCastingAction, 'container_factory': tuple}
329333

334+
if not args:
335+
# Bare tuple without type args -- treat as tuple[str, ...]
336+
nargs = '*' if has_default else '+'
337+
return {'is_collection': True, 'nargs': nargs, 'base_type': str, **cast_kwargs}
338+
330339
if len(args) == 2 and args[1] is Ellipsis:
331340
element_type, inner = _resolve_element(args[0])
332341
nargs = '*' if has_default else '+'
333342
return {**inner, 'is_collection': True, 'nargs': nargs, 'base_type': element_type, **cast_kwargs}
334343

335-
if args and Ellipsis not in args:
344+
if Ellipsis not in args:
336345
first = args[0]
337346
if not all(a == first for a in args[1:]):
338347
raise TypeError(
@@ -344,7 +353,7 @@ def _resolve_tuple(_tp: Any, args: tuple[Any, ...], *, has_default: bool = False
344353
_, inner = _resolve_element(first)
345354
return {**inner, 'is_collection': True, 'nargs': len(args), 'base_type': first, **cast_kwargs}
346355

347-
return {}
356+
return {} # pragma: no cover
348357

349358

350359
def _resolve_literal(_tp: Any, args: tuple[Any, ...], **_ctx: Any) -> dict[str, Any]:
@@ -425,8 +434,8 @@ def _resolve_annotation(
425434
*,
426435
has_default: bool = False,
427436
default: Any = None,
428-
) -> tuple[type, dict[str, Any], ArgMetadata, bool]:
429-
"""Decompose a type annotation into ``(base_type, type_kwargs, metadata, is_positional)``.
437+
) -> tuple[dict[str, Any], ArgMetadata, bool, bool]:
438+
"""Decompose a type annotation into ``(type_kwargs, metadata, is_positional, is_bool_flag)``.
430439
431440
Peels in order: Annotated → Optional → type resolution.
432441
"""
@@ -450,7 +459,9 @@ def _resolve_annotation(
450459
if len(args) == 1:
451460
tp = args[0]
452461
is_optional = True
453-
elif len(args) > 1:
462+
else:
463+
# len > 1: ambiguous union (e.g. str | int)
464+
# len == 0: all-None union -- unreachable via normal typing but handle defensively
454465
type_names = ' | '.join(a.__name__ if hasattr(a, '__name__') else str(a) for a in args)
455466
raise TypeError(f"Union type {type_names} is ambiguous for auto-resolution. ")
456467

@@ -490,6 +501,11 @@ def _resolve_annotation(
490501
return type_kwargs, metadata, is_positional, is_bool_flag
491502

492503

504+
# Parameter names that conflict with argparse internals and cannot be used
505+
# as annotated parameter names.
506+
_RESERVED_PARAM_NAMES = frozenset({'dest'})
507+
508+
493509
# ---------------------------------------------------------------------------
494510
# Signature → Parser conversion
495511
# ---------------------------------------------------------------------------
@@ -513,18 +529,26 @@ def build_parser_from_function(func: Callable[..., Any]) -> argparse.ArgumentPar
513529
sig = inspect.signature(func)
514530
try:
515531
hints = get_type_hints(func, include_extras=True)
516-
except (NameError, AttributeError, TypeError):
517-
hints = {}
532+
except (NameError, AttributeError, TypeError) as exc:
533+
raise TypeError(
534+
f"Failed to resolve type hints for {func.__qualname__}. Ensure all annotations use valid, importable types."
535+
) from exc
518536

519537
for name, param in sig.parameters.items():
520538
if name == 'self':
521539
continue
522540

541+
if name in _RESERVED_PARAM_NAMES:
542+
raise ValueError(
543+
f"Parameter name {name!r} in {func.__qualname__} is reserved by argparse "
544+
f"and cannot be used as an annotated parameter name."
545+
)
546+
523547
annotation = hints.get(name, param.annotation)
524548
has_default = param.default is not inspect.Parameter.empty
525549
default = param.default if has_default else None
526550

527-
kwargs, metadata, positional, is_bool_flag = _resolve_annotation(
551+
kwargs, metadata, positional, _is_bool_flag = _resolve_annotation(
528552
annotation,
529553
has_default=has_default,
530554
default=default,
@@ -533,12 +557,7 @@ def build_parser_from_function(func: Callable[..., Any]) -> argparse.ArgumentPar
533557
if positional:
534558
parser.add_argument(name, **kwargs)
535559
else:
536-
if isinstance(metadata, Option) and metadata.names:
537-
flags = list(metadata.names)
538-
elif is_bool_flag and has_default and default is True:
539-
flags = [f'--no-{name}']
540-
else:
541-
flags = [f'--{name}']
560+
flags = list(metadata.names) if isinstance(metadata, Option) and metadata.names else [f'--{name}']
542561
if isinstance(metadata, Option) and metadata.required:
543562
kwargs['required'] = True
544563
kwargs['dest'] = name

cmd2/decorators.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,9 @@ def cmd_wrapper(*args: Any, **kwargs: Any) -> bool | None:
411411
except SystemExit as exc:
412412
raise Cmd2ArgparseError from exc
413413

414-
# Unpack Namespace into function kwargs, filtering cmd2 internals
414+
# Unpack Namespace into function kwargs, filtering cmd2 internals.
415+
# Note: "dest" keys could conflict with annotation names -- may need validation.
416+
# Note: "required" handling when mixing with Options metadata needs verification.
415417
func_kwargs: dict[str, Any] = {}
416418
for key, value in vars(ns).items():
417419
if key.startswith('cmd2_') or key == constants.NS_ATTR_SUBCMD_HANDLER:

docs/features/argument_processing.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,7 @@ The decorator converts Python type annotations into `add_argument()` calls:
8989
| -------------------------------------- | --------------------------------------------------- |
9090
| `str` | default (no `type=` needed) |
9191
| `int`, `float` | `type=int` or `type=float` |
92-
| `bool` (default `False`) | `--flag` with `action='store_true'` |
93-
| `bool` (default `True`) | `--no-flag` with `action='store_false'` |
92+
| `bool` (with default) | `--flag / --no-flag` via `BooleanOptionalAction` |
9493
| positional `bool` | parsed from `true/false`, `yes/no`, `on/off`, `1/0` |
9594
| `Path` | `type=Path` |
9695
| `Enum` subclass | `type=converter`, `choices` from member values |

examples/annotated_example.py

Lines changed: 79 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,8 @@ def do_copy(self, src: Path, dst: Path) -> None:
110110

111111
# -- Bool flags ----------------------------------------------------------
112112
# With @with_argparser you'd set action='store_true' or 'store_false'.
113-
# Here bool defaults drive the flag style automatically.
114-
# False default -> --flag (store_true)
115-
# True default -> --no-flag (store_false)
113+
# Here bool defaults use BooleanOptionalAction automatically, giving
114+
# both --flag and --no-flag from a single parameter.
116115

117116
@cmd2.with_annotated
118117
@cmd2.with_category(ANNOTATED_CATEGORY)
@@ -122,10 +121,10 @@ def do_build(
122121
verbose: bool = False,
123122
color: bool = True,
124123
) -> None:
125-
"""Build a target. Bool flags are inferred from defaults.
124+
"""Build a target. Bool flags use BooleanOptionalAction.
126125
127-
``verbose: bool = False`` becomes ``--verbose`` (store_true).
128-
``color: bool = True`` becomes ``--no-color`` (store_false).
126+
``verbose: bool = False`` becomes ``--verbose / --no-verbose``.
127+
``color: bool = True`` becomes ``--color / --no-color``.
129128
130129
Try:
131130
build app --verbose --no-color
@@ -224,6 +223,80 @@ def do_echo(self, text: str) -> None:
224223
"""
225224
self.poutput(text)
226225

226+
# -- Subcommands ---------------------------------------------------------
227+
# @with_annotated can be the parent command that dispatches to
228+
# subcommands registered via @as_subcommand_to. The parent needs
229+
# a manual parser with add_subparsers() since subparser setup
230+
# can't be inferred from annotations.
231+
232+
@staticmethod
233+
def _build_team_parser() -> cmd2.Cmd2ArgumentParser:
234+
parser = cmd2.Cmd2ArgumentParser(description="Manage teams")
235+
parser.add_subparsers(dest='subcommand', metavar='SUBCOMMAND', required=True)
236+
return parser
237+
238+
@cmd2.with_argparser(_build_team_parser)
239+
@cmd2.with_category(ANNOTATED_CATEGORY)
240+
def do_team(self, args) -> None:
241+
"""Manage teams. Uses subcommands via @as_subcommand_to.
242+
243+
Try:
244+
team create Warriors
245+
team list
246+
help team
247+
help team create
248+
"""
249+
handler = args.cmd2_handler.get()
250+
if handler:
251+
handler(args)
252+
253+
_team_create_parser = cmd2.Cmd2ArgumentParser(description="Create a new team")
254+
_team_create_parser.add_argument('name', help='team name')
255+
_team_create_parser.add_argument('--city', default='Unknown', help='home city')
256+
257+
@cmd2.as_subcommand_to('team', 'create', _team_create_parser, help='create a new team')
258+
def team_create(self, args) -> None:
259+
self.poutput(f"Created team: {args.name} ({args.city})")
260+
self._sports.append(args.name)
261+
262+
_team_list_parser = cmd2.Cmd2ArgumentParser(description="List all teams")
263+
264+
@cmd2.as_subcommand_to('team', 'list', _team_list_parser, help='list all teams')
265+
def team_list(self, _args) -> None:
266+
for sport in self._sports:
267+
self.poutput(f" - {sport}")
268+
269+
# -- Argument groups (mutually exclusive) --------------------------------
270+
# Argument groups are a parser-level feature. With @with_argparser you
271+
# build them manually. They work alongside @with_annotated commands.
272+
273+
@staticmethod
274+
def _build_output_parser() -> cmd2.Cmd2ArgumentParser:
275+
parser = cmd2.Cmd2ArgumentParser(description="Output with format options")
276+
parser.add_argument('message', help='message to output')
277+
fmt_group = parser.add_mutually_exclusive_group()
278+
fmt_group.add_argument('--json', action='store_true', help='output as JSON')
279+
fmt_group.add_argument('--csv', action='store_true', help='output as CSV')
280+
fmt_group.add_argument('--plain', action='store_true', default=True, help='output as plain text')
281+
return parser
282+
283+
@cmd2.with_argparser(_build_output_parser)
284+
@cmd2.with_category(ANNOTATED_CATEGORY)
285+
def do_output(self, args) -> None:
286+
"""Output a message with mutually exclusive format options.
287+
288+
Try:
289+
output hello --json
290+
output hello --csv
291+
output hello --json --csv (error: mutually exclusive)
292+
"""
293+
if args.json:
294+
self.poutput(f'{{"message": "{args.message}"}}')
295+
elif args.csv:
296+
self.poutput(f'message,{args.message}')
297+
else:
298+
self.poutput(args.message)
299+
227300

228301
if __name__ == '__main__':
229302
app = AnnotatedExample()

0 commit comments

Comments
 (0)