cli icon indicating copy to clipboard operation
cli copied to clipboard

Add support for int/intslice flag validation

Open dearchap opened this issue 3 years ago • 5 comments

What type of PR is this?

  • feature

What this PR does / why we need it:

This PR adds basic support for flag validation.

Which issue(s) this PR fixes:

Fixes #786

Special notes for your reviewer:

Added support ONLY for int and intslice flags. This is for proof of concept. If this design is acceptable I can add support for other types in a similar fashion.

Testing

New tests have been added

  • TestValidateFlags
  • TestIntValidatorFunc
  • TestIntSliceValidatorFunc

Following tests have been updated to include flag validators

  • TestFlagsFromEnv

Release Notes


dearchap avatar Apr 02 '21 00:04 dearchap

Yeah i like this feature, if the design is approve i can help implementing other validator (string, time date/duration, net.IP)

instabledesign avatar Apr 07 '21 09:04 instabledesign

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.

stale[bot] avatar Jul 28 '21 04:07 stale[bot]

Closing this as it has become stale.

stale[bot] avatar Aug 28 '21 16:08 stale[bot]

bump

On Sat, Aug 28, 2021 at 12:07 PM stale[bot] @.***> wrote:

Closed #1261 https://github.com/urfave/cli/pull/1261.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/urfave/cli/pull/1261#event-5221864411, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYNLZRFJEVYABJL5BSKKJTT7ECS7ANCNFSM42H7Q4RA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

dearchap avatar Aug 28 '21 16:08 dearchap

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.

stale[bot] avatar Aug 30 '21 15:08 stale[bot]

@dearchap (paraphrasing from discussion) This and #1234 are similar ideas, and given that you're actively working on this one and #1234 has been seemingly abandoned, I favor moving forward with this PR. However, I'm concerned about the additional API surface area introduced here, so what do you think about targeting the v3 series and using generics instead?

meatballhat avatar Oct 02 '22 13:10 meatballhat

@meatballhat Yes that would be fine. However it might be worthwhile to add a light interface with implementation left to users for extending. And most common use cases can be provided with v3

dearchap avatar Oct 03 '22 12:10 dearchap

@meatballhat Yes that would be fine. However it might be worthwhile to add a light interface with implementation left to users for extending. And most common use cases can be provided with v3

This sounds good to me 🎉

meatballhat avatar Oct 03 '22 15:10 meatballhat

Will open a new PR with a small validation API

dearchap avatar Oct 15 '22 22:10 dearchap