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

Add "--no-flag" support for Boolean types #76

Open
elliot-nelson opened this issue Oct 1, 2020 · 5 comments
Open

Add "--no-flag" support for Boolean types #76

elliot-nelson opened this issue Oct 1, 2020 · 5 comments
Labels
Hacktoberfest Welcome to Hacktoberfest participants! help wanted

Comments

@elliot-nelson
Copy link
Member

SUMMARY

In many popular CLI frameworks, having a boolean flag --example also allows --no-example, as an alias for --example false. Sywac should support the same input for boolean flags.

DETAILS

We'll need to update the boolean type to slurp up this alternate alias, and make sure that it is handled properly when strict mode is turned on.

@elliot-nelson elliot-nelson added help wanted Hacktoberfest Welcome to Hacktoberfest participants! labels Oct 1, 2020
@imogenkinsman
Copy link

I'd love to help with this one ❤️

@nexdrew
Copy link
Member

nexdrew commented Oct 1, 2020

@immykins We would love for you to help with this!

It might get a little tricky diving into the codebase the first time, but hopefully we can give you some direction on how to get started.

One thing to be aware of is that configuration state of a CLI option should most likely live in the instance of the "type" that represents that CLI option (e.g. in the TypeBoolean class), but the parsing state of a CLI option should live in the Context instance that represents the result of all parsed CLI options for a single execution.

For this issue, I kinda think we should add a negationPrefix configuration option to the TypeBoolean class, and then somehow use that to automatically look for aliases that start with the prefix ('no-' by default) and, when encountered, just flip/negate the value that is given/set.

A couple things we'll need to address during this implementation:

  1. Hiding or displaying the special negated flag aliases in help text
  2. Whether the special negated aliases should end up as separate keys/entries in the parsed argv result

Hopefully I can provide some example code in the near future to help get you started.

In the meantime, feel free to ask questions or post comments in this issue.

Thanks again for your interest in contributing to sywac!

@imogenkinsman
Copy link

Thanks for the detailed information!

I'm thinking of tackling this as a few smaller PRs (adding tests for boolean, adding --no-flag support, adding / updating docs for boolean). I have a long train ride tomorrow to work on this - I'll let you know if I have questions.

@imogenkinsman
Copy link

imogenkinsman commented Oct 10, 2020

Okay, I have some tests in place for this. A few questions:

Let's say that a user has defined a boolean:
.boolean('-b, --bool')

My understanding is that if the parsed string doesn't use that boolean flag then it's set to false in the response argv, so using a negation prefix to set it to false would essentially be a noop. Can you help me understand the use case for this feature - maybe with an example? I'm probably misunderstanding something because of a lack of familiarity with the library.

Implementation questions:

If a boolean has an alias, should enabling negationPrefix make both the boolean and its alias have negations? In the previous example this would be -no-b and --no-bool.

How should we handle the case where both the flag and its negation are used simultaneously? My instinct would be to just fail with an error message.

@nexdrew
Copy link
Member

nexdrew commented Oct 14, 2020

@immykins Good questions!

In the terms of the use-case, I think supporting --no-bool is really only useful when --bool is defined as defaultValue: true, such that the CLI author wants their users to be able to say --no-bool to negate it instead of --bool=false or --bool false. Because of this, I think enabling use of a negation prefix should be opt-in only, meaning it should be disabled unless the CLI author explicitly turns it on via type configuration like negationPrefix: true or negationPrefix: 'no-'. (I also want to avoid breaking any CLIs that might already define explicit flags like --no-color.)

If a boolean has an alias, should enabling negationPrefix make both the boolean and its alias have negations? In the previous example this would be -no-b and --no-bool.

When configured, I think all aliases of the boolean option should support the negation prefix, but I would expect the use of a negation prefix to only work via a double-dash prefix, meaning I would expect --no-bool and --no-b to work but not -no-b or -no-bool. I believe that would be consistent with typical standards/expectations.

How should we handle the case where both the flag and its negation are used simultaneously? My instinct would be to just fail with an error message.

Each flag is encountered and parsed serially, so, for a boolean option, I would expect the last encountered value to "win". This should be consistent with current functionality when something like --bool --bool=false is given; the last given value for the same option is false, so the result should be a value of bool: false in the parsed argv. I don't think there's a need to fail/error in this scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Hacktoberfest Welcome to Hacktoberfest participants! help wanted
Projects
None yet
Development

No branches or pull requests

3 participants