Skip to content

Fixed issue where argument parsers for overridden commands were not being created. #1384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 2.5.7 (TBD)
* Bug Fixes
* Fixed issue where argument parsers for overridden commands were not being created.
* Fixed issue where `Cmd.ppaged()` was not writing to the passed in destination.

## 2.5.6 (November 14, 2024)
* Bug Fixes
* Fixed type hint for `with_default_category` decorator which caused type checkers to mistype
Expand Down
270 changes: 169 additions & 101 deletions cmd2/cmd2.py

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion cmd2/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,9 @@ def cmd_wrapper(*args: Any, **kwargs: Dict[str, Any]) -> Optional[bool]:
statement, parsed_arglist = cmd2_app.statement_parser.get_command_arg_list(
command_name, statement_arg, preserve_quotes
)
arg_parser = cmd2_app._command_parsers.get(command_name, None)

# Pass cmd_wrapper instead of func, since it contains the parser info.
arg_parser = cmd2_app._command_parsers.get(cmd_wrapper)
if arg_parser is None:
# This shouldn't be possible to reach
raise ValueError(f'No argument parser found for {command_name}') # pragma: no cover
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ dev-dependencies = [
"pytest",
"pytest-cov",
"pytest-mock",
"ruff",
"sphinx",
"sphinx-autobuild",
"sphinx-rtd-theme",
Expand Down
4 changes: 1 addition & 3 deletions tests/test_argparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
import cmd2

from .conftest import (
find_subcommand,
run_cmd,
)

Expand Down Expand Up @@ -402,8 +401,7 @@ def test_add_another_subcommand(subcommand_app):
This tests makes sure _set_parser_prog() sets _prog_prefix on every _SubParsersAction so that all future calls
to add_parser() write the correct prog value to the parser being added.
"""
base_parser = subcommand_app._command_parsers.get('base')
find_subcommand(subcommand_app._command_parsers.get('base'), [])
base_parser = subcommand_app._command_parsers.get(subcommand_app.do_base)
for sub_action in base_parser._actions:
if isinstance(sub_action, argparse._SubParsersAction):
new_parser = sub_action.add_parser('new_sub', help='stuff')
Expand Down
35 changes: 35 additions & 0 deletions tests/test_cmd2.py
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,13 @@ def do_multiline_docstr(self, arg):
"""
pass

parser_cmd_parser = cmd2.Cmd2ArgumentParser(description="This is the description.")

@cmd2.with_argparser(parser_cmd_parser)
def do_parser_cmd(self, args):
"""This is the docstring."""
pass


@pytest.fixture
def help_app():
Expand Down Expand Up @@ -1249,6 +1256,11 @@ def test_help_multiline_docstring(help_app):
assert help_app.last_result is True


def test_help_verbose_uses_parser_description(help_app: HelpApp):
out, err = run_cmd(help_app, 'help --verbose')
verify_help_text(help_app, out, verbose_strings=[help_app.parser_cmd_parser.description])


class HelpCategoriesApp(cmd2.Cmd):
"""Class for testing custom help_* methods which override docstring help."""

Expand Down Expand Up @@ -3016,3 +3028,26 @@ def test_columnize_too_wide(outsim_app):

expected = "\n".join(str_list) + "\n"
assert outsim_app.stdout.getvalue() == expected


def test_command_parser_retrieval(outsim_app: cmd2.Cmd):
# Pass something that isn't a method
not_a_method = "just a string"
assert outsim_app._command_parsers.get(not_a_method) is None

# Pass a non-command method
assert outsim_app._command_parsers.get(outsim_app.__init__) is None


def test_command_synonym_parser():
# Make sure a command synonym returns the same parser as what it aliases
class SynonymApp(cmd2.cmd2.Cmd):
do_synonym = cmd2.cmd2.Cmd.do_help

app = SynonymApp()

synonym_parser = app._command_parsers.get(app.do_synonym)
help_parser = app._command_parsers.get(app.do_help)

assert synonym_parser is not None
assert synonym_parser is help_parser
2 changes: 1 addition & 1 deletion tests/transcripts/from_cmdloop.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
# so you can see where they are.

(Cmd) help say
Usage: say [-h] [-p] [-s] [-r REPEAT]/ */
Usage: speak [-h] [-p] [-s] [-r REPEAT]/ */

Repeats what you tell me to./ */

Expand Down
60 changes: 57 additions & 3 deletions tests_isolated/test_commandset/test_commandset.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,52 @@ def test_autoload_commands(command_sets_app):
assert 'Command Set B' not in cmds_cats


def test_command_synonyms():
"""Test the use of command synonyms in CommandSets"""

class SynonymCommandSet(cmd2.CommandSet):
def __init__(self, arg1):
super().__init__()
self._arg1 = arg1

@cmd2.with_argparser(cmd2.Cmd2ArgumentParser(description="Native Command"))
def do_builtin(self, _):
pass

# Create a synonym to a command inside of this CommandSet
do_builtin_synonym = do_builtin

# Create a synonym to a command outside of this CommandSet with subcommands.
# This will best test the synonym check in cmd2.Cmd._check_uninstallable() when
# we unresgister this CommandSet.
do_alias_synonym = cmd2.Cmd.do_alias

cs = SynonymCommandSet("foo")
app = WithCommandSets(command_sets=[cs])

# Make sure the synonyms have the same parser as what they alias
builtin_parser = app._command_parsers.get(app.do_builtin)
builtin_synonym_parser = app._command_parsers.get(app.do_builtin_synonym)
assert builtin_parser is not None
assert builtin_parser is builtin_synonym_parser

alias_parser = app._command_parsers.get(cmd2.Cmd.do_alias)
alias_synonym_parser = app._command_parsers.get(app.do_alias_synonym)
assert alias_parser is not None
assert alias_parser is alias_synonym_parser

# Unregister the CommandSet and make sure built-in command and synonyms are gone
app.unregister_command_set(cs)
assert not hasattr(app, "do_builtin")
assert not hasattr(app, "do_builtin_synonym")
assert not hasattr(app, "do_alias_synonym")

# Make sure the alias command still exists, has the same parser, and works.
assert alias_parser is app._command_parsers.get(cmd2.Cmd.do_alias)
out, err = run_cmd(app, 'alias --help')
assert normalize(alias_parser.format_help())[0] in out


def test_custom_construct_commandsets():
command_set_b = CommandSetB('foo')

Expand Down Expand Up @@ -291,7 +337,7 @@ def test_load_commandset_errors(command_sets_manual, capsys):
cmd_set = CommandSetA()

# create a conflicting command before installing CommandSet to verify rollback behavior
command_sets_manual._install_command_function('durian', cmd_set.do_durian)
command_sets_manual._install_command_function('do_durian', cmd_set.do_durian)
with pytest.raises(CommandSetRegistrationError):
command_sets_manual.register_command_set(cmd_set)

Expand All @@ -316,13 +362,21 @@ def test_load_commandset_errors(command_sets_manual, capsys):
assert "Deleting alias 'banana'" in err
assert "Deleting macro 'apple'" in err

# verify command functions which don't start with "do_" raise an exception
with pytest.raises(CommandSetRegistrationError):
command_sets_manual._install_command_function('new_cmd', cmd_set.do_banana)

# verify methods which don't start with "do_" raise an exception
with pytest.raises(CommandSetRegistrationError):
command_sets_manual._install_command_function('do_new_cmd', cmd_set.on_register)

# verify duplicate commands are detected
with pytest.raises(CommandSetRegistrationError):
command_sets_manual._install_command_function('banana', cmd_set.do_banana)
command_sets_manual._install_command_function('do_banana', cmd_set.do_banana)

# verify bad command names are detected
with pytest.raises(CommandSetRegistrationError):
command_sets_manual._install_command_function('bad command', cmd_set.do_banana)
command_sets_manual._install_command_function('do_bad command', cmd_set.do_banana)

# verify error conflict with existing completer function
with pytest.raises(CommandSetRegistrationError):
Expand Down
Loading