axiros / terminal_markdown_viewer

Styled Terminal Markdown Viewer
Other
1.81k stars 106 forks source link

Potential code improvement: Replace docopt with argparse #33

Open smkent opened 8 years ago

smkent commented 8 years ago

Python ships with an argument parsing library called argparse which is pretty useful.

I think it would be good to investigate switching from docopt to argparse:

I'm thinking of doing some work around this, so I thought I'd file an issue to track it.

smkent commented 8 years ago

It would simplify calls to helper functions as option values wouldn't need to be passed to each individual function, either. The toplevel parsed arguments variable can simply be kept as a global and accessed as needed.

Looking further at how mdv is doing option parsing, this might be possible still using docopt. I still don't think it would be as readable since it would replace argument variables such as theme with args.get('-t', 'random'). Using argparse would allow the use of something like args.theme for this example.

axgkl commented 8 years ago

I like the idea of dropping the dependency (allthough I like also the idea of docopt to have -h output and parser in sync by design).

Also, think about people who use it as a library and want to call main, supplying all switches via named arguments. Listing them all in the main function has some advantages for e.g. code completing IDEs or REPLs like bpython....

smkent commented 8 years ago

I like the idea of dropping the dependency (allthough I like also the idea of docopt to have -h output and parser in sync by design).

This is definitely a nice feature, and it should be possible to do something similar with argparse's format_usage() function.

Also, think about people who use it as a library and want to call main, supplying all switches via named arguments. Listing them all in the main function has some advantages for e.g. code completing IDEs or REPLs like bpython....

Hm, true. argparse can be called with a manual list of command line switches to parse instead of reading them directly from sys.argv, so it would definitely still be possible to run mdv as a library this way. If I put together some code I'll keep this use case in mind.