Repository navigation
Negatable options in fluent command signatures - #51708
Closed
paulredmond wants to merge 4 commits into
Closed
paulredmond wants to merge 4 commits into
paulredmond wants to merge 4 commits into
Conversation
Member
|
I dunno - I just can't wrap my head around negatable options being a useful thing 😅 Seems like if you had a value like --cache... --no-cache is available by just not passing the option. I don't think I've ever needed something like this in my work. How can I be convinced? 😊 |
Contributor
Author
|
Yeah, you're probably right, thanks. |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was reaching for negatable options on an existing command and thought I'd propose a way to use them with the fluent signature:
Here's the simplest example:
Negatable options can have one of three states:
null,false, andtrue. Runningmy:commandwithout--cache|--no-cache, the default option value will benull. This is useful if you'd like to prompt the user of their preference if they don't pass an option:You can also get both variants of the option:
I like negatable options to unify toggling a boolean value on/off:

Using fluent signatures, you can also choose to define the default value as
trueorfalsewhen the user doesn't pass an option if you'd like:Handling an Invalid Default
Developers could misconfigure the fluent syntax. This implementation chooses to fall back to the default value of
nullwhenfilter_var()fails to filter a Boolean:Programmatically Calling Commands
You still need to pass
trueorfalsewhen using negatable options programmatically. However, you can use both flags:Possible B/C Issues?
The syntax could be whatever is desired if this PR is merged. The syntax I chose in this PR is the
!character (I also considered~). I did a quick Google search and found that~and¬are commonly used to represent negation. Although edge-case, my concern with~is a higher possibility that user-land signatures use~as the first character of a default value for the home folder on Unix systems:The above would be broken by this implementation, which would convert the option to a negatable option. That being said, there's a possibility that
!breaks user-land code too, but seems edge-case and odd that someone would start a default value with!:Fluent command signatures already support the
*symbol for arrays, so to me, the!doesn't feel too far a stretch for fluent negatable options:Reasons to Not Merge / Current Options using Fluent Signature
Without merging this PR, a developer can use a boolean "switch" by using only one of two options depending on what the desired default should be:
You could also define both flags, but it's possible that both flags are passed and both would be
true:I still prefer the DX of having separate flags to toggle a boolean—with the option of
nullby default when omitted.Another approach outside of this PR is not using the fluent syntax and instead defining command options using
configure():