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
Adding Flag Validators #786
Comments
These all sound like awesome features to add into this library. I think one of the biggest challenges facing adding in these is the way the flag structs are currently generated for this library. Currently a python script pops out the implementations for all of these and adding custom attributes like Min and Max that only apply to Ints/Floats and would make that interface significantly more complicated. The custom validator function however seems like something that could reasonably be added without causing significant rework of how flags are currently handled. It also gives end users the most functionality. |
@lynncyrin @AudriusButkevicius this sounds like a cool feature to implement |
I'd love to see someone work on the feature, yeah! |
dibs! |
This is how I think the validation should work:
I am also thinking of adding Uint Slice and Uint64 Slice flags to the current mix of already supported flags. If we find a more popular use case, we can have an out of the box validator for it shipped with the library. EDIT: Add Duration Flag |
I've made comments in the PR which I think this is a bad feature that should be avoided. People can do validation in their command, as flag validation might be contextual to the command. |
It looks like we've got 2 👍's (me + @johnwyles / @michaeljs1990 please post here with any more information you have about this feature / why you want it. I'll leave this open for a few weeks, and close it if it doesn't look like there's any consensus. |
I still throw in my vote for this feature obviously. I think validation would be a great trick up |
I'm in favor of this feature conceptually. That said, as a practical matter I think adding it to the package now would produce a ton of maintainer burden. I think we (eg. the maintainers) are very behind the curve as far as maintainence is concerned, so anything that adds maintainer burden should come attached with us getting more maintainers 🙂 I would be willing to reconsider this opinion if this feature got a ton of support, similarly to the 50 👍s for required flags #85. Given all that, I'm currently a 👎 on adding this, but very much want to keep this space open for conversation. |
@lynncyrin on your 👎 I would like to hear why such a simple PR couldn't be tossed together - again when I have time which always seems to be fleeting - it does seem to me this is a logical addition to this great library would should be incorporated. Would love to hear your opinion since a rejiggering of https://github.com/urfave/cli/blob/v2/flag.go or such (maybe |
you answered your own question 🙂
and my focus is #892 right now. After that is done, I'll remove my 👎 |
This issue or PR has been automatically marked as stale because it has not had recent activity. Please add a comment bumping this if you're still interested in it's resolution! Thanks for your help, please let us know if you need anything else. |
Closing this as it has become stale. |
is this feature implemented? |
no |
This issue or PR has been bumped and is no longer marked as stale! Feel free to bump it again in the future, if it's still relevant. |
Is there any interest in this ? I would like to add this feature. @asahasrabuddhe did you work on this previously ? |
Well. I tried and added a sample PR of how validation might work for int/intslice flags. No-one reviewed it so the PR got closed as stale. Most of these requested features seem to be nice-to-haves that occur on the spur of the moment. |
So the flag validator can be performed via the Flag Action handler. I've added an example in the documentation. That should suffice for most use cases. For v3 we can look into convenience functions like LT, GT etc for basic types |
I think it could be invaluable to have three features that instantly would be of use in a clean and succinct way when further examining my own use cases for this library around validation:
I illustrate this and provide comments in the pseudocode in this example:
Currently, as it stands, once the user supplies a value to the flag I have no way of knowing what the default value was associated with that flag as it seems to be overridden (I could be mistaken).
Secondly it would be a nice-to-have to have not only some custom validators for each type of flag (int: min, max, non-zero, etc., string: regex, non-nil, etc. float: percision, etc.).
Lastly having the ability supply a custom callback validator could allow users to have a catch-all for validation of the flag values.
I started to take a look into doing this but, again if I am not mistaken, it looked like the
flagSet
was using the underlying go built-inflag
library which may prohibit accessing the default value supplied without needing these features there first. If not keeping a copy of them before modification would be a workaround if that is possible.I would be glad to help and add more to this discussion!
The text was updated successfully, but these errors were encountered: