-
Notifications
You must be signed in to change notification settings - Fork 40
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
Move all ArgParser variants to single cli.py module #714
Conversation
OS:ubuntu-20.04 |
@angelhof It seems that some tests are failing despite the logic not having changed. My guess is that it fails because of the deprecated options throwing an error - is this a reasonable assuption? If so, I can also add them back in. |
OS = Debian 10
|
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.
Wow, how neat! This is a great PR :) Let's just make sure to readd the obsolete flags and mark them as such (maybe in a function/method that adds them and is clearly documented that they do nothing)! The reason is that it is a breaking change to anyone who uses PaSh to remove flags, and we can do it later at a point when we want to switch to a new major version.
OS:ubuntu-20.04 |
OS = Debian 10
|
@angelhof Obsolete flags have been readded under add_obsolete_flags(). Once they are actually phased out we can just not call that method |
I personally had great difficulty navigating multiple different files to figure out how to add flags, modify flags, etc. This PR gathers all the CLI argparsers into a single
compiler/cli.py
file.This also removes the deprecated flags from the argparsers and changes scripts accordingly.