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.

Sign up for GitHub

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

Jump to bottom

gh-107467: Restructure Argument Clinic command-line interface #107469

Merged
merged 8 commits into from
Aug 1, 2023
Merged

gh-107467: Restructure Argument Clinic command-line interface #107469

merged 8 commits into from
Aug 1, 2023

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jul 30, 2023 •
edited by bedevere-bot
Loading

  • Add and use CLIError exception for CLI usage errors
  • On CLI error, print to stderr instead of stdout
  • Put the entire CLI in main()
  • Rework ClinicExternalTest to call main() instead of using subprocesses

@erlend-aasland
pythongh-107467: Restructure Argument Clinic command-line interface
a16fe3f 
- Add and use CLIError exception for CLI usage errors
- On CLI error, print to stderr instead of stdout
- Put the entire CLI in main()
- Rework ClinicExternalTest to call main() instead of using subprocesses
@erlend-aasland
Add NEWS
665d44f
@erlend-aasland erlend-aasland changed the title clinic/rework cli gh-107467: Restructure Argument Clinic command-line interface Jul 30, 2023
@bedevere-bot bedevere-bot mentioned this pull request Jul 30, 2023
Closed
3 tasks
@erlend-aasland
Copy link
Contributor Author

@AlexWaygood, are you fine with this?

@AlexWaygood
Copy link
Member

@AlexWaygood, are you fine with this?

Haven't had a chance to look deeply yet, and would like to! :)

@AlexWaygood
Copy link
Member

I think some of this is reinventing the wheel a little bit, and can be simplified by making use of the ArgumentParser.error() method. I filed erlend-aasland#48 as a PR to your branch!

AlexWaygood
AlexWaygood approved these changes Aug 1, 2023
View reviewed changes
Copy link
Member

@AlexWaygood AlexWaygood left a comment

Choose a reason for hiding this comment

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

LGTM! Feel free to merge if you're happy with the changes I pushed :)

@erlend-aasland
Copy link
Contributor Author

Thanks for the argparse error trick; I was unaware of that :) Let's land this and continue with the error handling refactoring.

@erlend-aasland erlend-aasland merged commit 49f238e into python:main Aug 1, 2023
18 checks passed
@erlend-aasland erlend-aasland deleted the clinic/rework-cli branch August 1, 2023 16:24
@bedevere-bot

This comment was marked as off-topic.

Sign in to view
@AlexWaygood AlexWaygood mentioned this pull request Aug 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@erlend-aasland @AlexWaygood @bedevere-bot