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

Support default values for args #4

Merged
merged 2 commits into from
Jul 30, 2024
Merged

Support default values for args #4

merged 2 commits into from
Jul 30, 2024

Conversation

smores56
Copy link
Owner

@smores56 smores56 commented Jul 13, 2024

Support default values for options and parameters via a default ? [NoDefault, Value a, Generate ({} -> a)] field for single value fields (e.g. str, dec, etc.).

The thing I wanted to add but couldn't find a good way to do it was help text support. I could require the default type to implement Inspect, but that wouldn't even work for the Generate ({} -> a) variant of default since it's not known ahead of time. So for now the expectation is that the arg's help text is given a default: xyz suffix.

@isaacvando does this look good to you? I can make a release if so.

@isaacvando
Copy link

@smores56 This looks great! Thanks for adding it :)

For the help text, one idea is to require an explicit string to be added to the default tag like this:

{ default: Value 100 "100"}
# or
{ default: Value {val: 100, help: "100"}}

That being said, this idea is clunky and doesn't add much value so I think it would be better to leave it as it is now.

@smores56
Copy link
Owner Author

Yeah, I tried the same thing more or less and reached the same conclusion, it's much cleaner to just leave it to the developer to write their own docs

@smores56 smores56 force-pushed the default-values branch 2 times, most recently from 742e7a4 to dd2c12d Compare July 30, 2024 20:01
@smores56 smores56 merged commit bcf678d into main Jul 30, 2024
1 check passed
@smores56 smores56 deleted the default-values branch July 30, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants