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

Fix a bunch of processing issues #1687

Merged
merged 4 commits into from Oct 12, 2020
Merged

Fix a bunch of processing issues #1687

merged 4 commits into from Oct 12, 2020

Conversation

davidism
Copy link
Member

@davidism davidism commented Oct 12, 2020

This fixes the various processing issues I noticed while investigating #1649.

  • When multiple=True is set and nargs > 1, default must be a list of tuples. Setting such a default is detected correctly to produce a type of Tuple.
  • get_default is now tried in consume_value with the other sources, rather than being tried in full_process_value after calling process_value. This makes the processing more consistent. consume_value must now return a (value, ParameterSource) tuple.
  • UUID and DateTime types were adjusted so that their convert method allows a value that is already the correct type. This was already the case for other types.
  • Prompting is done in Option.consume_value instead of full_process_value to match the other sources.
  • Add ParameterSource.PROMPT to track that the value was prompted for. This is currently set even if the user confirms the default value.
  • Use the parameter source to determine if an argument was provided on the command line for completion, rather than checking if the value is None. This fixes an issue an argument with a default value wouldn't be completed. Also use the parameter source to check if a prompt should happen.
  • Return None if a default was not provided and nargs > 1. multiple=True or nargs=-1 returns an empty tuple. The workaround described in Tuples as Multi Value Options throw an TypeError #472 of setting default=(None, None) still works, but is no longer necessary.
  • Empty env vars are treated as if they weren't provided, rather than as an empty string (Path(exists=True, envvar='FOO') tries to interpret FOO="" as an existing path #1285). This was only applied to one branch in the env var processing code, it has been extended to the other cases.

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

This ensures the default value is processed like other values. Some
type converters were adjusted to accept values that are already the
correct type.

Use parameter source instead of value to determine if argument was
supplied on the command line during completion.

Add a parameter source for values from prompt.
@davidism davidism added this to the 8.0.0 milestone Oct 12, 2020
If a default isn't given, return () for multiple or nargs=-1, or return
None for nargs > 1.
@davidism davidism merged commit c067af7 into master Oct 12, 2020
@davidism davidism deleted the multiple-tuple branch October 12, 2020 01:34
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 13, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants