CommandLineUtils icon indicating copy to clipboard operation
CommandLineUtils copied to clipboard

Add support for --no-option

Open TheConstructor opened this issue 5 years ago • 17 comments
trafficstars

I got two ideas on how to do this:

  1. This could automatically be applied to bool?-type options making all three states reachable
  2. This could be enabled by specifying a no-value. This would enable ./configure like --no-dep/--dep=xyz stuff

I don't have an idea how to apply this to short-options (--no-s as negation of -s feels slightly off).

Currently you can define a second option to get a similar behavior. Main benefit would be less boilerplate and a shorter help-text.

I am willing to implement this myself if the idea sounds sound to you.

TheConstructor avatar Jan 16 '20 16:01 TheConstructor

I've seen the --[no-]option pattern before, but struggling to remember where and how it was useful. Do you have some concrete examples of what a user would do with this?

natemcmaster avatar Feb 07 '20 06:02 natemcmaster

After though: I've also seen --[disable/enable]-feature, so need to be careful about being too opinionated on naming conventions.

natemcmaster avatar Feb 07 '20 06:02 natemcmaster

We have some options wich csn be enabled in configuration. Now there might be an occasional call where you want to temporarily override configuration. Also if you are writing scripts and don't control the local configuration, you may want to explicitly state, that is is off. (Verbose, color-output, …)

You can do this with an (optional) value, but I prefer --no-force over --force false as it will not mess with arguments following it, and at the same time you have --force and not --force true. Lastly I would like to link enable-/disable-switches in help output.

While I may prefer --no-… adding a way to template/specify --disable-… should be straight forward.

TheConstructor avatar Feb 07 '20 06:02 TheConstructor

I think this idea needs to be fleshed out a little more. I thought I knew what you were asking, but then your response didn't fit with my original assumptions. Maybe I'm just dense. Can you share some code snippets of what you think the completed feature would look like for the end user?

natemcmaster avatar Feb 07 '20 06:02 natemcmaster

@natemcmaster this is a common pattern used in compilers and other such tools (configure, etc) and is used to invert the default meaning of a switch.

However I do believe @TheConstructor's definition is a little too broad and confusing. I suggest this feature be limited to boolean, long named switches. The absence of a valued option indicates --no-dep.

Controlling compiler warnings:

-Wall
-Wno-unused-parameter
-Wno-whatever

The first switch enables all warnings, then the following two disable specific warnings, and the opposite is also true:

-Wunused-parameter
-Wwhatever

By default all warnings are disabled, and we're enabling two of them.

I propose the following syntax:

(pseudo code, and naming conventions are just suggestions)

enum ToggleState
{
    Unchanged,
    No,
    Yes
}

var option1 = app.Option("--no-switch", CommandOptionType.Toggle);
var option2 = app.Option("--disable-other-switch", CommandOptionType.Toggle);

if (option1.ToggleValue == ToggleState.Yes)
{
    //do whatever makes sense
}

Usage is myapp --switch --other-switch or myapp -no-switch -disable-other-switch, and the meaning of the presence or absence of the prefix is determined by the developer.

The toggle prefix is declared by convention as the first stanza in the option declaration string. Presence of the prefixed switch indicates ToggleState.Yes, unprefixed switch ToggleState.No, and no switch ToggleState.Unchanged.

An alternative prefix specification is --enable|disable-switch to enable both --enable-switch and --disable-switch, which is also another well used convention.

The alternative usage of --switch true and --switch false could also be supported by matching against the switch name after the prefix definition.

JakeSays avatar Feb 13 '20 00:02 JakeSays

@JakeSays your suggestion on toggle enum is an interesting idea. I think I would implement it slightly differently though. What do you think of usage that looks like this?

Option<ToggleState> myMappedOption = app.AddMappedOption<ToggleState>(o =>
{
    o.Map("--yes", ToggleState.Yes);
    o.Map("--no", ToggleState.No);
    o.Map("--maybe-so", ToggleState.Maybe);
});

// or with attributes

public class Program
{
    [MappedOption("--yes", ToggleState.Yes)]
    [MappedOption("--no", ToggleState.No)]
    [MappedOption("--maybe-so", ToggleState.Maybe)]
    public ToggleState State { get; set; }
}

natemcmaster avatar Feb 18 '20 06:02 natemcmaster

@natemcmaster Is that mapping a prefix to a state? I am a little unclear on how it would be used.

How would I get from o.Map("--no", ToggleState.No) to --no-unused-parameter and --unused-parameter, or o.Map("--enable", ToggleState.Yes); o.Map("--disable", ToggleState.No) to --enable-touch-screen and --disable-touch-screen?

Also ToggleState.Maybe doesn't make sense to me, given my aversion to ambiguity. What would maybe mean in this context? Typically a prefix-switch defaults to one of yes/no, so the idea behind ToggleState.Unchanged is to indicate the user didn't specify anything, so go with the default. ToggleState.Default would make more sense now that I think about it.

JakeSays avatar Feb 18 '20 07:02 JakeSays

Sorry, allow me to clarify. I meant for ToggleState to be something from user code, not in this library. Maybe this makes it clearer?

Option<FruitOption> myMappedOption = app.AddMappedOption<FruitOption>(o =>
{
    o.Map("--banana", FruitOption.Banana);
    o.Map("--apple", FruitOption.Apple);
    o.Map("--pear", FruitOption.Pear);
});

// or with attributes

public class Program
{
    [MappedOption("--banana", FruitOption.Banana)]
    [MappedOption("--apple", FruitOption.Apple)]
    [MappedOption("--pear", FruitOption.Pear)]
    public FruitOption Fruit { get; set; }
}

