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

Switch from getopt to argparse #21

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

pinotree
Copy link
Contributor

@pinotree pinotree commented Jun 5, 2021

getopt is a low-level module for command line handling, and requires manual boilerplate to work. Instead, migrate to argparse, which is more high-level and provides a number of facilities:

  • a single place to define each option
  • streamlined help texts
  • enforcing of mandatory options
  • enforcing a value as certain type, or in a list of allowed values

@pinotree pinotree force-pushed the argparse branch 2 times, most recently from 15a3927 to b8c868b Compare June 6, 2021 06:11
@gapan
Copy link
Owner

gapan commented Jun 6, 2021

I'm mostly OK with this. However, one thing that I find I prefer with getopt, is that if you type just xdgmenumaker, you get the full help text, along with a message that -f is needed. Now, with argparse, you only get this:

usage: xdgmenumaker [OPTIONS]
xdgmenumaker: error: the following arguments are required: -f/--format

I think this is probably why I went with getopt in the first place.

getopt is a low-level module for command line handling, and requires
manual boilerplate to work. Instead, migrate to argparse, which is more
high-level and provides a number of facilities:
- a single place to define each option
- streamlined help texts
- enforcing of mandatory options
- enforcing a value as certain type, or in a list of allowed values
@pinotree
Copy link
Contributor Author

pinotree commented Jun 6, 2021

Not trying to undermine your feedback on this: how important is that you get the full help text on missing -f? At least from my experience, I have not seen that commonly in other CLI applications; even for xdgmenumaker itself, the users hopefully will either read the man page or the --help text on their own, and remember that in the next usages.

@gapan
Copy link
Owner

gapan commented Jun 6, 2021

I will turn the question around: how important is to switch parsing methods? Both work fine, so why go into the trouble, if the current one offers more flexibility too?

@pinotree
Copy link
Contributor Author

pinotree commented Jun 6, 2021

getopt is a low level method, and and such it leaves a lot to implemented by the applications:

  • the help text has to be manually provided by the user, and kept updated accordingly
  • there is a complete duplication of the short and long options in both the getopt call and the checks for the matched option, which means that a mere typo can make an option never used in practice
  • enforcing a certain type (like an integer) is left to the user; in this case there is only one, will add more boilerplate in case a new one is added
  • enforcing a certain value is only among a fixed choice is left to the user
  • the logic for default values is left to the user

In the end, it's much easier and less error-prone to use argparse; even the old and long-deprecated optparse is still a better choice than using getopt manually.

@gapan
Copy link
Owner

gapan commented Jun 6, 2021

Could you possibly look into this, so we can have the help message on error too?
https://stackoverflow.com/questions/4042452/display-help-message-with-python-argparse-when-script-is-called-without-any-argu#4042861

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.

None yet

2 participants