click-contrib / click_params

Bunch of click parameters to use
Apache License 2.0
31 stars 5 forks source link

Rename UnionParamType to FirstOf, change implementation to take positional args, fully typehint, more verbose errors, optional return tuple with param used for conversion. #17

Closed laundmo closed 2 years ago

laundmo commented 2 years ago

This PR does a few things with the UnionParamType which i accidentally re-implemented in a project of mine. I fully understand if the changes to the API/usage are not wanted, though personally i find they increase readability a lot.

Example error of a nested FirstOf (no names given, therefore default union name used):

Error: Invalid value for '-j': All possible options exhausted without any successful conversion:
 -  (INTEGER | (INTEGER | FLOAT)): All possible options exhausted without any successful conversion:
  -  INTEGER: 'asdas' is not a valid integer.
  -  (INTEGER | FLOAT): All possible options exhausted without any successful conversion:
   -  INTEGER: 'asdas' is not a valid integer.
   -  FLOAT: 'asdas' is not a valid float.
 -  INTEGER: 'asdas' is not a valid integer.

Checklist:

lewoudar commented 2 years ago

Hi @laundmo thank you for this well-defined pull request Honestly I don't have arguments and feedback against the changes to this parameter, so it is OK for me But the tests are not passing, can you fix them please?

laundmo commented 2 years ago

Of course, I'll try. They were passing locally though which makes this quite odd.

lewoudar commented 2 years ago

Thanks a lot for this PR @laundmo I have to refine a bit the code, but I will try to release a new version by the end of the week

laundmo commented 2 years ago

Just noticed i forgot to add to changelog, might be something you want to have a look at.

lewoudar commented 2 years ago

Yeah I thought about it :)