I think this would be generic enough to support the uses cases above

enum TouchScreen
{
   Enabled,
   Disabled
}

app.AddMappedOption<TouchScreen>(o =>
{
   o.Map("--disable-touch-screen", TouchScreen.Disabled);
   o.Map("--enable-touch-screen", TouchScreen.Enabled);
});

enum MyFeature
{
   Yes,
   No
}

app.AddMappedOption<MyFeature>(o =>
{
   o.Map("--feature", MyFeature.Yes);
   o.Map("--no-feature", MyFeature.No);
});

natemcmaster avatar Feb 18 '20 07:02 natemcmaster

@natemcmaster ahh ok. I see now.

Yes, I think that would work nicely.

The only scenario that isn't covered is --touch-screen=(enabled|disabled), but this could be accomplished with existing functionality. It would mean one more option declaration, but generally speaking I rarely see prefix switches and value switches being used for the same option. It's usually one style or the other. Therefore I personally don't see the need to explicitly accommodate this scenario.

I believe this could also solve #329.

Would it make sense to have AddMappedLongOption() and AddMappedShortOption()?

AddMappedShortOption(o =>
{
    o.Map("-T", TouchScreen.Enabled);
});

I personally wouldn't use it, but someone might.

JakeSays avatar Feb 18 '20 08:02 JakeSays

I'd rather just have one API which supports both short and long options.

Option<FruitOption> myMappedOption = app.AddMappedOption<FruitOption>(o =>
{
    o.Map("-b|--banana", FruitOption.Banana);
    o.Map("-a", FruitOption.Apple);
    o.Map("-p|--pear", FruitOption.Pear);
});

natemcmaster avatar Feb 19 '20 06:02 natemcmaster

Ah. Yes of course - makes total sense.

JakeSays avatar Feb 19 '20 12:02 JakeSays

While your suggestion would nicely cover the parsing (without another field), I have some questions:

  • is there any idea how to present this in help-output? As it would be defined I could imagine that you could either generically generate a header like
Usage: ConsoleApp1 [options]

Options:
  -i|--int      IntOption
  -?|-h|--help  Show help information

Fruit:
  -b|--banana   Banana
  -a            Apple
  -p|--pear     Pear

or

  (-b|--banana)|-a|(-p|--pear)   Fruit: Banana, Apple, Pear

my preferred way of

  --[no-]fruit  (don't) offer fruit

would probably require either a custom help-provider, or we could 'misue' the outer mapped-option to carry two fields for documentation purposes

  • should the parser handle/allow valued options? Example could be
public class Program
{
    [MappedOption("--banana", FruitOption.Banana)] // default NoValue
    [MappedOption("--apple", FruitOption.Apple, CommandOptionType.SingleValue)]
    [MappedOption("--pear", FruitOption.Pear, CommandOptionType.SingleOrNoValue)]
    public (FruitOption fruit, string color) Fruit { get; }
}
  • Should we offer the bool-first tupples? public (bool valueProvided, FruitOption fruit) / public (bool valueProvided, (FruitOption fruit, string color)) Fruit { get; } or would this be too complicated?

  • Especially if we allow values, we might want to support multiplicity-specification on the group of mapped options i.e. you may want to have a red apple, a green apple and a peach.

TheConstructor avatar Feb 19 '20 19:02 TheConstructor

@TheConstructor Some thoughts:

(-b|--banana)|-a|(-p|--pear) Fruit: Banana, Apple, Pear - I would hate to require my users to understand regular expression syntax. --[no-]fruit (don't) offer fruit - the presence or absence of any of the mapped switches would accomplish this need. Having an explicit option like this and the value switches would be confusing.

public (FruitOption fruit, string color) Fruit { get; }

What is the purpose of the above tuple? What would the string represent? If you are considering having both the value switches and --fruit=pear then this again is just more complexity for the end user. I would suggest only one of the two syntaxes.

For this:

Especially if we allow values, we might want to support multiplicity-specification on the group of mapped options i.e. you may want to have a red apple, a green apple and a peach.

It may make sense to base multiplicity off of the property type. If the property is a container then it would implicitly support multiple values.

JakeSays avatar Feb 27 '20 06:02 JakeSays

My two cents:

is there any idea how to present this in help-output?

I was planning to start with this for simplicity:

Usage: ConsoleApp1 [options]

Options:
  -i|--int      IntOption
  -?|-h|--help  Show help information
  -b|--banana   Banana
  -a            Apple
  -p|--pear     Pear

We can provide overrides for those who want to customize this more.

should the parser handle/allow valued options?

No, I don't think so. This scenario sounds more like PowerShell's parameter sets, which is a feature of PowerShell that I personally hate and find confusing whenever I've encountered it.

Should we offer the bool-first tupples?

Too complicated.

natemcmaster avatar Mar 08 '20 19:03 natemcmaster

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Please comment if you believe this should remain open, otherwise it will be closed in 14 days. Thank you for your contributions to this project.

stale[bot] avatar Jul 21 '21 02:07 stale[bot]

#451 is still work in progress

TheConstructor avatar Jul 22 '21 07:07 TheConstructor

This issue has been automatically marked as stale because it has no recent activity. It will be closed if no further activity occurs. Please comment if you believe this should remain open, otherwise it will be closed in 14 days. Thank you for your contributions to this project.

github-actions[bot] avatar Jul 23 '22 02:07 github-actions[bot]

Closing due to inactivity. If you are looking at this issue in the future and think it should be reopened, please make a commented here and mention natemcmaster so he sees the notification.

github-actions[bot] avatar Aug 07 '22 02:08 github-actions[bot]