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

Create an inverseBoolFlag #1643

Merged
merged 12 commits into from
Jan 17, 2023
Merged

Create an inverseBoolFlag #1643

merged 12 commits into from
Jan 17, 2023

Conversation

skelouse
Copy link
Contributor

@skelouse skelouse commented Jan 12, 2023

What type of PR is this?

This is a new feature for handling IsSet of a BoolFlag by introducing an inverseFlag alongside it.

  • feature

What this PR does / why we need it:

I made it here when trying to implement flags where the following commands are valid with a flag --local

$ tau local is not set, thus we prompt the user

$ tau --local local is set as true

$ tau --local=false local is set as false

I decided on an alternative where I take a flag, say local

$ tau --local local is set as true

$ tau --no-local local is set as false

Which issue(s) this PR fixes:

Somewhat Fixes #424

Testing

I simply tested the basic IsSet, value and an error with:

  • TestBoolWithInverse
  • Basic for testing the basic functionality
  • Action for testing the running of the provided BoolFlag's ActionFunc
  • EnvVars for testing positive and negative environment variables being set
  • WithPrefix for confirming the InversePrefix value is being used as expected
  • An example: ExampleNewBoolWithInverse

Release Notes

BoolWithInverseFlag is a new flag struct to handle IsSet of a BoolFlag by introducing an inverseFlag alongside it

TODO

Coverage

Options handling:

  • Negative Description
  • Negative suffix || prefix
  • Negative FilePath
    - Negative Destination && Count
  • Negative Usage

There are likely things I did not take into account which represent how a normal flag is expected to function.

@skelouse skelouse requested a review from a team as a code owner January 12, 2023 19:54
@skelouse skelouse mentioned this pull request Jan 12, 2023
flag_bool_with_inverse.go Outdated Show resolved Hide resolved
@dearchap
Copy link
Contributor

@skelouse Can you add more tests for to exercise new interface ?

…ct. Added more tests and solved logical issues.
@skelouse
Copy link
Contributor Author

@skelouse Can you add more tests for to exercise new interface ?

Added more tests and removed the interface so that it acts, and can be used as any other flag type.

@skelouse

This comment was marked as resolved.

flag_bool_with_inverse.go Outdated Show resolved Hide resolved
@dearchap dearchap changed the base branch from v3-dev-main to main January 13, 2023 22:03
@dearchap
Copy link
Contributor

@skelouse Also main is the default v3 branch now no need for v3-dev-main

@skelouse

This comment was marked as resolved.

@skelouse
Copy link
Contributor Author

Good to go. Swapped in place for my original BoolWithInverse interface, and ran e2e tests on current project.

@dearchap dearchap merged commit 68ec465 into urfave:main Jan 17, 2023
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.

Add support for flags that also behave as switches
2 participants