Added support for parameter choices #7
Conversation
We display the choices pipe delimited and block moving forwards without selecting valid option.
|
I just wanted to offer up my solution for supporting choices in the parameters. |
multimeric
left a comment
multimeric
left a comment
There was a problem hiding this comment.
Thanks for the PR!
I'm not opposed to looping until a valid argument is provided (although I might add the option to disable this later on).
Please also add a test or two that demonstrate this outputs the expected things.
|
|
||
| # Delegate to the parent class. Default must be '' in order to get the type function to be called | ||
| action = super().add_argument(*args, type=type, default="", **kwargs) | ||
| if(choices): |
There was a problem hiding this comment.
I'd rather you extract the kwargs into a dictionary here so that we're not duplicating code.
| # If the user provided no value for this argument, prompt them for it | ||
| if val == "": | ||
| prompt = "{}{}\n> {}".format(self.name, help_str, default_str) | ||
| prompt = "{}{}\n> {}".format(self.name, help_str, choices_options) |
There was a problem hiding this comment.
By removing default_str here it looks like this breaks the default prompt for non-choice args
| getpass.getpass(prompt=prompt) if self.secure else input(prompt) | ||
| ) | ||
|
|
||
| while choices_options and newval not in choices: |
There was a problem hiding this comment.
I don't mind having a loop here, but please refactor it such that the other arguments (without choices) also "loop until valid" (aka, until the self.type(newval) runs without failing).
|
I also accidentally forgot to run CI builds on pull requests but that should be enabled now, so hopefully it should show if any tests are failing once you push a new commit. |
|
News? I'd really like this feature |
|
You are welcome to fork this fork and submit another pull request. |
3 participants
We display the choices pipe delimited and block moving forwards without selecting valid option.