-
Notifications
You must be signed in to change notification settings - Fork 197
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
Migrated from Docopt to ArgParse #204
Conversation
calexandru2018
commented
Jun 29, 2020
- The CLI has been fully migrated to ArgParse
- CLI code has been written in OO paradigm
- None of the commands have been changed, fully backwards compatible with the previous version using Docopt
Have also added a custom help message.
Restored some CLI commands so that they are fully compatible with previous versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great approach! Definitely going with that :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
docopt is still mentioned as dependency in setup.py and requirements.txt. 😜 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM :)
Please let Alexandru squash-merge so it's properly counted as his contribution 👍
* Ported CLI to argparse, based on OO programming paradigm * Added descriptive comments * Updated usage string * Added logging * Updated exmaples * Removed function based CLI code * Removed docopt dependency * Added usage constant * Added back the PVPN_WAIT environment variable * Addressed Flake8 issues * Examples are now inline with the CLI * Removed unncesessary comment * Cleaned up code and improved readability * Removed dependencies still they are no longer needed * Updated inline command * Allow uppercase protocol with -p * Allign help message * Return missing -p to example Co-authored-by: Alexandru Cheltuitor <[email protected]>
The second to last commit was force-pushed and ignored this PR which was previously merged. I was wondering if that was intentional to revert this back? Getting back to Docopt again? |
We are preparing the official linux CLI (currently on beta: https://protonvpn.com/blog/linux-vpn-cli-beta/). For technical reasons (as we need to prepare for smooth update from this CLI to the official one) we needed to make a small technical release as part of the planned upgrade. Unfortunately master branch already had this docopt-to-argparse branch merged in, but was too risky to make the technical release to also include this branch. |