Skip to content
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

Add required args to subcommand program #1293

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rdhammond15
Copy link

Issue

When a positional comes before a subcommand, the help output for the subcommand will be missing the positional in the usage. This does not match up with argparse functionality.

Expected Output

(Cmd) top middle test test lower1 -h
Usage: top middle required required2 lower1 [-h]

optional arguments:
  -h, --help  show this help message and exit

Actual Output

(Cmd) top middle test test lower1 -h
Usage: top middle lower1 [-h]

optional arguments:
  -h, --help  show this help message and exit

Sample Test Program

import cmd2


class App(cmd2.Cmd):

    top_parser = cmd2.Cmd2ArgumentParser(description="Top level parser")
    top_subparser = top_parser.add_subparsers(dest="middle", required=True, title="Middle level command")

    @cmd2.with_argparser(top_parser)
    def do_top(self, opts):
        print("In top")
        opts.cmd2_handler.get()(opts)


@cmd2.with_default_category('middle')
class Middle(cmd2.CommandSet):

    middle_parser = cmd2.Cmd2ArgumentParser(description="Middle level command to top")
    middle_parser.add_argument("required", help="Required arg between subparsers")
    middle_parser.add_argument("required2", help="Required arg between subparsers")
    middle_subparser = middle_parser.add_subparsers(dest="lower", required=True, title="Lower level commands")
    middle_subparser.add_parser("lower1", help="First lower command")
    middle_subparser.add_parser("lower2", help="Second lower command")

    @cmd2.as_subcommand_to('top', 'middle', middle_parser, help="Middle and lower commands")
    def do_middle(self, opts):
        print("Lower commands")


if __name__ == "__main__":
    app = App()
    app.cmdloop()

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (be15939) 98.56% compared to head (2bc8b6b) 98.56%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1293      +/-   ##
==========================================
- Coverage   98.56%   98.56%   -0.01%     
==========================================
  Files          22       22              
  Lines        5780     5776       -4     
==========================================
- Hits         5697     5693       -4     
  Misses         83       83              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@anselor anselor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run black and add type annotations to address the mypy errors. The changes look reasonable to me. Thanks for finding and addressing this!

@rdhammond15
Copy link
Author

rdhammond15 commented Feb 16, 2024

Please run black and add type annotations to address the mypy errors. The changes look reasonable to me. Thanks for finding and addressing this!

Seems like flake8 and black are competing against each other? Format passes now, but Lint fails due to the formatting fixes.

@rdhammond15
Copy link
Author

@anselor Do you have any guidance for the conflicting flake8 and black rules?

@anselor
Copy link
Contributor

anselor commented Jul 2, 2024

Sorry, this slipped out of my brain and I didn't notice until now. It looks like the build log is gone and I'm not sure how to rerun the build...

@rdhammond15
Copy link
Author

@anselor I've repushed to kick off the workflow again.

@anselor
Copy link
Contributor

anselor commented Jul 4, 2024

@rdhammond15 It looks like flake8 recently become opinionated about that. I disagree with E704 being an antipattern. Just go ahead and disable E704 for flake8 globally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants